From 03e62b56ad47b95587700cfb228a1fccd3ac7b5e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 22:17:35 -0500 Subject: [PATCH] Enforce TOTP second factor on SSO login (#31) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SSO login minted a full session immediately, ignoring the account's totp_enabled flag — so a 2FA admin with a linked Google/Discord/OIDC identity could sign in without their authenticator code, silently downgrading the account to single-factor (the strength of the IdP login). The local password flow already gates on needsTotp(); SSO did not. Wire SSO through the same staged-TOTP gate: - ssoState: createTotpPending/verifyTotpPending + a short-lived httpOnly sso_totp cookie. The pending token carries stage:'totp' (session validation rejects it) + kind:'sso_totp' (scoped to the SSO endpoint) plus the resolved context (userId, provider, authMethod, returnTo). - sso.controller: finishLogin now stages the challenge and redirects to /admin/login?sso_totp=1 instead of creating a session when the account has TOTP on. New finishSsoTotp verifies the code (backoff + bot-scoring on failure, mirroring loginTotp) and only then mints the session. - sso.routes: POST /auth/sso/totp behind the same backoff/slow/limiter stack and code validation as the local TOTP endpoint. - client: AdminLogin detects ?sso_totp=1 and completes over fetch via api.ssoLoginTotp; the challenge never touches the URL or JS. Keeps the second factor httpOnly throughout, consistent with the SSO tx cookie. 12 new tests; full suite 106/106. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019rao86n5cXpwAyjdBFEshV --- client/src/api/client.js | 3 + client/src/contexts/AuthContext.jsx | 10 ++- client/src/routes/admin/AdminLogin.jsx | 33 ++++++++-- server/src/auth/ssoState.js | 44 ++++++++++++- server/src/router/v1/auth/sso.controller.js | 68 ++++++++++++++++++- server/src/router/v1/auth/sso.routes.js | 27 +++++++- server/test/ssoCallback.test.js | 73 +++++++++++++++++++++ server/test/ssoState.test.js | 34 +++++++++- 8 files changed, 281 insertions(+), 11 deletions(-) diff --git a/client/src/api/client.js b/client/src/api/client.js index fd027ba..1c87b58 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -46,6 +46,9 @@ export const api = { req('/auth/login', { method: 'POST', body: { username, password, ...extra } }), loginTotp: (challenge, code) => req('/auth/login/totp', { method: 'POST', body: { challenge, code } }), + // Second factor for an SSO login (challenge is held in an httpOnly cookie set by + // the callback, so only the code is sent). Returns { user, returnTo }. + ssoLoginTotp: (code) => req('/auth/sso/totp', { method: 'POST', body: { code } }), logout: () => req('/auth/logout', { method: 'POST' }), // Public SSO provider discovery — drives the login-page provider buttons. authProviders: () => req('/auth/providers'), diff --git a/client/src/contexts/AuthContext.jsx b/client/src/contexts/AuthContext.jsx index 5a78b14..42f25ee 100644 --- a/client/src/contexts/AuthContext.jsx +++ b/client/src/contexts/AuthContext.jsx @@ -37,6 +37,14 @@ export function AuthProvider({ children }) { return data.user }, []) + // Step 2 for SSO logins whose account has 2FA on. The pending challenge lives in + // an httpOnly cookie, so only the code is sent. Returns { user, returnTo }. + const ssoLoginTotp = useCallback(async (code) => { + const data = await api.ssoLoginTotp(code) + setUser(data.user) + return data + }, []) + const logout = useCallback(async () => { try { await api.logout() @@ -46,7 +54,7 @@ export function AuthProvider({ children }) { }, []) return ( - + {children} ) diff --git a/client/src/routes/admin/AdminLogin.jsx b/client/src/routes/admin/AdminLogin.jsx index 0954c8d..73ec8d2 100644 --- a/client/src/routes/admin/AdminLogin.jsx +++ b/client/src/routes/admin/AdminLogin.jsx @@ -31,7 +31,7 @@ const honeypotStyle = { } export default function AdminLogin() { - const { user, login, loginTotp } = useAuth() + const { user, login, loginTotp, ssoLoginTotp } = useAuth() const navigate = useNavigate() const location = useLocation() const dest = location.state?.from?.pathname || '/admin' @@ -42,10 +42,12 @@ export default function AdminLogin() { const [error, setError] = useState('') const [busy, setBusy] = useState(false) - // Two-factor step state. + // Two-factor step state. `ssoTotp` marks the SSO variant: the challenge lives in + // an httpOnly cookie (not React state), so the code posts to a different endpoint. const [stage, setStage] = useState('creds') // 'creds' | 'totp' const [challenge, setChallenge] = useState('') const [code, setCode] = useState('') + const [ssoTotp, setSsoTotp] = useState(false) // SSO providers to offer (empty if none configured) + any error the callback // bounced us back with (?sso_error=...). @@ -57,6 +59,16 @@ export default function AdminLogin() { if (user) navigate(dest, { replace: true }) }, [user, dest, navigate]) + // The SSO callback bounces 2FA accounts back here with ?sso_totp=1 after the IdP + // step: it has staged an httpOnly TOTP challenge and needs the authenticator code + // before it will issue a session. Jump straight to the code step. + useEffect(() => { + if (new URLSearchParams(location.search).get('sso_totp')) { + setStage('totp') + setSsoTotp(true) + } + }, [location.search]) + // Load enabled SSO providers for the buttons. Failure is non-fatal — the page // still works with password login and simply shows no provider buttons. useEffect(() => { @@ -101,16 +113,25 @@ export default function AdminLogin() { setError('') setBusy(true) try { - await loginTotp(challenge, code) - navigate(dest, { replace: true }) + if (ssoTotp) { + const { returnTo } = await ssoLoginTotp(code) + navigate(returnTo || '/admin', { replace: true }) + } else { + await loginTotp(challenge, code) + navigate(dest, { replace: true }) + } } catch (err) { + const expired = err.status === 401 && /expired/i.test(err.message) setError( - err.status === 401 && /expired/i.test(err.message) + expired ? 'Your verification session expired. Please sign in again.' : 'Invalid verification code.', ) setBusy(false) - if (err.status === 401 && /expired/i.test(err.message)) setStage('creds') + if (expired) { + setStage('creds') + setSsoTotp(false) + } } } diff --git a/server/src/auth/ssoState.js b/server/src/auth/ssoState.js index 33ef40a..9bbf2d5 100644 --- a/server/src/auth/ssoState.js +++ b/server/src/auth/ssoState.js @@ -19,6 +19,14 @@ const token = require('./token') const TX_COOKIE = 'sso_tx' const TX_TTL = '10m' // a login round-trip is quick; abandon after 10 minutes +// Second leg of an SSO login for an account that has TOTP enabled. The callback +// authenticated the user with the IdP but must NOT bypass their second factor +// (see issue #31), so instead of minting a session it stages this signed, +// httpOnly cookie and routes the browser through the TOTP form — mirroring the +// local password→TOTP gate. TTL matches the local challenge window. +const TOTP_COOKIE = 'sso_totp' +const TOTP_TTL = '5m' + // base64url of random bytes — used for the nonce and the PKCE verifier. function randomUrlSafe(bytes = 32) { return crypto.randomBytes(bytes).toString('base64url') @@ -56,4 +64,38 @@ function verifyTx(txToken, stateNonce) { return decoded } -module.exports = { TX_COOKIE, TX_TTL, createTx, verifyTx, codeChallengeFor, randomUrlSafe } +// Stage the pending second factor for an SSO login. Carries the context the +// callback already resolved (userId, provider, authMethod, returnTo) so that +// presenting a valid code alone finishes the login. It is deliberately NOT a +// session: `stage: 'totp'` makes session validation reject it (same marker the +// local TOTP challenge uses), and `kind: 'sso_totp'` both reinforces that and +// scopes it to the SSO completion endpoint. +function createTotpPending({ userId, provider, authMethod, returnTo }) { + return token.signToken( + { id: userId }, // subject only; identity is re-loaded fresh when the code is verified + { stage: 'totp', kind: 'sso_totp', provider, authMethod, returnTo }, + { expiresIn: TOTP_TTL }, + ) +} + +// Verify a pending-TOTP cookie. Returns the payload +// ({ id, provider, authMethod, returnTo, ... }) or null if missing/expired/wrong-kind. +function verifyTotpPending(pendingToken) { + if (!pendingToken) return null + const decoded = token.verifyToken(pendingToken) + if (!decoded || decoded.stage !== 'totp' || decoded.kind !== 'sso_totp') return null + return decoded +} + +module.exports = { + TX_COOKIE, + TX_TTL, + TOTP_COOKIE, + TOTP_TTL, + createTx, + verifyTx, + createTotpPending, + verifyTotpPending, + codeChallengeFor, + randomUrlSafe, +} diff --git a/server/src/router/v1/auth/sso.controller.js b/server/src/router/v1/auth/sso.controller.js index 9775702..b10606b 100644 --- a/server/src/router/v1/auth/sso.controller.js +++ b/server/src/router/v1/auth/sso.controller.js @@ -19,6 +19,10 @@ const registry = require('../../../auth/providers/registry') const sessionService = require('../../../auth/session.service') const ssoState = require('../../../auth/ssoState') const token = require('../../../auth/token') +const totp = require('../../../utils/totp') +const botScore = require('../../../middleware/botScore') +const loginProtection = require('../../../middleware/loginProtection') +const { needsTotp } = require('./auth.controller') const log = require('../../../utils/logger')('sso') @@ -56,6 +60,13 @@ function txCookieOptions(req) { return { ...token.cookieOptions(req), maxAge: 10 * 60 * 1000 } } +// httpOnly cookie carrying the staged pending-TOTP token for the second-factor +// step. Same standard options; TTL matches the token so a stale cookie can't +// outlive the challenge it holds. +function totpCookieOptions(req) { + return { ...token.cookieOptions(req), maxAge: 5 * 60 * 1000 } +} + // GET /auth/providers — public discovery. Never touches secrets. async function listProviders(req, res) { try { @@ -148,6 +159,23 @@ async function finishLogin(req, res, providerId, kind, tx, profile) { if (!user) return res.redirect(loginError('not_linked')) const authMethod = sessionService.AUTH_METHODS.includes(kind) ? kind : 'sso' + + // 2FA parity with the local login (auth.controller): if the account has TOTP + // enabled, an SSO sign-in must NOT bypass the second factor. Stage a signed, + // httpOnly challenge and route the browser through the TOTP form instead of + // minting a session here. See issue #31. + if (needsTotp(user)) { + const pending = ssoState.createTotpPending({ + userId: user.id, + provider: providerId, + authMethod, + returnTo: sanitizeReturn(tx.returnTo) || undefined, + }) + res.cookie(ssoState.TOTP_COOKIE, pending, totpCookieOptions(req)) + log.info('sso login: awaiting TOTP', { provider: providerId, id: user.id, ip: req.ip }) + return res.redirect('/admin/login?sso_totp=1') + } + const { token: sessionToken } = sessionService.createSession(user, authMethod) token.setAuthCookie(req, res, sessionToken) await users.recordLogin(user.id) @@ -156,6 +184,44 @@ async function finishLogin(req, res, providerId, kind, tx, profile) { return res.redirect(sanitizeReturn(tx.returnTo) || '/admin') } +// POST /auth/sso/totp — second factor for an SSO login whose account has TOTP on. +// Reads the staged pending-TOTP cookie, verifies the authenticator code, then +// mints the full session. Mirrors auth.controller.loginTotp: a wrong code is a +// failed attempt (backoff + bot score), and the response is JSON (the login page +// completes this step over fetch and then navigates to returnTo). +async function finishSsoTotp(req, res) { + const pending = ssoState.verifyTotpPending(req.cookies && req.cookies[ssoState.TOTP_COOKIE]) + if (!pending) { + return res.status(401).json({ message: 'Your verification session expired. Please sign in again.' }) + } + try { + const user = await users.getRawById(pending.id) + if (!user || !user.totp_enabled || !totp.verifyCode(user.totp_secret, req.body.code)) { + botScore.recordLoginFailure(req.ip) + loginProtection.recordFailure(req.ip) + log.warn('sso TOTP verify failed', { id: pending.id, ip: req.ip }) + return res.status(401).json({ message: 'Invalid verification code.' }) + } + + // Second factor satisfied — clear the staged cookie and issue the real session. + res.clearCookie(ssoState.TOTP_COOKIE, token.cookieOptions(req)) + loginProtection.recordSuccess(req.ip) + const authMethod = sessionService.AUTH_METHODS.includes(pending.authMethod) ? pending.authMethod : 'sso' + const { token: sessionToken } = sessionService.createSession(user, authMethod) + token.setAuthCookie(req, res, sessionToken) + await users.recordLogin(user.id) + await activity.log({ req, userId: user.id, action: 'auth.sso.login', detail: { provider: pending.provider, totp: true } }) + log.info('sso login success (2fa)', { provider: pending.provider, id: user.id, ip: req.ip }) + return res.json({ + user: { id: user.id, username: user.username, role: user.role }, + returnTo: sanitizeReturn(pending.returnTo) || '/admin', + }) + } catch (err) { + log.error('sso totp error', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + // Attach the external identity to the account that initiated linking (tx.linkUserId // was captured behind requireAuth at /link start, so the signed tx authorizes it). async function finishLink(req, res, providerId, tx, profile) { @@ -173,4 +239,4 @@ async function finishLink(req, res, providerId, tx, profile) { return res.redirect(`/admin/account?linked=${providerId}`) } -module.exports = { listProviders, start, linkStart, callback, beginFlow, finishLogin, finishLink } +module.exports = { listProviders, start, linkStart, callback, beginFlow, finishLogin, finishSsoTotp, finishLink } diff --git a/server/src/router/v1/auth/sso.routes.js b/server/src/router/v1/auth/sso.routes.js index 40b6549..bf30848 100644 --- a/server/src/router/v1/auth/sso.routes.js +++ b/server/src/router/v1/auth/sso.routes.js @@ -1,11 +1,18 @@ const express = require('express') +const { body } = require('express-validator') const ctrl = require('./sso.controller') const { requireAuth } = require('../../../auth/session.middleware') -const { ssoStartLimiter } = require('../../../middleware/rateLimit') +const { ssoStartLimiter, loginLimiter } = require('../../../middleware/rateLimit') +const { slowLogin, backoffGuard } = require('../../../middleware/loginProtection') +const validate = require('../../../middleware/validate') const ssoRouter = express.Router() +// Same throttling stack the local login/TOTP endpoints use — the SSO TOTP step is +// a code-guessing surface too (cheapest rejection first). +const loginGuards = [backoffGuard, slowLogin, loginLimiter] + // Public discovery — the login page reads this to render provider buttons. ssoRouter.get( '/providers', @@ -57,4 +64,22 @@ ssoRouter.get( ctrl.callback, ) +// Second factor for an SSO login whose account has TOTP enabled. The callback +// stages an httpOnly pending-TOTP cookie and bounces the browser to the login +// page (?sso_totp=1); the page posts the code here to finish and receive a session. +ssoRouter.post( + '/sso/totp', + // #swagger.tags = ['Auth · SSO'] + // #swagger.summary = 'Complete an SSO login with a TOTP code' + // #swagger.description = 'Second step when a linked account has 2FA enabled. Reads the staged pending-TOTP cookie set by the callback plus the current authenticator code, and on success sets the session cookie. Rate limited and behind bot/backoff guards.' + /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { type: "object", required: ["code"], properties: { code: { type: "string" } } } } } } */ + /* #swagger.responses[200] = { description: 'Session issued', content: { "application/json": { schema: { type: "object", properties: { user: { $ref: "#/components/schemas/User" }, returnTo: { type: "string" } } } } } } */ + /* #swagger.responses[401] = { description: 'Invalid code or expired challenge', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[429] = { description: 'Too many attempts (rate limited / backoff)', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + ...loginGuards, + body('code').isString().trim().isLength({ min: 6, max: 8 }), + validate, + ctrl.finishSsoTotp, +) + module.exports = ssoRouter diff --git a/server/test/ssoCallback.test.js b/server/test/ssoCallback.test.js index 1c96429..ca46f74 100644 --- a/server/test/ssoCallback.test.js +++ b/server/test/ssoCallback.test.js @@ -15,6 +15,7 @@ const activity = require('../src/model/activity/activity.model') const authProviders = require('../src/model/authProviders/authProviders.model') const userIdentities = require('../src/model/userIdentities/userIdentities.model') const registry = require('../src/auth/providers/registry') +const totp = require('../src/utils/totp') const db = require('../src/utils/db') after(() => db.close()) @@ -114,3 +115,75 @@ test('bad state (CSRF) → rejected before any provider work', async () => { assert.equal(res.redirectedTo, '/admin/login?sso_error=bad_state') assert.equal(res.cookies[token.COOKIE_NAME], undefined) }) + +// ── 2FA parity: SSO must not bypass TOTP (issue #31) ──────────────────────── + +test('linked account with TOTP → staged challenge, NO session, routed to TOTP', async () => { + userIdentities.findByProviderSubject = async () => ({ user_id: 7 }) + users.getById = async (id) => ({ id, username: 'alice', role: 'admin', totp_enabled: 1 }) + const tx = ssoState.createTx({ provider: 'google', mode: 'login', returnTo: '/admin/posts' }) + const res = mockRes() + await ssoCtrl.callback(makeReq(tx), res) + + assert.equal(res.cookies[token.COOKIE_NAME], undefined, 'no full session before 2FA') + assert.ok(res.cookies[ssoState.TOTP_COOKIE], 'pending-TOTP cookie staged') + assert.equal(res.redirectedTo, '/admin/login?sso_totp=1') + assert.equal(logged.length, 0, 'login not logged until the second factor passes') + // The staged cookie carries the resolved context and is not a usable session. + const pending = ssoState.verifyTotpPending(res.cookies[ssoState.TOTP_COOKIE]) + assert.equal(pending.id, 7) + assert.equal(pending.provider, 'google') + assert.equal(pending.returnTo, '/admin/posts') +}) + +function makeTotpReq(pendingToken, code) { + return { + cookies: pendingToken ? { [ssoState.TOTP_COOKIE]: pendingToken } : {}, + body: { code }, + ip: '127.0.0.1', protocol: 'http', get: () => 'localhost', headers: {}, + } +} + +test('finishSsoTotp: correct code → session issued, pending cookie cleared, login logged', async () => { + users.getRawById = async (id) => ({ id, username: 'alice', role: 'admin', totp_enabled: 1, totp_secret: 'S' }) + totp.verifyCode = () => true + const pending = ssoState.createTotpPending({ userId: 7, provider: 'google', authMethod: 'google', returnTo: '/admin/posts' }) + const res = mockRes() + await ssoCtrl.finishSsoTotp(makeTotpReq(pending, '123456'), res) + + assert.ok(res.cookies[token.COOKIE_NAME], 'session cookie set after 2FA') + assert.ok(res.cleared.includes(ssoState.TOTP_COOKIE), 'pending-TOTP cookie cleared') + assert.equal(res.body.returnTo, '/admin/posts') + assert.equal(res.body.user.id, 7) + assert.equal(logged.at(-1).action, 'auth.sso.login') + assert.equal(logged.at(-1).detail.totp, true) +}) + +test('finishSsoTotp: wrong code → 401, no session', async () => { + users.getRawById = async (id) => ({ id, username: 'alice', role: 'admin', totp_enabled: 1, totp_secret: 'S' }) + totp.verifyCode = () => false + const pending = ssoState.createTotpPending({ userId: 7, provider: 'google', authMethod: 'google' }) + const res = mockRes() + await ssoCtrl.finishSsoTotp(makeTotpReq(pending, '000000'), res) + + assert.equal(res.statusCode, 401) + assert.match(res.body.message, /Invalid verification code/) + assert.equal(res.cookies[token.COOKIE_NAME], undefined) +}) + +test('finishSsoTotp: missing/expired pending cookie → 401 expired', async () => { + const res = mockRes() + await ssoCtrl.finishSsoTotp(makeTotpReq(null, '123456'), res) + assert.equal(res.statusCode, 401) + assert.match(res.body.message, /expired/i) + assert.equal(res.cookies[token.COOKIE_NAME], undefined) +}) + +test('finishSsoTotp: a local /login/totp challenge is not accepted here', async () => { + // A stage:'totp' token without kind:'sso_totp' must be rejected by this endpoint. + const localChallenge = token.signTotpChallenge({ id: 7 }) + const res = mockRes() + await ssoCtrl.finishSsoTotp(makeTotpReq(localChallenge, '123456'), res) + assert.equal(res.statusCode, 401) + assert.match(res.body.message, /expired/i) +}) diff --git a/server/test/ssoState.test.js b/server/test/ssoState.test.js index abd1dce..e64c6cd 100644 --- a/server/test/ssoState.test.js +++ b/server/test/ssoState.test.js @@ -1,10 +1,18 @@ +// Point the DB at a closed port before requiring the auth layer: session.service +// (used by one test below) pulls in the users model → db pool at load, and an idle +// pool would keep this process alive. None of these tests touch the database. process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-secret' +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' -const { test } = require('node:test') +const { test, after } = require('node:test') const assert = require('node:assert/strict') const crypto = require('crypto') const ssoState = require('../src/auth/ssoState') +const db = require('../src/utils/db') + +after(() => db.close()) test('createTx → verifyTx round-trips the flow payload', () => { const tx = ssoState.createTx({ provider: 'google', mode: 'login', returnTo: '/admin/posts' }) @@ -36,3 +44,27 @@ test('verifyTx rejects a non-tx token', () => { const notTx = token.signToken({ id: 1, username: 'a', role: 'admin' }) assert.equal(ssoState.verifyTx(notTx, 'anything'), null) }) + +test('createTotpPending → verifyTotpPending round-trips the SSO 2FA context', () => { + const pending = ssoState.createTotpPending({ userId: 7, provider: 'google', authMethod: 'google', returnTo: '/admin/posts' }) + const payload = ssoState.verifyTotpPending(pending) + assert.ok(payload) + assert.equal(payload.id, 7) + assert.equal(payload.provider, 'google') + assert.equal(payload.authMethod, 'google') + assert.equal(payload.returnTo, '/admin/posts') + assert.equal(payload.stage, 'totp') +}) + +test('a pending-TOTP token is NOT accepted as a session (stage + kind reject it)', () => { + const sessionService = require('../src/auth/session.service') + const pending = ssoState.createTotpPending({ userId: 7, provider: 'google', authMethod: 'google' }) + assert.equal(sessionService.decodeIdentity(pending), null) +}) + +test('verifyTotpPending rejects a plain session and a bare TOTP challenge', () => { + const token = require('../src/auth/token') + assert.equal(ssoState.verifyTotpPending(token.signToken({ id: 1, username: 'a', role: 'admin' })), null) + assert.equal(ssoState.verifyTotpPending(token.signTotpChallenge({ id: 1 })), null) + assert.equal(ssoState.verifyTotpPending(null), null) +})