From 9fde9a011950ea6819ff52c1bb47954172295ff6 Mon Sep 17 00:00:00 2001
From: David Pate <davidtpate@gmail.com>
Date: Mon, 12 Oct 2015 23:53:46 -0400
Subject: [PATCH 1/3] Add the ability to specify a secret as a function which
 is executed with `req` so that the secret can be generated dynamically if
 desired. Added tests around the functionality and updated the documentation
 to reflect it.

---
 README.md       |  19 ++++-
 index.js        |  29 +++++--
 test/session.js | 209 ++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 244 insertions(+), 13 deletions(-)

diff --git a/README.md b/README.md
index abcd337b..84c71e6f 100644
--- a/README.md
+++ b/README.md
@@ -135,9 +135,24 @@ it to be saved.
 **Required option**
 
 This is the secret used to sign the session ID cookie. This can be either a string
-for a single secret, or an array of multiple secrets. If an array of secrets is
+for a single secret, an array of multiple secrets, or a function. If an array of secrets is
 provided, only the first element will be used to sign the session ID cookie, while
-all the elements will be considered when verifying the signature in requests.
+all the elements will be considered when verifying the signature in requests. If a function
+is provided then it will be executed with `req` as the first parameter for each request.
+The function should return a string or array of strings to be used as the secret for 
+signing the cookie.
+
+```js
+app.use(function(req, res, next) {
+  var subdomain = subdomain[0];
+  req.clientSecret = getClientKey(subdomain);
+});
+app.use(session({
+  secret: function (req) { 
+    return 'Danger Zone!' + req.clientSecret;
+  }
+}))
+```
 
 ##### store
 
diff --git a/index.js b/index.js
index 5e12c9fc..ca2aef40 100644
--- a/index.js
+++ b/index.js
@@ -117,14 +117,20 @@ function session(options){
   // TODO: switch to "destroy" on next major
   var unsetDestroy = options.unset === 'destroy';
 
-  if (Array.isArray(secret) && secret.length === 0) {
+  if (Array.isArray(secret)) {
+    if (secret.length === 0) {
+      throw new TypeError('secret option array must contain one or more strings');
+    }
+    // Make sure that we have a string for each item in the array.
+    for (var i = 0; i < secret.length; i++) {
+      if (typeof secret[i] !== 'string') {
+        throw new TypeError('secret option array must only contain strings');
+      }
+    }
+  } else if(typeof secret !== 'function' && (secret && typeof secret !== 'string')) {
     throw new TypeError('secret option array must contain one or more strings');
   }
 
-  if (secret && !Array.isArray(secret)) {
-    secret = [secret];
-  }
-
   if (!secret) {
     deprecate('req.secret; provide secret option');
   }
@@ -166,7 +172,18 @@ function session(options){
 
     // backwards compatibility for signed cookies
     // req.secret is passed from the cookie parser middleware
-    var secrets = secret || [req.secret];
+    var secrets = typeof secret === 'function' ? secret(req) : (secret || [req.secret]);
+
+    if (!Array.isArray(secrets)) {
+      secrets = [secrets];
+    }
+
+    for (var i = 0; i < secrets.length; i++) {
+      if (typeof secrets[i] !== 'string') {
+        next(new Error('secret must be a string'));
+        return;
+      }
+    }
 
     var originalHash;
     var originalId;
diff --git a/test/session.js b/test/session.js
index f592ef3f..a436ef67 100644
--- a/test/session.js
+++ b/test/session.js
@@ -1011,11 +1011,27 @@ describe('session()', function(){
   });
 
   describe('secret option', function () {
+    it('shouldn\'t reject string', function () {
+      assert.doesNotThrow(createServer.bind(null, { secret: 'keyboard cat' }));
+    });
+
     it('should reject empty arrays', function () {
-      assert.throws(createServer.bind(null, { secret: [] }), /secret option array/);
-    })
+      assert.throws(createServer.bind(null, { secret: [] }), /secret option array must contain one or more strings/);
+    });
+
+    it('should reject object secret', function () {
+      assert.throws(createServer.bind(null, { secret: {} }), /secret option array must contain one or more strings/);
+    });
 
     describe('when an array', function () {
+      it('should reject array with function', function () {
+        assert.throws(createServer.bind(null, { secret: ['keyboard cat', function() {} ] }), /secret option array must only contain strings/);
+      });
+
+      it('should reject array with object', function () {
+        assert.throws(createServer.bind(null, { secret: ['keyboard cat', {} ] }), /secret option array must only contain strings/);
+      });
+
       it('should sign cookies', function (done) {
         var server = createServer({ secret: ['keyboard cat', 'nyan cat'] }, function (req, res) {
           req.session.user = 'bob';
@@ -1026,7 +1042,7 @@ describe('session()', function(){
         .get('/')
         .expect(shouldSetCookie('connect.sid'))
         .expect(200, 'bob', done);
-      })
+      });
 
       it('should sign cookies with first element', function (done) {
         var store = new session.MemoryStore();
@@ -1075,8 +1091,178 @@ describe('session()', function(){
           .expect(200, 'bob', done);
         });
       });
-    })
-  })
+    });
+
+    describe('when a function', function () {
+      it('should sign cookie with secret from function', function (done) {
+        var server = createServer({ secret: function(req) {
+          return 'Danger Zone!' + subdomains(hostname(req));
+        }}, function (req, res) {
+          req.session.user = 'bob';
+          res.end(req.session.user);
+        });
+
+        request(server)
+        .get('/')
+        .set('Host', 'test1.doamin.com')
+        .expect(shouldSetCookie('connect.sid'))
+        .expect(200, 'bob', done);
+      });
+
+      it('should load session from cookie sid', function (done) {
+        var count = 0;
+        var server = createServer({ secret: function(req) {
+          return 'Danger Zone!' + subdomains(hostname(req));
+        }}, function (req, res) {
+          req.session.num = req.session.num || ++count;
+          res.end('session ' + req.session.num)
+        });
+
+        request(server)
+            .get('/')
+            .set('Host', 'test1.doamin.com')
+            .expect(shouldSetCookie('connect.sid'))
+            .expect(200, 'session 1', function (err, res) {
+              if (err) return done(err);
+              request(server)
+                  .get('/')
+                  .set('Host', 'test1.doamin.com')
+                  .set('Cookie', cookie(res))
+                  .expect(200, 'session 1', done)
+            })
+      });
+
+      it('should not load session from cookie sid when secret is different', function (done) {
+        var count = 0;
+        var server = createServer({ secret: function(req) {
+          return 'Danger Zone!' + subdomains(hostname(req));
+        }}, function (req, res) {
+          req.session.num = req.session.num || ++count;
+          res.end('session ' + req.session.num)
+        });
+
+        request(server)
+            .get('/')
+            .set('Host', 'test1.doamin.com')
+            .expect(shouldSetCookie('connect.sid'))
+            .expect(200, 'session 1', function (err, res) {
+              if (err) return done(err);
+              request(server)
+                  .get('/')
+                  .set('Host', 'test2.doamin.com')
+                  .set('Cookie', cookie(res))
+                  .expect(200, 'session 2', done)
+            })
+      });
+
+      it('should sign cookie with secret from function', function (done) {
+        var server = createServer({ secret: function(req) {
+          return 'Danger Zone!' + subdomains(hostname(req));
+        }}, function (req, res) {
+          req.session.user = 'bob';
+          res.end(req.session.user);
+        });
+
+        request(server)
+        .get('/')
+        .set('Host', 'test1.doamin.com')
+        .expect(shouldSetCookie('connect.sid'))
+        .expect(200, 'bob', done);
+      });
+
+      it('should sign cookies with different session ids', function (done) {
+        var server = createServer({ secret: function(req) {
+          return 'Danger Zone!' + subdomains(hostname(req));
+        } }, function (req, res) {
+          req.session.user = 'bob';
+          res.end(req.session.user);
+        });
+
+        request(server)
+        .get('/test1')
+        .set('Host', 'test1.doamin.com')
+        .expect(shouldSetCookie('connect.sid'))
+        .expect(200, 'bob', function (err, test1Res) {
+              if (err) return done(err);
+              request(server)
+                  .get('/test2')
+                  .set('Host', 'test2.doamin.com')
+                  .set('Cookie', cookie(test1Res))
+                  .expect(shouldSetCookie('connect.sid'))
+                  .expect(200, 'bob', function (err, test2Res) {
+                    if (err) return done(err);
+                    var test1Sid = sid(test1Res);
+                    var test2Sid = sid(test2Res);
+                    assert.ok(test1Sid !== test2Sid, 'session ids should not be equal for different secrets');
+                    done();
+                  });
+            });
+      });
+
+      it('shouldn\'t sign when function doesn\'t return string', function (done) {
+        var server = createServer({ secret: function(req) {
+          return {};
+        }}, function (req, res) {
+          req.session.user = 'bob';
+          res.end(req.session.user);
+        });
+
+        request(server)
+            .get('/')
+            .set('Host', 'test1.doamin.com')
+            .expect(shouldNotHaveHeader('cookie'))
+            .expect(500, /secret must be a string/, done);
+      });
+
+      it('should sign cookies with first element', function (done) {
+        var store = new session.MemoryStore();
+
+        var server1 = createServer({ secret: function(req) { return [ 'Danger Zone!' + subdomains(hostname(req)), 'keyboard cat' ] }, store: store }, function (req, res) {
+          req.session.user = 'bob';
+          res.end(req.session.user);
+        });
+
+        var server2 = createServer({ secret: 'keyboard cat', store: store }, function (req, res) {
+          res.end(String(req.session.user));
+        });
+
+        request(server1)
+            .get('/')
+            .expect(shouldSetCookie('connect.sid'))
+            .expect(200, 'bob', function (err, res) {
+              if (err) return done(err);
+              request(server2)
+                  .get('/')
+                  .set('Cookie', cookie(res))
+                  .expect(200, 'undefined', done);
+            });
+      });
+
+      it('should read cookies using all elements', function (done) {
+        var store = new session.MemoryStore();
+
+        var server1 = createServer({ secret: 'nyan cat', store: store }, function (req, res) {
+          req.session.user = 'bob';
+          res.end(req.session.user);
+        });
+
+        var server2 = createServer({ secret: function(req) { return [ 'Danger Zone!' + subdomains(hostname(req)), 'nyan cat' ] }, store: store }, function (req, res) {
+          res.end(String(req.session.user));
+        });
+
+        request(server1)
+            .get('/')
+            .expect(shouldSetCookie('connect.sid'))
+            .expect(200, 'bob', function (err, res) {
+              if (err) return done(err);
+              request(server2)
+                  .get('/')
+                  .set('Cookie', cookie(res))
+                  .expect(200, 'bob', done);
+            });
+      });
+    });
+  });
 
   describe('unset option', function () {
     it('should reject unknown values', function(){
@@ -2078,6 +2264,19 @@ function sid(res) {
   return val
 }
 
+function subdomains(hostname) {
+  if (!hostname) {
+    return [];
+  }
+
+  var offset = 2;
+  return hostname.split('.').reverse().slice(offset);
+}
+
+function hostname(req) {
+  return req.headers.host;
+}
+
 function writePatch() {
   var ended = false
   return function addWritePatch(req, res, next) {

From 10db3a3e7e75c7299c463c7c72244991b3315958 Mon Sep 17 00:00:00 2001
From: David Pate <davidtpate@gmail.com>
Date: Thu, 18 Aug 2016 13:32:30 -0400
Subject: [PATCH 2/3] Modify the example and tests to use a primitive key
 rotation via `setInterval`

---
 README.md       | 14 ++++++++---
 test/session.js | 62 ++++++++++++++++++++++++++++++-------------------
 2 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/README.md b/README.md
index 84c71e6f..5d6e8b1a 100644
--- a/README.md
+++ b/README.md
@@ -143,13 +143,21 @@ The function should return a string or array of strings to be used as the secret
 signing the cookie.
 
 ```js
+var rotatingSecretKey;
+function rotateKey() {
+    rotatingSecretKey = Math.random();
+}
+// Initial rotation.
+rotateKey();
+
+setInterval(rotateKey, 60 * 60 * 1000) // Rotate the key at least once an hour.
 app.use(function(req, res, next) {
   var subdomain = subdomain[0];
-  req.clientSecret = getClientKey(subdomain);
 });
+
 app.use(session({
-  secret: function (req) { 
-    return 'Danger Zone!' + req.clientSecret;
+  secret: function () { 
+    return rotatingSecretKey;
   }
 }))
 ```
diff --git a/test/session.js b/test/session.js
index a436ef67..ceff5aa5 100644
--- a/test/session.js
+++ b/test/session.js
@@ -1094,9 +1094,22 @@ describe('session()', function(){
     });
 
     describe('when a function', function () {
+      var rotatingSecretKey;
+
+      function rotateKey() {
+        rotatingSecretKey = Math.random() + '';
+      }
+
+      rotateKey();
+      var rotateIntervalId = setInterval(rotateKey, 5000);
+
+      after(function() {
+        clearInterval(rotateIntervalId);
+      });
+
       it('should sign cookie with secret from function', function (done) {
-        var server = createServer({ secret: function(req) {
-          return 'Danger Zone!' + subdomains(hostname(req));
+        var server = createServer({ secret: function() {
+          return rotatingSecretKey;
         }}, function (req, res) {
           req.session.user = 'bob';
           res.end(req.session.user);
@@ -1104,15 +1117,14 @@ describe('session()', function(){
 
         request(server)
         .get('/')
-        .set('Host', 'test1.doamin.com')
         .expect(shouldSetCookie('connect.sid'))
         .expect(200, 'bob', done);
       });
 
       it('should load session from cookie sid', function (done) {
         var count = 0;
-        var server = createServer({ secret: function(req) {
-          return 'Danger Zone!' + subdomains(hostname(req));
+        var server = createServer({ secret: function() {
+          return rotatingSecretKey;
         }}, function (req, res) {
           req.session.num = req.session.num || ++count;
           res.end('session ' + req.session.num)
@@ -1120,13 +1132,11 @@ describe('session()', function(){
 
         request(server)
             .get('/')
-            .set('Host', 'test1.doamin.com')
             .expect(shouldSetCookie('connect.sid'))
             .expect(200, 'session 1', function (err, res) {
               if (err) return done(err);
               request(server)
                   .get('/')
-                  .set('Host', 'test1.doamin.com')
                   .set('Cookie', cookie(res))
                   .expect(200, 'session 1', done)
             })
@@ -1134,8 +1144,8 @@ describe('session()', function(){
 
       it('should not load session from cookie sid when secret is different', function (done) {
         var count = 0;
-        var server = createServer({ secret: function(req) {
-          return 'Danger Zone!' + subdomains(hostname(req));
+        var server = createServer({ secret: function() {
+          return rotatingSecretKey;
         }}, function (req, res) {
           req.session.num = req.session.num || ++count;
           res.end('session ' + req.session.num)
@@ -1147,17 +1157,19 @@ describe('session()', function(){
             .expect(shouldSetCookie('connect.sid'))
             .expect(200, 'session 1', function (err, res) {
               if (err) return done(err);
+
+              rotateKey(); // Rotate the key so the old session is now invalid.
+
               request(server)
                   .get('/')
-                  .set('Host', 'test2.doamin.com')
                   .set('Cookie', cookie(res))
                   .expect(200, 'session 2', done)
             })
       });
 
       it('should sign cookie with secret from function', function (done) {
-        var server = createServer({ secret: function(req) {
-          return 'Danger Zone!' + subdomains(hostname(req));
+        var server = createServer({ secret: function() {
+          return rotatingSecretKey;
         }}, function (req, res) {
           req.session.user = 'bob';
           res.end(req.session.user);
@@ -1165,14 +1177,13 @@ describe('session()', function(){
 
         request(server)
         .get('/')
-        .set('Host', 'test1.doamin.com')
         .expect(shouldSetCookie('connect.sid'))
         .expect(200, 'bob', done);
       });
 
       it('should sign cookies with different session ids', function (done) {
-        var server = createServer({ secret: function(req) {
-          return 'Danger Zone!' + subdomains(hostname(req));
+        var server = createServer({ secret: function() {
+          return rotatingSecretKey;
         } }, function (req, res) {
           req.session.user = 'bob';
           res.end(req.session.user);
@@ -1180,17 +1191,21 @@ describe('session()', function(){
 
         request(server)
         .get('/test1')
-        .set('Host', 'test1.doamin.com')
         .expect(shouldSetCookie('connect.sid'))
-        .expect(200, 'bob', function (err, test1Res) {
-              if (err) return done(err);
+            .expect(200, 'bob', function (err, test1Res) {
+              if (err) {
+                return done(err);
+              }
+
+              rotateKey(); // Rotate the key so we generate a session id with a new signature.
               request(server)
                   .get('/test2')
-                  .set('Host', 'test2.doamin.com')
                   .set('Cookie', cookie(test1Res))
                   .expect(shouldSetCookie('connect.sid'))
                   .expect(200, 'bob', function (err, test2Res) {
-                    if (err) return done(err);
+                    if (err) {
+                      return done(err);
+                    }
                     var test1Sid = sid(test1Res);
                     var test2Sid = sid(test2Res);
                     assert.ok(test1Sid !== test2Sid, 'session ids should not be equal for different secrets');
@@ -1200,7 +1215,7 @@ describe('session()', function(){
       });
 
       it('shouldn\'t sign when function doesn\'t return string', function (done) {
-        var server = createServer({ secret: function(req) {
+        var server = createServer({ secret: function() {
           return {};
         }}, function (req, res) {
           req.session.user = 'bob';
@@ -1209,7 +1224,6 @@ describe('session()', function(){
 
         request(server)
             .get('/')
-            .set('Host', 'test1.doamin.com')
             .expect(shouldNotHaveHeader('cookie'))
             .expect(500, /secret must be a string/, done);
       });
@@ -1217,7 +1231,7 @@ describe('session()', function(){
       it('should sign cookies with first element', function (done) {
         var store = new session.MemoryStore();
 
-        var server1 = createServer({ secret: function(req) { return [ 'Danger Zone!' + subdomains(hostname(req)), 'keyboard cat' ] }, store: store }, function (req, res) {
+        var server1 = createServer({ secret: function() { return [ rotatingSecretKey, 'keyboard cat' ] }, store: store }, function (req, res) {
           req.session.user = 'bob';
           res.end(req.session.user);
         });
@@ -1246,7 +1260,7 @@ describe('session()', function(){
           res.end(req.session.user);
         });
 
-        var server2 = createServer({ secret: function(req) { return [ 'Danger Zone!' + subdomains(hostname(req)), 'nyan cat' ] }, store: store }, function (req, res) {
+        var server2 = createServer({ secret: function() { return [ rotatingSecretKey, 'nyan cat' ] }, store: store }, function (req, res) {
           res.end(String(req.session.user));
         });
 

From 06bf3abb80d1cdb5f3c38497f996dc4b1c6c44e2 Mon Sep 17 00:00:00 2001
From: David Pate <davidtpate@gmail.com>
Date: Thu, 18 Aug 2016 13:33:09 -0400
Subject: [PATCH 3/3] Remove now unused functions.

---
 test/session.js | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/test/session.js b/test/session.js
index ceff5aa5..fef601e0 100644
--- a/test/session.js
+++ b/test/session.js
@@ -2278,19 +2278,6 @@ function sid(res) {
   return val
 }
 
-function subdomains(hostname) {
-  if (!hostname) {
-    return [];
-  }
-
-  var offset = 2;
-  return hostname.split('.').reverse().slice(offset);
-}
-
-function hostname(req) {
-  return req.headers.host;
-}
-
 function writePatch() {
   var ended = false
   return function addWritePatch(req, res, next) {