From 1a4afa24b7835c2c5a91180277a2cbd07894b296 Mon Sep 17 00:00:00 2001 From: Paul McClean Date: Fri, 27 Jan 2017 15:35:53 +0000 Subject: [PATCH] feat: Option to allow regenerate() to preserve old session Add preserve option to session.regenerate() function for use cases when it is desirable to regenerate the session, but not destroy the old session immediately. For example when there could be multiple ajax requests inflight with the old session id. Existing functionality is preserved if the option is omitted. Existing test was improved to cover session destuction and new test was added for the preserve case. --- README.md | 22 ++++++++++++++--- session/session.js | 4 +-- session/store.js | 29 ++++++++++++++++++---- test/session.js | 61 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 105 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 2be21d2c..e4054921 100644 --- a/README.md +++ b/README.md @@ -304,15 +304,31 @@ app.get('/', function(req, res, next) { }) ``` -#### Session.regenerate(callback) +#### Session.regenerate([options], callback) -To regenerate the session simply invoke the method. Once complete, +Regenerates the session for the current request. Once complete, a new SID and `Session` instance will be initialized at `req.session` and the `callback` will be invoked. +#### Options + +If the options argument is omitted, the options will take their default values. +The following options are supported: + +##### destroy + +Specifies whether the existing session should be destroyed or not. Defaults +to `true`, and the existing session will be destroyed. If this is not desirable, +set to `false` to preserve the existing session. + ```js req.session.regenerate(function(err) { - // will have a new session here + // existing session is destroyed and will have a new session here +}) +``` +```js +req.session.regenerate({ destroy: false }, function(err) { + // existing session is preserved but will have a new session here }) ``` diff --git a/session/session.js b/session/session.js index fee7608c..c4b7068c 100644 --- a/session/session.js +++ b/session/session.js @@ -120,8 +120,8 @@ defineMethod(Session.prototype, 'destroy', function destroy(fn) { * @api public */ -defineMethod(Session.prototype, 'regenerate', function regenerate(fn) { - this.req.sessionStore.regenerate(this.req, fn); +defineMethod(Session.prototype, 'regenerate', function regenerate(options, fn) { + this.req.sessionStore.regenerate(this.req, options, fn); return this; }); diff --git a/session/store.js b/session/store.js index 00f3ac41..215c2c3e 100644 --- a/session/store.js +++ b/session/store.js @@ -47,12 +47,31 @@ util.inherits(Store, EventEmitter) * @api public */ -Store.prototype.regenerate = function(req, fn){ +Store.prototype.regenerate = function regenerate (req, options, fn) { + var cb = fn + var opts = options || {} var self = this; - this.destroy(req.sessionID, function(err){ - self.generate(req); - fn(err); - }); + + if (typeof options === 'function') { + cb = options + opts = {} + } + + var destroy = opts.destroy !== undefined + ? opts.destroy + : true + + if (destroy) { + this.destroy(req.sessionID, function (err) { + self.generate(req) + cb(err) + }) + } else { + process.nextTick(function () { + self.generate(req) + cb(null) + }) + } }; /** diff --git a/test/session.js b/test/session.js index 6862ccc4..ee81b3b3 100644 --- a/test/session.js +++ b/test/session.js @@ -1558,7 +1558,8 @@ describe('session()', function(){ describe('.regenerate()', function(){ it('should destroy/replace the previous session', function(done){ - var server = createServer(null, function (req, res) { + var store = new session.MemoryStore() + var server = createServer({store: store}, function (req, res) { var id = req.session.id req.session.regenerate(function (err) { if (err) res.statusCode = 500 @@ -1574,11 +1575,41 @@ describe('session()', function(){ request(server) .get('/') .set('Cookie', cookie(res)) + .expect(shouldDestroySessionInStore(store)) .expect(shouldSetCookie('connect.sid')) .expect(shouldSetCookieToDifferentSessionId(sid(res))) .expect(200, 'false', done) }); }) + + it('should replace but not destroy the previous session when preserve flag is set', function(done){ + var store = new session.MemoryStore() + var server = createServer({store: store}, function (req, res) { + var id = req.session.id + req.session.regenerate({destroy: false}, function (err) { + if (err) res.statusCode = 500 + res.end(String(req.session.id === id)) + }) + }) + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, function (err, res) { + if (err) return done(err) + var id = sid(res) + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(shouldSetCookie('connect.sid')) + .expect(shouldNotDestroySessionInStore(store)) + .expect(200, 'false', function (err, res) { + if (err) return done(err) + assert.notEqual(sid(res), id) + done(); + }); + }); + }) }) describe('.reload()', function () { @@ -2216,6 +2247,34 @@ function parseSetCookie (header) { return cookie } +function shouldDestroySessionInStore(store) { + var _destroy = store.destroy + var count = 0 + + store.destroy = function destroy () { + count++ + return _destroy.apply(this, arguments) + } + + return function () { + assert.ok(count === 1, 'should destroy session in store') + } +} + +function shouldNotDestroySessionInStore(store) { + var _destroy = store.destroy + var count = 0 + + store.destroy = function destroy () { + count++ + return _destroy.apply(this, arguments) + } + + return function () { + assert.ok(count === 0, 'should not destroy session in store') + } +} + function shouldNotHaveHeader(header) { return function (res) { assert.ok(!(header.toLowerCase() in res.headers), 'should not have ' + header + ' header')