fix(auth): give OIDC/SAML logins a real MFA challenge instead of a hard block (#536)

Fixes #533, found while working on #529/#531.

OIDC and SAML logins hard-blocked on the MFA policy with zero check of whether the user actually enrolled TOTP, and no challenge step at all. Once an admin turned on an MFA-required policy, every SSO user was permanently locked out regardless of enrollment status.

- Extract the post-auth MFA decision (challenge / enrollment-required / proceed) into a shared, unit-tested function so OIDC and SAML can't independently diverge again
- An already-enrolled user now gets a real challenge (reusing the existing, auth-method-agnostic MFA completion flow) instead of being blocked
- An unenrolled user under a required policy gets a distinct, correctly mapped error instead of the old generic one
- Fix a real fail-open regression caught in review: a transient DB error during the enrollment-status check could have silently skipped MFA entirely for an enrolled user; now it fails closed and logs
- Strip the one-time challenge token from the URL after consuming it

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<void> {

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<void> {
 
       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

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<void> {
 
       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)

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<HTMLInputElement>(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<string, string> = {
@@ -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();

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<string, unknown> = 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<void>((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<void>((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<string, unknown> | 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();
+    }
+  });
+});

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",
+      );
+    });
+  });
 });

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(
+    <MemoryRouter initialEntries={[path]}>
+      {onLocationChange && <LocationProbe onChange={onLocationChange} />}
+      <LoginPage />
+    </MemoryRouter>,
+  );
+}
+
+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(""));
+  });
+});