Move session handling code into a SessionManager class

Author: timleslie

.changeset/e3ae8a80/changes.json

@@ -0,0 +1,31 @@
+{
+  "releases": [
+    { "name": "@voussoir/admin-ui", "type": "patch" },
+    { "name": "@voussoir/core", "type": "major" },
+    { "name": "@voussoir/fields", "type": "patch" },
+    { "name": "@voussoir/server", "type": "patch" },
+    { "name": "@voussoir/cypress-project-basic", "type": "patch" },
+    { "name": "@voussoir/cypress-project-login", "type": "patch" },
+    { "name": "@voussoir/cypress-project-twitter-login", "type": "patch" }
+  ],
+  "dependents": [
+    { "name": "@voussoir/adapter-mongoose", "type": "patch", "dependencies": ["@voussoir/core"] },
+    {
+      "name": "@voussoir/test-utils",
+      "type": "patch",
+      "dependencies": ["@voussoir/adapter-mongoose", "@voussoir/core", "@voussoir/server"]
+    },
+    {
+      "name": "@voussoir/cypress-project-access-control",
+      "type": "patch",
+      "dependencies": [
+        "@voussoir/adapter-mongoose",
+        "@voussoir/test-utils",
+        "@voussoir/admin-ui",
+        "@voussoir/core",
+        "@voussoir/fields",
+        "@voussoir/server"
+      ]
+    }
+  ]
+}

.changeset/e3ae8a80/changes.md

@@ -0,0 +1,4 @@
+- Rename keystone.session to keystone.sessionManager
+  - Rename keystone.session.validate to keystone.sessionManager.populateAuthedItemMiddleware
+  - Rename keystone.session.create to keystone.sessionManager.startAuthedSession
+  - Rename keystone.session.destroy to keystone.sessionManager.endAuthedSession

packages/admin-ui/server/AdminUI.js

@@ -55,7 +55,7 @@ module.exports = class AdminUI {
         });
       }
 
-      await this.keystone.session.create(req, result);
+      await this.keystone.sessionManager.startAuthedSession(req, result);
     } catch (e) {
       return next(e);
     }
@@ -75,7 +75,7 @@ module.exports = class AdminUI {
   async signout(req, res, next) {
     let success;
     try {
-      await this.keystone.session.destroy(req);
+      await this.keystone.sessionManager.endAuthedSession(req);
       success = true;
     } catch (e) {
       success = false;

packages/admin-ui/tests/server/AdminUI.test.js

@@ -19,9 +19,7 @@ jest.doMock('html-webpack-plugin', () => {
 const AdminUI = require('../../server/AdminUI.js');
 
 const keystone = {
-  session: {
-    validate: () => jest.fn(),
-  },
+  sessionManager: {},
   getAdminSchema: jest.fn(),
   getAdminMeta: jest.fn(),
 };

packages/core/Keystone/index.js

@@ -14,7 +14,7 @@ const {
   mergeRelationships,
 } = require('./relationship-utils');
 const List = require('../List');
-const bindSession = require('./session');
+const SessionManager = require('./session');
 
 const unique = arr => [...new Set(arr)];
 
@@ -30,7 +30,7 @@ module.exports = class Keystone {
     this.lists = {};
     this.listsArray = [];
     this.getListByKey = key => this.lists[key];
-    this.session = bindSession(this);
+    this.sessionManager = new SessionManager(this);
 
     if (config.adapters) {
       this.adapters = config.adapters;

packages/core/Keystone/session.js

@@ -1,47 +1,48 @@
-const noop = () => undefined;
-
-const validate = keystone => ({ valid = noop, invalid = noop }) => async (req, res, next) => {
-  if (!req.session || !req.session.keystoneItemId) {
-    invalid({ req, reason: 'empty' });
-    return next();
+class SessionManager {
+  constructor(keystone) {
+    this.keystone = keystone;
+    this.populateAuthedItemMiddleware = this.populateAuthedItemMiddleware.bind(this);
   }
-  const list = keystone.lists[req.session.keystoneListKey];
-  if (!list) {
-    // TODO: probably destroy the session
-    invalid({ req, reason: 'invalid-list' });
-    return next();
+
+  async populateAuthedItemMiddleware(req, res, next) {
+    if (!req.session || !req.session.keystoneItemId) {
+      return next();
+    }
+    const list = this.keystone.lists[req.session.keystoneListKey];
+    if (!list) {
+      // TODO: probably destroy the session
+      return next();
+    }
+    const item = await list.adapter.findById(req.session.keystoneItemId);
+    if (!item) {
+      // TODO: probably destroy the session
+      return next();
+    }
+    req.user = item;
+    req.authedListKey = list.key;
+
+    next();
   }
-  const item = await list.adapter.findById(req.session.keystoneItemId);
-  if (!item) {
-    // TODO: probably destroy the session
-    invalid({ req, reason: 'invalid-item' });
-    return next();
+
+  startAuthedSession(req, { item, list }) {
+    return new Promise((resolve, reject) =>
+      req.session.regenerate(err => {
+        if (err) return reject(err);
+        req.session.keystoneListKey = list.key;
+        req.session.keystoneItemId = item.id;
+        resolve();
+      })
+    );
   }
-  valid({ req, list, item });
-  next();
-};
 
-function create(req, { item, list }) {
-  return new Promise((resolve, reject) =>
-    req.session.regenerate(err => {
-      if (err) return reject(err);
-      req.session.keystoneListKey = list.key;
-      req.session.keystoneItemId = item.id;
-      resolve();
-    })
-  );
-}
-function destroy(req) {
-  return new Promise((resolve, reject) =>
-    req.session.regenerate(err => {
-      if (err) return reject(err);
-      resolve({ success: true });
-    })
-  );
+  endAuthedSession(req) {
+    return new Promise((resolve, reject) =>
+      req.session.regenerate(err => {
+        if (err) return reject(err);
+        resolve({ success: true });
+      })
+    );
+  }
 }
 
-module.exports = keystone => ({
-  create: create,
-  destroy: destroy,
-  validate: validate(keystone),
-});
+module.exports = SessionManager;

packages/core/auth/index.md

@@ -127,10 +127,8 @@ The time spend verifying an actors credentials should be constant-time regardles
 
 ```javascript
 server.app.use(
-  keystone.session.validate({
-    // When logged in, we'll get a req.user object
-    valid: ({ req, item }) => (req.user = item),
-  })
+  // When logged in, we'll get a req.user object
+  keystone.sessionManager.populateAuthedItemMiddleware
 );
 
 const twitterAuth = keystone.createAuthStrategy({
@@ -168,7 +166,7 @@ server.app.get(
 
       try {
         await keystone.auth.User.twitter.connectItem(req, { item: authedItem });
-        await keystone.session.create(req, {
+        await keystone.sessionManager.startAuthedSession(req, {
           item: authedItem,
           list: info.list,
         });
@@ -198,10 +196,8 @@ bank's password over multiple server requests / page refreshes:
 
 ```javascript
 server.app.use(
-  keystone.session.validate({
-    // When logged in, we'll get a req.user object
-    valid: ({ req, item }) => (req.user = item),
-  })
+  // When logged in, we'll get a req.user object
+  keystone.sessionManager.populateAuthedItemMiddleware
 );
 
 const twitterAuth = keystone.createAuthStrategy({
@@ -280,7 +276,7 @@ server.app.post('/auth/twitter/complete', async (req, res, next) => {
     });
 
     await keystone.auth.User.twitter.connectItem(req, { item });
-    await keystone.session.create(req, { item, list });
+    await keystone.sessionManager.startAuthedSession(req, { item, list });
     res.redirect('/api/session');
   } catch (createError) {
     next(createError);

packages/fields/tests/fields.test.js

@@ -77,7 +77,6 @@ describe('Test CRUD for all fields', () => {
             // Set up a server (but do not .listen(), we will use supertest to access the app)
             const server = new WebServer(keystone, {
               'cookie secret': 'qwerty',
-              session: false,
             });
 
             return { server, keystone };

packages/server/WebServer/index.js

@@ -78,14 +78,7 @@ module.exports = class WebServer {
       this.app.use(injectAuthCookieMiddleware, sessionMiddleware);
 
       // Attach the user to the request for all following route handlers
-      this.app.use(
-        this.keystone.session.validate({
-          valid: ({ req, list, item }) => {
-            req.user = item;
-            req.authedListKey = list.key;
-          },
-        })
-      );
+      this.app.use(this.keystone.sessionManager.populateAuthedItemMiddleware);
     }
 
     if (adminUI && this.config.authStrategy) {

projects/basic/index.js

@@ -151,12 +151,6 @@ const server = new WebServer(keystone, {
   port,
 });
 
-server.app.use(
-  keystone.session.validate({
-    valid: ({ req, item }) => (req.user = item),
-  })
-);
-
 server.app.get('/reset-db', (req, res) => {
   const reset = async () => {
     Object.values(keystone.adapters).forEach(async adapter => {

projects/login/index.js

@@ -65,7 +65,7 @@ server.app.post(
   bodyParser.urlencoded({ extended: true }),
   async (req, res, next) => {
     // Cleanup any previous session
-    await keystone.session.destroy(req);
+    await keystone.sessionManager.endAuthedSession(req);
 
     try {
       const result = await keystone.auth.User.password.validate({
@@ -77,7 +77,7 @@ server.app.post(
           success: false,
         });
       }
-      await keystone.session.create(req, result);
+      await keystone.sessionManager.startAuthedSession(req, result);
       res.json({
         success: true,
         itemId: result.item.id,
@@ -91,7 +91,7 @@ server.app.post(
 
 server.app.get('/signout', async (req, res, next) => {
   try {
-    await keystone.session.destroy(req);
+    await keystone.sessionManager.endAuthedSession(req);
     res.json({
       success: true,
     });

projects/login/tests/auth-header.test.js

@@ -48,7 +48,7 @@ function setupKeystone() {
     bodyParser.urlencoded({ extended: true }),
     async (req, res, next) => {
       // Cleanup any previous session
-      await keystone.session.destroy(req);
+      await keystone.sessionManager.endAuthedSession(req);
 
       try {
         const result = await keystone.auth.User.password.validate({
@@ -60,7 +60,7 @@ function setupKeystone() {
             success: false,
           });
         }
-        await keystone.session.create(req, result);
+        await keystone.sessionManager.startAuthedSession(req, result);
         res.json({
           success: true,
           token: req.sessionID,

projects/twitter-login/twitter.js

@@ -78,7 +78,7 @@ exports.configureTwitterAuth = function(keystone, server) {
         });
 
         await keystone.auth.User.twitter.connectItem(req, { item });
-        await keystone.session.create(req, { item, list });
+        await keystone.sessionManager.startAuthedSession(req, { item, list });
         res.redirect('/api/session');
       } catch (createError) {
         next(createError);