Skip to content

Commit ad8c8b8

Browse files
JamesMGreeneJamesMGreene
committed
Expire cookie immediately upon destroying session
Fixes #241.
1 parent 50cdae2 commit ad8c8b8

File tree

2 files changed

+314
-16
lines changed

2 files changed

+314
-16
lines changed

Diff for: ‎index.js

+47-12
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,7 @@ function session(options){
142142
store.generate = function(req){
143143
req.sessionID = generateId(req);
144144
req.session = new Session(req);
145-
req.session.cookie = new Cookie(cookieOptions);
146-
147-
if (cookieOptions.secure === 'auto') {
148-
req.session.cookie.secure = issecure(req, trustProxy);
149-
}
145+
req.session.cookie = createCookie(cookieOptions, req, trustProxy);
150146
};
151147

152148
var storeImplementsTouch = typeof store.touch === 'function';
@@ -187,26 +183,46 @@ function session(options){
187183

188184
// set-cookie
189185
onHeaders(res, function(){
186+
// Is this an existing session that is being destroyed?
187+
var isBeingDestroyed = !!cookieId && shouldDestroy(req);
188+
190189
if (!req.session) {
191190
debug('no session');
192-
return;
191+
if (!isBeingDestroyed) {
192+
return;
193+
}
193194
}
194195

195-
if (!shouldSetCookie(req)) {
196-
return;
196+
var cookie = req.session ? req.session.cookie : undefined;
197+
198+
if (isBeingDestroyed) {
199+
if (cookie == null) {
200+
debug('creating expired cookie');
201+
cookie = createCookie(cookieOptions, req, trustProxy);
202+
}
203+
204+
// Set the cookie to immediately expire on the client
205+
cookie.maxAge = 0;
197206
}
198207

199208
// only send secure cookies via https
200-
if (req.session.cookie.secure && !issecure(req, trustProxy)) {
209+
if (cookie.secure && !issecure(req, trustProxy)) {
201210
debug('not secured');
202211
return;
203212
}
204213

205-
// touch session
206-
req.session.touch();
214+
if (!isBeingDestroyed) {
215+
if (!shouldSetCookie(req)) {
216+
return;
217+
}
218+
else {
219+
// touch session
220+
req.session.touch();
221+
}
222+
}
207223

208224
// set cookie
209-
setcookie(res, name, req.sessionID, secrets[0], req.session.cookie.data);
225+
setcookie(res, name, req.sessionID, secrets[0], cookie.data);
210226
});
211227

212228
// proxy end() to commit the session
@@ -436,6 +452,25 @@ function session(options){
436452
};
437453
};
438454

455+
/**
456+
* Create a new Cookie for this request.
457+
*
458+
* @param {Object} cookieOptions
459+
* @param {Object} req
460+
* @param {Boolean} [trustProxy]
461+
* @return {Cookie}
462+
* @private
463+
*/
464+
function createCookie(cookieOptions, req, trustProxy) {
465+
var cookieOpts = cookieOptions || {};
466+
467+
var cookie = new Cookie(cookieOpts);
468+
if (cookieOpts.secure === 'auto') {
469+
cookie.secure = issecure(req, trustProxy);
470+
}
471+
return cookie;
472+
}
473+
439474
/**
440475
* Generate a session ID for a new session.
441476
*

Diff for: ‎test/session.js

+267-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
process.env.NO_DEPRECATION = 'express-session';
32

43
var after = require('after')
@@ -1301,6 +1300,165 @@ describe('session()', function(){
13011300
});
13021301
});
13031302

1303+
it('should set expired cookie to override persistent cookie on req.session = null', function(done){
1304+
var store = new session.MemoryStore();
1305+
var app = express()
1306+
.use(session({ store: store, unset: 'destroy', secret: 'keyboard cat', cookie: { maxAge: min } }))
1307+
.use(function(req, res, next){
1308+
req.session.count = req.session.count || 0;
1309+
req.session.count++;
1310+
if (req.session.count === 2) req.session = null;
1311+
res.end();
1312+
});
1313+
1314+
request(app)
1315+
.get('/')
1316+
.expect(shouldSetCookie('connect.sid'))
1317+
.expect(200, function(err, res){
1318+
if (err) return done(err);
1319+
1320+
var now = new Date();
1321+
1322+
var expiry = expires(res);
1323+
assert.ok(expiry);
1324+
1325+
var a = new Date(expiry);
1326+
1327+
var delta = a.valueOf() - now.valueOf();
1328+
assert.ok(delta > (min - 1000) && delta <= min);
1329+
1330+
request(app)
1331+
.get('/')
1332+
.set('Cookie', cookie(res))
1333+
.expect(shouldSetCookie('connect.sid'))
1334+
.expect(200, function(err, res){
1335+
if (err) return done(err);
1336+
1337+
now = new Date();
1338+
1339+
expiry = expires(res);
1340+
assert.ok(expiry);
1341+
1342+
var b = new Date(expiry);
1343+
1344+
delta = b.valueOf() - a.valueOf();
1345+
assert.ok(delta <= 0);
1346+
1347+
delta = b.valueOf() - now.valueOf();
1348+
assert.ok(delta <= 0);
1349+
1350+
done();
1351+
});
1352+
});
1353+
});
1354+
1355+
it('should set expired cookie to override session cookie on req.session = null', function(done){
1356+
var store = new session.MemoryStore();
1357+
var app = express()
1358+
.use(session({ store: store, unset: 'destroy', secret: 'keyboard cat' }))
1359+
.use(function(req, res, next){
1360+
req.session.count = req.session.count || 0;
1361+
req.session.count++;
1362+
if (req.session.count === 2) req.session = null;
1363+
res.end();
1364+
});
1365+
1366+
request(app)
1367+
.get('/')
1368+
.expect(shouldSetCookie('connect.sid'))
1369+
.expect(200, function(err, res){
1370+
if (err) return done(err);
1371+
1372+
assert.ok(!expires(res));
1373+
1374+
request(app)
1375+
.get('/')
1376+
.set('Cookie', cookie(res))
1377+
.expect(shouldSetCookie('connect.sid'))
1378+
.expect(200, function(err, res){
1379+
if (err) return done(err);
1380+
1381+
var now = new Date();
1382+
1383+
var expiry = expires(res);
1384+
assert.ok(expiry);
1385+
1386+
var b = new Date(expiry);
1387+
1388+
var delta = b.valueOf() - now.valueOf();
1389+
assert.ok(delta <= 0);
1390+
1391+
done();
1392+
});
1393+
});
1394+
});
1395+
1396+
it('should not set an expired cookie on destroyed session if session is regenerated', function(done){
1397+
var store = new session.MemoryStore();
1398+
var app = express()
1399+
.use(session({ store: store, unset: 'destroy', secret: 'keyboard cat', cookie: { maxAge: min } }))
1400+
.use(function(req, res, next){
1401+
req.session.count = req.session.count || 0;
1402+
req.session.count++;
1403+
if (req.session.count === 2) {
1404+
req.session.destroy(function(err) {
1405+
if (err) return done(err);
1406+
1407+
req.sessionStore.regenerate(req, function(err) {
1408+
if (err) return done(err);
1409+
1410+
req.session.now = Date.now();
1411+
res.end();
1412+
});
1413+
});
1414+
}
1415+
else {
1416+
res.end();
1417+
}
1418+
});
1419+
1420+
request(app)
1421+
.get('/')
1422+
.expect(shouldSetCookie('connect.sid'))
1423+
.expect(200, function(err, res){
1424+
if (err) return done(err);
1425+
1426+
var now = new Date();
1427+
1428+
var expiry = expires(res);
1429+
assert.ok(expiry);
1430+
1431+
var a = new Date(expiry);
1432+
1433+
var delta = a.valueOf() - now.valueOf();
1434+
assert.ok(delta > (min - 1000) && delta <= min);
1435+
1436+
request(app)
1437+
.get('/')
1438+
.set('Cookie', cookie(res))
1439+
.expect(shouldSetCookie('connect.sid'))
1440+
.expect(200, function(err, res){
1441+
if (err) return done(err);
1442+
1443+
now = new Date();
1444+
1445+
expiry = expires(res);
1446+
assert.ok(expiry);
1447+
1448+
var b = new Date(expiry);
1449+
1450+
// The session regenerated so the expiration date should be the same as the original
1451+
delta = b.valueOf() - a.valueOf();
1452+
assert.equal(delta, 0);
1453+
1454+
delta = b.valueOf() - now.valueOf();
1455+
assert.ok(delta > 0);
1456+
1457+
done();
1458+
});
1459+
});
1460+
});
1461+
13041462
it('should not set cookie if initial session destroyed', function(done){
13051463
var store = new session.MemoryStore();
13061464
var app = express()
@@ -1481,9 +1639,114 @@ describe('session()', function(){
14811639
request(app)
14821640
.get('/')
14831641
.expect(shouldNotHaveHeader('Set-Cookie'))
1484-
.expect(200, done)
1485-
})
1486-
})
1642+
.expect(200, done);
1643+
});
1644+
1645+
it('should set expired cookie to override persistent cookie', function(done) {
1646+
var store = new session.MemoryStore();
1647+
var app = express()
1648+
.use(session({ store: store, secret: 'keyboard cat', cookie: { maxAge: min } }))
1649+
.use(function(req, res, next){
1650+
req.session.count = req.session.count || 0;
1651+
req.session.count++;
1652+
if (req.session.count === 2) {
1653+
req.session.destroy(function(err) {
1654+
if (err) return next(err);
1655+
res.end();
1656+
});
1657+
} else {
1658+
res.end();
1659+
}
1660+
});
1661+
1662+
request(app)
1663+
.get('/')
1664+
.expect(shouldSetCookie('connect.sid'))
1665+
.expect(200, function(err, res){
1666+
if (err) return done(err);
1667+
1668+
var now = new Date();
1669+
1670+
var expiry = expires(res);
1671+
assert.ok(expiry);
1672+
1673+
var a = new Date(expiry);
1674+
1675+
var delta = a.valueOf() - now.valueOf();
1676+
assert.ok(delta > (min - 1000) && delta <= min);
1677+
1678+
request(app)
1679+
.get('/')
1680+
.set('Cookie', cookie(res))
1681+
.expect(shouldSetCookie('connect.sid'))
1682+
.expect(200, function(err, res){
1683+
if (err) return done(err);
1684+
1685+
now = new Date();
1686+
1687+
expiry = expires(res);
1688+
assert.ok(expiry);
1689+
1690+
var b = new Date(expiry);
1691+
1692+
delta = b.valueOf() - a.valueOf();
1693+
assert.ok(delta <= 0);
1694+
1695+
delta = b.valueOf() - now.valueOf();
1696+
assert.ok(delta <= 0);
1697+
1698+
done();
1699+
});
1700+
});
1701+
});
1702+
1703+
it('should set expired cookie to override session cookie', function(done) {
1704+
var store = new session.MemoryStore();
1705+
var app = express()
1706+
.use(session({ store: store, secret: 'keyboard cat' }))
1707+
.use(function(req, res, next){
1708+
req.session.count = req.session.count || 0;
1709+
req.session.count++;
1710+
if (req.session.count === 2) {
1711+
req.session.destroy(function(err) {
1712+
if (err) return next(err);
1713+
res.end();
1714+
});
1715+
} else {
1716+
res.end();
1717+
}
1718+
});
1719+
1720+
request(app)
1721+
.get('/')
1722+
.expect(shouldSetCookie('connect.sid'))
1723+
.expect(200, function(err, res){
1724+
if (err) return done(err);
1725+
1726+
assert.ok(!expires(res));
1727+
1728+
request(app)
1729+
.get('/')
1730+
.set('Cookie', cookie(res))
1731+
.expect(shouldSetCookie('connect.sid'))
1732+
.expect(200, function(err, res){
1733+
if (err) return done(err);
1734+
1735+
var now = new Date();
1736+
1737+
var expiry = expires(res);
1738+
assert.ok(expiry);
1739+
1740+
var b = new Date(expiry);
1741+
1742+
var delta = b.valueOf() - now.valueOf();
1743+
assert.ok(delta <= 0);
1744+
1745+
done();
1746+
});
1747+
});
1748+
});
1749+
});
14871750

14881751
describe('.regenerate()', function(){
14891752
it('should destroy/replace the previous session', function(done){

0 commit comments

Comments
 (0)