Enforce TOTP second factor on SSO login (#31)
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019rao86n5cXpwAyjdBFEshV
This commit is contained in:
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user