From 06b66c38cd5868e54d75a5f650ba6b66524bfd65 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Mon, 16 Mar 2020 15:43:30 +1100 Subject: [PATCH] Replace session package API with a SessionManager class --- .changeset/smooth-doors-juggle.md | 7 + packages/auth-passport/lib/Passport.js | 12 +- packages/keystone/lib/Keystone/index.js | 37 ++--- packages/session/index.js | 4 +- packages/session/lib/session.js | 202 +++++++++++++----------- 5 files changed, 135 insertions(+), 127 deletions(-) create mode 100644 .changeset/smooth-doors-juggle.md diff --git a/.changeset/smooth-doors-juggle.md b/.changeset/smooth-doors-juggle.md new file mode 100644 index 00000000000..754b6f244ed --- /dev/null +++ b/.changeset/smooth-doors-juggle.md @@ -0,0 +1,7 @@ +--- +'@keystonejs/keystone': major +'@keystonejs/session': major +'@keystonejs/auth-passport': patch +--- + +`@keystonejs/session` now provides a `SessionManager` class which replaces the former function based API. The method `keystone.getCookieSecret()` has been removed. diff --git a/packages/auth-passport/lib/Passport.js b/packages/auth-passport/lib/Passport.js index 6c547de269e..f5c887922d4 100644 --- a/packages/auth-passport/lib/Passport.js +++ b/packages/auth-passport/lib/Passport.js @@ -1,7 +1,6 @@ const express = require('express'); const passport = require('passport'); const assert = require('nanoassert'); -const { startAuthedSession } = require('@keystonejs/session'); const FIELD_SERVICE_NAME = 'service'; const FIELD_USER_ID = 'serviceUserId'; @@ -81,7 +80,7 @@ class PassportAuthStrategy { this._keystone = keystone; this._listKey = listKey; this._ServiceStrategy = ServiceStrategy; - this._cookieSecret = keystone.getCookieSecret(); + this._sessionManager = keystone._sessionManager; // Pull all the required data off the `config` object this._serviceAppId = config.appId; @@ -506,11 +505,10 @@ class PassportAuthStrategy { } async _authenticateItem(item, accessToken, isNewItem, req, res, next) { - const token = await startAuthedSession( - req, - { item, list: this._getList() }, - this._cookieSecret - ); + const token = await this._sessionManager.startAuthedSession(req, { + item, + list: this._getList(), + }); this._onAuthenticated({ token, item, isNewItem }, req, res, next); } } diff --git a/packages/keystone/lib/Keystone/index.js b/packages/keystone/lib/Keystone/index.js index 36c14c768b8..3daaf84263a 100644 --- a/packages/keystone/lib/Keystone/index.js +++ b/packages/keystone/lib/Keystone/index.js @@ -22,11 +22,7 @@ const { validateCustomAccessControl, validateAuthAccessControl, } = require('@keystonejs/access-control'); -const { - startAuthedSession, - endAuthedSession, - commonSessionMiddleware, -} = require('@keystonejs/session'); +const { SessionManager } = require('@keystonejs/session'); const { AppVersionProvider, appVersionMiddleware } = require('@keystonejs/app-version'); const { @@ -65,10 +61,12 @@ module.exports = class Keystone { this.listsArray = []; this.getListByKey = key => this.lists[key]; this._schemas = {}; - this._cookieSecret = cookieSecret; - this._secureCookies = secureCookies; - this._cookieMaxAge = cookieMaxAge; - this._sessionStore = sessionStore; + this._sessionManager = new SessionManager({ + cookieSecret, + secureCookies, + cookieMaxAge, + sessionStore, + }); this.eventHandlers = { onConnect }; this.registeredTypes = new Set(); this._schemaNames = schemaNames; @@ -118,13 +116,6 @@ module.exports = class Keystone { }; } - getCookieSecret() { - if (!this._cookieSecret) { - throw new Error('No cookieSecret set in Keystone constructor'); - } - return this._cookieSecret; - } - _executeOperation({ requestString, rootValue = null, @@ -222,11 +213,7 @@ module.exports = class Keystone { return { schemaName, - startAuthedSession: ({ item, list }) => - startAuthedSession(req, { item, list }, this._cookieSecret), - endAuthedSession: endAuthedSession.bind(null, req), - authedItem: req.user, - authedListKey: req.authedListKey, + ...this._sessionManager.getContext(req), getCustomAccessControlForUser, getListAccessControlForUser, getFieldAccessControlForUser, @@ -521,13 +508,7 @@ module.exports = class Keystone { // to be first so the methods added to `req` are available further down // the request pipeline. // TODO: set up a session test rig (maybe by wrapping an in-memory store) - commonSessionMiddleware({ - keystone: this, - cookieSecret: this._cookieSecret, - sessionStore: this._sessionStore, - secureCookies: this._secureCookies, - cookieMaxAge: this._cookieMaxAge, - }), + this._sessionManager.getSessionMiddleware({ keystone: this }), falsey(process.env.DISABLE_LOGGING) && require('express-pino-logger')(pinoOptions), cors && createCorsMiddleware(cors), ...(await Promise.all( diff --git a/packages/session/index.js b/packages/session/index.js index 0862405ab82..233618fa1ae 100644 --- a/packages/session/index.js +++ b/packages/session/index.js @@ -1,3 +1,3 @@ -const { commonSessionMiddleware, startAuthedSession, endAuthedSession } = require('./lib/session'); +const { SessionManager } = require('./lib/session'); -module.exports = { commonSessionMiddleware, startAuthedSession, endAuthedSession }; +module.exports = { SessionManager }; diff --git a/packages/session/lib/session.js b/packages/session/lib/session.js index f937811d05c..8455f0d8bb1 100644 --- a/packages/session/lib/session.js +++ b/packages/session/lib/session.js @@ -2,76 +2,94 @@ const cookieSignature = require('cookie-signature'); const expressSession = require('express-session'); const cookie = require('cookie'); -const commonSessionMiddleware = ({ - keystone, - cookieSecret, - sessionStore, - secureCookies, - cookieMaxAge, -}) => { - const COOKIE_NAME = 'keystone.sid'; - - // We have at least one auth strategy - // Setup the session as the very first thing. - // The way express works, the `req.session` (and, really, anything added - // to `req`) will be available to all sub `express()` instances. - // This way, we have one global setting for authentication / sessions that - // all routes on the server can utilize. - function injectAuthCookieMiddleware(req, res, next) { - if (!req.headers) { - return next(); - } - - const authHeader = req.headers.authorization || req.headers.Authorization; - - if (!authHeader) { - return next(); - } - - const [type, token] = req.headers['authorization'].split(' '); - - if (type !== 'Bearer') { - // TODO: Use logger - console.warn(`Got Authorization header of type ${type}, but expected Bearer`); - return next(); - } - - // Split the cookies out - const cookies = cookie.parse(req.headers.cookie || ''); - - // Construct a "fake" session cookie based on the authorization token - cookies[COOKIE_NAME] = `s:${token}`; - - // Then reset the cookies so the session middleware can read it. - req.headers.cookie = Object.entries(cookies) - .map(([name, value]) => `${name}=${value}`) - .join('; '); - - // Always call next - next(); +class SessionManager { + constructor({ + cookieSecret = 'qwerty', + secureCookies = process.env.NODE_ENV === 'production', // Default to true in production + cookieMaxAge = 1000 * 60 * 60 * 24 * 30, // 30 days + sessionStore, + }) { + this._cookieSecret = cookieSecret; + this._secureCookies = secureCookies; + this._cookieMaxAge = cookieMaxAge; + this._sessionStore = sessionStore; } - const sessionMiddleware = expressSession({ - secret: cookieSecret, - resave: false, - saveUninitialized: false, - name: COOKIE_NAME, - cookie: { secure: secureCookies, maxAge: cookieMaxAge }, - store: sessionStore, - }); - - return [injectAuthCookieMiddleware, sessionMiddleware, populateAuthedItemMiddleware(keystone)]; -}; + getSessionMiddleware({ keystone }) { + const COOKIE_NAME = 'keystone.sid'; + + // We have at least one auth strategy + // Setup the session as the very first thing. + // The way express works, the `req.session` (and, really, anything added + // to `req`) will be available to all sub `express()` instances. + // This way, we have one global setting for authentication / sessions that + // all routes on the server can utilize. + const injectAuthCookieMiddleware = (req, res, next) => { + if (!req.headers) { + return next(); + } + + const authHeader = req.headers.authorization || req.headers.Authorization; + + if (!authHeader) { + return next(); + } + + const [type, token] = req.headers['authorization'].split(' '); + + if (type !== 'Bearer') { + // TODO: Use logger + console.warn(`Got Authorization header of type ${type}, but expected Bearer`); + return next(); + } + + // Split the cookies out + const cookies = cookie.parse(req.headers.cookie || ''); + + // Construct a "fake" session cookie based on the authorization token + cookies[COOKIE_NAME] = `s:${token}`; + + // Then reset the cookies so the session middleware can read it. + req.headers.cookie = Object.entries(cookies) + .map(([name, value]) => `${name}=${value}`) + .join('; '); + + // Always call next + next(); + }; + + const sessionMiddleware = expressSession({ + secret: this._cookieSecret, + resave: false, + saveUninitialized: false, + name: COOKIE_NAME, + cookie: { secure: this._secureCookies, maxAge: this._cookieMaxAge }, + store: this._sessionStore, + }); + + const _populateAuthedItemMiddleware = async (req, res, next) => { + const item = await this._getAuthedItem(req, keystone); + if (!item) { + // TODO: probably destroy the session + return next(); + } + + req.user = item; + req.authedListKey = req.session.keystoneListKey; + + next(); + }; + + return [injectAuthCookieMiddleware, sessionMiddleware, _populateAuthedItemMiddleware]; + } -function populateAuthedItemMiddleware(keystone) { - return async (req, res, next) => { + async _getAuthedItem(req, keystone) { if (!req.session || !req.session.keystoneItemId) { - return next(); + return; } const list = keystone.lists[req.session.keystoneListKey]; if (!list) { - // TODO: probably destroy the session - return next(); + return; } let item; try { @@ -81,38 +99,42 @@ function populateAuthedItemMiddleware(keystone) { info: {}, }); } catch (e) { - // If the item no longer exists, getAccessControlledItem() will throw an exception - return next(); + return; } if (!item) { - // TODO: probably destroy the session - return next(); + return; } - req.user = item; - req.authedListKey = list.key; + return item; + } - 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(cookieSignature.sign(req.session.id, this._cookieSecret)); + }) + ); + } -function startAuthedSession(req, { item, list }, cookieSecret) { - 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(cookieSignature.sign(req.session.id, cookieSecret)); - }) - ); -} + endAuthedSession(req) { + return new Promise((resolve, reject) => + req.session.regenerate(err => { + if (err) return reject(err); + resolve({ success: true }); + }) + ); + } -function endAuthedSession(req) { - return new Promise((resolve, reject) => - req.session.regenerate(err => { - if (err) return reject(err); - resolve({ success: true }); - }) - ); + getContext(req) { + return { + startAuthedSession: ({ item, list }) => this.startAuthedSession(req, { item, list }), + endAuthedSession: () => this.endAuthedSession(req), + authedItem: req.user, + authedListKey: req.authedListKey, + }; + } } -module.exports = { commonSessionMiddleware, startAuthedSession, endAuthedSession }; +module.exports = { SessionManager };