From f22214afbea0be084ce7815d74217d4c5f3ae655 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Mon, 6 Jul 2026 00:29:40 -0500 Subject: [PATCH 1/2] feat: expire session cookie on destruction --- HISTORY.md | 3 +++ README.md | 7 ++++-- index.js | 30 +++++++++++++++++++++++++ test/session.js | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 238498ec..4629a7d5 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,9 @@ unreleased ========== + * Expire the session cookie on the response when the session is destroyed + - Applies to `req.session.destroy()` and to `unset: 'destroy'`, when the request + came in with a session cookie * Replace `uid-safe` dependency with built-in `crypto.randomBytes` for session ID generation - Session IDs keep the same format as before (32-character base64url strings) diff --git a/README.md b/README.md index 1e30b86b..7ef4f81c 100644 --- a/README.md +++ b/README.md @@ -337,7 +337,8 @@ etc.). The default value is `'keep'`. - - `'destroy'` The session will be destroyed (deleted) when the response ends. + - `'destroy'` The session will be destroyed (deleted) when the response ends, + and the response will set an expired cookie to remove it from the client. - `'keep'` The session in the store will be kept, but modifications made during the request are ignored and not saved. @@ -381,7 +382,9 @@ req.session.regenerate(function(err) { #### Session.destroy(callback) Destroys the session and will unset the `req.session` property. -Once complete, the `callback` will be invoked. +Once complete, the `callback` will be invoked. If the request came +in with a session cookie, the response will set an expired cookie +to remove it from the client. ```js req.session.destroy(function(err) { diff --git a/index.js b/index.js index c61dce0a..50b32310 100644 --- a/index.js +++ b/index.js @@ -186,6 +186,7 @@ function session(options) { return } + var destroyed = false var originalHash; var originalId; var savedHash; @@ -200,6 +201,20 @@ function session(options) { // set-cookie onHeaders(res, function(){ if (!req.session) { + // expire the cookie when the session was destroyed + if (destroyed && cookieId) { + debug('expire cookie') + var expired = new Cookie(cookieOptions) + expired.expires = new Date(0) + + try { + setcookie(res, name, '', secrets[0], expired.data) + } catch (err) { + setImmediate(next, err) + } + return + } + debug('no session'); return; } @@ -294,6 +309,7 @@ function session(options) { if (shouldDestroy(req)) { // destroy session debug('destroying'); + destroyed = true store.destroy(req.sessionID, function ondestroy(err) { if (err) { setImmediate(next, err); @@ -379,9 +395,16 @@ function session(options) { // wrap session methods function wrapmethods(sess) { + var _destroy = sess.destroy var _reload = sess.reload var _save = sess.save; + function destroy() { + debug('destroying %s', this.id) + destroyed = true + return _destroy.apply(this, arguments) + } + function reload(callback) { debug('reloading %s', this.id) _reload.call(this, rewrapmethods(this, callback)) @@ -393,6 +416,13 @@ function session(options) { _save.apply(this, arguments); } + Object.defineProperty(sess, 'destroy', { + configurable: true, + enumerable: false, + value: destroy, + writable: true + }) + Object.defineProperty(sess, 'reload', { configurable: true, enumerable: false, diff --git a/test/session.js b/test/session.js index 1e851dcd..cea7825b 100644 --- a/test/session.js +++ b/test/session.js @@ -1351,6 +1351,7 @@ describe('session()', function(){ request(server) .get('/') .set('Cookie', cookie(res)) + .expect(shouldSetCookieToExpired('connect.sid')) .expect(200, function(err, res){ if (err) return done(err); store.length(function(err, len){ @@ -1363,6 +1364,27 @@ describe('session()', function(){ }); }); + it('should not expire cookie on req.session = null when set to keep', function(done){ + var store = new session.MemoryStore(); + var server = createServer({ store: store, unset: 'keep' }, function (req, res) { + req.session.count = req.session.count || 0 + req.session.count++ + if (req.session.count === 2) req.session = null + res.end() + }) + + request(server) + .get('/') + .expect(200, function(err, res){ + if (err) return done(err); + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(shouldNotHaveHeader('Set-Cookie')) + .expect(200, done) + }); + }); + it('should not set cookie if initial session destroyed', function(done){ var store = new session.MemoryStore(); var server = createServer({ store: store, unset: 'destroy' }, function (req, res) { @@ -1604,6 +1626,32 @@ describe('session()', function(){ .expect(shouldNotHaveHeader('Set-Cookie')) .expect(200, 'undefined', done) }) + + it('should expire the cookie of an existing session', function (done) { + var server = createServer(null, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + req.session.destroy(function (err) { + if (err) res.statusCode = 500 + res.end('destroyed') + }) + }) + + request(server) + .get('/') + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + request(server) + .get('/foo') + .set('Cookie', cookie(res)) + .expect(shouldSetCookieToExpired('connect.sid')) + .expect(200, 'destroyed', done) + }) + }) }) describe('.regenerate()', function(){ @@ -2459,6 +2507,17 @@ function shouldSetCookieToDifferentSessionId (id) { } } +function shouldSetCookieToExpired (name) { + return function (res) { + var header = cookie(res) + var data = header && utils.parseSetCookie(header) + assert.ok(header, 'should have a cookie header') + assert.strictEqual(data.name, name, 'should set cookie ' + name) + assert.ok(('expires' in data), 'should set cookie with attribute Expires') + assert.strictEqual(Date.parse(data.expires), 0, 'should set cookie ' + name + ' to expired') + } +} + function shouldSetCookieToExpireIn (name, delta) { return function (res) { var header = cookie(res) From 22237b6c3c69b6807362c63cf79219ca83455fe4 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Mon, 6 Jul 2026 00:37:16 -0500 Subject: [PATCH 2/2] feat: enhance cookie expiration logic for secure connections --- index.js | 13 ++++++- test/session.js | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 50b32310..15895243 100644 --- a/index.js +++ b/index.js @@ -203,10 +203,21 @@ function session(options) { if (!req.session) { // expire the cookie when the session was destroyed if (destroyed && cookieId) { - debug('expire cookie') var expired = new Cookie(cookieOptions) expired.expires = new Date(0) + if (expired.secure === 'auto') { + expired.secure = issecure(req, trustProxy) + } + + // only send secure cookies via https + if (expired.secure && !issecure(req, trustProxy)) { + debug('not secured, cannot expire cookie'); + return; + } + + debug('expire cookie') + try { setcookie(res, name, '', secrets[0], expired.data) } catch (err) { diff --git a/test/session.js b/test/session.js index cea7825b..10b70d83 100644 --- a/test/session.js +++ b/test/session.js @@ -1652,6 +1652,98 @@ describe('session()', function(){ .expect(200, 'destroyed', done) }) }) + + it('should not send expired secure cookie when insecure', function (done) { + function setup (req) { + req.secure = JSON.parse(req.headers['x-secure']) + } + + var server = createServer(setup, { cookie: { secure: true, maxAge: min } }, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + req.session.destroy(function (err) { + if (err) res.statusCode = 500 + res.end('destroyed') + }) + }) + + request(server) + .get('/') + .set('X-Secure', 'true') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + request(server) + .get('/foo') + .set('Cookie', cookie(res)) + .set('X-Secure', 'false') + .expect(shouldNotHaveHeader('Set-Cookie')) + .expect(200, 'destroyed', done) + }) + }) + + describe('when cookie "secure" set to "auto"', function () { + function setup (req) { + req.secure = JSON.parse(req.headers['x-secure']) + } + + function createDestroyServer () { + return createServer(setup, { cookie: { secure: 'auto', maxAge: min } }, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + req.session.destroy(function (err) { + if (err) res.statusCode = 500 + res.end('destroyed') + }) + }) + } + + it('should expire cookie with Secure when connection is secure', function (done) { + var server = createDestroyServer() + + request(server) + .get('/') + .set('X-Secure', 'true') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + request(server) + .get('/foo') + .set('Cookie', cookie(res)) + .set('X-Secure', 'true') + .expect(shouldSetCookieToExpired('connect.sid')) + .expect(shouldSetCookieWithAttribute('connect.sid', 'Secure')) + .expect(200, 'destroyed', done) + }) + }) + + it('should expire cookie without Secure when connection is insecure', function (done) { + var server = createDestroyServer() + + request(server) + .get('/') + .set('X-Secure', 'false') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + request(server) + .get('/foo') + .set('Cookie', cookie(res)) + .set('X-Secure', 'false') + .expect(shouldSetCookieToExpired('connect.sid')) + .expect(shouldSetCookieWithoutAttribute('connect.sid', 'Secure')) + .expect(200, 'destroyed', done) + }) + }) + }) }) describe('.regenerate()', function(){