Added check for verified sessions

refs ENG-1610
This commit is contained in:
Sam Lord 2024-10-09 13:43:07 +01:00 committed by Kevin Ansfield
parent 7a18e829c5
commit 0b852bcb38
6 changed files with 80 additions and 53 deletions

View File

@ -1,5 +1,4 @@
const {Store} = require('express-session');
const {InternalServerError} = require('@tryghost/errors');
module.exports = class SessionStore extends Store {
constructor(SessionModel) {
@ -29,11 +28,6 @@ module.exports = class SessionStore extends Store {
}
set(sid, sessionData, callback) {
if (!sessionData.user_id) {
return callback(new InternalServerError({
message: 'Cannot create a session with no user_id'
}));
}
this.SessionModel
.upsert({session_data: sessionData}, {session_id: sid})
.then(() => {

View File

@ -1,8 +1,25 @@
const errors = require('@tryghost/errors');
const labs = require('../../../../shared/labs');
function SessionMiddleware({sessionService}) {
async function createSession(req, res, next) {
try {
await sessionService.createSessionForUser(req, res, req.user);
res.sendStatus(201);
if (labs.isSet('staff2fa')) {
const isVerified = await sessionService.isVerifiedSession(req, res);
if (isVerified) {
res.sendStatus(201);
} else {
throw new errors.NoPermissionError({
code: '2FA_TOKEN_REQUIRED',
errorType: 'Needs2FAError',
message: 'User must verify session to login.'
});
}
} else {
res.sendStatus(201);
}
} catch (err) {
next(err);
}
@ -13,7 +30,11 @@ function SessionMiddleware({sessionService}) {
await sessionService.removeUserForSession(req, res);
res.sendStatus(204);
} catch (err) {
next(err);
if (errors.utils.isGhostError(err)) {
next(err);
} else {
next(new errors.InternalServerError({err}));
}
}
}

View File

@ -109,12 +109,12 @@ describe('Session controller', function () {
const fakeReq = {};
const fakeRes = {};
const fakeNext = () => {};
const destroySessionStub = sinon.stub(sessionServiceMiddleware, 'destroySession');
const logoutSessionStub = sinon.stub(sessionServiceMiddleware, 'logout');
return sessionController.delete().then((fn) => {
fn(fakeReq, fakeRes, fakeNext);
}).then(function () {
const destroySessionStubCall = destroySessionStub.getCall(0);
const destroySessionStubCall = logoutSessionStub.getCall(0);
should.equal(destroySessionStubCall.args[0], fakeReq);
should.equal(destroySessionStubCall.args[1], fakeRes);
should.equal(destroySessionStubCall.args[2], fakeNext);

View File

@ -1,7 +1,9 @@
const sessionMiddleware = require('../../../../../../core/server/services/auth').session;
const SessionMiddlware = require('../../../../../../core/server/services/auth/session/middleware');
const models = require('../../../../../../core/server/models');
const sinon = require('sinon');
const should = require('should');
const labs = require('../../../../../../core/shared/labs');
describe('Session Service', function () {
before(function () {
@ -14,9 +16,7 @@ describe('Session Service', function () {
const fakeReq = function fakeReq() {
return {
session: {
destroy() {}
},
session: {},
user: null,
body: {},
get() {}
@ -74,54 +74,63 @@ describe('Session Service', function () {
sessionMiddleware.createSession(req, res);
});
it('errors with a 403 when signing in while not verified', function (done) {
sinon.stub(labs, 'isSet').returns(true);
const req = fakeReq();
const res = fakeRes();
sinon.stub(req, 'get')
.withArgs('origin').returns('http://host.tld')
.withArgs('user-agent').returns('bububang');
req.ip = '127.0.0.1';
req.user = models.User.forge({id: 23});
sessionMiddleware.createSession(req, res, (err) => {
should.equal(err.statusCode, 403);
should.equal(err.code, '2FA_TOKEN_REQUIRED');
done();
});
});
});
describe('destroySession', function () {
it('calls req.session.destroy', function (done) {
describe('logout', function () {
it('calls next with InternalServerError if removeSessionForUser errors', function (done) {
const req = fakeReq();
const res = fakeRes();
const destroyStub = sinon.stub(req.session, 'destroy')
.callsFake(function (fn) {
fn();
});
const middleware = SessionMiddlware({
sessionService: {
removeUserForSession: function () {
return Promise.reject(new Error('oops'));
}
}
});
sinon.stub(res, 'sendStatus')
.callsFake(function () {
should.equal(destroyStub.callCount, 1);
done();
});
sessionMiddleware.destroySession(req, res);
});
it('calls next with InternalServerError if destroy errors', function (done) {
const req = fakeReq();
const res = fakeRes();
sinon.stub(req.session, 'destroy')
.callsFake(function (fn) {
fn(new Error('oops'));
});
sessionMiddleware.destroySession(req, res, function next(err) {
middleware.logout(req, res, function next(err) {
should.equal(err.errorType, 'InternalServerError');
done();
});
});
it('calls sendStatus with 204 if destroy does not error', function (done) {
it('calls sendStatus with 204 if removeUserForSession does not error', function (done) {
const req = fakeReq();
const res = fakeRes();
sinon.stub(req.session, 'destroy')
.callsFake(function (fn) {
fn();
});
sinon.stub(res, 'sendStatus')
.callsFake(function (status) {
should.equal(status, 204);
done();
});
sessionMiddleware.destroySession(req, res);
const middleware = SessionMiddlware({
sessionService: {
removeUserForSession: function () {
return Promise.resolve();
}
}
});
middleware.logout(req, res);
});
});
});

View File

@ -127,16 +127,6 @@ describe('Auth Service SessionStore', function () {
});
describe('SessionStore#set', function () {
it('calls back with an error if there is no user_id on the session_data', function (done) {
const store = new SessionStore(models.Session);
const sid = 1;
const session_data = {};
store.set(sid, session_data, function (err) {
should.exist(err);
done();
});
});
it('calls upsert on the model with the session_id and the session_data', function (done) {
const upsertStub = sinon.stub(models.Session, 'upsert')
.resolves();

View File

@ -31,6 +31,7 @@ const {totp} = require('otplib');
* @prop {(req: Req, res: Res) => Promise<void>} verifySession
* @prop {(req: Req, res: Res) => Promise<void>} sendAuthCodeToUser
* @prop {(req: Req, res: Res) => Promise<boolean>} verifyAuthCodeForUser
* @prop {(req: Req, res: Res) => Promise<boolean>} isVerifiedSession
*/
/**
@ -168,6 +169,17 @@ module.exports = function createSessionService({
session.verified = true;
}
/**
* isVerifiedSession
*
* @param {Req} req
* @param {Res} res
*/
async function isVerifiedSession(req, res) {
const session = await getSession(req, res);
return session.verified;
}
/**
* removeUserForSession
*
@ -217,6 +229,7 @@ module.exports = function createSessionService({
createSessionForUser,
removeUserForSession,
verifySession,
isVerifiedSession,
sendAuthCodeToUser,
verifyAuthCodeForUser,
generateAuthCodeForUser