From 4e4ed80d1669f89079d9ea700a3e3239dff355da Mon Sep 17 00:00:00 2001 From: "Merrifield, Jay" Date: Tue, 15 Sep 2015 15:06:50 -0400 Subject: [PATCH] Added ability to automatically set the secure cookie value based on the trust proxy settings assuming the cookie secure option isn't already set --- README.md | 8 ++++ index.js | 4 ++ test/session.js | 121 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+) diff --git a/README.md b/README.md index abcd337b..0199b4cb 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,14 @@ The default value is `undefined`. if there is a direct TLS/SSL connection. - `undefined` Uses the "trust proxy" setting from express +##### autoSecure + +Setting this option will turn the cookie into a secure cookie if the request is secure. +It decides whether a request is secure based on the proxy setting. This setting overrides +the "secure" cookie option. + +The default value is `false`. + ##### resave Forces the session to be saved back to the session store, even if the session diff --git a/index.js b/index.js index 5e12c9fc..5bf5a1d3 100644 --- a/index.js +++ b/index.js @@ -88,6 +88,7 @@ function session(options){ , store = options.store || new MemoryStore , cookie = options.cookie || {} , trustProxy = options.proxy + , autoSecure = options.autoSecure || false , storeReady = true , rollingSessions = options.rolling || false; var resaveSession = options.resave; @@ -140,6 +141,9 @@ function session(options){ req.sessionID = generateId(req); req.session = new Session(req); req.session.cookie = new Cookie(cookie); + if (autoSecure) { + req.session.cookie.secure = issecure(req, trustProxy); + } }; var storeImplementsTouch = typeof store.touch === 'function'; diff --git a/test/session.js b/test/session.js index f592ef3f..fd24411b 100644 --- a/test/session.js +++ b/test/session.js @@ -579,6 +579,45 @@ describe('session()', function(){ .expect(shouldNotHaveHeader('Set-Cookie')) .expect(200, done) }) + describe('auto-set secure cookie', function() { + before(function () { + server = createServer({ proxy: true, autoSecure: true, cookie: { maxAge: 5 }}) + }) + it('should set secure cookie when X-Forwarded-Proto is https', function(done){ + request(server) + .get('/') + .set('X-Forwarded-Proto', 'https') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.notEqual(val.indexOf('Secure'), -1, 'should be Secure cookie') + done(); + }) + }) + + it('should trust X-Forwarded-Proto when comma-separated list', function(done){ + request(server) + .get('/') + .set('X-Forwarded-Proto', 'https,http') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.notEqual(val.indexOf('Secure'), -1, 'should be Secure cookie') + done(); + }) + }) + + it('should work when no header', function(done){ + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.equal(val.indexOf('Secure'), -1, 'should not be Secure cookie') + done(); + }) + }) + }) }) describe('when disabled', function(){ @@ -607,6 +646,45 @@ describe('session()', function(){ .expect(shouldNotHaveHeader('Set-Cookie')) .expect(200, 'true', done) }) + describe('never auto-set secure cookie', function() { + before(function () { + server = createServer({ proxy: false, autoSecure: true, cookie: { maxAge: 5 }}) + }) + it('should set secure cookie when X-Forwarded-Proto is https', function(done){ + request(server) + .get('/') + .set('X-Forwarded-Proto', 'https') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.equal(val.indexOf('Secure'), -1, 'should not be a Secure cookie') + done(); + }) + }) + + it('should trust X-Forwarded-Proto when comma-separated list', function(done){ + request(server) + .get('/') + .set('X-Forwarded-Proto', 'https,http') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.equal(val.indexOf('Secure'), -1, 'should not be Secure cookie') + done(); + }) + }) + + it('should work when no header', function(done){ + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.equal(val.indexOf('Secure'), -1, 'should not be Secure cookie') + done(); + }) + }) + }) }) describe('when unspecified', function(){ @@ -635,6 +713,49 @@ describe('session()', function(){ .expect(shouldSetCookie('connect.sid')) .expect(200, 'true', done) }) + describe('auto-set secure cookie', function() { + var app; + before(function () { + app = express() + .use(session({ secret: 'keyboard cat', autoSecure: true, cookie: { maxAge: min }})) + .use(function(req, res) { res.json(req.secure); }); + app.enable('trust proxy'); + }) + it('should set secure cookie when X-Forwarded-Proto is https', function(done){ + request(app) + .get('/') + .set('X-Forwarded-Proto', 'https') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.notEqual(val.indexOf('Secure'), -1, 'should be Secure cookie') + done(); + }) + }) + + it('should trust X-Forwarded-Proto when comma-separated list', function(done){ + request(app) + .get('/') + .set('X-Forwarded-Proto', 'https,http') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.notEqual(val.indexOf('Secure'), -1, 'should be Secure cookie') + done(); + }) + }) + + it('should work when no header', function(done){ + request(app) + .get('/') + .expect(shouldNotHaveHeader('Set-Cookie')) + .expect(200, 'true', function(err, res) { + var val = cookie(res); + assert.equal(val.indexOf('Secure'), -1, 'should not be Secure cookie') + done(); + }) + }) + }) }) })