Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session manager updates #551

Merged
merged 2 commits into from
Dec 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .changeset/e3ae8a80/changes.json
Original file line number Diff line number Diff line change
@@ -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"
]
}
]
}
4 changes: 4 additions & 0 deletions .changeset/e3ae8a80/changes.md
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions packages/admin-ui/server/AdminUI.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down
4 changes: 1 addition & 3 deletions packages/admin-ui/tests/server/AdminUI.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
Expand Down
4 changes: 2 additions & 2 deletions packages/core/Keystone/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)];

Expand All @@ -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;
Expand Down
83 changes: 42 additions & 41 deletions packages/core/Keystone/session.js
Original file line number Diff line number Diff line change
@@ -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;
16 changes: 6 additions & 10 deletions packages/core/auth/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion packages/fields/tests/fields.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
9 changes: 1 addition & 8 deletions packages/server/WebServer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 0 additions & 6 deletions projects/basic/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
6 changes: 3 additions & 3 deletions projects/login/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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,
Expand All @@ -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,
});
Expand Down
4 changes: 2 additions & 2 deletions projects/login/tests/auth-header.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion projects/twitter-login/twitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down