diff --git a/apps/api/src/plugins/mfa.ts b/apps/api/src/plugins/mfa.ts index 0b1d2fcc..314c657f 100644 --- a/apps/api/src/plugins/mfa.ts +++ b/apps/api/src/plugins/mfa.ts @@ -106,6 +106,22 @@ export function isMfaRequiredForUser(policy: MfaPolicy, userRole: string): boole return false; } +// The post-authentication MFA decision, shared by every login path (local +// password, OIDC, SAML) so a new auth method can't silently diverge from the +// others the way OIDC/SAML once did (they blocked on policy alone, with no +// totpEnabled check and no challenge step -- snapotter-hq/SnapOtter#533). +export type ExternalMfaOutcome = "proceed" | "challenge" | "enrollment_required"; + +export function resolveExternalLoginMfaOutcome( + policy: MfaPolicy, + userRole: string, + totpEnabled: boolean, +): ExternalMfaOutcome { + if (totpEnabled) return "challenge"; + if (isMfaRequiredForUser(policy, userRole)) return "enrollment_required"; + return "proceed"; +} + // ── MFA plugin registration ─────────────────────────────────────── export async function registerMfa(app: FastifyInstance): Promise { diff --git a/apps/api/src/plugins/oidc.ts b/apps/api/src/plugins/oidc.ts index d659cb61..6fddcf39 100644 --- a/apps/api/src/plugins/oidc.ts +++ b/apps/api/src/plugins/oidc.ts @@ -1,8 +1,11 @@ +import { randomUUID } from "node:crypto"; import type {} from "@fastify/cookie"; +import { eq } from "drizzle-orm"; import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; import * as oidc from "openid-client"; import { env } from "../config.js"; import { db, schema } from "../db/index.js"; +import { sharedRedis } from "../jobs/connection.js"; import { auditFromRequest, sanitizeAuditInput } from "../lib/audit.js"; import { resolveExternalUser, sanitizeUsername } from "../lib/external-auth-resolver.js"; import { authAttempts } from "../lib/metrics.js"; @@ -274,22 +277,62 @@ export async function oidcRoutes(app: FastifyInstance): Promise { const resolvedUser = result.user; - let mfaRequired = false; + // Unguarded on purpose: this read decides whether MFA gets checked at + // all, so a DB error here must fail the login, not silently skip MFA + // for an enrolled user. The try/catch below is scoped only to the + // optional MFA plugin/policy lookup, same as it always was. + let dbUser: { totpEnabled: boolean } | undefined; try { - const { getMfaPolicy, isMfaRequiredForUser } = await import("./mfa.js"); - const policy = await getMfaPolicy(); - mfaRequired = isMfaRequiredForUser(policy, resolvedUser.role); - } catch { - // MFA plugin not loaded - } - if (mfaRequired) { + [dbUser] = await db + .select({ totpEnabled: schema.users.totpEnabled }) + .from(schema.users) + .where(eq(schema.users.id, resolvedUser.id)); + } catch (err) { + request.log.error( + { err, userId: resolvedUser.id }, + "OIDC callback: failed to read MFA enrollment status", + ); authAttempts.inc({ method: "oidc", result: "failure" }); await audit("OIDC_LOGIN_FAILED", { userId: resolvedUser.id, username: resolvedUser.username, - reason: "mfa_required", + reason: "mfa_check_error", }); - return redirectToLogin(reply, "mfa_required"); + return redirectToLogin(reply, "oidc_auth_failed"); + } + + let mfaOutcome: "proceed" | "challenge" | "enrollment_required" = "proceed"; + try { + const { getMfaPolicy, resolveExternalLoginMfaOutcome } = await import("./mfa.js"); + const policy = await getMfaPolicy(); + mfaOutcome = resolveExternalLoginMfaOutcome( + policy, + resolvedUser.role, + dbUser?.totpEnabled ?? false, + ); + } catch { + // MFA plugin not loaded + } + + if (mfaOutcome === "challenge") { + const mfaToken = randomUUID(); + const redis = sharedRedis(); + await redis.setex(`mfa:${mfaToken}`, 300, resolvedUser.id); + await audit("MFA_CHALLENGE_ISSUED", { + userId: resolvedUser.id, + username: resolvedUser.username, + }); + return reply.redirect(`/login?mfaToken=${mfaToken}`); + } + + if (mfaOutcome === "enrollment_required") { + authAttempts.inc({ method: "oidc", result: "failure" }); + await audit("OIDC_LOGIN_FAILED", { + userId: resolvedUser.id, + username: resolvedUser.username, + reason: "mfa_enrollment_required", + }); + return redirectToLogin(reply, "mfa_enrollment_required"); } // 5. Create session diff --git a/apps/api/src/plugins/saml.ts b/apps/api/src/plugins/saml.ts index f5da1bc1..d3000b43 100644 --- a/apps/api/src/plugins/saml.ts +++ b/apps/api/src/plugins/saml.ts @@ -1,9 +1,12 @@ +import { randomUUID } from "node:crypto"; import { parse as parseQs } from "node:querystring"; import type {} from "@fastify/cookie"; import { SAML } from "@node-saml/node-saml"; +import { eq } from "drizzle-orm"; import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; import { env } from "../config.js"; import { db, schema } from "../db/index.js"; +import { sharedRedis } from "../jobs/connection.js"; import { auditFromRequest } from "../lib/audit.js"; import { findUniqueUsername, @@ -164,22 +167,62 @@ export async function registerSaml(app: FastifyInstance): Promise { const resolvedUser = result.user; - let mfaRequired = false; + // Unguarded on purpose: this read decides whether MFA gets checked at + // all, so a DB error here must fail the login, not silently skip MFA + // for an enrolled user. The try/catch below is scoped only to the + // optional MFA plugin/policy lookup, same as it always was. + let dbUser: { totpEnabled: boolean } | undefined; try { - const { getMfaPolicy, isMfaRequiredForUser } = await import("./mfa.js"); - const policy = await getMfaPolicy(); - mfaRequired = isMfaRequiredForUser(policy, resolvedUser.role); - } catch { - // MFA plugin not loaded - } - if (mfaRequired) { + [dbUser] = await db + .select({ totpEnabled: schema.users.totpEnabled }) + .from(schema.users) + .where(eq(schema.users.id, resolvedUser.id)); + } catch (err) { + request.log.error( + { err, userId: resolvedUser.id }, + "SAML callback: failed to read MFA enrollment status", + ); authAttempts.inc({ method: "saml", result: "failure" }); await audit("SAML_LOGIN_FAILED", { userId: resolvedUser.id, username: resolvedUser.username, - reason: "mfa_required", + reason: "mfa_check_error", }); - return redirectToLogin(reply, "mfa_required"); + return redirectToLogin(reply, "saml_auth_failed"); + } + + let mfaOutcome: "proceed" | "challenge" | "enrollment_required" = "proceed"; + try { + const { getMfaPolicy, resolveExternalLoginMfaOutcome } = await import("./mfa.js"); + const policy = await getMfaPolicy(); + mfaOutcome = resolveExternalLoginMfaOutcome( + policy, + resolvedUser.role, + dbUser?.totpEnabled ?? false, + ); + } catch { + // MFA plugin not loaded + } + + if (mfaOutcome === "challenge") { + const mfaToken = randomUUID(); + const redis = sharedRedis(); + await redis.setex(`mfa:${mfaToken}`, 300, resolvedUser.id); + await audit("MFA_CHALLENGE_ISSUED", { + userId: resolvedUser.id, + username: resolvedUser.username, + }); + return reply.redirect(`/login?mfaToken=${mfaToken}`); + } + + if (mfaOutcome === "enrollment_required") { + authAttempts.inc({ method: "saml", result: "failure" }); + await audit("SAML_LOGIN_FAILED", { + userId: resolvedUser.id, + username: resolvedUser.username, + reason: "mfa_enrollment_required", + }); + return redirectToLogin(reply, "mfa_enrollment_required"); } // Create session (same pattern as OIDC) diff --git a/apps/web/src/pages/login-page.tsx b/apps/web/src/pages/login-page.tsx index 26521fc1..6e27633a 100644 --- a/apps/web/src/pages/login-page.tsx +++ b/apps/web/src/pages/login-page.tsx @@ -129,7 +129,7 @@ function LanguageSelector() { export function LoginPage() { const { t } = useTranslation(); const { oidcEnabled, oidcProviderName, samlEnabled, samlProviderName, ssoEnforced } = useAuth(); - const [searchParams] = useSearchParams(); + const [searchParams, setSearchParams] = useSearchParams(); const [username, setUsername] = useState(""); const [password, setPassword] = useState(""); const [error, setError] = useState(""); @@ -141,6 +141,23 @@ export function LoginPage() { const mfaInputRef = useRef(null); useEffect(() => { + // A successful OIDC/SAML login for an already-enrolled user redirects + // here with a one-time mfaToken instead of completing the session + // directly, so the TOTP challenge can be completed the same way a local + // login's challenge is. + const redirectedMfaToken = searchParams.get("mfaToken"); + if (redirectedMfaToken) { + setMfaToken(redirectedMfaToken); + setShowMfaPrompt(true); + setTimeout(() => mfaInputRef.current?.focus(), 100); + // Drop it from the URL: it's a one-time credential and has no business + // sitting in browser history or a Referer header for the rest of the + // challenge. Also stops a later effect re-run (e.g. a locale switch) + // from reopening the prompt after the user has moved past it. + setSearchParams({}, { replace: true }); + return; + } + const authError = searchParams.get("error"); if (authError) { const errorMessages: Record = { @@ -152,10 +169,12 @@ export function LoginPage() { saml_auth_failed: t.auth.samlAuthFailed, saml_user_not_authorized: t.auth.samlUserNotAuthorized, saml_user_limit_reached: t.auth.samlUserLimitReached, + mfa_enrollment_required: t.auth.mfaEnrollmentRequired, }; setError(errorMessages[authError] || t.auth.oidcGenericError); + setSearchParams({}, { replace: true }); } - }, [searchParams, t]); + }, [searchParams, setSearchParams, t]); const handleSubmit = async (e: FormEvent) => { e.preventDefault(); diff --git a/tests/integration/platform/oidc-mfa-callback.test.ts b/tests/integration/platform/oidc-mfa-callback.test.ts new file mode 100644 index 00000000..c8b795d2 --- /dev/null +++ b/tests/integration/platform/oidc-mfa-callback.test.ts @@ -0,0 +1,337 @@ +/** + * End-to-end reproduction + regression coverage for the OIDC MFA gap + * (snapotter-hq/SnapOtter#533): before the fix, a successful OIDC login for a + * user under an MFA-required policy was unconditionally blocked, with no + * check of whether the user had actually enrolled TOTP and no challenge step. + * + * The real cryptographic token exchange (PKCE/nonce/JWT signature + * verification) is mocked at the `openid-client` boundary so this test can + * drive the REAL callback route, REAL cookie/state handling, REAL Redis + * challenge token, and REAL MFA decision code end to end without needing a + * full mock IdP with signed JWTs. + */ +import { randomUUID } from "node:crypto"; +import { createServer, type Server } from "node:http"; +import { eq } from "drizzle-orm"; +import * as OTPAuth from "otpauth"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; + +const authorizationCodeGrantMock = vi.hoisted(() => vi.fn()); + +vi.mock("openid-client", async (importOriginal) => { + const actual: Record = await importOriginal(); + return { ...actual, authorizationCodeGrant: authorizationCodeGrantMock }; +}); + +vi.resetModules(); +const { mockEnterpriseFeatures } = await import("../../helpers/enterprise-mock.js"); +mockEnterpriseFeatures(["mfa"]); + +const { env } = await import("../../../apps/api/src/config.js"); +const { db, schema } = await import("../../../apps/api/src/db/index.js"); +const { sharedRedis } = await import("../../../apps/api/src/jobs/connection.js"); +const { buildTestApp } = await import("../test-server.js"); + +import type { TestApp } from "../test-server.js"; + +let oidcApp: TestApp; +let mockServer: Server; +let mockPort: number; + +const origOidcEnabled = env.OIDC_ENABLED; +const origExternalUrl = env.EXTERNAL_URL; +const origIssuerUrl = env.OIDC_ISSUER_URL; +const origClientId = env.OIDC_CLIENT_ID; +const origClientSecret = env.OIDC_CLIENT_SECRET; + +async function startOidcLoginAndGetStateCookie() { + const loginRes = await oidcApp.app.inject({ method: "GET", url: "/api/auth/oidc/login" }); + expect(loginRes.statusCode).toBe(302); + const rawCookies = loginRes.headers["set-cookie"]; + const cookieStr = Array.isArray(rawCookies) ? rawCookies[0] : rawCookies || ""; + const cookieMatch = cookieStr.match(/oidc-state=([^;]+)/); + const cookieValue = decodeURIComponent(cookieMatch?.[1] ?? ""); + const redirectUrl = new URL(loginRes.headers.location as string); + const state = redirectUrl.searchParams.get("state") ?? ""; + return { cookieValue, state }; +} + +function generateTotpCode(uri: string): string { + const totp = OTPAuth.URI.parse(uri) as OTPAuth.TOTP; + return totp.generate(); +} + +async function insertOidcUser(opts: { role?: string; totpEnabled?: boolean } = {}) { + const externalId = `sub-${randomUUID()}`; + const userId = randomUUID(); + const username = `oidc_mfa_${Date.now()}_${Math.random().toString(36).slice(2, 6)}`; + await db.insert(schema.users).values({ + id: userId, + username, + passwordHash: null, + role: opts.role ?? "user", + team: "default-team-00000000", + mustChangePassword: false, + authProvider: "oidc", + externalId, + email: `${username}@example.com`, + totpEnabled: opts.totpEnabled ?? false, + }); + return { userId, username, externalId }; +} + +/** + * Inserts an OIDC user and drives them through the REAL self-service TOTP + * enrollment flow (enroll -> generate a real code -> verify), the same way + * `apps/web/src/components/settings/two-factor-settings.tsx` does it. This + * leaves a genuine, working, encrypted totpSecret in the DB -- not just a + * `totpEnabled: true` flag -- so a challenge issued for this user can + * actually be completed with a generated code, proving the OIDC-issued + * challenge is real and not just a Redis key that happens to exist. + */ +async function insertAndEnrollOidcUser(opts: { role?: string } = {}) { + const { userId, username, externalId } = await insertOidcUser({ + role: opts.role, + totpEnabled: false, + }); + + const sessionToken = randomUUID(); + await db.insert(schema.sessions).values({ + id: sessionToken, + userId, + expiresAt: new Date(Date.now() + 3_600_000), + }); + + const enrollRes = await oidcApp.app.inject({ + method: "POST", + url: "/api/auth/mfa/enroll", + headers: { authorization: `Bearer ${sessionToken}` }, + }); + const { uri } = JSON.parse(enrollRes.body) as { uri: string }; + + const verifyRes = await oidcApp.app.inject({ + method: "POST", + url: "/api/auth/mfa/verify", + headers: { authorization: `Bearer ${sessionToken}` }, + payload: { code: generateTotpCode(uri) }, + }); + expect(verifyRes.statusCode).toBe(200); + + // This bootstrap session isn't the one under test; drop it so it can't + // mask a bug where the OIDC callback fails to mint its own. + await db.delete(schema.sessions).where(eq(schema.sessions.id, sessionToken)); + + return { userId, username, externalId, totpUri: uri }; +} + +async function callbackAsUser(externalId: string) { + authorizationCodeGrantMock.mockResolvedValueOnce({ + claims: () => ({ sub: externalId, email: `${externalId}@example.com` }), + id_token: "fake-id-token", + }); + const { cookieValue, state } = await startOidcLoginAndGetStateCookie(); + return oidcApp.app.inject({ + method: "GET", + url: `/api/auth/oidc/callback?code=abc&state=${state}`, + cookies: { "oidc-state": cookieValue }, + }); +} + +async function setMfaPolicy(value: "optional" | "admins_only" | "required") { + await db + .insert(schema.settings) + .values({ key: "mfaPolicy", value }) + .onConflictDoUpdate({ target: schema.settings.key, set: { value } }); +} + +beforeAll(async () => { + mockServer = createServer((req, res) => { + if (req.url === "/.well-known/openid-configuration") { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + issuer: `http://localhost:${mockPort}`, + authorization_endpoint: `http://localhost:${mockPort}/authorize`, + token_endpoint: `http://localhost:${mockPort}/token`, + jwks_uri: `http://localhost:${mockPort}/jwks`, + response_types_supported: ["code"], + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], + code_challenge_methods_supported: ["S256"], + }), + ); + return; + } + if (req.url === "/jwks") { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ keys: [] })); + return; + } + res.writeHead(404); + res.end(); + }); + + await new Promise((resolve) => { + mockServer.listen(0, "127.0.0.1", () => { + const addr = mockServer.address(); + mockPort = typeof addr === "object" && addr ? addr.port : 0; + resolve(); + }); + }); + + (env as any).OIDC_ENABLED = true; + (env as any).EXTERNAL_URL = "http://localhost:9999"; + (env as any).OIDC_ISSUER_URL = `http://localhost:${mockPort}`; + (env as any).OIDC_CLIENT_ID = "test-client-id"; + (env as any).OIDC_CLIENT_SECRET = "test-client-secret"; + + oidcApp = await buildTestApp(); +}, 30_000); + +afterEach(async () => { + await setMfaPolicy("optional"); + authorizationCodeGrantMock.mockReset(); +}); + +afterAll(async () => { + (env as any).OIDC_ENABLED = origOidcEnabled; + (env as any).EXTERNAL_URL = origExternalUrl; + (env as any).OIDC_ISSUER_URL = origIssuerUrl; + (env as any).OIDC_CLIENT_ID = origClientId; + (env as any).OIDC_CLIENT_SECRET = origClientSecret; + await oidcApp.cleanup(); + await new Promise((resolve) => mockServer.close(() => resolve())); +}, 10_000); + +describe("OIDC callback MFA outcomes", () => { + it("creates a session directly when MFA is optional and the user isn't enrolled", async () => { + const { externalId } = await insertOidcUser({ totpEnabled: false }); + await setMfaPolicy("optional"); + + const res = await callbackAsUser(externalId); + + expect(res.statusCode).toBe(302); + expect(res.headers.location).toBe("/"); + const setCookie = res.headers["set-cookie"]; + expect(String(setCookie)).toContain("snapotter-session="); + }); + + it("blocks with a distinct enrollment-required error when policy requires MFA and the user hasn't enrolled -- this is the bug in #533", async () => { + const { externalId } = await insertOidcUser({ role: "user", totpEnabled: false }); + await setMfaPolicy("required"); + + const res = await callbackAsUser(externalId); + + expect(res.statusCode).toBe(302); + expect(res.headers.location).toBe("/login?error=mfa_enrollment_required"); + // Must NOT be the old generic code that the frontend doesn't even map to a message. + expect(res.headers.location).not.toBe("/login?error=mfa_required"); + const setCookie = res.headers["set-cookie"]; + expect(String(setCookie ?? "")).not.toContain("snapotter-session="); + }); + + it("issues an MFA challenge that is genuinely completable end to end with a real TOTP code", async () => { + const { username, externalId, totpUri } = await insertAndEnrollOidcUser({ role: "user" }); + await setMfaPolicy("required"); + + const res = await callbackAsUser(externalId); + + expect(res.statusCode).toBe(302); + const location = res.headers.location as string; + expect(location).toMatch(/^\/login\?mfaToken=/); + const setCookie = res.headers["set-cookie"]; + expect(String(setCookie ?? "")).not.toContain("snapotter-session="); + + const mfaToken = location.split("mfaToken=")[1]; + const redis = sharedRedis(); + expect(await redis.get(`mfa:${mfaToken}`)).toBeTruthy(); + + // Actually complete it -- this is the real end-to-end proof, not just + // that a Redis key exists. + const completeRes = await oidcApp.app.inject({ + method: "POST", + url: "/api/auth/mfa/complete", + payload: { mfaToken, code: generateTotpCode(totpUri) }, + }); + expect(completeRes.statusCode).toBe(200); + const body = JSON.parse(completeRes.body); + expect(body.token).toBeDefined(); + expect(body.user.username).toBe(username); + }); + + it("rejects completing an OIDC-issued challenge with an invalid code, and does not create a session", async () => { + const { externalId } = await insertAndEnrollOidcUser({ role: "user" }); + await setMfaPolicy("required"); + + const res = await callbackAsUser(externalId); + const mfaToken = (res.headers.location as string).split("mfaToken=")[1]; + + const completeRes = await oidcApp.app.inject({ + method: "POST", + url: "/api/auth/mfa/complete", + payload: { mfaToken, code: "000000" }, + }); + expect(completeRes.statusCode).toBe(401); + expect(JSON.parse(completeRes.body).code).toBe("INVALID_CODE"); + }); + + it("rejects completing with an unknown/expired mfaToken", async () => { + const completeRes = await oidcApp.app.inject({ + method: "POST", + url: "/api/auth/mfa/complete", + payload: { mfaToken: randomUUID(), code: "123456" }, + }); + expect(completeRes.statusCode).toBe(401); + expect(JSON.parse(completeRes.body).code).toBe("MFA_EXPIRED"); + }); + + it("challenges an enrolled user even when policy is optional (enrollment beats policy)", async () => { + const { externalId } = await insertAndEnrollOidcUser({}); + await setMfaPolicy("optional"); + + const res = await callbackAsUser(externalId); + + const location = res.headers.location as string; + expect(location).toMatch(/^\/login\?mfaToken=/); + }); + + it("does not block a user outside the policy's scope (admins_only, non-admin role)", async () => { + const { externalId } = await insertOidcUser({ role: "user", totpEnabled: false }); + await setMfaPolicy("admins_only"); + + const res = await callbackAsUser(externalId); + + expect(res.headers.location).toBe("/"); + }); + + it("fails closed (does not create a session) when the MFA enrollment-status query throws", async () => { + const { externalId } = await insertOidcUser({ role: "user", totpEnabled: true }); + await setMfaPolicy("required"); + + const originalSelect = db.select.bind(db); + const selectSpy = vi.spyOn(db, "select").mockImplementation((...args: unknown[]) => { + const selection = args[0] as Record | undefined; + if (selection && "totpEnabled" in selection) { + throw new Error("simulated DB failure"); + } + // biome-ignore lint/suspicious/noExplicitAny: passthrough to the real overloaded implementation + return (originalSelect as any)(...args); + }); + + try { + const res = await callbackAsUser(externalId); + + // Must NOT silently proceed without MFA and must NOT issue a challenge + // either -- a broken enrollment-status read means the login fails, + // full stop, not "let them in" or "pretend they're unenrolled". + expect(res.statusCode).toBe(302); + const location = res.headers.location as string; + expect(location).not.toBe("/"); + expect(location).not.toMatch(/^\/login\?mfaToken=/); + const setCookie = res.headers["set-cookie"]; + expect(String(setCookie ?? "")).not.toContain("snapotter-session="); + } finally { + selectSpy.mockRestore(); + } + }); +}); diff --git a/tests/unit/api/mfa.test.ts b/tests/unit/api/mfa.test.ts index 24857863..d45094d2 100644 --- a/tests/unit/api/mfa.test.ts +++ b/tests/unit/api/mfa.test.ts @@ -4,6 +4,7 @@ import { createTotp, hashRecoveryCodes, isMfaRequiredForUser, + resolveExternalLoginMfaOutcome, verifyRecoveryCode, verifyTotpCode, } from "../../../apps/api/src/plugins/mfa.js"; @@ -188,4 +189,35 @@ describe("MFA", () => { expect(isMfaRequiredForUser("admins_only", "user")).toBe(false); }); }); + + describe("resolveExternalLoginMfaOutcome", () => { + // Exhaustive over the full input space: 3 policies x 2 roles x 2 + // totpEnabled states. Role only matters via isMfaRequiredForUser, which + // branches solely on role === "admin", so {admin, user} covers it. + it.each([ + ["optional", "user", false, "proceed"], + ["optional", "user", true, "challenge"], + ["optional", "admin", false, "proceed"], + ["optional", "admin", true, "challenge"], + ["admins_only", "user", false, "proceed"], + ["admins_only", "user", true, "challenge"], + ["admins_only", "admin", false, "enrollment_required"], + ["admins_only", "admin", true, "challenge"], + // required+user+enrolled is the exact shape of the #533 bug: an + // enrolled non-admin user under a required policy must be challenged, + // not hard-blocked. + ["required", "user", false, "enrollment_required"], + ["required", "user", true, "challenge"], + ["required", "admin", false, "enrollment_required"], + ["required", "admin", true, "challenge"], + ] as const)("policy=%s role=%s totpEnabled=%s -> %s", (policy, role, totpEnabled, expected) => { + expect(resolveExternalLoginMfaOutcome(policy, role, totpEnabled)).toBe(expected); + }); + + it("enrollment takes priority: an enrolled user is challenged even under a required policy", () => { + expect(resolveExternalLoginMfaOutcome("required", "admin", true)).not.toBe( + "enrollment_required", + ); + }); + }); }); diff --git a/tests/unit/web/login-page-oidc-mfa-redirect.test.tsx b/tests/unit/web/login-page-oidc-mfa-redirect.test.tsx new file mode 100644 index 00000000..bd940802 --- /dev/null +++ b/tests/unit/web/login-page-oidc-mfa-redirect.test.tsx @@ -0,0 +1,79 @@ +// @vitest-environment jsdom + +import "@testing-library/jest-dom/vitest"; +import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import { MemoryRouter, useLocation } from "react-router-dom"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +const useAuth = vi.hoisted(() => vi.fn()); + +vi.mock("@/hooks/use-auth", () => ({ useAuth })); + +import { LoginPage } from "@/pages/login-page"; + +afterEach(() => { + cleanup(); + useAuth.mockReset(); +}); + +function LocationProbe({ onChange }: { onChange: (search: string) => void }) { + const location = useLocation(); + onChange(location.search); + return null; +} + +function renderLoginPage(path: string, onLocationChange?: (search: string) => void) { + useAuth.mockReturnValue({ + oidcEnabled: true, + oidcProviderName: "Test IdP", + samlEnabled: false, + samlProviderName: null, + ssoEnforced: false, + }); + return render( + + {onLocationChange && } + + , + ); +} + +describe("LoginPage OIDC/SAML MFA redirect handling", () => { + it("shows the TOTP prompt automatically when redirected back with an mfaToken", () => { + renderLoginPage("/login?mfaToken=abc-123"); + expect(screen.getByText(/enter your authentication code/i)).toBeInTheDocument(); + expect(screen.getByPlaceholderText("000000")).toBeInTheDocument(); + }); + + it("shows the enrollment-required message for the mfa_enrollment_required error code", () => { + renderLoginPage("/login?error=mfa_enrollment_required"); + expect(screen.getByText(/multi-factor authentication/i)).toBeInTheDocument(); + }); + + it("does not show the TOTP prompt for an empty mfaToken param", () => { + renderLoginPage("/login?mfaToken="); + expect(screen.queryByPlaceholderText("000000")).not.toBeInTheDocument(); + }); + + it("prioritizes an mfaToken over a simultaneous error param", () => { + renderLoginPage("/login?mfaToken=abc-123&error=oidc_auth_failed"); + expect(screen.getByText(/enter your authentication code/i)).toBeInTheDocument(); + expect(screen.queryByText(/authentication error/i)).not.toBeInTheDocument(); + }); + + it("strips mfaToken from the URL after consuming it", async () => { + let currentSearch = ""; + renderLoginPage("/login?mfaToken=abc-123", (search) => { + currentSearch = search; + }); + await waitFor(() => expect(currentSearch).toBe("")); + }); + + it("strips the error param from the URL after consuming it", async () => { + let currentSearch = ""; + renderLoginPage("/login?error=mfa_enrollment_required", (search) => { + currentSearch = search; + }); + await waitFor(() => expect(currentSearch).toBe("")); + }); +});