fix(auth): OIDC sessions never satisfy MFA step-up; 2FA accounts get the challenge step

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.
This commit is contained in:
Marc-André Bombeck committed 2026-07-14 11:46:40 +02:00
1 parent fa326c398a
commit 9b8b31c864
4 files changed
+206 -5

No files matched your search

@@ -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());
+73 -5
View File
@@ -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 });
+30
View File
@@ -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.
+17
View File
@@ -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;