Merge pull request 'Enforce TOTP second factor on SSO login (#31)' (#39) from bugfix/sso-totp-bypass-31 into main

Reviewed-on: UOM/website#39
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

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'),

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 (
-    <AuthContext.Provider value={{ user, loading, login, loginTotp, logout, refresh }}>
+    <AuthContext.Provider value={{ user, loading, login, loginTotp, ssoLoginTotp, logout, refresh }}>
       {children}
     </AuthContext.Provider>
   )

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 {
+      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)
+      }
     }
   }
 

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,
+}

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 }

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

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)
+})

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)
+})