From 01b4edba601efe624cb87ee956adb8818ae58495 Mon Sep 17 00:00:00 2001 From: roblabla Date: Mon, 1 Aug 2016 15:41:08 +0200 Subject: [PATCH 1/4] Implement custom cookie signature --- README.md | 7 +++++++ index.js | 56 ++++++++++++++++++++++++++----------------------- test/session.js | 42 +++++++++++++++++++++++++++++++++---- 3 files changed, 75 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index b88ed824..a5967baa 100644 --- a/README.md +++ b/README.md @@ -276,6 +276,13 @@ The default value is `'keep'`. - `'keep'` The session in the store will be kept, but modifications made during the request are ignored and not saved. +##### signature + +Pass in your own cookie signing object/module here that implements the +`sign(value, secret)` and `unsign(value, secret)` functions. + +The default value is the `node-cookie-signature` module. + ### req.session To store or access session data, simply use the request property `req.session`, diff --git a/index.js b/index.js index 5794118f..27cd95a9 100644 --- a/index.js +++ b/index.js @@ -78,6 +78,8 @@ var defer = typeof setImmediate === 'function' * @param {Boolean} [options.saveUninitialized] Save uninitialized sessions to the store * @param {String|Array} [options.secret] Secret for signing session ID * @param {Object} [options.store=MemoryStore] Session store + * @param {Object} [options.signature] Object that has the same API as node-cookie-signature + if you need to implement your own signing mechanism * @param {String} [options.unset] * @return {Function} middleware * @public @@ -131,6 +133,16 @@ function session(options) { throw new TypeError('unset option must be "destroy" or "keep"'); } + var signer = signature; + if (typeof opts.signature === 'object') { + if (typeof opts.signature.sign !== 'function' || + typeof opts.signature.unsign !== 'function') { + throw new TypeError('signature option object must have sign and unsign functions'); + } + + signer = options.signature; + } + // TODO: switch to "destroy" on next major var unsetDestroy = opts.unset === 'destroy' @@ -212,7 +224,7 @@ function session(options) { req.sessionStore = store; // get the session ID from the cookie - var cookieId = req.sessionID = getcookie(req, name, secrets); + var cookieId = req.sessionID = getcookie(req, name, secrets, signer); // set-cookie onHeaders(res, function(){ @@ -235,7 +247,7 @@ function session(options) { req.session.touch(); // set cookie - setcookie(res, name, req.sessionID, secrets[0], req.session.cookie.data); + setcookie(res, name, req.sessionID, secrets[0], req.session.cookie.data, signer); }); // proxy end() to commit the session @@ -483,7 +495,7 @@ function generateSessionId(sess) { * @private */ -function getcookie(req, name, secrets) { +function getcookie(req, name, secrets, signer) { var header = req.headers.cookie; var raw; var val; @@ -495,15 +507,11 @@ function getcookie(req, name, secrets) { raw = cookies[name]; if (raw) { - if (raw.substr(0, 2) === 's:') { - val = unsigncookie(raw.slice(2), secrets); + val = unsigncookie(raw, secrets, signer); - if (val === false) { - debug('cookie signature invalid'); - val = undefined; - } - } else { - debug('cookie unsigned') + if (val === false) { + debug('cookie signature missing or invalid'); + val = undefined; } } } @@ -522,19 +530,15 @@ function getcookie(req, name, secrets) { raw = req.cookies[name]; if (raw) { - if (raw.substr(0, 2) === 's:') { - val = unsigncookie(raw.slice(2), secrets); + val = unsigncookie(raw, secrets, signer); - if (val) { - deprecate('cookie should be available in req.headers.cookie'); - } + if (val) { + deprecate('cookie should be available in req.headers.cookie'); + } - if (val === false) { - debug('cookie signature invalid'); - val = undefined; - } - } else { - debug('cookie unsigned') + if (val === false) { + debug('cookie signature missing or invalid'); + val = undefined; } } } @@ -602,8 +606,8 @@ function issecure(req, trustProxy) { * @private */ -function setcookie(res, name, val, secret, options) { - var signed = 's:' + signature.sign(val, secret); +function setcookie(res, name, val, secret, options, signer) { + var signed = signer.sign(val, secret); var data = cookie.serialize(name, signed, options); debug('set-cookie %s', data); @@ -624,9 +628,9 @@ function setcookie(res, name, val, secret, options) { * @returns {String|Boolean} * @private */ -function unsigncookie(val, secrets) { +function unsigncookie(val, secrets, signer) { for (var i = 0; i < secrets.length; i++) { - var result = signature.unsign(val, secrets[i]); + var result = signer.unsign(val, secrets[i]); if (result !== false) { return result; diff --git a/test/session.js b/test/session.js index f9419d48..d50b266c 100644 --- a/test/session.js +++ b/test/session.js @@ -745,7 +745,7 @@ describe('session()', function(){ request(createServer({ genid: genid })) .get('/') - .expect(shouldSetCookieToValue('connect.sid', 's%3Aapple.D8Y%2BpkTAmeR0PobOhY4G97PRW%2Bj7bUnP%2F5m6%2FOn1MCU')) + .expect(shouldSetCookieToValue('connect.sid', 'apple.D8Y%2BpkTAmeR0PobOhY4G97PRW%2Bj7bUnP%2F5m6%2FOn1MCU')) .expect(200, done) }); @@ -754,7 +754,7 @@ describe('session()', function(){ request(createServer({ genid: genid })) .get('/') - .expect(shouldSetCookieToValue('connect.sid', 's%3A%25.kzQ6x52kKVdF35Qh62AWk4ZekS28K5XYCXKa%2FOTZ01g')) + .expect(shouldSetCookieToValue('connect.sid', '%25.kzQ6x52kKVdF35Qh62AWk4ZekS28K5XYCXKa%2FOTZ01g')) .expect(200, done) }); @@ -763,7 +763,7 @@ describe('session()', function(){ request(createServer({ genid: genid })) .get('/foo') - .expect(shouldSetCookieToValue('connect.sid', 's%3A%2Ffoo.paEKBtAHbV5s1IB8B2zPnzAgYmmnRPIqObW4VRYj%2FMQ')) + .expect(shouldSetCookieToValue('connect.sid', '%2Ffoo.paEKBtAHbV5s1IB8B2zPnzAgYmmnRPIqObW4VRYj%2FMQ')) .expect(200, done) }); }); @@ -2179,6 +2179,40 @@ describe('session()', function(){ }) }) + it('should use a custom signature object if passed in', function(done){ + var app = express() + .use(cookieParser()) + .use(function(req, res, next){ req.headers.cookie = 'foo=bar'; next() }) + .use(session({ + secret: 'keyboard cat', + key: 'sessid', + signature: { + sign: function(val, secret) { + return secret + val + secret; + }, + unsign: function(val, secret) { + return val.replace(/secret/g, '') + } + } + })) + .use(function(req, res, next){ + req.session.count = req.session.count || 0 + req.session.count++ + res.end(req.session.count.toString()) + }) + + request(app) + .get('/') + .expect(200, '1', function (err, res) { + if (err) return done(err) + var val = cookie(res).replace(/...\./, '.') + request(app) + .get('/') + .set('Cookie', val) + .expect(200, '1', done) + }) + }) + it('should read from req.signedCookies', function(done){ var app = express() .use(cookieParser('keyboard cat')) @@ -2301,7 +2335,7 @@ function shouldSetSecureCookie(name) { } function sid(res) { - var match = /^[^=]+=s%3A([^;\.]+)[\.;]/.exec(cookie(res)) + var match = /^[^=]+=([^;\.]+)[\.;]/.exec(cookie(res)) var val = match ? match[1] : undefined return val } From a955d257a419b32fabd8a6dd7be30e909af71148 Mon Sep 17 00:00:00 2001 From: roblabla Date: Tue, 16 Aug 2016 10:21:19 +0200 Subject: [PATCH 2/4] Fix documentation about the signature feature --- README.md | 2 +- index.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a5967baa..f31dfb2c 100644 --- a/README.md +++ b/README.md @@ -281,7 +281,7 @@ The default value is `'keep'`. Pass in your own cookie signing object/module here that implements the `sign(value, secret)` and `unsign(value, secret)` functions. -The default value is the `node-cookie-signature` module. +The default value is the [`cookie-signature`](https://github.com/tj/node-cookie-signature) module. ### req.session diff --git a/index.js b/index.js index 27cd95a9..32a2079f 100644 --- a/index.js +++ b/index.js @@ -78,8 +78,8 @@ var defer = typeof setImmediate === 'function' * @param {Boolean} [options.saveUninitialized] Save uninitialized sessions to the store * @param {String|Array} [options.secret] Secret for signing session ID * @param {Object} [options.store=MemoryStore] Session store - * @param {Object} [options.signature] Object that has the same API as node-cookie-signature - if you need to implement your own signing mechanism + * @param {Object} [options.signature=cookie-signature] Object that has two functions, + * sign(val, secret) and unsign(val, secret). Compatible with cookie-signature * @param {String} [options.unset] * @return {Function} middleware * @public From 3f880c86421480c03fa97cf10327aed03dd7d3c7 Mon Sep 17 00:00:00 2001 From: roblabla Date: Tue, 16 Aug 2016 10:25:02 +0200 Subject: [PATCH 3/4] Restore compatibility with previous versions --- index.js | 18 +++++++++++++++++- test/session.js | 8 ++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 32a2079f..119f86b7 100644 --- a/index.js +++ b/index.js @@ -20,7 +20,7 @@ var deprecate = require('depd')('express-session'); var parseUrl = require('parseurl'); var uid = require('uid-safe').sync , onHeaders = require('on-headers') - , signature = require('cookie-signature') + , cookiesignature = require('cookie-signature') var Session = require('./session/session') , MemoryStore = require('./session/memory') @@ -65,6 +65,22 @@ var defer = typeof setImmediate === 'function' ? setImmediate : function(fn){ process.nextTick(fn.bind.apply(fn, arguments)) } +/** + * Updated signature that prefixes s: to signed cookies. Done for compatibility + * with previous version of session + */ +var signature = { + sign: function(val, sig) { + return 's:' + cookiesignature.sign(val, sig); + }, + unsign: function(val, sig) { + if (val.substr(0, 2) === 's:') + return cookiesignature.unsign(val.slice(2), sig); + else + return false; + } +}; + /** * Setup session store with the given `options`. * diff --git a/test/session.js b/test/session.js index d50b266c..ffaad5c5 100644 --- a/test/session.js +++ b/test/session.js @@ -745,7 +745,7 @@ describe('session()', function(){ request(createServer({ genid: genid })) .get('/') - .expect(shouldSetCookieToValue('connect.sid', 'apple.D8Y%2BpkTAmeR0PobOhY4G97PRW%2Bj7bUnP%2F5m6%2FOn1MCU')) + .expect(shouldSetCookieToValue('connect.sid', 's%3Aapple.D8Y%2BpkTAmeR0PobOhY4G97PRW%2Bj7bUnP%2F5m6%2FOn1MCU')) .expect(200, done) }); @@ -754,7 +754,7 @@ describe('session()', function(){ request(createServer({ genid: genid })) .get('/') - .expect(shouldSetCookieToValue('connect.sid', '%25.kzQ6x52kKVdF35Qh62AWk4ZekS28K5XYCXKa%2FOTZ01g')) + .expect(shouldSetCookieToValue('connect.sid', 's%3A%25.kzQ6x52kKVdF35Qh62AWk4ZekS28K5XYCXKa%2FOTZ01g')) .expect(200, done) }); @@ -763,7 +763,7 @@ describe('session()', function(){ request(createServer({ genid: genid })) .get('/foo') - .expect(shouldSetCookieToValue('connect.sid', '%2Ffoo.paEKBtAHbV5s1IB8B2zPnzAgYmmnRPIqObW4VRYj%2FMQ')) + .expect(shouldSetCookieToValue('connect.sid', 's%3A%2Ffoo.paEKBtAHbV5s1IB8B2zPnzAgYmmnRPIqObW4VRYj%2FMQ')) .expect(200, done) }); }); @@ -2335,7 +2335,7 @@ function shouldSetSecureCookie(name) { } function sid(res) { - var match = /^[^=]+=([^;\.]+)[\.;]/.exec(cookie(res)) + var match = /^[^=]+=s%3A([^;\.]+)[\.;]/.exec(cookie(res)) var val = match ? match[1] : undefined return val } From c8223e95f98a6cf35260a1ae68552e24fa0e3de5 Mon Sep 17 00:00:00 2001 From: roblabla Date: Tue, 16 Aug 2016 10:36:21 +0200 Subject: [PATCH 4/4] 100% coverage --- test/session.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/session.js b/test/session.js index ffaad5c5..95a1ff78 100644 --- a/test/session.js +++ b/test/session.js @@ -2179,6 +2179,10 @@ describe('session()', function(){ }) }) + it('should reject invalid custom signature objects', function(){ + assert.throws(session.bind(null, { signature: { sign: 'bogus!' } }), /signature.*sign.*unsign/) + }); + it('should use a custom signature object if passed in', function(done){ var app = express() .use(cookieParser())