0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-10 23:36:14 -05:00

Moved 2fa labs flag usage to avoid logging out users

After migrations run, any sessions made with the labs flag turned off
will have the verified flag set. We also need new sessions made after
that to gain the verified flag, so that they aren't logged out at the
point that the labs flag is enabled (or removed).
This commit is contained in:
Sam Lord 2024-10-10 15:10:59 +01:00 committed by Kevin Ansfield
parent 16b0ef352f
commit 1f687ae466
4 changed files with 59 additions and 29 deletions

View file

@ -5,6 +5,7 @@ const createSessionMiddleware = require('./middleware');
const settingsCache = require('../../../../shared/settings-cache');
const {GhostMailer} = require('../../mail');
const {t} = require('../../i18n');
const labs = require('../../../../shared/labs');
const expressSession = require('./express-session');
@ -12,6 +13,9 @@ const models = require('../../../models');
const urlUtils = require('../../../../shared/url-utils');
const url = require('url');
// TODO: We have too many lines here, should move functions out into a utils module
/* eslint-disable max-lines */
function getOriginOfRequest(req) {
const origin = req.get('origin');
const referrer = req.get('referrer') || urlUtils.getAdminUrl() || urlUtils.getSiteUrl();
@ -44,6 +48,7 @@ const sessionService = createSessionService({
},
mailer,
urlUtils,
labs,
t
});
@ -59,8 +64,5 @@ module.exports.createSessionFromToken = sessionFromToken({
getTokenFromRequest: ssoAdapter.getRequestCredentials.bind(ssoAdapter)
});
// TODO: We have 51 lines here, should move functions out into a utils module
/* eslint-disable max-lines */
module.exports.sessionService = sessionService;
module.exports.deleteAllSessions = expressSession.deleteAllSessions;

View file

@ -1,25 +1,20 @@
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);
if (labs.isSet('staff2fa')) {
const isVerified = await sessionService.isVerifiedSession(req, res);
if (isVerified) {
res.sendStatus(201);
} else {
await sessionService.sendAuthCodeToUser(req, res);
throw new errors.NoPermissionError({
code: '2FA_TOKEN_REQUIRED',
errorType: 'Needs2FAError',
message: 'User must verify session to login.'
});
}
} else {
const isVerified = await sessionService.isVerifiedSession(req, res);
if (isVerified) {
res.sendStatus(201);
} else {
await sessionService.sendAuthCodeToUser(req, res);
throw new errors.NoPermissionError({
code: '2FA_TOKEN_REQUIRED',
errorType: 'Needs2FAError',
message: 'User must verify session to login.'
});
}
} catch (err) {
next(err);
@ -43,12 +38,11 @@ function SessionMiddleware({sessionService}) {
try {
const user = await sessionService.getUserForSession(req, res);
if (user) {
if (labs.isSet('staff2fa')) {
const isVerified = await sessionService.isVerifiedSession(req, res);
if (!isVerified) {
return next();
}
const isVerified = await sessionService.isVerifiedSession(req, res);
if (!isVerified) {
return next();
}
// Do not nullify `req.user` as it might have been already set
// in a previous middleware (authorize middleware).
req.user = user;

View file

@ -49,6 +49,7 @@ totp.options = {
* @param {(req: Req) => string} deps.getOriginOfRequest
* @param {(key: string) => string} deps.getSettingsCache
* @param {import('../../core/core/server/services/mail').GhostMailer} deps.mailer
* @param {import('../../core/core/shared/labs')} deps.labs
* @param {import('../../core/core/server/services/i18n').t} deps.t
* @param {import('../../core/core/shared/url-utils')} deps.urlUtils
* @returns {SessionService}
@ -61,6 +62,7 @@ module.exports = function createSessionService({
getSettingsCache,
mailer,
urlUtils,
labs,
t
}) {
/**
@ -107,6 +109,10 @@ module.exports = function createSessionService({
session.origin = origin;
session.user_agent = req.get('user-agent');
session.ip = req.ip;
if (!labs.isSet('staff2fa')) {
session.verified = true;
}
}
/**

View file

@ -16,11 +16,15 @@ describe('SessionService', function () {
};
const findUserById = sinon.spy(async ({id}) => ({id}));
const getOriginOfRequest = sinon.stub().returns('origin');
const labs = {
isSet: () => false
};
const sessionService = SessionService({
getSession,
findUserById,
getOriginOfRequest
getOriginOfRequest,
labs
});
const req = Object.create(express.request, {
@ -68,11 +72,15 @@ describe('SessionService', function () {
};
const findUserById = sinon.spy(async ({id}) => ({id}));
const getOriginOfRequest = sinon.stub().returns('other-origin');
const labs = {
isSet: () => false
};
const sessionService = SessionService({
getSession,
findUserById,
getOriginOfRequest
getOriginOfRequest,
labs
});
const req = Object.create(express.request, {
@ -107,11 +115,15 @@ describe('SessionService', function () {
};
const findUserById = sinon.spy(async ({id}) => ({id}));
const getOriginOfRequest = sinon.stub().returns('other-origin');
const labs = {
isSet: () => false
};
const sessionService = SessionService({
getSession,
findUserById,
getOriginOfRequest
getOriginOfRequest,
labs
});
const req = Object.create(express.request, {
@ -147,11 +159,15 @@ describe('SessionService', function () {
};
const findUserById = sinon.spy(async ({id}) => ({id}));
const getOriginOfRequest = sinon.stub().returns('origin');
const labs = {
isSet: () => true
};
const sessionService = SessionService({
getSession,
findUserById,
getOriginOfRequest
getOriginOfRequest,
labs
});
const req = Object.create(express.request, {
@ -200,12 +216,16 @@ describe('SessionService', function () {
const findUserById = sinon.spy(async ({id}) => ({id}));
const getOriginOfRequest = sinon.stub().returns('origin');
const getSettingsCache = sinon.stub().returns('secret-key');
const labs = {
isSet: () => true
};
const sessionService = SessionService({
getSession,
findUserById,
getOriginOfRequest,
getSettingsCache
getSettingsCache,
labs
});
const req = Object.create(express.request, {
@ -250,12 +270,16 @@ describe('SessionService', function () {
const findUserById = sinon.spy(async ({id}) => ({id}));
const getOriginOfRequest = sinon.stub().returns('origin');
const getSettingsCache = sinon.stub().returns('secret-key');
const labs = {
isSet: true
};
const sessionService = SessionService({
getSession,
findUserById,
getOriginOfRequest,
getSettingsCache
getSettingsCache,
labs
});
const req = Object.create(express.request, {
@ -299,6 +323,9 @@ describe('SessionService', function () {
};
const findUserById = sinon.spy(async ({id}) => ({id}));
const getOriginOfRequest = sinon.stub().returns('origin');
const labs = {
isSet: () => true
};
const req = Object.create(express.request, {
ip: {
@ -321,7 +348,8 @@ describe('SessionService', function () {
getSession,
findUserById,
getOriginOfRequest,
getSettingsCache: getSecretFirst
getSettingsCache: getSecretFirst,
labs
});
const authCodeFirst = await sessionServiceFirst.generateAuthCodeForUser(req, res);