diff --git a/src/app/api/auth/me/mfa/totp/setup/route.ts b/src/app/api/auth/me/mfa/totp/setup/route.ts index e1ac624ea..9c94d919e 100644 --- a/src/app/api/auth/me/mfa/totp/setup/route.ts +++ b/src/app/api/auth/me/mfa/totp/setup/route.ts @@ -16,8 +16,10 @@ * factor that later satisfies every step-up gate, so a stolen session must not * be able to mint one: the cookie arm also passes `checkCookieEnrollmentProof` * — a sign-in or second factor inside the last five minutes, or a password / - * factor proof in the body (the step-up mint's shapes). The Bearer arm already - * carries that proof in its elevation. The recovery-code batch is + * factor proof in the body (the step-up mint's shapes); on an account that + * already has a second factor, only that factor or a passkey. The Bearer arm + * carries the proof in its elevation, under the same rule: on an account with + * a second factor the elevation must come from one. The recovery-code batch is * issued at `/confirm` (after the factor is proven), not here, so an abandoned * setup never persists codes. */ @@ -41,7 +43,7 @@ const SETUP_RATE_LIMIT = 5; const SETUP_WINDOW_MS = 15 * 60 * 1000; export const POST = apiHandler(async (req: Request) => { - const auth = await requireMfaManagementAuth(); + const auth = await requireMfaManagementAuth({ freshFactorIfEnrolled: true }); const { user } = auth; const rl = await checkRateLimit( diff --git a/src/app/api/auth/me/mfa/webauthn/register/options/route.ts b/src/app/api/auth/me/mfa/webauthn/register/options/route.ts index 998c2f3b5..ff09e4f79 100644 --- a/src/app/api/auth/me/mfa/webauthn/register/options/route.ts +++ b/src/app/api/auth/me/mfa/webauthn/register/options/route.ts @@ -8,7 +8,10 @@ * * The cookie arm additionally passes `checkCookieEnrollmentProof`: a session * alone cannot add a security key, for the same reason it cannot enroll TOTP — - * the new key would satisfy every step-up gate from then on. + * the new key would satisfy every step-up gate from then on. On an account that + * already has a second factor, both arms want that factor (or a passkey): a + * password, or a session that skipped the factor on a remembered browser, does + * not add a key. */ import { apiHandler, requireMfaManagementAuth } from "@/lib/api-handler"; import { apiError, apiSuccess, getClientIp } from "@/lib/api-response"; @@ -20,7 +23,7 @@ import { createMfaRegistrationOptions } from "@/lib/auth/mfa/webauthn"; export const dynamic = "force-dynamic"; export const POST = apiHandler(async (req: Request) => { - const auth = await requireMfaManagementAuth(); + const auth = await requireMfaManagementAuth({ freshFactorIfEnrolled: true }); const { user } = auth; const rl = await checkRateLimit( diff --git a/src/app/api/auth/passkey/register-options/route.ts b/src/app/api/auth/passkey/register-options/route.ts index ac848880c..b72bfff13 100644 --- a/src/app/api/auth/passkey/register-options/route.ts +++ b/src/app/api/auth/passkey/register-options/route.ts @@ -5,10 +5,16 @@ import { } from "@/lib/auth/passkey"; import { createMfaAuthenticationOptions } from "@/lib/auth/mfa/webauthn"; import { + isSecondFactorProof, recordReproofFailure, + refundReproof, throttleReproof, verifyExistingFactorProof, } from "@/lib/auth/existing-factor-proof"; +import { + hasSecondFactorEnrolled, + recentProofMethods, +} from "@/lib/auth/second-factor"; import { prisma } from "@/lib/db"; import { apiError, @@ -55,6 +61,25 @@ export const POST = apiHandler(async (request: NextRequest) => { const parsed = stepUpMintSchema.safeParse(body); if (!parsed.success) return proofRequired(); + // On an account with a second factor a password does not add a sign-in + // credential, exactly as it does not satisfy the second factor at sign-in: + // a passkey registered on the strength of the password alone would from then + // on sign in without the second factor at all. Refused before a guess is + // spent, with the proofs that would work. + if ( + !isSecondFactorProof(parsed.data.method) && + (await hasSecondFactorEnrolled(user)) + ) { + annotate({ + action: { name: "auth.reproof.method_refused" }, + meta: { stage: "passkey_enroll", method: parsed.data.method }, + }); + return apiError("Confirm with your second factor or a passkey", 422, { + errorCode: "auth.reproof.too_weak", + methods: await recentProofMethods(user, true), + }); + } + // Every proof counts against the account's shared re-proof bucket and every // refusal is audited. Without both, a stolen session could guess the account // password or a TOTP code here as fast as it could send requests. @@ -73,6 +98,7 @@ export const POST = apiHandler(async (request: NextRequest) => { ); return proofRequired(); } + await refundReproof(user.id); // A strong existing factor refreshes the session's MFA stamp. Password proof // deliberately clears it: registration is authorized by this single-use, diff --git a/src/app/api/auth/password/__tests__/route.test.ts b/src/app/api/auth/password/__tests__/route.test.ts index e13ecff29..557ac34e8 100644 --- a/src/app/api/auth/password/__tests__/route.test.ts +++ b/src/app/api/auth/password/__tests__/route.test.ts @@ -50,6 +50,7 @@ vi.mock("@/lib/auth/password", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn(), + refundRateLimit: vi.fn(), })); vi.mock("@/lib/i18n/server-locale", () => ({ @@ -77,7 +78,7 @@ vi.mock("next/headers", () => ({ import { POST } from "../route"; import { prisma } from "@/lib/db"; import { getSession } from "@/lib/auth/session"; -import { checkRateLimit } from "@/lib/rate-limit"; +import { checkRateLimit, refundRateLimit } from "@/lib/rate-limit"; import { verifyPassword, checkPasswordStrength } from "@/lib/auth/password"; import { requireFreshMfaIfEnrolled, @@ -146,6 +147,9 @@ describe("POST /api/auth/password — MFA step-up (v1.25)", () => { expect(res.status).toBe(200); expect(requireFreshMfaIfEnrolled).toHaveBeenCalledWith(expect.any(Number)); expect(prisma.user.update).toHaveBeenCalled(); + // The right current password was not a guess: the shared re-proof + // bucket gets its slot back. + expect(refundRateLimit).toHaveBeenCalledWith("auth:step-up:user-1"); }); }); diff --git a/src/app/api/auth/password/route.ts b/src/app/api/auth/password/route.ts index 47906bc9f..a8935d912 100644 --- a/src/app/api/auth/password/route.ts +++ b/src/app/api/auth/password/route.ts @@ -16,6 +16,7 @@ import { } from "@/lib/api-response"; import { recordReproofFailure, + refundReproof, throttleReproof, } from "@/lib/auth/existing-factor-proof"; import { @@ -72,6 +73,10 @@ export const POST = apiHandler(async (request: NextRequest) => { ); return apiError("Current password is incorrect", 401); } + // The current password was right, so this attempt was not a guess. Give the + // shared re-proof budget its slot back; otherwise a password change followed + // by a few confirmations elsewhere runs the owner out of attempts. + await refundReproof(user.id); if (currentPassword === newPassword) { return apiError("New password must differ from current password", 422); diff --git a/src/app/api/auth/reproof/route.ts b/src/app/api/auth/reproof/route.ts index 0f39e7d13..f01203d96 100644 --- a/src/app/api/auth/reproof/route.ts +++ b/src/app/api/auth/reproof/route.ts @@ -52,6 +52,7 @@ import { annotate } from "@/lib/logging/context"; import { REPROOF_FAILED_CODE, recordReproofFailure, + refundReproof, throttleReproof, verifyExistingFactorProof, } from "@/lib/auth/existing-factor-proof"; @@ -110,6 +111,8 @@ export const POST = apiHandler(async (request: NextRequest) => { }); } + await refundReproof(user.id); + const now = new Date(); await prisma.session.update({ where: { id: session.id }, diff --git a/src/components/settings/security-section/existing-factor-reauth-dialog.tsx b/src/components/settings/security-section/existing-factor-reauth-dialog.tsx index 88fd9c3b7..553fba98f 100644 --- a/src/components/settings/security-section/existing-factor-reauth-dialog.tsx +++ b/src/components/settings/security-section/existing-factor-reauth-dialog.tsx @@ -44,6 +44,28 @@ export function isReproofRequired(err: unknown): boolean { ); } +const REAUTH_METHODS: readonly ReauthMethod[] = [ + "password", + "totp", + "passkey", + "webauthn", +]; + +/** + * The proofs the refusal says this account can give, or null when it names + * none. On an account with a second factor the server leaves the password out, + * so the dialog does not offer a choice that can only be refused. + */ +export function offeredReauthMethods(err: unknown): ReauthMethod[] | null { + if (!(err instanceof ApiError)) return null; + const raw = err.meta?.methods; + if (!Array.isArray(raw)) return null; + const offered = raw.filter((m): m is ReauthMethod => + REAUTH_METHODS.includes(m as ReauthMethod), + ); + return offered.length > 0 ? offered : null; +} + /** * Collect a fresh proof of a credential the account already holds, before a * new factor is added. A passkey or security-key assertion begins at the diff --git a/src/components/settings/security-section/security-keys-card.tsx b/src/components/settings/security-section/security-keys-card.tsx index deeaca438..7dcb374f2 100644 --- a/src/components/settings/security-section/security-keys-card.tsx +++ b/src/components/settings/security-section/security-keys-card.tsx @@ -36,6 +36,7 @@ import { formatDate } from "@/lib/format"; import { ExistingFactorReauthDialog, isReproofRequired, + offeredReauthMethods, type ExistingFactorProof, type ReauthMethod, } from "./existing-factor-reauth-dialog"; @@ -98,8 +99,14 @@ export function SecurityKeysCard({ // options call is retried with it from the dialog. const [reauthOpen, setReauthOpen] = useState(false); const [reauthError, setReauthError] = useState(null); - const reauthMethods: ReauthMethod[] = [ - "password", + // An account with a second factor confirms with that factor or a passkey; + // the server refuses a password there and names what it will take. + const hasSecondFactor = totpEnabled || keys.length > 0; + const [offeredMethods, setOfferedMethods] = useState( + null, + ); + const reauthMethods: ReauthMethod[] = offeredMethods ?? [ + ...(hasSecondFactor ? [] : (["password"] as const)), ...(totpEnabled ? (["totp"] as const) : []), "passkey", ...(keys.length > 0 ? (["webauthn"] as const) : []), @@ -126,6 +133,7 @@ export function SecurityKeysCard({ }, onError: (err, proof) => { if (isReproofRequired(err)) { + setOfferedMethods(offeredReauthMethods(err)); setError(null); setReauthError(null); setReauthOpen(true); diff --git a/src/components/settings/security-section/totp-card.tsx b/src/components/settings/security-section/totp-card.tsx index e35a43d40..7495a9c93 100644 --- a/src/components/settings/security-section/totp-card.tsx +++ b/src/components/settings/security-section/totp-card.tsx @@ -36,7 +36,9 @@ import { RecoveryCodesPanel } from "./recovery-codes-panel"; import { ExistingFactorReauthDialog, isReproofRequired, + offeredReauthMethods, type ExistingFactorProof, + type ReauthMethod, } from "./existing-factor-reauth-dialog"; interface SetupData { @@ -128,6 +130,11 @@ export function TotpCard({ // call is retried with it from the dialog. const [reauthOpen, setReauthOpen] = useState(false); const [reauthError, setReauthError] = useState(null); + const [reauthMethods, setReauthMethods] = useState([ + "password", + "passkey", + "webauthn", + ]); const beginSetup = useMutation({ mutationFn: async (proof?: ExistingFactorProof) => { @@ -148,6 +155,8 @@ export function TotpCard({ }, onError: (err, proof) => { if (isReproofRequired(err)) { + const offered = offeredReauthMethods(err); + if (offered) setReauthMethods(offered); setError(null); setReauthError(null); setReauthOpen(true); @@ -278,7 +287,7 @@ export function TotpCard({ setReauthOpen(open); if (!open) setReauthError(null); }} - methods={["password", "passkey", "webauthn"]} + methods={reauthMethods} pending={beginSetup.isPending} error={reauthError} onProof={(proof) => beginSetup.mutate(proof)} diff --git a/src/lib/__tests__/require-recent-proof.test.ts b/src/lib/__tests__/require-recent-proof.test.ts index 5f3716fb2..8c0d09fd1 100644 --- a/src/lib/__tests__/require-recent-proof.test.ts +++ b/src/lib/__tests__/require-recent-proof.test.ts @@ -38,7 +38,11 @@ vi.mock("next/headers", () => ({ })), })); -import { requireRecentProof, StepUpRequiredError } from "../api-handler"; +import { + requireMfaManagementAuth, + requireRecentProof, + StepUpRequiredError, +} from "../api-handler"; import { getSession } from "@/lib/auth/session"; import { prisma } from "@/lib/db"; import { headers } from "next/headers"; @@ -219,3 +223,44 @@ describe("Bearer", () => { await refusal(requireRecentProof({ bearer: "elevation" })); }); }); + +describe("adding a factor over Bearer (`freshFactorIfEnrolled`)", () => { + const passwordElevation = { + userId: "user-1", + apiTokenId: "tok-1", + consumedAt: null, + expiresAt: new Date(Date.now() + 60_000), + method: "password", + }; + + it("a password-proved elevation still adds a first factor", async () => { + bearer(PLAIN_USER, `hle_${"a".repeat(64)}`); + vi.mocked(prisma.stepUpElevation.findUnique).mockResolvedValue( + passwordElevation as never, + ); + const auth = await requireMfaManagementAuth({ + freshFactorIfEnrolled: true, + }); + expect(auth.transport).toBe("bearer"); + }); + + it("a password-proved elevation does not add a factor beside an existing one", async () => { + bearer(MFA_USER, `hle_${"a".repeat(64)}`); + vi.mocked(prisma.stepUpElevation.findUnique).mockResolvedValue( + passwordElevation as never, + ); + await refusal(requireMfaManagementAuth({ freshFactorIfEnrolled: true })); + }); + + it("a second-factor elevation does", async () => { + bearer(MFA_USER, `hle_${"a".repeat(64)}`); + vi.mocked(prisma.stepUpElevation.findUnique).mockResolvedValue({ + ...passwordElevation, + method: "totp", + } as never); + const auth = await requireMfaManagementAuth({ + freshFactorIfEnrolled: true, + }); + expect(auth.transport).toBe("bearer"); + }); +}); diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index db1087f5a..cf464dea2 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -12,6 +12,13 @@ import { annotate, eventStorage, getEvent } from "./logging/context"; import { emitIfSampled } from "./logging/transports"; import { redactOptional, redactSecrets } from "./logging/redact"; import { getSession } from "./auth/session"; +import { + hasSecondFactorEnrolled, + recentProofMethods, + type RecentProofMethod, +} from "./auth/second-factor"; + +export { recentProofMethods, type RecentProofMethod }; import { auditLog, recordDelegatedAccess } from "./auth/audit"; import { resolveBearerToken, @@ -1388,13 +1395,6 @@ export async function requireFreshMfaOrElevationIfEnrolled( return { ...auth, commitElevation }; } -/** - * A proof the recent-proof gate can ask for. `password` and `passkey` on an - * account without a second factor; `totp`, `webauthn` and `passkey` on one - * with a second factor, where a password alone has never been enough. - */ -export type RecentProofMethod = "password" | "totp" | "webauthn" | "passkey"; - /** * How the recent-proof gate treats a Bearer caller. * @@ -1508,63 +1508,6 @@ export async function assertRecentCookieProof( ); } -/** - * The proofs that would satisfy {@link assertRecentCookieProof} for this - * account. Also what `POST /api/auth/reproof` accepts, so the dialog and the - * endpoint cannot disagree. - */ -export async function recentProofMethods( - user: Pick, - enrolled?: boolean, -): Promise { - const hasSecond = enrolled ?? (await hasSecondFactorEnrolled(user)); - const [keys, passkeys] = await Promise.all([ - prisma.webauthnMfaCredential.count({ where: { userId: user.id } }), - prisma.passkey.count({ where: { userId: user.id } }), - ]); - const methods: RecentProofMethod[] = []; - if (hasSecond) { - if (user.totpConfirmedAt) methods.push("totp"); - if (keys > 0) methods.push("webauthn"); - } else if (user.passwordHash) { - methods.push("password"); - } - if (passkeys > 0) methods.push("passkey"); - return methods; -} - -/** - * Does this account have a SECOND FACTOR at all? - * - * Either factor enrols it: a confirmed TOTP secret OR a registered WebAuthn - * security key. A webauthn-only account must clear step-up too, so every - * boundary that asks this question tracks `requireFreshMfa`'s either-factor - * rule rather than reading `totpConfirmedAt` alone. It was written out three - * times before it was a function, which is two more places for the second arm - * to be forgotten. - * - * DELIBERATELY DOES NOT COUNT A PRIMARY PASSKEY, even though a passkey login - * stamps `mfaVerifiedAt` and could therefore satisfy the gate. This predicate - * answers "is there a second factor", and that is the question - * `requireFreshMfaIfEnrolled` asks before deciding whether account deletion, the - * data reset, the password change, the encrypted export and key rotation demand - * a step-up at all. Counting a passkey here would silently pull every - * passkey-holding account into a gate those routes have never applied to them — - * a behaviour change to five destructive actions, made in passing, for the - * benefit of a sixth. {@link canProveFreshFactor} is where the wider question - * lives. - */ -async function hasSecondFactorEnrolled(user: { - id: string; - totpConfirmedAt: Date | null; -}): Promise { - if (user.totpConfirmedAt) return true; - const webauthnKeyCount = await prisma.webauthnMfaCredential.count({ - where: { userId: user.id }, - }); - return webauthnKeyCount > 0; -} - /** * Can this account produce a fresh-factor proof at all? * @@ -1719,10 +1662,17 @@ export type MfaManagementContext = { * set. Passkey removal passes `"any-possession"` because a primary passkey IS * the proof there — see {@link FreshFactorProofSource}. It changes only the * reachability pre-check; the gate itself is identical either way. + * @param options.freshFactorIfEnrolled the Bearer arm of ADDING a factor + * (TOTP setup, security-key registration). On an account that already has a + * second factor, the elevation must come from a second-factor or passkey + * proof, the rule `checkCookieEnrollmentProof` holds on the cookie arm; on + * one without, a password-proved elevation is still enough. The cookie arm + * is not changed by it: those routes run their own enrollment proof there. */ export async function requireMfaManagementAuth( options: { freshFactor?: boolean; + freshFactorIfEnrolled?: boolean; proofSource?: FreshFactorProofSource; } = {}, ): Promise { @@ -1747,9 +1697,13 @@ export async function requireMfaManagementAuth( // Bearer path. Resolution first: an unknown, revoked, expired, or narrow-scope // token is refused here and never gets as far as presenting an elevation. const auth = await requireBearerAuth(); + const bearerFreshFactor = + freshFactor || + (options.freshFactorIfEnrolled === true && + (await hasSecondFactorEnrolled(auth.user))); const commitElevation = await resolveBearerElevation( { user: auth.user, apiTokenId: auth.apiTokenId }, - { freshFactor, proofSource }, + { freshFactor: bearerFreshFactor, proofSource }, ); return { diff --git a/src/lib/auth/__tests__/enrollment-proof-second-factor.test.ts b/src/lib/auth/__tests__/enrollment-proof-second-factor.test.ts new file mode 100644 index 000000000..5cdabf23a --- /dev/null +++ b/src/lib/auth/__tests__/enrollment-proof-second-factor.test.ts @@ -0,0 +1,228 @@ +/** + * Adding a credential to an account that already has a second factor. + * + * A password, a young session or a password re-proof used to be enough to + * add a security key, a TOTP authenticator or a passkey even when the account + * was protected by a second factor. A session that signed in on a remembered + * browser skips the factor and is still young, so the password alone could + * add a key that then satisfies every gate the factor protected. On such an + * account only possession of a factor counts now; the account without one is + * unchanged. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + session: { findUnique: vi.fn(), update: vi.fn() }, + webauthnMfaCredential: { count: vi.fn() }, + passkey: { count: vi.fn() }, + authChallenge: { findUnique: vi.fn() }, + }, +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(), + rateLimitHeaders: vi.fn(() => ({})), + refundRateLimit: vi.fn(), +})); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/auth/password", () => ({ verifyPassword: vi.fn() })); +vi.mock("@/lib/auth/passkey", () => ({ + verifyAuthentication: vi.fn(), + createAuthenticationOptions: vi.fn(), + createRegistrationOptions: vi.fn(), +})); +vi.mock("@/lib/auth/mfa/webauthn", () => ({ + verifyMfaAuthentication: vi.fn(), + createMfaAuthenticationOptions: vi.fn(), +})); +vi.mock("@/lib/auth/mfa/verify-factor", () => ({ verifyMfaFactor: vi.fn() })); +vi.mock("@/lib/api-handler", () => ({ + apiHandler: (fn: unknown) => fn, + requireCookieAuth: vi.fn(), +})); + +import { prisma } from "@/lib/db"; +import { checkRateLimit, refundRateLimit } from "@/lib/rate-limit"; +import { verifyPassword } from "@/lib/auth/password"; +import { verifyMfaFactor } from "@/lib/auth/mfa/verify-factor"; +import { createRegistrationOptions } from "@/lib/auth/passkey"; +import { requireCookieAuth } from "@/lib/api-handler"; +import { checkCookieEnrollmentProof } from "../existing-factor-proof"; +import { POST as PASSKEY_REGISTER_OPTIONS } from "@/app/api/auth/passkey/register-options/route"; + +const NOW = Date.now(); +const fresh = new Date(NOW - 60_000); +const stale = new Date(NOW - 60 * 60_000); + +const ENROLLED: { + id: string; + username: string; + email: string; + passwordHash: string; + totpSecretEncrypted: string | null; + totpLastStep: null; + totpConfirmedAt: Date | null; +} = { + id: "user-1", + username: "u", + email: "u@example.com", + passwordHash: "hash", + totpSecretEncrypted: "enc", + totpLastStep: null, + totpConfirmedAt: new Date("2026-01-01"), +}; +const PLAIN = { ...ENROLLED, totpSecretEncrypted: null, totpConfirmedAt: null }; + +function req(body?: unknown): Request { + return new Request("http://localhost/api/auth/me/mfa/totp/setup", { + method: "POST", + headers: body === undefined ? {} : { "content-type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }); +} + +function session(stamps: { + mfaVerifiedAt?: Date | null; + createdAt?: Date; + reproofAt?: Date | null; +}) { + vi.mocked(prisma.session.findUnique).mockResolvedValue({ + mfaVerifiedAt: stamps.mfaVerifiedAt ?? null, + createdAt: stamps.createdAt ?? stale, + reproofAt: stamps.reproofAt ?? null, + } as never); +} + +function gate(user: typeof ENROLLED | typeof PLAIN, request: Request) { + return checkCookieEnrollmentProof({ + user: user as never, + sessionId: "sess-1", + request, + ipAddress: "203.0.113.9", + stage: "totp_enroll", + }); +} + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(0 as never); + vi.mocked(prisma.passkey.count).mockResolvedValue(0 as never); + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: true, + limit: 5, + remaining: 4, + resetAt: NOW + 1000, + }); +}); + +describe("account with a second factor", () => { + it("a session signed in a minute ago is not enough (remembered browser)", async () => { + session({ createdAt: fresh }); + const res = await gate(ENROLLED, req()); + expect(res?.status).toBe(401); + const body = await res!.json(); + expect(body.meta.errorCode).toBe("auth.reproof.required"); + expect(body.meta.methods).toEqual(["totp"]); + }); + + it("a password re-proof stamp is not enough", async () => { + session({ reproofAt: fresh }); + const res = await gate(ENROLLED, req()); + expect(res?.status).toBe(401); + }); + + it("a password in the body is refused before it is verified", async () => { + session({ createdAt: fresh }); + const res = await gate( + ENROLLED, + req({ method: "password", password: "correct horse" }), + ); + expect(res?.status).toBe(401); + expect(verifyPassword).not.toHaveBeenCalled(); + expect(checkRateLimit).not.toHaveBeenCalled(); + }); + + it("a completed second factor within five minutes passes", async () => { + session({ mfaVerifiedAt: fresh }); + expect(await gate(ENROLLED, req())).toBeNull(); + }); + + it("a TOTP code in the body passes and gives the attempt back", async () => { + session({}); + vi.mocked(verifyMfaFactor).mockResolvedValue({ ok: true } as never); + expect(await gate(ENROLLED, req({ method: "totp", code: "123456" }))).toBe( + null, + ); + expect(refundRateLimit).toHaveBeenCalledWith("auth:step-up:user-1"); + }); + + it("a security key alone makes the account enrolled", async () => { + vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(1 as never); + session({ createdAt: fresh }); + const res = await gate(PLAIN, req()); + expect(res?.status).toBe(401); + expect((await res!.json()).meta.methods).toEqual(["webauthn"]); + }); +}); + +describe("account without a second factor (unchanged)", () => { + it("a session signed in a minute ago passes", async () => { + session({ createdAt: fresh }); + expect(await gate(PLAIN, req())).toBeNull(); + }); + + it("a password in the body passes", async () => { + session({}); + vi.mocked(verifyPassword).mockResolvedValue(true); + expect( + await gate(PLAIN, req({ method: "password", password: "pw" })), + ).toBeNull(); + expect(verifyPassword).toHaveBeenCalled(); + }); +}); + +describe("POST /api/auth/passkey/register-options", () => { + function passkeyReq(body: unknown) { + return new Request("http://localhost/api/auth/passkey/register-options", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + } + + it("refuses a password on an account with a second factor, before verifying it", async () => { + vi.mocked(requireCookieAuth).mockResolvedValue({ + user: ENROLLED, + session: { id: "sess-1" }, + } as never); + const res = await PASSKEY_REGISTER_OPTIONS( + passkeyReq({ method: "password", password: "correct horse" }) as never, + ); + expect(res.status).toBe(422); + const body = await res.json(); + expect(body.meta.errorCode).toBe("auth.reproof.too_weak"); + expect(body.meta.methods).toEqual(["totp"]); + expect(verifyPassword).not.toHaveBeenCalled(); + expect(createRegistrationOptions).not.toHaveBeenCalled(); + }); + + it("still takes a password on an account without one", async () => { + vi.mocked(requireCookieAuth).mockResolvedValue({ + user: PLAIN, + session: { id: "sess-1" }, + } as never); + vi.mocked(verifyPassword).mockResolvedValue(true); + vi.mocked(createRegistrationOptions).mockResolvedValue({ + options: {}, + challengeId: "ch-1", + } as never); + const res = await PASSKEY_REGISTER_OPTIONS( + passkeyReq({ method: "password", password: "pw" }) as never, + ); + expect(res.status).toBe(200); + expect(refundRateLimit).toHaveBeenCalledWith("auth:step-up:user-1"); + }); +}); diff --git a/src/lib/auth/existing-factor-proof.ts b/src/lib/auth/existing-factor-proof.ts index 64915503d..e44c3b53c 100644 --- a/src/lib/auth/existing-factor-proof.ts +++ b/src/lib/auth/existing-factor-proof.ts @@ -22,7 +22,15 @@ import { apiError } from "@/lib/api-response"; import { REPROOF_REQUIRED_CODE } from "@/lib/api-errors"; import { auditLog } from "@/lib/auth/audit"; import { annotate } from "@/lib/logging/context"; -import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; +import { + checkRateLimit, + rateLimitHeaders, + refundRateLimit, +} from "@/lib/rate-limit"; +import { + hasSecondFactorEnrolled, + recentProofMethods, +} from "@/lib/auth/second-factor"; import { verifyPassword } from "@/lib/auth/password"; import { verifyAuthentication } from "@/lib/auth/passkey"; import { verifyMfaAuthentication } from "@/lib/auth/mfa/webauthn"; @@ -73,6 +81,16 @@ export async function throttleReproof( }); } +/** + * Give back the attempt a proof charged once it verified. The bucket exists to + * stop guessing; a proof that verified was not a guess, and without the refund + * an owner who confirms a few actions in a row (an export, a share link, a + * password change) runs the shared budget dry and is told to wait. + */ +export async function refundReproof(userId: string): Promise { + await refundRateLimit(reproofBucket(userId)); +} + export type ProofOutcome = { ok: true } | { ok: false; reason: string }; /** @@ -184,14 +202,15 @@ export const RECENT_PROOF_MAX_AGE_MS = 5 * 60 * 1000; * session could enroll their own authenticator, and from then on hold a factor * that satisfies every step-up gate, registers a passkey and survives the * owner changing the password. So the caller must show that the person at the - * keyboard can authenticate as the account owner right now, in one of three - * ways: + * keyboard can authenticate as the account owner right now. * - * - the session completed a second factor or passkey sign-in within the last - * five minutes (`Session.mfaVerifiedAt`); + * What counts depends on whether the account already has a second factor. + * + * Without one (password, passkey or single sign-on only), any of: + * - the session completed a passkey sign-in or re-proof within the last five + * minutes (`Session.mfaVerifiedAt`); * - the session re-proved a credential at `POST /api/auth/reproof` within - * the last five minutes (`Session.reproofAt`), which accepts the same - * proofs this gate takes in the body; + * the last five minutes (`Session.reproofAt`); * - the session itself was created by a sign-in within the last five * minutes (`Session.createdAt`, which no later write touches) — this is * also what lets an SSO-only account with nothing else to re-prove enroll @@ -200,6 +219,13 @@ export const RECENT_PROOF_MAX_AGE_MS = 5 * 60 * 1000; * `{ method: "totp", code }` or a passkey / security-key assertion, the * same shapes the step-up mint accepts. * + * With one, only a proof of possession of a factor: `Session.mfaVerifiedAt` + * within five minutes, or a TOTP code, security-key or passkey assertion in + * the body. A password is not enough and neither is a young session: signing + * in on a remembered browser skips the second factor and still creates a + * fresh session, so `createdAt` would let the password alone add a new key, + * which then satisfies every gate the second factor was protecting. + * * Returns the refusal to send, or null to proceed. Proofs in the body are * throttled and audited through the shared re-proof bucket. */ @@ -212,6 +238,7 @@ export async function checkCookieEnrollmentProof(args: { }): Promise { const { user, sessionId, request, ipAddress, stage } = args; + const enrolled = await hasSecondFactorEnrolled(user); const row = await prisma.session.findUnique({ where: { id: sessionId }, select: { mfaVerifiedAt: true, createdAt: true, reproofAt: true }, @@ -219,20 +246,25 @@ export async function checkCookieEnrollmentProof(args: { const now = Date.now(); const recent = (d: Date | null | undefined) => d != null && now - d.getTime() <= RECENT_PROOF_MAX_AGE_MS; - if ( - recent(row?.mfaVerifiedAt) || - recent(row?.createdAt) || - recent(row?.reproofAt) - ) { + const sessionSatisfies = enrolled + ? recent(row?.mfaVerifiedAt) + : recent(row?.mfaVerifiedAt) || + recent(row?.createdAt) || + recent(row?.reproofAt); + if (sessionSatisfies) { annotate({ meta: { enrollment_proof: "recent_session" } }); return null; } const proof = await readProof(request); if (proof === "absent") { - annotate({ action: { name: "auth.reproof.required" }, meta: { stage } }); + annotate({ + action: { name: "auth.reproof.required" }, + meta: { stage, second_factor: enrolled }, + }); return apiError("Confirm it is you before adding a sign-in method", 401, { errorCode: REPROOF_REQUIRED_CODE, + methods: await recentProofMethods(user, enrolled), }); } if (proof === "invalid") { @@ -240,6 +272,18 @@ export async function checkCookieEnrollmentProof(args: { errorCode: REPROOF_REQUIRED_CODE, }); } + // Refused before a guess is spent: a password that verified would still not + // open this gate on an account with a second factor. + if (enrolled && !isSecondFactorProof(proof.method)) { + annotate({ + action: { name: "auth.reproof.method_refused" }, + meta: { stage, method: proof.method }, + }); + return apiError("Confirm with your second factor or a passkey", 401, { + errorCode: REPROOF_REQUIRED_CODE, + methods: await recentProofMethods(user, enrolled), + }); + } const limited = await throttleReproof(user.id, ipAddress, stage); if (limited) return limited; @@ -257,10 +301,22 @@ export async function checkCookieEnrollmentProof(args: { errorCode: REPROOF_FAILED_CODE, }); } + await refundReproof(user.id); annotate({ meta: { enrollment_proof: proof.method } }); return null; } +/** + * The proofs that stand for possession of a factor rather than knowledge of + * the password. On an account with a second factor these are the only ones + * that may add a credential. + */ +export function isSecondFactorProof( + method: ExistingFactorProof["method"], +): boolean { + return method === "totp" || method === "webauthn" || method === "passkey"; +} + async function readProof( request: Request, ): Promise { diff --git a/src/lib/auth/second-factor.ts b/src/lib/auth/second-factor.ts new file mode 100644 index 000000000..48634c9fa --- /dev/null +++ b/src/lib/auth/second-factor.ts @@ -0,0 +1,66 @@ +import type { User } from "@/generated/prisma/client"; +import { prisma } from "@/lib/db"; + +/** + * Does this account have a SECOND FACTOR at all? + * + * Either factor enrols it: a confirmed TOTP secret OR a registered WebAuthn + * security key. A webauthn-only account must clear step-up too, so every + * boundary that asks this question tracks `requireFreshMfa`'s either-factor + * rule rather than reading `totpConfirmedAt` alone. It was written out three + * times before it was a function, which is two more places for the second arm + * to be forgotten. + * + * DELIBERATELY DOES NOT COUNT A PRIMARY PASSKEY, even though a passkey login + * stamps `mfaVerifiedAt` and could therefore satisfy the gate. This predicate + * answers "is there a second factor", and that is the question + * `requireFreshMfaIfEnrolled` asks before deciding whether account deletion, the + * data reset, the password change, the encrypted export and key rotation demand + * a step-up at all. Counting a passkey here would silently pull every + * passkey-holding account into a gate those routes have never applied to them — + * a behaviour change to five destructive actions, made in passing, for the + * benefit of a sixth. `canProveFreshFactor` in `api-handler.ts` is where the wider question + * lives. + */ +export async function hasSecondFactorEnrolled(user: { + id: string; + totpConfirmedAt: Date | null; +}): Promise { + if (user.totpConfirmedAt) return true; + const webauthnKeyCount = await prisma.webauthnMfaCredential.count({ + where: { userId: user.id }, + }); + return webauthnKeyCount > 0; +} + +/** + * A proof the recent-proof gate can ask for. `password` and `passkey` on an + * account without a second factor; `totp`, `webauthn` and `passkey` on one + * with a second factor, where a password alone has never been enough. + */ +export type RecentProofMethod = "password" | "totp" | "webauthn" | "passkey"; + +/** + * The proofs that would satisfy `assertRecentCookieProof` for this + * account. Also what `POST /api/auth/reproof` accepts, so the dialog and the + * endpoint cannot disagree. + */ +export async function recentProofMethods( + user: Pick, + enrolled?: boolean, +): Promise { + const hasSecond = enrolled ?? (await hasSecondFactorEnrolled(user)); + const [keys, passkeys] = await Promise.all([ + prisma.webauthnMfaCredential.count({ where: { userId: user.id } }), + prisma.passkey.count({ where: { userId: user.id } }), + ]); + const methods: RecentProofMethod[] = []; + if (hasSecond) { + if (user.totpConfirmedAt) methods.push("totp"); + if (keys > 0) methods.push("webauthn"); + } else if (user.passwordHash) { + methods.push("password"); + } + if (passkeys > 0) methods.push("passkey"); + return methods; +}