diff --git a/ghost/core/core/server/services/auth/session/index.js b/ghost/core/core/server/services/auth/session/index.js index d225854ca7..be9f5ef1b3 100644 --- a/ghost/core/core/server/services/auth/session/index.js +++ b/ghost/core/core/server/services/auth/session/index.js @@ -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; diff --git a/ghost/core/core/server/services/auth/session/middleware.js b/ghost/core/core/server/services/auth/session/middleware.js index 415d040ead..1a502a1f4d 100644 --- a/ghost/core/core/server/services/auth/session/middleware.js +++ b/ghost/core/core/server/services/auth/session/middleware.js @@ -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; diff --git a/ghost/session-service/lib/session-service.js b/ghost/session-service/lib/session-service.js index 51f0b0e4cc..71ac6d369f 100644 --- a/ghost/session-service/lib/session-service.js +++ b/ghost/session-service/lib/session-service.js @@ -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; + } } /** diff --git a/ghost/session-service/test/SessionService.test.js b/ghost/session-service/test/SessionService.test.js index 3e821e8a66..bbe85280fe 100644 --- a/ghost/session-service/test/SessionService.test.js +++ b/ghost/session-service/test/SessionService.test.js @@ -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);