From ee11c6670c4f6892a0cd353c9ea0c06e5c1f60e2 Mon Sep 17 00:00:00 2001 From: niftylettuce Date: Wed, 8 Jan 2020 11:28:17 -0600 Subject: [PATCH] Revert "feat: add secure cookie override to agent (#1515)" This reverts commit 737697fa559d3c92b12fec4519ed030ed604968f. --- src/agent-base.js | 3 +- src/node/agent.js | 9 +---- src/node/index.js | 13 ------ test/node/secure-cookie.js | 82 -------------------------------------- 4 files changed, 2 insertions(+), 105 deletions(-) delete mode 100644 test/node/secure-cookie.js diff --git a/src/agent-base.js b/src/agent-base.js index be189aa60..45bd6d470 100644 --- a/src/agent-base.js +++ b/src/agent-base.js @@ -24,8 +24,7 @@ function Agent() { 'key', 'pfx', 'cert', - 'disableTLSCerts', - 'sendSecureCookie' + 'disableTLSCerts' ].forEach(fn => { // Default setting for all requests from this agent Agent.prototype[fn] = function(...args) { diff --git a/src/node/agent.js b/src/node/agent.js index d5573458c..7aa5fd037 100644 --- a/src/node/agent.js +++ b/src/node/agent.js @@ -50,10 +50,6 @@ function Agent(options) { if (options.rejectUnauthorized === false) { this.disableTLSCerts(); } - - if (options.sendSecureCookie) { - this.sendSecureCookie(); - } } } @@ -80,14 +76,11 @@ Agent.prototype._saveCookies = function(res) { */ Agent.prototype._attachCookies = function(req) { - const sendSecureCookie = Boolean( - this._defaults.find(current => current.fn === 'sendSecureCookie') - ); const url = parse(req.url); const access = new CookieAccessInfo( url.hostname, url.pathname, - url.protocol === 'https:' || sendSecureCookie + url.protocol === 'https:' ); const cookies = this.jar.getCookies(access).toValueString(); req.cookies = cookies; diff --git a/src/node/index.js b/src/node/index.js index 614a0255c..7c3e0cd9b 100644 --- a/src/node/index.js +++ b/src/node/index.js @@ -646,19 +646,6 @@ Request.prototype.disableTLSCerts = function() { return this; }; -/** - * Sends secure cookies on http and https requests - * Be warned this allows cookie hijacking - * - * @return {Request} for chaining - * @api public - */ - -Request.prototype.sendSecureCookie = function() { - this._sendSecureCookie = true; - return this; -}; - /** * Return an http[s] request. * diff --git a/test/node/secure-cookie.js b/test/node/secure-cookie.js deleted file mode 100644 index 8bb782042..000000000 --- a/test/node/secure-cookie.js +++ /dev/null @@ -1,82 +0,0 @@ -const request = require('../support/client'); -const express = require('express'); -const cookieParser = require('cookie-parser'); -const http = require('http'); - -const app = express(); - -app.use(cookieParser()); - -app.get('/', (req, res) => { - const cookie = req.header('cookie'); - if (cookie === undefined) { - res.cookie('test', 1, { maxAge: 900000, httpOnly: true, secure: true }); - res.send('cookie set'); - } else { - res.send('cookie sent'); - } -}); - -let base = 'http://localhost'; -let server; -before(function listen(done) { - server = http.createServer(app); - server = server.listen(0, function listening() { - base += `:${server.address().port}`; - done(); - }); -}); - -const agent1 = request.agent(); -const agent2 = request.agent({ sendSecureCookie: true }); -const agent3 = request.agent(); - -describe('Secure cookie', () => { - it('Should receive a secure cookie', () => { - agent1.get(`${base}/`).then(res => { - res.should.have.status(200); - should.exist(res.headers['set-cookie']); - res.headers['set-cookie'][0].should.containEql('Secure'); - res.text.should.containEql('cookie set'); - }); - - agent2.get(`${base}/`).then(res => { - res.should.have.status(200); - should.exist(res.headers['set-cookie']); - res.headers['set-cookie'][0].should.containEql('Secure'); - res.text.should.containEql('cookie set'); - }); - - agent3.get(`${base}/`).then(res => { - res.should.have.status(200); - should.exist(res.headers['set-cookie']); - res.headers['set-cookie'][0].should.containEql('Secure'); - res.text.should.containEql('cookie set'); - }); - }); - - it('Should send secure cookie on configured agents', () => { - agent1 - .sendSecureCookie() - .get(`${base}/`) - .then(res => { - res.should.have.status(200); - should.not.exist(res.headers['set-cookie']); - res.text.should.containEql('cookie sent'); - }); - - agent2.get(`${base}/`).then(res => { - res.should.have.status(200); - should.not.exist(res.headers['set-cookie']); - res.text.should.containEql('cookie sent'); - }); - }); - - it('Should not send secure cookie on default agent', () => { - agent3.get(`${base}/`).then(res => { - res.should.have.status(200); - should.exist(res.headers['set-cookie']); - res.text.should.containEql('cookie set'); - }); - }); -});