add tls options

[cgx9]

i need use key and cert at self-sign certificate for https

[kornelski]

It allows arbitrary options, and tls options can override all other properties, e.g. tls({host:'xxx'}), which doesn't seem right. I'd prefer an API that is limited to only specific useful whitelisted options.

[timon]

This PR works for me,

and I also find it concerning that current implementation will happily override any options.

At least cert and key should be exposed (I believe they could be made separate arguments), maybe some tis peer checking callbacks could be exported as well.

This is sample naïve code I have used (that's basis upon which I'm building up now):

var fs = require('fs');
var request = require('superagent');

var url = 'https://server.local/client_auth';

var myCA = fs.readFileSync('server-ca.crt');
var myCert = fs.readFileSync('client.crt');
var myKey = fs.readFileSync('client.key');

request.get(url).
  ca(myCA). // <-- already present
  tls({ key: myKey, cert: myCert }). // <-- required to  work
  end(function(err, res) {
  if (err) {
    console.log("Err: ", err);
    return;
  }
  console.log(res);
});

[Whoaa512]

This approach seems most amicable and forward thinking in the case that the options for tls.connect() change.

I agree that overriding the other options is troublesome, however this could easily be avoided by simply moving the loop above the property assignments which should not be overridden.

The only other suggestion would be to add a comment above the method for documentation purposes and maybe a test.

@cgx9, you could adapt my test from #832

[ericnewton76]

ca(...) and tls(...) shouldn't be on the returned object from get(...) but should be on request(...) so that wasteful redundancy in testing code is reduced

var fs = require('fs');
var request = require('superagent');

var url = 'https://server.local/client_auth';

var myCA = fs.readFileSync('server-ca.crt');
var myCert = fs.readFileSync('client.crt');
var myKey = fs.readFileSync('client.key');

request
  .ca(myCA) // <-- already present
  .tls({ key: myKey, cert: myCert }) // <-- required to  work
  .get(url)
  .end(function(err, res) {
  if (err) {
    console.log("Err: ", err);
    return;
  }
  console.log(res);
});

so that potentially (what i do in my test files)

var fs = require('fs');
var request = require('superagent');

var url = 'https://server.local/client_auth';
var myCA = fs.readFileSync('server-ca.crt');
var myCert = fs.readFileSync('client.crt');
var myKey = fs.readFileSync('client.key');

request = request
  .ca(myCA) // <-- already present
  .tls({ key: myKey, cert: myCert }) // <-- required to  work

//now the request object is setup same for all subsequent GETs POSTs, etc
it('test1',function() {
request
  .get(url)
  .end(function(err, res) {
  if (err) { done(err); }
  console.log(res);
});
})
it('test2',function() {
request
  .post(url)
  .end(function(err, res) {
  if (err) { done(err); }
  console.log(res);
});

[matteofigus]

Hi, any news about this? It would be a good feature to add for me. Thanks for the great work.

[kornelski]

This will not be merged in the current state, because it allows change of arbitrary options on the agent, and we don't want to support everything, just properties we know are useful and have tests for.

• Please make it only accept whitelist of properties (without rejectUnauthorized allow rejectUnauthorized to be set to false for https request #832)
• Please add tests for the properties accepted
• Please add a dummy function in browser implementation

[kornelski]

Closed in favor of #1057

If you'd like to add more options, see #681 (comment)

[niftylettuce]

v5.1.1 released that resolves this issue