Expire cookie immediately upon destroying session

Fixes #241.

Author: JamesMGreene

Diff for: index.js

@@ -139,11 +139,7 @@ function session(options){
   store.generate = function(req){
     req.sessionID = generateId(req);
     req.session = new Session(req);
-    req.session.cookie = new Cookie(cookieOptions);
-
-    if (cookieOptions.secure === 'auto') {
-      req.session.cookie.secure = issecure(req, trustProxy);
-    }
+    req.session.cookie = createCookie(cookieOptions, req, trustProxy);
   };
 
   var storeImplementsTouch = typeof store.touch === 'function';
@@ -184,20 +180,35 @@ function session(options){
 
     // set-cookie
     onHeaders(res, function(){
+      // Is this an existing session that is being destroyed?
+      var isBeingDestroyed = !!cookieId && shouldDestroy(req);
+
       if (!req.session) {
         debug('no session');
-        return;
+        if (!isBeingDestroyed) {
+          return;
+        }
       }
 
-      var cookie = req.session.cookie;
+      var cookie = req.session ? req.session.cookie : undefined;
+
+      if (isBeingDestroyed) {
+        if (cookie == null) {
+          debug('creating expired cookie');
+          cookie = createCookie(cookieOptions, req, trustProxy);
+        }
+
+        // Set the cookie to immediately expire on the client
+        cookie.maxAge = 0;
+      }
 
       // only send secure cookies via https
       if (cookie.secure && !issecure(req, trustProxy)) {
         debug('not secured');
         return;
       }
 
-      if (!shouldSetCookie(req)) {
+      if (!isBeingDestroyed && !shouldSetCookie(req)) {
         return;
       }
 
@@ -570,6 +581,25 @@ function issecure(req, trustProxy) {
   return proto === 'https';
 }
 
+/**
+ * Create a new Cookie for this request.
+ *
+ * @param {Object} cookieOptions
+ * @param {Object} req
+ * @param {Boolean} [trustProxy]
+ * @return {Cookie}
+ * @private
+ */
+function createCookie(cookieOptions, req, trustProxy) {
+  cookieOptions = cookieOptions || {};
+
+  var cookie = new Cookie(cookieOptions);
+  if (cookieOptions.secure === 'auto') {
+    cookie.secure = issecure(req, trustProxy);
+  }
+  return cookie;
+}
+
 /**
  * Set cookie on response.
  *

Diff for: test/session.js

@@ -1210,6 +1210,165 @@ describe('session()', function(){
       });
     });
 
+    it('should set expired cookie to override persistent cookie on req.session = null', function(done){
+      var store = new session.MemoryStore();
+      var app = express()
+        .use(session({ store: store, unset: 'destroy', secret: 'keyboard cat', cookie: { maxAge: min } }))
+        .use(function(req, res, next){
+          req.session.count = req.session.count || 0;
+          req.session.count++;
+          if (req.session.count === 2) req.session = null;
+          res.end();
+        });
+
+      request(app)
+      .get('/')
+      .expect(shouldSetCookie('connect.sid'))
+      .expect(200, function(err, res){
+        if (err) return done(err);
+
+        var now = new Date();
+
+        var expiry = expires(res);
+        assert.ok(expiry);
+
+        var a = new Date(expiry);
+
+        var delta = a.valueOf() - now.valueOf();
+        assert.ok(delta > (min - 1000) && delta <= min);
+
+        request(app)
+        .get('/')
+        .set('Cookie', cookie(res))
+        .expect(shouldSetCookie('connect.sid'))
+        .expect(200, function(err, res){
+          if (err) return done(err);
+
+          now = new Date();
+
+          expiry = expires(res);
+          assert.ok(expiry);
+
+          var b = new Date(expiry);
+
+          delta = b.valueOf() - a.valueOf();
+          assert.ok(delta <= 0);
+
+          delta = b.valueOf() - now.valueOf();
+          assert.ok(delta <= 0);
+
+          done();
+        });
+      });
+    });
+
+    it('should set expired cookie to override session cookie on req.session = null', function(done){
+      var store = new session.MemoryStore();
+      var app = express()
+        .use(session({ store: store, unset: 'destroy', secret: 'keyboard cat' }))
+        .use(function(req, res, next){
+          req.session.count = req.session.count || 0;
+          req.session.count++;
+          if (req.session.count === 2) req.session = null;
+          res.end();
+        });
+
+      request(app)
+      .get('/')
+      .expect(shouldSetCookie('connect.sid'))
+      .expect(200, function(err, res){
+        if (err) return done(err);
+
+        assert.ok(!expires(res));
+
+        request(app)
+        .get('/')
+        .set('Cookie', cookie(res))
+        .expect(shouldSetCookie('connect.sid'))
+        .expect(200, function(err, res){
+          if (err) return done(err);
+
+          var now = new Date();
+
+          var expiry = expires(res);
+          assert.ok(expiry);
+
+          var b = new Date(expiry);
+
+          var delta = b.valueOf() - now.valueOf();
+          assert.ok(delta <= 0);
+
+          done();
+        });
+      });
+    });
+
+    it('should not set an expired cookie on destroyed session if session is regenerated', function(done){
+      var store = new session.MemoryStore();
+      var app = express()
+        .use(session({ store: store, unset: 'destroy', secret: 'keyboard cat', cookie: { maxAge: min } }))
+        .use(function(req, res, next){
+          req.session.count = req.session.count || 0;
+          req.session.count++;
+          if (req.session.count === 2) {
+            req.session.destroy(function(err) {
+              if (err) return done(err);
+
+              req.sessionStore.regenerate(req, function(err) {
+                if (err) return done(err);
+
+                req.session.now = Date.now();
+                res.end();
+              });
+            });
+          }
+          else {
+            res.end();
+          }
+        });
+
+      request(app)
+      .get('/')
+      .expect(shouldSetCookie('connect.sid'))
+      .expect(200, function(err, res){
+        if (err) return done(err);
+
+        var now = new Date();
+
+        var expiry = expires(res);
+        assert.ok(expiry);
+
+        var a = new Date(expiry);
+
+        var delta = a.valueOf() - now.valueOf();
+        assert.ok(delta > (min - 1000) && delta <= min);
+
+        request(app)
+        .get('/')
+        .set('Cookie', cookie(res))
+        .expect(shouldSetCookie('connect.sid'))
+        .expect(200, function(err, res){
+          if (err) return done(err);
+
+          now = new Date();
+
+          expiry = expires(res);
+          assert.ok(expiry);
+
+          var b = new Date(expiry);
+
+          // The session regenerated so the expiration date should be the same as the original
+          delta = b.valueOf() - a.valueOf();
+          assert.equal(delta, 0);
+
+          delta = b.valueOf() - now.valueOf();
+          assert.ok(delta > 0);
+
+          done();
+        });
+      });
+    });
+
     it('should not set cookie if initial session destroyed', function(done){
       var store = new session.MemoryStore();
       var app = express()