From 9b8b31c864d03f51b180bab7c8404bda857920f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 14 Jul 2026 11:46:40 +0200 Subject: [PATCH] fix(auth): OIDC sessions never satisfy MFA step-up; 2FA accounts get the challenge step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two second-factor gaps in the OIDC callback: The session was minted with mfaVerifiedAt stamped to the login instant, letting a bare SSO login satisfy requireFreshMfa step-up (MFA disable, encryption-key rotation, encrypted export, account deletion). OIDC is a delegated factor — whatever the IdP checked, this app did not see a second factor — so the stamp is now null, the same treatment a trusted-device login gets. An account with a confirmed TOTP secret or a registered security key was logged straight in, silently downgrading its native 2FA to IdP-only. The callback now mints the same single-use MFA ticket password login uses and hands it to the login page through a short-lived, login-page-scoped cookie (the callback ends on a browser redirect, so the ticket cannot ride an envelope meta like the password flow). The login page picks it up once, clears it, and renders the same challenge step; the login completes at /api/auth/mfa/verify with identical continuation semantics. The cookie is deliberately not httpOnly — the login page script must read the ticket, exactly the exposure the password flow already has — and the ticket alone carries no session authority (single-use, hashed at rest, 5-minute TTL, attempt-capped). Direct (factor-less) OIDC logins now also sync the admin-enforced-MFA enrollment hint cookie, matching password login. --- .../oidc/callback/__tests__/route.test.ts | 86 +++++++++++++++++++ src/app/api/auth/oidc/callback/route.ts | 78 +++++++++++++++-- src/app/auth/login/page.tsx | 30 +++++++ src/lib/auth/oidc-cookie.ts | 17 ++++ 4 files changed, 206 insertions(+), 5 deletions(-) diff --git a/src/app/api/auth/oidc/callback/__tests__/route.test.ts b/src/app/api/auth/oidc/callback/__tests__/route.test.ts index fbc327556..fd48ea383 100644 --- a/src/app/api/auth/oidc/callback/__tests__/route.test.ts +++ b/src/app/api/auth/oidc/callback/__tests__/route.test.ts @@ -28,12 +28,25 @@ vi.mock("@/lib/db", () => ({ update: vi.fn(), }, appSettings: { findUnique: vi.fn() }, + webauthnMfaCredential: { count: vi.fn() }, }, })); vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() })); vi.mock("@/lib/auth/session", () => ({ createSession: vi.fn() })); vi.mock("@/lib/auth/login-alert", () => ({ recordSignInDevice: vi.fn() })); +vi.mock("@/lib/auth/mfa/challenge", () => ({ + createMfaChallenge: vi.fn(async () => ({ + ticket: "mfa-ticket-1", + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + })), +})); +vi.mock("@/lib/auth/mfa-enrollment", () => ({ + syncMfaEnrollCookie: vi.fn(), +})); +vi.mock("@/lib/auth/secure-cookie", () => ({ + shouldEmitSecureCookie: () => false, +})); vi.mock("@/lib/tz/resolver", () => ({ resolveServerDefaultTimezone: vi.fn(async () => "Europe/Berlin"), })); @@ -54,6 +67,7 @@ import { checkAuthSurfaceRateLimit } from "@/lib/rate-limit"; import { prisma } from "@/lib/db"; import { auditLog } from "@/lib/auth/audit"; import { createSession } from "@/lib/auth/session"; +import { createMfaChallenge } from "@/lib/auth/mfa/challenge"; import { getOidcConfig, discoverOidcMetadata, @@ -160,6 +174,11 @@ beforeEach(() => { } as never); vi.mocked(getOidcConfig).mockReturnValue(CONFIG); vi.mocked(discoverOidcMetadata).mockResolvedValue(METADATA); + vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(0); + vi.mocked(createMfaChallenge).mockResolvedValue({ + ticket: "mfa-ticket-1", + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + }); }); describe("GET /api/auth/oidc/callback", () => { @@ -368,6 +387,73 @@ describe("GET /api/auth/oidc/callback", () => { ); }); + it("never stamps the session step-up fresh — the 5th createSession arg is null", async () => { + mockIdentity(); + mockUserLookups({ byIdentity: null, byEmail: userRow() }); + vi.mocked(prisma.user.update).mockResolvedValue( + userRow({ oidcIssuer: METADATA.issuer, oidcSub: "sub-1" }) as never, + ); + + await GET(validRequest()); + + expect(createSession).toHaveBeenCalledWith( + "user-1", + false, + "1.2.3.4", + null, + null, + ); + }); + + it("routes a TOTP-enrolled account through the MFA challenge instead of logging in", async () => { + mockIdentity(); + mockUserLookups({ + byIdentity: userRow({ + oidcIssuer: METADATA.issuer, + oidcSub: "sub-1", + totpConfirmedAt: new Date(), + }), + }); + + const res = await GET(validRequest("/dashboard")); + + // No session — the partial state lives in the single-use ticket. + expect(createSession).not.toHaveBeenCalled(); + expect(createMfaChallenge).toHaveBeenCalledWith("user-1", "login"); + + const location = res.headers.get("location")!; + expect(location).toContain("/auth/login"); + expect(location).not.toContain("error="); + // `next` continuation rides the query string into the login page. + expect(location).toContain(`next=${encodeURIComponent("/dashboard")}`); + + // The ticket + methods ride the login-page-scoped handoff cookie. + const setCookie = res.headers.getSetCookie().join("\n"); + expect(setCookie).toContain("oidc_mfa_handoff="); + expect(setCookie).toContain(encodeURIComponent("mfa-ticket-1")); + expect(setCookie.toLowerCase()).toContain("path=/auth/login"); + expect(decodeURIComponent(setCookie)).toContain('"totp"'); + }); + + it("offers only the security-key method for a WebAuthn-only account", async () => { + mockIdentity(); + mockUserLookups({ + byIdentity: userRow({ + oidcIssuer: METADATA.issuer, + oidcSub: "sub-1", + totpConfirmedAt: null, + }), + }); + vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(1); + + const res = await GET(validRequest()); + + expect(createSession).not.toHaveBeenCalled(); + const setCookie = decodeURIComponent(res.headers.getSetCookie().join("\n")); + expect(setCookie).toContain('"webauthn"'); + expect(setCookie).not.toContain('"totp"'); + }); + it("redirects with oidc_failed when the token exchange throws", async () => { vi.mocked(exchangeCodeForTokens).mockRejectedValue(new Error("network")); const res = await GET(validRequest()); diff --git a/src/app/api/auth/oidc/callback/route.ts b/src/app/api/auth/oidc/callback/route.ts index 8a348151e..f1d8bca08 100644 --- a/src/app/api/auth/oidc/callback/route.ts +++ b/src/app/api/auth/oidc/callback/route.ts @@ -17,7 +17,15 @@ import { getOidcRedirectUri, verifyIdToken, } from "@/lib/auth/oidc"; -import { OIDC_STATE_COOKIE } from "@/lib/auth/oidc-cookie"; +import { + OIDC_MFA_COOKIE, + OIDC_MFA_COOKIE_PATH, + OIDC_MFA_TTL_MS, + OIDC_STATE_COOKIE, +} from "@/lib/auth/oidc-cookie"; +import { createMfaChallenge } from "@/lib/auth/mfa/challenge"; +import { syncMfaEnrollCookie } from "@/lib/auth/mfa-enrollment"; +import { shouldEmitSecureCookie } from "@/lib/auth/secure-cookie"; import { resolveServerDefaultTimezone } from "@/lib/tz/resolver"; const LOGIN_ERROR_URL = "/auth/login"; @@ -256,16 +264,76 @@ export const GET = apiHandler(async (req: NextRequest) => { } } + // Native second factor. OIDC is a DELEGATED factor: it proves the IdP + // authenticated someone, but it never substitutes for this app's own + // second factor. An account with a confirmed TOTP secret or a + // registered security key gets the same MFA challenge step password + // login gets — no session is minted here; the single-use ticket rides + // the handoff cookie to the login page, which renders the same + // challenge UI and completes at `/api/auth/mfa/verify`. (The + // trusted-device skip cannot apply on this path: its cookie is + // SameSite=Strict, which the browser withholds on an IdP-initiated + // redirect chain.) + const hasTotp = Boolean(user.totpConfirmedAt); + const webauthnKeyCount = await prisma.webauthnMfaCredential.count({ + where: { userId: user.id }, + }); + const hasWebauthn = webauthnKeyCount > 0; + + if (hasTotp || hasWebauthn) { + const challenge = await createMfaChallenge(user.id, "login"); + const methods: ("totp" | "recovery" | "webauthn")[] = []; + if (hasTotp) methods.push("totp", "recovery"); + if (hasWebauthn) methods.push("webauthn"); + await auditLog("auth.mfa.challenge", { + userId: user.id, + ipAddress: ip, + details: { source: "login.oidc" }, + }); + annotate({ + action: { name: "auth.mfa.challenge" }, + meta: { mfa_required: true }, + }); + const loginUrl = new URL(LOGIN_ERROR_URL, req.url); + if (stored.next !== "/") { + loginUrl.searchParams.set("next", stored.next); + } + const response = NextResponse.redirect(loginUrl); + response.cookies.delete(OIDC_STATE_COOKIE); + response.cookies.set( + OIDC_MFA_COOKIE, + JSON.stringify({ ticket: challenge.ticket, methods }), + { + // Not httpOnly by design — see the constant's doc comment. + httpOnly: false, + secure: shouldEmitSecureCookie(), + sameSite: "lax", + maxAge: Math.floor(OIDC_MFA_TTL_MS / 1000), + path: OIDC_MFA_COOKIE_PATH, + }, + ); + return response; + } + const ua = req.headers.get("user-agent"); - // The IdP already performed its own authentication — OIDC login - // satisfies HealthLog's own MFA step-up, the same treatment passkey - // login gives its own factor. + + // v1.23 parity with password login — the admin-enforced-MFA hint cookie + // sends a single-factor account into forced enrollment after sign-in. + await syncMfaEnrollCookie(user.id, { + totpConfirmedAt: user.totpConfirmedAt, + mfaEnforced: user.mfaEnforced, + }); + + // The 5th argument stays null: OIDC is a delegated factor, so an SSO + // session must never satisfy `requireFreshMfa` step-up (MFA disable, + // encryption-key rotation, encrypted export, account deletion) — the + // same treatment a trusted-device login gets. await createSession( user.id, user.onboardingCompletedAt === null, ip, ua, - new Date(), + null, ); void recordSignInDevice({ userId: user.id, ip, userAgent: ua }); diff --git a/src/app/auth/login/page.tsx b/src/app/auth/login/page.tsx index 17db0b74f..87e6a6ead 100644 --- a/src/app/auth/login/page.tsx +++ b/src/app/auth/login/page.tsx @@ -20,6 +20,7 @@ import { apiPost, } from "@/lib/api/api-fetch"; import { MfaLoginStep, type MfaMethod } from "@/components/auth/mfa-login-step"; +import { OIDC_MFA_COOKIE, OIDC_MFA_COOKIE_PATH } from "@/lib/auth/oidc-cookie"; import { isDashboardSnapshotEnabled } from "@/lib/dashboard/snapshot-flag"; import { prefetchDashboardSnapshot } from "@/lib/queries/use-dashboard-snapshot"; import { clearOfflineCachesForSessionEnd } from "@/lib/pwa/query-persister"; @@ -218,6 +219,35 @@ export default function LoginPage() { } } + // OIDC → second-factor handoff. The OAuth callback cannot render UI: for + // a 2FA-enrolled account it mints the same single-use MFA ticket password + // login uses and parks it in a short-lived, login-page-scoped cookie + // (`oidc-cookie.ts` documents why it is readable here). Pick it up once, + // clear it, and enter the exact same MfaLoginStep the password flow uses — + // `next` continuation rides the query string as usual. + useEffect(() => { + const raw = document.cookie + .split("; ") + .find((c) => c.startsWith(`${OIDC_MFA_COOKIE}=`)) + ?.slice(OIDC_MFA_COOKIE.length + 1); + if (!raw) return; + document.cookie = `${OIDC_MFA_COOKIE}=; path=${OIDC_MFA_COOKIE_PATH}; max-age=0`; + try { + const parsed = JSON.parse(decodeURIComponent(raw)) as { + ticket?: string; + methods?: MfaMethod[]; + }; + if (parsed.ticket) { + setMfaChallenge({ + ticket: parsed.ticket, + methods: parsed.methods ?? ["totp", "recovery"], + }); + } + } catch { + // Malformed handoff — fall through to the normal login UI. + } + }, []); + // v1.23 — passkey conditional-UI autofill. When the browser supports it, // arm a discoverable-credential assertion bound to the username field so a // returning user can pick their passkey straight from the autofill prompt. diff --git a/src/lib/auth/oidc-cookie.ts b/src/lib/auth/oidc-cookie.ts index 3aae3d1e8..8a860905a 100644 --- a/src/lib/auth/oidc-cookie.ts +++ b/src/lib/auth/oidc-cookie.ts @@ -5,3 +5,20 @@ */ export const OIDC_STATE_COOKIE = "oidc_auth_state"; export const OIDC_STATE_TTL_MS = 10 * 60 * 1000; + +/** + * MFA handoff between `/api/auth/oidc/callback` (writer) and the login page + * (reader). When the OIDC account has a native second factor, the callback + * mints the same single-use MFA ticket password login uses — but it ends on + * a browser redirect, not a JSON response, so the ticket travels in this + * short-lived, login-page-scoped cookie instead of an envelope `meta`. + * Deliberately NOT httpOnly: the login page's script must read the ticket — + * exactly the exposure the password flow already has, where the ticket + * arrives in a JSON body read by the same script. The ticket alone carries + * no session authority (single-use, hashed at rest, ~5-minute TTL, + * attempt-capped — see `src/lib/auth/mfa/challenge.ts`). + */ +export const OIDC_MFA_COOKIE = "oidc_mfa_handoff"; +export const OIDC_MFA_COOKIE_PATH = "/auth/login"; +/** Mirrors MFA_CHALLENGE_TTL_MS — the cookie dies with the ticket. */ +export const OIDC_MFA_TTL_MS = 5 * 60 * 1000;