From 07afa6500d032ae6d4eb22575bd679ca63d16a86 Mon Sep 17 00:00:00 2001 From: Sam Lord Date: Mon, 11 Nov 2024 22:26:40 +0000 Subject: [PATCH] Changed SSO adapter to automatically verify sessions (#21388) ref ENG-1680 SSO is a different flow that wouldn't need the extra email verification flow --- .../server/services/auth/session/index.js | 18 +++-- ghost/core/core/server/web/parent/backend.js | 2 +- .../admin/__snapshots__/sso.test.js.snap | 16 ++++ ghost/core/test/e2e-api/admin/sso.test.js | 77 +++++++++++++++++++ ghost/session-service/lib/session-service.js | 15 ++++ .../test/SessionService.test.js | 45 +++++++++++ 6 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 ghost/core/test/e2e-api/admin/__snapshots__/sso.test.js.snap create mode 100644 ghost/core/test/e2e-api/admin/sso.test.js diff --git a/ghost/core/core/server/services/auth/session/index.js b/ghost/core/core/server/services/auth/session/index.js index ad3bccc51e..c5ecc69362 100644 --- a/ghost/core/core/server/services/auth/session/index.js +++ b/ghost/core/core/server/services/auth/session/index.js @@ -59,15 +59,17 @@ const sessionService = createSessionService({ module.exports = createSessionMiddleware({sessionService}); -const ssoAdapter = adapterManager.getAdapter('sso'); // Looks funky but this is a "custom" piece of middleware -module.exports.createSessionFromToken = sessionFromToken({ - callNextWithError: false, - createSession: sessionService.createSessionForUser, - findUserByLookup: ssoAdapter.getUserForIdentity.bind(ssoAdapter), - getLookupFromToken: ssoAdapter.getIdentityFromCredentials.bind(ssoAdapter), - getTokenFromRequest: ssoAdapter.getRequestCredentials.bind(ssoAdapter) -}); +module.exports.createSessionFromToken = () => { + const ssoAdapter = adapterManager.getAdapter('sso'); + return sessionFromToken({ + callNextWithError: false, + createSession: sessionService.createVerifiedSessionForUser, + findUserByLookup: ssoAdapter.getUserForIdentity.bind(ssoAdapter), + getLookupFromToken: ssoAdapter.getIdentityFromCredentials.bind(ssoAdapter), + getTokenFromRequest: ssoAdapter.getRequestCredentials.bind(ssoAdapter) + }); +}; module.exports.sessionService = sessionService; module.exports.deleteAllSessions = expressSession.deleteAllSessions; diff --git a/ghost/core/core/server/web/parent/backend.js b/ghost/core/core/server/web/parent/backend.js index 66c1019340..46c484ce1e 100644 --- a/ghost/core/core/server/web/parent/backend.js +++ b/ghost/core/core/server/web/parent/backend.js @@ -15,7 +15,7 @@ module.exports = () => { backendApp.lazyUse(BASE_API_PATH, require('../api')); backendApp.lazyUse('/ghost/.well-known', require('../well-known')); - backendApp.use('/ghost', require('../../services/auth/session').createSessionFromToken, require('../admin')()); + backendApp.use('/ghost', require('../../services/auth/session').createSessionFromToken(), require('../admin')()); return backendApp; }; diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/sso.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/sso.test.js.snap new file mode 100644 index 0000000000..b3b92ea8de --- /dev/null +++ b/ghost/core/test/e2e-api/admin/__snapshots__/sso.test.js.snap @@ -0,0 +1,16 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`SSO API SSO with 2FA enabled can sign in with SSO when 2FA is enabled 1: [headers] 1`] = ` +Object { + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": 156, + "content-security-policy": "default-src 'none'", + "content-type": "text/html; charset=utf-8", + "set-cookie": Array [ + StringMatching /\\^ghost-admin-api-session=/, + ], + "vary": "Accept-Encoding", + "x-content-type-options": "nosniff", + "x-powered-by": "Express", +} +`; diff --git a/ghost/core/test/e2e-api/admin/sso.test.js b/ghost/core/test/e2e-api/admin/sso.test.js new file mode 100644 index 0000000000..068717f278 --- /dev/null +++ b/ghost/core/test/e2e-api/admin/sso.test.js @@ -0,0 +1,77 @@ +const {agentProvider, fixtureManager, matchers} = require('../../utils/e2e-framework'); +const {mockLabsEnabled, mockLabsDisabled, restore} = require('../../utils/e2e-framework-mock-manager'); +const {stringMatching} = matchers; +const sinon = require('sinon'); +const adapterManager = require('../../../core/server/services/adapter-manager'); +const models = require('../../../core/server/models'); + +describe('SSO API', function () { + let agent; + + before(async function () { + // Configure mock SSO adapter that always returns owner + const owner = await models.User.getOwnerUser(); + + // Create a mock adapter that always returns the owner + class MockSSOAdapter { + async getRequestCredentials() { + return { + id: 'mock-credentials' + }; + } + + async getIdentityFromCredentials() { + return { + id: 'mock-identity' + }; + } + + async getUserForIdentity() { + return owner; + } + } + + // Stub adapter manager to return mock SSO adapter + const originalGetAdapter = adapterManager.getAdapter; + sinon.stub(adapterManager, 'getAdapter').callsFake((name) => { + if (name === 'sso') { + return new MockSSOAdapter(); + } + return originalGetAdapter.call(this, name); + }); + + agent = await agentProvider.getGhostAPIAgent(); + await fixtureManager.init(); + }); + + after(function () { + restore(); + sinon.restore(); + }); + + describe('SSO with 2FA enabled', function () { + beforeEach(async function () { + mockLabsEnabled('staff2fa'); + }); + + afterEach(async function () { + mockLabsDisabled('staff2fa'); + }); + + it('can sign in with SSO when 2FA is enabled', async function () { + await agent + .post('/') + .expectEmptyBody() + .matchHeaderSnapshot({ + 'set-cookie': [ + stringMatching(/^ghost-admin-api-session=/) + ] + }); + + // Verify we can access authenticated endpoints after SSO login + await agent + .get('api/admin/users/me') + .expectStatus(200); + }); + }); +}); diff --git a/ghost/session-service/lib/session-service.js b/ghost/session-service/lib/session-service.js index 5dfa571934..da1ccf117d 100644 --- a/ghost/session-service/lib/session-service.js +++ b/ghost/session-service/lib/session-service.js @@ -41,6 +41,7 @@ totp.options = { * @prop {(req: Req, res: Res) => Promise} getUserForSession * @prop {(req: Req, res: Res) => Promise} removeUserForSession * @prop {(req: Req, res: Res, user: User) => Promise} createSessionForUser + * @prop {(req: Req, res: Res) => Promise} createVerifiedSessionForUser * @prop {(req: Req, res: Res) => Promise} verifySession * @prop {(req: Req, res: Res) => Promise} sendAuthCodeToUser * @prop {(req: Req, res: Res) => Promise} verifyAuthCodeForUser @@ -122,6 +123,19 @@ module.exports = function createSessionService({ } } + /** + * createVerifiedSessionForUser + * + * @param {Req} req + * @param {Res} res + * @param {User} user + * @returns {Promise} + */ + async function createVerifiedSessionForUser(req, res, user) { + await createSessionForUser(req, res, user); + await verifySession(req, res); + } + /** * generateAuthCodeForUser * @@ -328,6 +342,7 @@ module.exports = function createSessionService({ return { getUserForSession, createSessionForUser, + createVerifiedSessionForUser, removeUserForSession, verifySession, isVerifiedSession, diff --git a/ghost/session-service/test/SessionService.test.js b/ghost/session-service/test/SessionService.test.js index c841c11e1b..ed66b0b3b4 100644 --- a/ghost/session-service/test/SessionService.test.js +++ b/ghost/session-service/test/SessionService.test.js @@ -472,4 +472,49 @@ describe('SessionService', function () { message: 'Failed to send email. Please check your site configuration and try again.' }); }); + + it('Can create a verified session for SSO', async function () { + const getSession = async (req) => { + if (req.session) { + return req.session; + } + req.session = { + destroy: sinon.spy(cb => cb()) + }; + return req.session; + }; + const findUserById = sinon.spy(async ({id}) => ({id})); + const getOriginOfRequest = sinon.stub().returns('origin'); + const labs = { + isSet: () => false + }; + + const sessionService = SessionService({ + getSession, + findUserById, + getOriginOfRequest, + labs + }); + + const req = Object.create(express.request, { + ip: { + value: '0.0.0.0' + }, + headers: { + value: { + cookie: 'thing' + } + }, + get: { + value: () => 'Fake' + } + }); + const res = Object.create(express.response); + const user = {id: 'egg'}; + + await sessionService.createVerifiedSessionForUser(req, res, user); + + should.equal(req.session.user_id, 'egg'); + should.equal(req.session.verified, true); + }); });