Issue 227 - allow custom session id generation (#252)

This commit address [issue
227](#227)
allowing an implementation of genid to be used when a custom session
store is used.  This is fully compatibile with the existing
implementation and aligns with how expression-session allows for
overriding the way session ids are generated. Note because of how
session ids are generated in this library, it is not possible to simply
fall into the custom store's genid function.

In implementing these changes, the following was done:
- Moved the default session id generation into config to make it clear
what said default is
- Added a new parameter genid compatible with how it works for
express-session
- Updated the appsession code to take advantage of the provided value and
fallback to the default.
- Added unit tests validating the genid parameter being used when
specified and not having any effect on the encrypted cookie
implementation.
- Reordered the spread operators in the appsession custom store tests
so that overrides are respected.  This changed no other existing tests.
- Updated documentation with the additional, optional parameter along
with links to the compatabile usage in express-session.

Co-authored-by: Adam Mcgrath <adam.mcgrath@auth0.com>

index.d.ts

@@ -536,6 +536,16 @@ interface SessionConfigParams {
    */
   store?: SessionStore;
 
+  /**
+   * A Function for generating a session id when using a custom session store.
+   * For full details see the documentation for express-session
+   * at [genid](https://github.com/expressjs/session/blob/master/README.md#genid).
+   * If encrypted cookie storage is used or no value is provided, a default implementation is used.
+   * Be aware the default implmentation is  slightly different in this library as compared to the
+   * default session id generation used express-session.
+   */
+  genid?: (req: OpenidRequest) => string;
+
   /**
    * If you want your session duration to be rolling, eg reset everytime the
    * user is active on your site, set this to a `true`. If you want the session

lib/appSession.js

@@ -5,7 +5,6 @@ const {
   JWE,
   errors: { JOSEError },
 } = require('jose');
-const crypto = require('crypto');
 const { promisify } = require('util');
 const cookie = require('cookie');
 const onHeaders = require('on-headers');
@@ -44,15 +43,20 @@ module.exports = (config) => {
   const sessionName = config.session.name;
   const cookieConfig = config.session.cookie;
   const {
+    genid: generateId,
     absoluteDuration,
     rolling: rollingEnabled,
     rollingDuration,
   } = config.session;
 
-  const { transient: emptyTransient , ...emptyCookieOptions } = cookieConfig;
+  const { transient: emptyTransient, ...emptyCookieOptions } = cookieConfig;
   emptyCookieOptions.expires = emptyTransient ? 0 : new Date();
 
-  const emptyCookie = cookie.serialize(`${sessionName}.0`, '', emptyCookieOptions);
+  const emptyCookie = cookie.serialize(
+    `${sessionName}.0`,
+    '',
+    emptyCookieOptions
+  );
   const cookieChunkSize = MAX_COOKIE_SIZE - emptyCookie.length;
 
   let keystore = new JWKS.KeyStore();
@@ -97,7 +101,7 @@ module.exports = (config) => {
     { uat = epoch(), iat = uat, exp = calculateExp(iat, uat) }
   ) {
     const cookies = req[COOKIES];
-    const { transient: cookieTransient , ...cookieOptions } = cookieConfig;
+    const { transient: cookieTransient, ...cookieOptions } = cookieConfig;
     cookieOptions.expires = cookieTransient ? 0 : new Date(exp * 1000);
 
     // session was deleted or is empty, this matches all session cookies (chunked or unchunked)
@@ -143,7 +147,7 @@ module.exports = (config) => {
           debug('replacing non chunked cookie with chunked cookies');
           res.clearCookie(sessionName, {
             domain: cookieConfig.domain,
-            path: cookieConfig.path
+            path: cookieConfig.path,
           });
         }
       } else {
@@ -153,7 +157,7 @@ module.exports = (config) => {
           if (cookieName.match(`^${sessionName}\\.\\d$`)) {
             res.clearCookie(cookieName, {
               domain: cookieConfig.domain,
-              path: cookieConfig.path
+              path: cookieConfig.path,
             });
           }
         }
@@ -325,7 +329,7 @@ module.exports = (config) => {
       attachSessionObject(req, sessionName, {});
     }
 
-    const id = existingSessionValue || crypto.randomBytes(16).toString('hex');
+    const id = existingSessionValue || generateId(req);
 
     onHeaders(res, () => store.setCookie(id, req, res, { iat }));
 

lib/config.js

@@ -1,7 +1,10 @@
 const Joi = require('joi');
+const crypto = require('crypto');
 const { defaultState: getLoginState } = require('./hooks/getLoginState');
 const isHttps = /^https:/i;
 
+const defaultSessionIdGenerator = () => crypto.randomBytes(16).toString('hex');
+
 const paramsSchema = Joi.object({
   secret: Joi.alternatives([
     Joi.string().min(8),
@@ -38,6 +41,10 @@ const paramsSchema = Joi.object({
       .default(7 * 24 * 60 * 60), // 7 days,
     name: Joi.string().token().optional().default('appSession'),
     store: Joi.object().optional(),
+    genid: Joi.function()
+      .maxArity(1)
+      .optional()
+      .default(() => defaultSessionIdGenerator),
     cookie: Joi.object({
       domain: Joi.string().optional(),
       transient: Joi.boolean().optional().default(false),

test/appSession.customStore.tests.js

@@ -60,8 +60,8 @@ describe('appSession custom store', () => {
 
     const conf = getConfig({
       ...defaultConfig,
-      session: { store, ...(config && config.session) },
       ...config,
+      session: { ...(config && config.session), store },
     });
 
     server = await createServer(appSession(conf));
@@ -155,6 +155,31 @@ describe('appSession custom store', () => {
     assert.isEmpty(jar.getCookies(baseUrl));
   });
 
+  it('uses custom session id generator when provided', async () => {
+    const immId = 'apple';
+    await setup({
+      session: { genid: () => immId },
+    });
+    const jar = await login({
+      sub: '__foo_user__',
+      role: 'test',
+      userid: immId,
+    });
+    const res = await request.get('/session', {
+      baseUrl,
+      jar,
+      json: true,
+    });
+    assert.equal(res.statusCode, 200);
+    const storedSessionJson = await redisClient.asyncGet(immId);
+    const { data: sessionValues } = JSON.parse(storedSessionJson);
+    assert.deepEqual(sessionValues, {
+      sub: '__foo_user__',
+      role: 'test',
+      userid: immId,
+    });
+  });
+
   it('should handle storage errors', async () => {
     const store = {
       get(id, cb) {

test/appSession.tests.js

@@ -124,8 +124,10 @@ describe('appSession', () => {
 
   it('should limit total cookie size to 4096 Bytes', async () => {
     const path =
-    '/some-really-really-really-really-really-really-really-really-really-really-really-really-really-long-path';
-    server = await createServer(appSession(getConfig({ ...defaultConfig, session: { cookie: { path } } })));
+      '/some-really-really-really-really-really-really-really-really-really-really-really-really-really-long-path';
+    server = await createServer(
+      appSession(getConfig({ ...defaultConfig, session: { cookie: { path } } }))
+    );
     const jar = request.jar();
 
     await request.post('session', {
@@ -139,24 +141,30 @@ describe('appSession', () => {
 
     const cookies = jar
       .getCookies(`${baseUrl}${path}`)
-      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
+      .reduce(
+        (obj, value) => Object.assign(obj, { [value.key]: value + '' }),
+        {}
+      );
 
     assert.exists(cookies);
     assert.equal(cookies['appSession.0'].length, 4096);
     assert.equal(cookies['appSession.1'].length, 4096);
     assert.equal(cookies['appSession.2'].length, 4096);
-    assert.isTrue(cookies['appSession.3'].length <= 4096)
+    assert.isTrue(cookies['appSession.3'].length <= 4096);
   });
 
   it('should clean up single cookie when switching to chunked', async () => {
     server = await createServer(appSession(getConfig(defaultConfig)));
     const jar = request.jar();
-    jar.setCookie(`appSession=foo`, baseUrl)
+    jar.setCookie(`appSession=foo`, baseUrl);
 
     const firstCookies = jar
       .getCookies(baseUrl)
-      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
-    assert.property(firstCookies, 'appSession')
+      .reduce(
+        (obj, value) => Object.assign(obj, { [value.key]: value + '' }),
+        {}
+      );
+    assert.property(firstCookies, 'appSession');
 
     await request.post('session', {
       baseUrl,
@@ -169,23 +177,29 @@ describe('appSession', () => {
 
     const cookies = jar
       .getCookies(baseUrl)
-      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
+      .reduce(
+        (obj, value) => Object.assign(obj, { [value.key]: value + '' }),
+        {}
+      );
 
-    assert.property(cookies, 'appSession.0')
-    assert.notProperty(cookies, 'appSession')
+    assert.property(cookies, 'appSession.0');
+    assert.notProperty(cookies, 'appSession');
   });
 
   it('should clean up chunked cookies when switching to single cookie', async () => {
     server = await createServer(appSession(getConfig(defaultConfig)));
     const jar = request.jar();
-    jar.setCookie(`appSession.0=foo`, baseUrl)
-    jar.setCookie(`appSession.1=foo`, baseUrl)
+    jar.setCookie(`appSession.0=foo`, baseUrl);
+    jar.setCookie(`appSession.1=foo`, baseUrl);
 
     const firstCookies = jar
       .getCookies(baseUrl)
-      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
-    assert.property(firstCookies, 'appSession.0')
-    assert.property(firstCookies, 'appSession.1')
+      .reduce(
+        (obj, value) => Object.assign(obj, { [value.key]: value + '' }),
+        {}
+      );
+    assert.property(firstCookies, 'appSession.0');
+    assert.property(firstCookies, 'appSession.1');
 
     await request.post('session', {
       baseUrl,
@@ -197,10 +211,13 @@ describe('appSession', () => {
 
     const cookies = jar
       .getCookies(baseUrl)
-      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
+      .reduce(
+        (obj, value) => Object.assign(obj, { [value.key]: value + '' }),
+        {}
+      );
 
-    assert.property(cookies, 'appSession')
-    assert.notProperty(cookies, 'appSession.0')
+    assert.property(cookies, 'appSession');
+    assert.notProperty(cookies, 'appSession.0');
   });
 
   it('should handle unordered chunked cookies', async () => {
@@ -323,6 +340,33 @@ describe('appSession', () => {
     });
   });
 
+  it('should disregard custom id generation without a custom store', async () => {
+    server = await createServer(
+      appSession(
+        getConfig({
+          ...defaultConfig,
+          session: {
+            genid: () => {
+              throw 'this should not be called';
+            }, //consider using chai-spies
+          },
+        })
+      )
+    );
+    const jar = request.jar();
+    const res = await request.get('/session', {
+      baseUrl,
+      json: true,
+      jar,
+      headers: {
+        cookie: `appSession=${encrypted}`,
+      },
+    });
+
+    assert.equal(res.statusCode, 200);
+    assert.equal(res.body.sub, '__test_sub__');
+  });
+
   it('should use a custom cookie name', async () => {
     server = await createServer(
       appSession(

test/config.tests.js

@@ -165,12 +165,14 @@ describe('get config', () => {
   });
 
   it('should set custom cookie configuration', () => {
+    const sessionIdGenerator = () => '1235';
     const config = getConfig({
       ...defaultConfig,
       secret: ['__test_session_secret_1__', '__test_session_secret_2__'],
       session: {
         name: '__test_custom_session_name__',
         rollingDuration: 1234567890,
+        genid: sessionIdGenerator,
         cookie: {
           domain: '__test_custom_domain__',
           transient: true,
@@ -188,6 +190,7 @@ describe('get config', () => {
         rollingDuration: 1234567890,
         absoluteDuration: 604800,
         rolling: true,
+        genid: sessionIdGenerator,
         cookie: {
           domain: '__test_custom_domain__',
           transient: true,