diff --git a/prisma/migrations/0256_step_up_elevation/migration.sql b/prisma/migrations/0260_step_up_elevation/migration.sql similarity index 78% rename from prisma/migrations/0256_step_up_elevation/migration.sql rename to prisma/migrations/0260_step_up_elevation/migration.sql index c16c4f3da..bf827994a 100644 --- a/prisma/migrations/0256_step_up_elevation/migration.sql +++ b/prisma/migrations/0260_step_up_elevation/migration.sql @@ -19,6 +19,14 @@ CREATE TABLE "step_up_elevations" ( ); CREATE UNIQUE INDEX "step_up_elevations_token_hash_key" ON "step_up_elevations"("token_hash"); + +-- At most ONE redeemable elevation per token, enforced by the database rather +-- than by the mint's delete-then-create ordering. Without it two concurrent +-- mints can interleave so that both rows survive, or the second mint's delete +-- strands the value the first mint already handed to the client. +CREATE UNIQUE INDEX "step_up_elevations_live_per_token_key" + ON "step_up_elevations"("api_token_id") WHERE "consumed_at" IS NULL; + CREATE INDEX "step_up_elevations_user_id_idx" ON "step_up_elevations"("user_id"); CREATE INDEX "step_up_elevations_api_token_id_idx" ON "step_up_elevations"("api_token_id"); CREATE INDEX "step_up_elevations_expires_at_idx" ON "step_up_elevations"("expires_at"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 9a645a2c4..a43118859 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -3159,6 +3159,13 @@ model StepUpElevation { @@index([userId]) @@index([apiTokenId]) @@index([expiresAt]) + // NOTE: migration 0260 additionally creates a PARTIAL unique index — + // `(api_token_id) WHERE consumed_at IS NULL` — so at most one elevation per + // token is ever redeemable. Prisma cannot express a partial index, and a + // plain `@@unique([apiTokenId])` would be wrong: it would also forbid minting + // a second elevation after the first was legitimately consumed. The + // constraint therefore lives only in the migration, and the mint below + // handles its unique violation explicitly. @@map("step_up_elevations") } diff --git a/src/__tests__/step-up-elevation-guard.test.ts b/src/__tests__/step-up-elevation-guard.test.ts index 89c4a71d1..7b0581c8f 100644 --- a/src/__tests__/step-up-elevation-guard.test.ts +++ b/src/__tests__/step-up-elevation-guard.test.ts @@ -7,10 +7,20 @@ * unable to grow by accident. Behavioural tests prove the mechanism works; * these prove nobody quietly pointed it somewhere else. * - * Like the Bearer-scope guards next door, these are tripwires rather than - * proofs. They cannot show the allowlist is correct — only that it has not - * changed without someone editing this file, which is where the reviewer's - * attention is supposed to land. + * MATCHING IS ON THE IMPORT, NOT THE CALL. An earlier revision grepped for + * `requireMfaManagementAuth(` and was evadable in one line: + * + * import { requireMfaManagementAuth as gate } from "@/lib/api-handler"; + * const auth = await gate(); + * + * A call-shaped regex sees nothing there. An import-shaped one cannot be dodged + * that way, because the binding has to be imported from the module under some + * local name before it can be called at all. The helpers below parse the import + * specifier — including its `as` alias — and check calls against the LOCAL name, + * so a rename is followed rather than lost. + * + * They remain tripwires, not proofs. They cannot show an allowlist is correct, + * only that it has not changed without someone editing this file. */ import { describe, it, expect } from "vitest"; import { readFileSync } from "node:fs"; @@ -34,19 +44,65 @@ function read(rel: string): string { return readFileSync(join(SRC, rel), "utf8"); } -function filesMatching(re: RegExp): string[] { - return sourceFiles().filter((rel) => re.test(read(rel))); +/** Local names bound for `exportName` from any module matching `moduleRe`. */ +function importedAs( + rel: string, + exportName: string, + moduleRe: RegExp, +): string[] { + const src = read(rel); + const names: string[] = []; + for (const m of src.matchAll( + /import\s*\{([\s\S]*?)\}\s*from\s*["']([^"']+)["']/g, + )) { + const [, clause, specifier] = m; + if (!moduleRe.test(specifier)) continue; + for (const part of clause.split(",")) { + const piece = part.trim().replace(/^type\s+/, ""); + if (!piece) continue; + const aliased = piece.match( + /^([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)$/, + ); + if (aliased) { + if (aliased[1] === exportName) names.push(aliased[2]); + } else if (piece === exportName) { + names.push(piece); + } + } + } + return names; +} + +const API_HANDLER = /api-handler$/; +const STEP_UP_MODULE = /auth\/step-up$/; + +/** Files that import `exportName` from the module AND call it. */ +function callers(exportName: string, moduleRe: RegExp): string[] { + return sourceFiles() + .filter((rel) => { + const locals = importedAs(rel, exportName, moduleRe); + if (locals.length === 0) return false; + const src = read(rel); + return locals.some((name) => new RegExp(`\\b${name}\\s*\\(`).test(src)); + }) + .sort(); } /** * Every route an elevation can unlock. This is the whole set — the second-factor - * management surface and nothing besides. + * management MUTATIONS and nothing besides. * * What is deliberately ABSENT is as much the point as what is present: + * - `GET /api/auth/me/mfa`. The status read is plain `requireAuth()`: it + * carries no credential material (no secret, no code, no public key, no + * credential id — only names, dates, and a count), the web shows the same + * screen to any authenticated session, and `/api/auth/me` already exposes + * `totpConfirmedAt` over Bearer. Gating it would force a factor re-proof to + * render a screen and buy nothing; * - every `/api/admin/*` route. `requireAdmin` is cookie-only and does not * consult this mechanism at all; - * - the trusted-device routes. "Remember this browser" is a browser concept; - * a native client has no equivalent to manage; + * - the trusted-device routes. "Remember this browser" is a browser concept + * with no native equivalent to manage; * - `requireFreshMfaIfEnrolled`'s callers (password change, account deletion, * data reset, encrypted export, encryption-key rotation). Those are * destructive account actions, not second-factor management, and letting an @@ -54,7 +110,6 @@ function filesMatching(re: RegExp): string[] { * exists to prevent. */ const ELEVATION_ROUTES = [ - "app/api/auth/me/mfa/route.ts", "app/api/auth/me/mfa/disable/route.ts", "app/api/auth/me/mfa/recovery-codes/regenerate/route.ts", "app/api/auth/me/mfa/totp/confirm/route.ts", @@ -65,56 +120,98 @@ const ELEVATION_ROUTES = [ ].sort(); describe("S1 — the elevation-accepting route set is frozen", () => { - it("only the known MFA-management routes call the gate", () => { - const callers = filesMatching(/requireMfaManagementAuth\(/).filter( - // The gate's own definition names itself. + it("only the known MFA-management mutations call the gate", () => { + const found = callers("requireMfaManagementAuth", API_HANDLER).filter( (rel) => rel !== "lib/api-handler.ts", ); - expect(callers).toEqual(ELEVATION_ROUTES); + expect(found).toEqual(ELEVATION_ROUTES); + }); + + it("follows an aliased import rather than losing it", () => { + // Guards the guard. Without this the allowlist above is one rename away + // from meaningless, which is precisely how the previous revision was + // evadable. + const probe = ` + import { requireMfaManagementAuth as gate } from "@/lib/api-handler"; + const auth = await gate(); + `; + const names: string[] = []; + for (const m of probe.matchAll( + /import\s*\{([\s\S]*?)\}\s*from\s*["']([^"']+)["']/g, + )) { + for (const part of m[1].split(",")) { + const aliased = part + .trim() + .match(/^([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)$/); + if (aliased && aliased[1] === "requireMfaManagementAuth") { + names.push(aliased[2]); + } + } + } + expect(names).toEqual(["gate"]); + expect(new RegExp(`\\b${names[0]}\\s*\\(`).test(probe)).toBe(true); }); it("no admin route calls the gate", () => { - const admin = filesMatching(/requireMfaManagementAuth\(/).filter((rel) => - rel.startsWith("app/api/admin/"), + const admin = callers("requireMfaManagementAuth", API_HANDLER).filter( + (rel) => rel.startsWith("app/api/admin/"), ); expect(admin).toEqual([]); }); it("the old cookie-only gates no longer linger on the MFA surface", () => { - // A route that still called `requireCookieAuth` / `requireFreshMfa` - // directly would be outside the single gate, so its posture could drift - // from the rest of the set without tripping the allowlist above. for (const rel of ELEVATION_ROUTES) { - const src = read(rel); - expect(src).not.toMatch(/await requireCookieAuth\(/); - expect(src).not.toMatch(/await requireFreshMfa\(/); + expect(importedAs(rel, "requireCookieAuth", API_HANDLER)).toEqual([]); + expect(importedAs(rel, "requireFreshMfa", API_HANDLER)).toEqual([]); + } + }); + + it("every gated route spends its elevation", () => { + // `commitElevation` is what makes the proof single-use. A route that gates + // but never commits would leave the elevation redeemable for the rest of + // its window — the one fail-open the two-phase split introduces, closed + // here. + for (const rel of ELEVATION_ROUTES) { + expect(read(rel)).toMatch(/\.commitElevation\(\)/); } }); }); -describe("S2 — redemption happens in exactly one place", () => { - it("only the gate redeems an elevation", () => { - const redeemers = filesMatching(/redeemStepUpElevation\(/).filter( +describe("S2 — mint and redeem happen in exactly one place each", () => { + it("only the gate claims an elevation", () => { + const found = callers("claimStepUpElevation", STEP_UP_MODULE).filter( (rel) => rel !== "lib/auth/step-up.ts", ); - expect(redeemers).toEqual(["lib/api-handler.ts"]); + expect(found).toEqual(["lib/api-handler.ts"]); + }); + + it("only the gate validates one", () => { + const found = callers("validateStepUpElevation", STEP_UP_MODULE).filter( + (rel) => rel !== "lib/auth/step-up.ts", + ); + expect(found).toEqual(["lib/api-handler.ts"]); }); it("only the mint route mints one", () => { - const minters = filesMatching(/mintStepUpElevation\(/).filter( + const found = callers("mintStepUpElevation", STEP_UP_MODULE).filter( (rel) => rel !== "lib/auth/step-up.ts", ); - expect(minters).toEqual(["app/api/auth/step-up/route.ts"]); + expect(found).toEqual(["app/api/auth/step-up/route.ts"]); }); - it("no route reads the elevation header for itself", () => { - // The header is the gate's private input. A route reading it directly - // would be hand-rolling an authorisation decision outside the one place - // that is reviewed for it. - const readers = sourceFiles() - .filter((p) => p.endsWith("/route.ts")) - .filter((rel) => /["']x-step-up["']/i.test(read(rel))); - expect(readers).toEqual([]); + it("no file outside the gate reads the elevation header", () => { + // Both spellings — the literal string AND the exported constant — across + // every source file, not only `route.ts`. A helper could read it too. + const offenders = sourceFiles() + .filter((rel) => rel !== "lib/api-handler.ts") + .filter((rel) => { + const src = read(rel); + if (/["']x-step-up["']/i.test(src)) return true; + return ( + importedAs(rel, "STEP_UP_ELEVATION_HEADER", API_HANDLER).length > 0 + ); + }); + expect(offenders).toEqual([]); }); }); @@ -126,9 +223,10 @@ describe("S3 — requireAdmin stays cookie-only", () => { src.indexOf("export async function requireCookieAuth("), ); expect(body).toContain("await getSession()"); - // No Bearer resolution, and no elevation redemption, inside the admin gate. expect(body).not.toMatch(/authenticateBearer|resolveBearerToken/); - expect(body).not.toMatch(/redeemStepUpElevation|STEP_UP_ELEVATION_HEADER/); + expect(body).not.toMatch( + /claimStepUpElevation|validateStepUpElevation|STEP_UP_ELEVATION_HEADER|requireMfaManagementAuth/, + ); }); }); @@ -138,9 +236,11 @@ describe("S4 — the mint surface refuses a cookie", () => { "app/api/auth/step-up/route.ts", "app/api/auth/step-up/options/route.ts", ]) { - const src = read(rel); - expect(src).toMatch(/await requireBearerAuth\(\)/); - expect(src).not.toMatch(/requireAuth\(|requireCookieAuth\(/); + expect(importedAs(rel, "requireBearerAuth", API_HANDLER)).toEqual([ + "requireBearerAuth", + ]); + expect(importedAs(rel, "requireAuth", API_HANDLER)).toEqual([]); + expect(importedAs(rel, "requireCookieAuth", API_HANDLER)).toEqual([]); } }); @@ -153,3 +253,39 @@ describe("S4 — the mint surface refuses a cookie", () => { expect(body).not.toContain("getSession("); }); }); + +describe("S5 — a password proof cannot satisfy a fresh-factor gate", () => { + it("the fresh-factor set excludes password", async () => { + // The B1 invariant, pinned at the definition. The web stamps + // `mfaVerifiedAt` only for a completed second factor or a primary passkey + // login; the Bearer arm must not be looser. + const { FRESH_FACTOR_METHODS, isFreshFactorMethod } = + await import("@/lib/auth/step-up"); + expect([...FRESH_FACTOR_METHODS].sort()).toEqual([ + "passkey", + "totp", + "webauthn", + ]); + expect(isFreshFactorMethod("password")).toBe(false); + }); + + it("the destructive routes all request the fresh-factor arm", () => { + // Miss this on one route and a password-proved elevation tears down the + // second factor there. + for (const rel of [ + "app/api/auth/me/mfa/disable/route.ts", + "app/api/auth/me/mfa/recovery-codes/regenerate/route.ts", + "app/api/auth/me/mfa/webauthn/[id]/route.ts", + ]) { + expect(read(rel)).toMatch(/freshFactor:\s*true/); + } + }); + + it("the claim re-checks the factor rather than trusting the validation", () => { + const src = read("lib/auth/step-up.ts"); + const claim = src.slice( + src.indexOf("export async function claimStepUpElevation("), + ); + expect(claim).toMatch(/requireFreshFactor\s*&&\s*!isFreshFactorMethod/); + }); +}); diff --git a/src/app/api/auth/me/mfa/disable/route.ts b/src/app/api/auth/me/mfa/disable/route.ts index 144eb4c1b..50d22d12a 100644 --- a/src/app/api/auth/me/mfa/disable/route.ts +++ b/src/app/api/auth/me/mfa/disable/route.ts @@ -38,9 +38,8 @@ const DISABLE_WINDOW_MS = 15 * 60 * 1000; export const POST = apiHandler(async (req: Request) => { // Step-up gate first — throws StepUpRequiredError (401 + errorCode) if the // session is not freshly second-factor-verified. - const { user, session } = await requireMfaManagementAuth({ - freshFactor: true, - }); + const auth = await requireMfaManagementAuth({ freshFactor: true }); + const { user } = auth; const rl = await checkRateLimit( `mfa:disable:${user.id}`, @@ -85,6 +84,11 @@ export const POST = apiHandler(async (req: Request) => { throw new HttpError(401, "Invalid code"); } + // Every cheap check has passed and the teardown is next, so spend the + // elevation now. Placing it here rather than at the gate means a wrong code or + // a 429 above costs the caller nothing. + await auth.commitElevation(); + await prisma.$transaction(async (tx) => { await tx.user.update({ where: { id: user.id }, @@ -102,7 +106,12 @@ export const POST = apiHandler(async (req: Request) => { // every "remember this device" trusted device (all handled by // destroyOtherSessions), keeping only the caller's current session. A stale // session or a trusted-device cookie must not survive the factor's removal. - await destroyOtherSessions(user.id, session.id); + await destroyOtherSessions( + user.id, + auth.transport === "cookie" + ? { kind: "session", sessionId: auth.session.id } + : { kind: "accessToken", accessTokenHash: auth.accessTokenHash }, + ); await auditLog("auth.mfa.disabled", { userId: user.id, diff --git a/src/app/api/auth/me/mfa/recovery-codes/regenerate/route.ts b/src/app/api/auth/me/mfa/recovery-codes/regenerate/route.ts index e6c97c3d5..341515dc8 100644 --- a/src/app/api/auth/me/mfa/recovery-codes/regenerate/route.ts +++ b/src/app/api/auth/me/mfa/recovery-codes/regenerate/route.ts @@ -24,7 +24,8 @@ const REGEN_RATE_LIMIT = 5; const REGEN_WINDOW_MS = 15 * 60 * 1000; export const POST = apiHandler(async (req: Request) => { - const { user } = await requireMfaManagementAuth({ freshFactor: true }); + const auth = await requireMfaManagementAuth({ freshFactor: true }); + const { user } = auth; // Defence in depth — regeneration only makes sense for an active factor. if (!user.totpConfirmedAt) { @@ -44,6 +45,8 @@ export const POST = apiHandler(async (req: Request) => { return res; } + await auth.commitElevation(); + const recoveryCodes = await regenerateRecoveryCodes(user.id); await auditLog("auth.mfa.recovery_regenerated", { diff --git a/src/app/api/auth/me/mfa/route.ts b/src/app/api/auth/me/mfa/route.ts index df6c775f4..d283a10d8 100644 --- a/src/app/api/auth/me/mfa/route.ts +++ b/src/app/api/auth/me/mfa/route.ts @@ -3,12 +3,21 @@ * * Second-factor status for the security settings hub: whether TOTP is active, * how many recovery codes remain, and the registered WebAuthn security keys. - * Gated by `requireMfaManagementAuth`: a cookie session, or a Bearer token - * presenting a single-use step-up elevation. A token alone is not enough. + * Plain `requireAuth()` — a cookie session or a cookie-equivalent token. This + * read is NOT elevation-gated, and deliberately so: it returns metadata only + * (whether TOTP is on, how many recovery codes remain, and each key's name and + * dates) and no credential material at all — no secret, no code, no public key, + * no credential id. The web shows the same screen to any authenticated session, + * and `/api/auth/me` already exposes `totpConfirmedAt` over Bearer, so gating it + * would only force a factor re-proof to render a screen and buy nothing. + * + * Every MUTATION on this surface stays behind `requireMfaManagementAuth`. The + * argument-less `requireAuth()` keeps the fail-closed scope default, so a + * narrow-scope token gains no reach here. * * Returns only metadata: no secret, no recovery-code plaintext, no public key. */ -import { apiHandler, requireMfaManagementAuth } from "@/lib/api-handler"; +import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; import { prisma } from "@/lib/db"; @@ -17,7 +26,7 @@ import { countRemainingRecoveryCodes } from "@/lib/auth/mfa/recovery-codes"; export const dynamic = "force-dynamic"; export const GET = apiHandler(async () => { - const { user } = await requireMfaManagementAuth(); + const { user } = await requireAuth(); const [recoveryCodesRemaining, webauthn] = await Promise.all([ countRemainingRecoveryCodes(user.id), diff --git a/src/app/api/auth/me/mfa/totp/confirm/route.ts b/src/app/api/auth/me/mfa/totp/confirm/route.ts index 8bad88724..8e701bf04 100644 --- a/src/app/api/auth/me/mfa/totp/confirm/route.ts +++ b/src/app/api/auth/me/mfa/totp/confirm/route.ts @@ -46,7 +46,8 @@ const CONFIRM_RATE_LIMIT = 10; const CONFIRM_WINDOW_MS = 15 * 60 * 1000; export const POST = apiHandler(async (req: Request) => { - const { user, session } = await requireMfaManagementAuth(); + const auth = await requireMfaManagementAuth(); + const { user } = auth; const rl = await checkRateLimit( `mfa:confirm:${user.id}`, @@ -103,6 +104,10 @@ export const POST = apiHandler(async (req: Request) => { const recoveryCodes = generateRecoveryCodes(); + // The code verified and the activation is next — spend the elevation here so + // a wrong code above costs the caller nothing. + await auth.commitElevation(); + await prisma.$transaction(async (tx) => { await tx.user.update({ where: { id: user.id }, @@ -115,11 +120,14 @@ export const POST = apiHandler(async (req: Request) => { await tx.mfaRecoveryCode.deleteMany({ where: { userId: user.id } }); await persistRecoveryCodes(tx, user.id, recoveryCodes); // Stamp the completing session so the user can immediately manage the - // factor under step-up without re-verifying. - await tx.session.updateMany({ - where: { id: session.id }, - data: { mfaVerifiedAt: new Date() }, - }); + // factor under step-up without re-verifying. Cookie transport only — a + // Bearer caller has no session row, and mints a fresh elevation per action. + if (auth.transport === "cookie") { + await tx.session.updateMany({ + where: { id: auth.session.id }, + data: { mfaVerifiedAt: new Date() }, + }); + } }); // v1.23 — the account now has an active second factor, so any 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 662aa2206..a9ef6b5ed 100644 --- a/src/app/api/auth/me/mfa/totp/setup/route.ts +++ b/src/app/api/auth/me/mfa/totp/setup/route.ts @@ -33,7 +33,8 @@ const SETUP_RATE_LIMIT = 5; const SETUP_WINDOW_MS = 15 * 60 * 1000; export const POST = apiHandler(async (req: Request) => { - const { user } = await requireMfaManagementAuth(); + const auth = await requireMfaManagementAuth(); + const { user } = auth; const rl = await checkRateLimit( `mfa:setup:${user.id}`, @@ -56,6 +57,9 @@ export const POST = apiHandler(async (req: Request) => { throw new HttpError(409, "A second factor is already active"); } + // Rate limit and the already-active check have passed; the write is next. + await auth.commitElevation(); + const secret = generateTotpSecret(); const account = user.email ?? user.username; const otpauthUri = buildOtpauthUri(secret, account); diff --git a/src/app/api/auth/me/mfa/webauthn/[id]/route.ts b/src/app/api/auth/me/mfa/webauthn/[id]/route.ts index 6aab56136..61af3c671 100644 --- a/src/app/api/auth/me/mfa/webauthn/[id]/route.ts +++ b/src/app/api/auth/me/mfa/webauthn/[id]/route.ts @@ -28,7 +28,8 @@ export const PATCH = apiHandler( request: NextRequest, { params }: { params: Promise<{ id: string }> }, ) => { - const { user } = await requireMfaManagementAuth(); + const auth = await requireMfaManagementAuth(); + const { user } = auth; const { id } = await params; const { data: body, error: jsonError } = await safeJson(request, { @@ -49,6 +50,8 @@ export const PATCH = apiHandler( return apiError("Security key not found", 404); } + await auth.commitElevation(); + const updated = await prisma.webauthnMfaCredential.update({ where: { id }, data: { name: parsed.data.name }, @@ -67,7 +70,8 @@ export const DELETE = apiHandler( ) => { // Step-up gate first — throws StepUpRequiredError (401 + errorCode) if the // session is not freshly second-factor-verified. - const { user } = await requireMfaManagementAuth({ freshFactor: true }); + const auth = await requireMfaManagementAuth({ freshFactor: true }); + const { user } = auth; const { id } = await params; const existing = await prisma.webauthnMfaCredential.findUnique({ @@ -78,6 +82,10 @@ export const DELETE = apiHandler( return apiError("Security key not found", 404); } + // Ownership resolved and the row is about to go — spend the elevation now, + // so a 404 for someone else's key id does not burn it. + await auth.commitElevation(); + await prisma.webauthnMfaCredential.delete({ where: { id } }); await auditLog("auth.mfa.webauthn.remove", { 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 109acbbf3..a472d8d55 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 @@ -15,7 +15,8 @@ import { createMfaRegistrationOptions } from "@/lib/auth/mfa/webauthn"; export const dynamic = "force-dynamic"; export const POST = apiHandler(async () => { - const { user } = await requireMfaManagementAuth(); + const auth = await requireMfaManagementAuth(); + const { user } = auth; const rl = await checkRateLimit( `mfa:webauthn:register:${user.id}`, @@ -30,6 +31,8 @@ export const POST = apiHandler(async () => { return res; } + await auth.commitElevation(); + const { options, challengeId } = await createMfaRegistrationOptions( user.id, user.email ?? user.username, diff --git a/src/app/api/auth/me/mfa/webauthn/register/verify/route.ts b/src/app/api/auth/me/mfa/webauthn/register/verify/route.ts index b3b651ca3..371e91174 100644 --- a/src/app/api/auth/me/mfa/webauthn/register/verify/route.ts +++ b/src/app/api/auth/me/mfa/webauthn/register/verify/route.ts @@ -24,7 +24,8 @@ import { setMfaEnrollCookie } from "@/lib/auth/mfa-enrollment"; export const dynamic = "force-dynamic"; export const POST = apiHandler(async (request: NextRequest) => { - const { user } = await requireMfaManagementAuth(); + const auth = await requireMfaManagementAuth(); + const { user } = auth; const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 64 * 1024, @@ -46,6 +47,10 @@ export const POST = apiHandler(async (request: NextRequest) => { return apiError("Security key verification failed", 400); } + // The attestation verified and the credential is about to be stored — a + // failed ceremony above must not have cost the caller their elevation. + await auth.commitElevation(); + const { registrationInfo } = verification; const created = await prisma.webauthnMfaCredential.create({ data: { diff --git a/src/app/api/auth/me/sessions/route.ts b/src/app/api/auth/me/sessions/route.ts index 440f4d44a..24f41a095 100644 --- a/src/app/api/auth/me/sessions/route.ts +++ b/src/app/api/auth/me/sessions/route.ts @@ -88,7 +88,10 @@ export const GET = apiHandler(async () => { export const DELETE = apiHandler(async (request: NextRequest) => { const { user, session } = await requireAuth(); - const { sessionsRevoked } = await destroyOtherSessions(user.id, session.id); + const { sessionsRevoked } = await destroyOtherSessions(user.id, { + kind: "session", + sessionId: session.id, + }); await auditLog("auth.session.revoke_others", { userId: user.id, diff --git a/src/app/api/auth/step-up/options/route.ts b/src/app/api/auth/step-up/options/route.ts index f9caae99d..30392af0c 100644 --- a/src/app/api/auth/step-up/options/route.ts +++ b/src/app/api/auth/step-up/options/route.ts @@ -1,23 +1,30 @@ /** * POST /api/auth/step-up/options * - * Begin a primary-passkey assertion for the step-up mint below. Returns - * SimpleWebAuthn assertion options scoped to the calling account's registered - * passkeys, plus the server-issued challenge id to present back at - * `POST /api/auth/step-up`. + * Begin an assertion ceremony for the step-up mint. Two kinds: + * + * `passkey` — against the account's PRIMARY passkeys. At parity with + * `/api/auth/passkey/login-verify`, which the web treats as a + * second-factor-grade proof. + * `webauthn` — against the account's registered SECOND-FACTOR security keys. + * + * Returns SimpleWebAuthn assertion options plus the server-issued challenge id + * to present back at `POST /api/auth/step-up`. * * Bearer-only (`requireBearerAuth`) — a browser has no use for this surface; it * re-proves a factor at login and carries the result on its session row. The - * allow-list is never empty by construction: an account with no passkey gets a - * 409 telling the client to use the password arm instead. + * allow-list is never empty by construction: an account with no credential of + * the requested kind gets a 409 so the client can offer another arm. */ import { NextRequest } from "next/server"; import { apiHandler, requireBearerAuth } from "@/lib/api-handler"; -import { apiError, apiSuccess } from "@/lib/api-response"; +import { apiError, apiSuccess, safeJson } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; import { prisma } from "@/lib/db"; import { checkAuthSurfaceRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; import { createAuthenticationOptions } from "@/lib/auth/passkey"; +import { createMfaAuthenticationOptions } from "@/lib/auth/mfa/webauthn"; +import { stepUpOptionsSchema } from "@/lib/validations/step-up"; export const dynamic = "force-dynamic"; @@ -36,6 +43,29 @@ export const POST = apiHandler(async (request: NextRequest) => { }); } + const { data: body, error: jsonError } = await safeJson(request, { + maxBytes: 4 * 1024, + }); + if (jsonError) return jsonError; + + const parsed = stepUpOptionsSchema.safeParse(body); + if (!parsed.success) { + return apiError("Invalid request", 422); + } + + if (parsed.data.method === "webauthn") { + const result = await createMfaAuthenticationOptions(user.id); + if (!result) { + annotate({ action: { name: "auth.stepup.webauthn.no_key" } }); + return apiError("No security key registered", 409); + } + annotate({ action: { name: "auth.stepup.webauthn.options" } }); + return apiSuccess({ + options: result.options, + challengeId: result.challengeId, + }); + } + const passkeyCount = await prisma.passkey.count({ where: { userId: user.id }, }); diff --git a/src/app/api/auth/step-up/route.ts b/src/app/api/auth/step-up/route.ts index ec1e6e228..7744cc320 100644 --- a/src/app/api/auth/step-up/route.ts +++ b/src/app/api/auth/step-up/route.ts @@ -4,6 +4,15 @@ * Re-prove a factor and receive a single-use, token-bound step-up elevation for * one second-factor-management call. * + * WHICH factor is re-proved decides WHAT the elevation reaches. `password` + * reaches exactly what a plain cookie session reaches. `totp`, `webauthn`, and + * `passkey` additionally satisfy the fresh-factor routes — disable, recovery-code + * rotation, security-key removal — and that set is precisely the set of + * ceremonies for which the web stamps a session second-factor-verified. If a + * password-proved elevation opened those routes, a stolen token plus the account + * password would rotate the recovery codes (which take no factor in the body) + * and then spend one of them to disable the second factor outright. + * * This is the whole point of the mechanism, so it is worth being blunt about * what it is not: it is NOT a way to turn a token into a stronger credential. * Presenting the token gets you as far as this endpoint and no further — the @@ -37,8 +46,11 @@ import { } from "@/lib/rate-limit"; import { verifyPassword } from "@/lib/auth/password"; import { verifyAuthentication } from "@/lib/auth/passkey"; +import { verifyMfaAuthentication } from "@/lib/auth/mfa/webauthn"; +import { verifyMfaFactor } from "@/lib/auth/mfa/verify-factor"; import { mintStepUpElevation, + isFreshFactorMethod, STEP_UP_ELEVATION_TTL_SECONDS, type StepUpMethod, } from "@/lib/auth/step-up"; @@ -103,32 +115,62 @@ export const POST = apiHandler(async (request: NextRequest) => { return apiError("Invalid request", 422); } + const method: StepUpMethod = parsed.data.method; let proved = false; - let method: StepUpMethod = parsed.data.method; // Audit-only detail. Never reaches the response — see the file header. let failure = "invalid"; + /** + * Resolve a WebAuthn challenge and confirm it belongs to THIS account before + * any verification runs. Both ceremony helpers resolve a challenge by id + * alone, so without this a challenge minted for another account could be + * carried into the ceremony. + */ + const challengeBelongsToCaller = async (id: string): Promise => { + const row = await prisma.authChallenge.findUnique({ + where: { id }, + select: { userId: true }, + }); + return Boolean(row && row.userId === user.id); + }; + if (parsed.data.method === "password") { if (!user.passwordHash) { // An SSO-provisioned account has no password to re-prove. Same refusal as // a wrong one: the account's credential shape is not a token holder's to // enumerate. `/api/auth/step-up/options` is the honest discovery path — - // it 409s when there is no passkey either. + // it 409s when there is no credential of the requested kind either. failure = "no_password"; } else { proved = await verifyPassword(user.passwordHash, parsed.data.password); if (!proved) failure = "bad_password"; } + } else if (parsed.data.method === "totp") { + // The shared factor verifier, so the replay guard and the accepted-step + // burn behave exactly as they do at login and at MFA-disable. A code spent + // here cannot be replayed there. + const result = await verifyMfaFactor(user, "totp", parsed.data.code); + proved = result.ok; + if (!proved) failure = result.replay ? "totp_replay" : "bad_totp"; + } else if (parsed.data.method === "webauthn") { + if (!(await challengeBelongsToCaller(parsed.data.challengeId))) { + failure = "foreign_challenge"; + } else { + try { + // Scoped to the caller's own second-factor credentials by the verifier. + proved = await verifyMfaAuthentication( + parsed.data.challengeId, + user.id, + parsed.data.credential, + ); + if (!proved) failure = "bad_assertion"; + } catch { + // A malformed / expired challenge is a failed attempt, not a 500. + failure = "bad_assertion"; + } + } } else { - method = "passkey"; - // Bind the challenge to the caller BEFORE verifying. `verifyAuthentication` - // resolves a challenge by id alone; pinning ownership here means a challenge - // minted for another account cannot be carried into this ceremony. - const challenge = await prisma.authChallenge.findUnique({ - where: { id: parsed.data.challengeId }, - select: { userId: true }, - }); - if (!challenge || challenge.userId !== user.id) { + if (!(await challengeBelongsToCaller(parsed.data.challengeId))) { failure = "foreign_challenge"; } else { try { @@ -142,7 +184,6 @@ export const POST = apiHandler(async (request: NextRequest) => { result.verification.verified && result.passkey.userId === user.id; if (!proved) failure = "bad_assertion"; } catch { - // A malformed / expired challenge is a failed attempt, not a 500. failure = "bad_assertion"; } } @@ -184,5 +225,12 @@ export const POST = apiHandler(async (request: NextRequest) => { elevation: token, expiresAt: expiresAt.toISOString(), expiresInSeconds: STEP_UP_ELEVATION_TTL_SECONDS, + method, + // Whether this elevation reaches the fresh-factor routes (disable, + // recovery-code rotation, security-key removal). A password-proved + // elevation does not, exactly as a password login does not on the web. + // Surfaced so the client can pick the right ceremony up front instead of + // discovering the refusal after spending a proof. + satisfiesFreshFactor: isFreshFactorMethod(method), }); }); diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index df6d0bdd7..96b32e4a9 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -14,9 +14,11 @@ import { type ScopeRequirement, } from "./auth/bearer"; import { - redeemStepUpElevation, + claimStepUpElevation, + validateStepUpElevation, STEP_UP_ELEVATION_TTL_SECONDS, } from "./auth/step-up"; +import { hashToken } from "./auth/hmac"; import { AssistantDisabledError } from "./feature-flags"; import { ConsentRequiredError } from "./ai/consent-guard"; import { SCOPE_HEALTH_READ, SCOPE_HEALTH_WRITE } from "./mcp/oauth/config"; @@ -624,6 +626,27 @@ export async function requireFreshMfaIfEnrolled( return auth; } +/** + * A Bearer-resolved caller. + * + * `apiTokenId` and `accessTokenHash` are named rather than smuggled through + * `AuthContext.session.id`. That field means "the session row id" on the cookie + * path and "the ApiToken row id" on the Bearer path, and code that forgot which + * one it held passed a token id to a session-scoped query — deleting every one + * of the user's browser sessions and revoking the caller's own refresh token. + * Naming the fields is what stops that recurring. + */ +export interface BearerAuthContext extends AuthContext { + /** The `ApiToken` row id — the binding a step-up elevation is tied to. */ + apiTokenId: string; + /** + * HMAC of the presented access token, which is what `RefreshToken` + * cross-references in `accessTokenHash`. Lets a route identify the CALLER's + * own device login and spare it when revoking every other one. + */ + accessTokenHash: string; +} + /** * v1.30.34 — resolve a caller by Bearer token ONLY, refusing a cookie session. * @@ -639,7 +662,7 @@ export async function requireFreshMfaIfEnrolled( * a cookie-equivalent (`["*"]`) token is admitted. A narrow token — an MCP grant, * a medication-ingest grant — is refused 403 by the resolver. */ -export async function requireBearerAuth(): Promise { +export async function requireBearerAuth(): Promise { let authHeader: string | null = null; try { const headerList = await headers(); @@ -650,7 +673,13 @@ export async function requireBearerAuth(): Promise { if (!authHeader?.startsWith("Bearer ")) { throw new HttpError(401, "Not authenticated"); } - return await authenticateBearer(authHeader.slice(7), undefined); + const raw = authHeader.slice(7); + const auth = await authenticateBearer(raw, undefined); + return { + ...auth, + apiTokenId: auth.session.id, + accessTokenHash: hashToken(raw), + }; } /** @@ -660,10 +689,40 @@ export async function requireBearerAuth(): Promise { */ export const STEP_UP_ELEVATION_HEADER = "x-step-up"; +/** + * What an MFA-management route received, with the transport made explicit. + * + * The two arms carry DIFFERENT fields on purpose. A cookie caller has a session + * row and a Bearer caller does not, so `session` exists only on the cookie arm + * and the compiler refuses to read it on the other. This is the fix for a real + * defect: the MFA-disable route passed `session.id` to `destroyOtherSessions`, + * which on the Bearer path was an ApiToken id — matching no session row, so the + * "keep the current one" exclusion excluded nothing and the caller revoked its + * own device login. A comment would not have caught that; a type does. + */ +export type MfaManagementContext = { + user: User; + /** + * Spend the elevation. Call it immediately BEFORE the mutation, once every + * cheap validation has passed — a 429, a 422, or a wrong factor code must not + * burn a proof the user then has to mint again against a 5-per-15-minute + * ceiling. + * + * A no-op on the cookie arm (the session stamp is not consumable). On the + * Bearer arm it is the atomic single-use claim, and it THROWS + * `StepUpRequiredError` if the claim is lost — so a concurrent redemption + * still yields exactly one winner even though validation happened earlier. + */ + commitElevation: () => Promise; +} & ( + | { transport: "cookie"; session: { id: string; expiresAt: Date } } + | { transport: "bearer"; apiTokenId: string; accessTokenHash: string } +); + /** * v1.30.34 — the single gate for second-factor management. * - * Every MFA-management route goes through here and nothing else does. The set + * Every MFA-management MUTATION goes through here and nothing else does. The set * is frozen by `src/__tests__/step-up-elevation-guard.test.ts`, so a future * route cannot quietly join it: widening the reach of an elevation has to be a * visible edit to that allowlist. @@ -677,35 +736,48 @@ export const STEP_UP_ELEVATION_HEADER = "x-step-up"; * all. * * BEARER + ELEVATION — a token that resolves cleanly AND presents a valid, - * unconsumed elevation minted for that same token against a re-proved factor. - * The token alone is never enough: with no header, or a header that does not - * redeem, the caller gets the same `auth.stepup.required` a stale cookie - * session gets. + * unconsumed elevation minted for that same token against a re-proved factor + * of sufficient strength. The token alone is never enough. + * + * FRESH FACTOR IS ABOUT WHICH FACTOR, NOT JUST HOW RECENT. On the cookie path + * `requireFreshMfa` reads `Session.mfaVerifiedAt`, and only a completed second + * factor or a primary passkey login ever writes it — a password login does not. + * The Bearer arm holds the identical line through `FRESH_FACTOR_METHODS`: a + * password-proved elevation reaches what a plain cookie session reaches and + * stops there. Without that rule, a stolen token plus the account password could + * rotate the recovery codes and spend one to disable the second factor. * * `requireAdmin` is untouched and stays cookie-only. An elevation cannot reach * it — not because a check refuses one, but because `requireAdmin` resolves * through `getSession()` and never consults this function or the header. * - * @param options.freshFactor mirrors the cookie path's `requireFreshMfa`: the - * account must actually have a second factor enrolled. Set by the destructive - * routes (disable, recovery-code rotation, security-key removal). + * @param options.freshFactor mirrors the cookie path's `requireFreshMfa`. Set by + * the destructive routes (disable, recovery-code rotation, security-key + * removal). */ export async function requireMfaManagementAuth( options: { freshFactor?: boolean } = {}, -): Promise { +): Promise { + const freshFactor = options.freshFactor === true; + // Cookie first, and via the original helpers — the web path runs the same // code it always did. const sessionData = await getSession(); if (sessionData) { - return options.freshFactor + const resolved = freshFactor ? await requireFreshMfa(MFA_STEP_UP_MAX_AGE_SECONDS) : await requireCookieAuth(); + return { + transport: "cookie", + user: resolved.user, + session: resolved.session, + commitElevation: async () => {}, + }; } // 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 apiTokenId = auth.session.id; let raw: string | null = null; try { @@ -718,36 +790,43 @@ export async function requireMfaManagementAuth( if (!raw) { annotate({ action: { name: "auth.stepup.elevation.missing" }, - meta: { maxAgeSeconds: STEP_UP_ELEVATION_TTL_SECONDS }, + meta: { maxAgeSeconds: STEP_UP_ELEVATION_TTL_SECONDS, freshFactor }, }); throw new StepUpRequiredError(); } - const redeemed = await redeemStepUpElevation({ - rawToken: raw, - userId: auth.user.id, - apiTokenId, - }); - - if (!redeemed.ok) { + const refusal = (reason: string): StepUpRequiredError => { // One audit row with the machine reason, one generic refusal on the wire. // A prober learns only "not accepted" — never whether the elevation was - // unknown, already spent, expired, or minted for a different token. + // unknown, already spent, expired, minted for a different token, or minted + // from a factor too weak for this route. auditLog("auth.stepup.elevation.rejected", { userId: auth.user.id, - details: { reason: redeemed.reason }, + details: { reason, freshFactor }, }).catch(() => {}); annotate({ action: { name: "auth.stepup.elevation.rejected" }, - meta: { reason: redeemed.reason }, + meta: { reason, freshFactor }, }); - throw new StepUpRequiredError(); - } + // Returned rather than thrown so every call site reads `throw refusal(...)` + // and the compiler narrows the result union afterwards. + return new StepUpRequiredError(); + }; + + // Validate WITHOUT consuming. The route runs its own cheap checks next and + // spends the elevation only when it is about to act. + const validated = await validateStepUpElevation({ + rawToken: raw, + userId: auth.user.id, + apiTokenId: auth.apiTokenId, + requireFreshFactor: freshFactor, + }); + if (!validated.ok) throw refusal(validated.reason); // Parity with the cookie path: `requireFreshMfa` refuses an account with no // second factor enrolled, because a step-up-gated action is meaningless there. // The Bearer path holds the same line rather than becoming the softer route. - if (options.freshFactor) { + if (freshFactor) { let enrolled = Boolean(auth.user.totpConfirmedAt); if (!enrolled) { const keys = await prisma.webauthnMfaCredential.count({ @@ -760,12 +839,25 @@ export async function requireMfaManagementAuth( } } - annotate({ - action: { name: "auth.stepup.elevation.accepted" }, - meta: { method: redeemed.method }, - }); - - return auth; + return { + transport: "bearer", + user: auth.user, + apiTokenId: auth.apiTokenId, + accessTokenHash: auth.accessTokenHash, + commitElevation: async () => { + const claimed = await claimStepUpElevation({ + rawToken: raw, + userId: auth.user.id, + apiTokenId: auth.apiTokenId, + requireFreshFactor: freshFactor, + }); + if (!claimed.ok) throw refusal(claimed.reason); + annotate({ + action: { name: "auth.stepup.elevation.accepted" }, + meta: { method: claimed.method, freshFactor }, + }); + }, + }; } /** diff --git a/src/lib/auth/__tests__/step-up.test.ts b/src/lib/auth/__tests__/step-up.test.ts index 70a46d2b0..f37365ddb 100644 --- a/src/lib/auth/__tests__/step-up.test.ts +++ b/src/lib/auth/__tests__/step-up.test.ts @@ -1,23 +1,33 @@ /** * Unit-level checks on the elevation primitive that need no database. * - * The behavioural work — binding, single use, expiry, the concurrent race — - * lives in `tests/integration/step-up-elevation.test.ts`, because every one of - * those claims is a claim about what Postgres does with a WHERE clause and a - * mocked client cannot answer it. What is worth pinning here is the pair of - * constants that must not drift apart, and the shape of the value the mint - * hands out. + * The behavioural work — binding, single use, expiry, the concurrent race, the + * fresh-factor rule end to end — lives in + * `tests/integration/step-up-elevation.test.ts`, because every one of those + * claims is a claim about what Postgres does with a WHERE clause and a mocked + * client cannot answer it. What is worth pinning here is the arithmetic and the + * constants, both of which a mock CAN answer honestly. */ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { readFileSync } from "node:fs"; + +const createMock = vi.fn().mockResolvedValue({ id: "elevation-1" }); vi.mock("@/lib/db", () => ({ prisma: { + $transaction: vi.fn(async (fn: (tx: unknown) => Promise) => + fn({ + stepUpElevation: { + deleteMany: vi.fn().mockResolvedValue({ count: 0 }), + create: createMock, + }, + }), + ), stepUpElevation: { - create: vi.fn().mockResolvedValue({ id: "elevation-1" }), deleteMany: vi.fn().mockResolvedValue({ count: 0 }), - updateMany: vi.fn().mockResolvedValue({ count: 0 }), findUnique: vi.fn().mockResolvedValue(null), }, + $queryRaw: vi.fn().mockResolvedValue([]), }, })); @@ -26,10 +36,32 @@ process.env.API_TOKEN_HMAC_KEY ??= const { mintStepUpElevation, - redeemStepUpElevation, + claimStepUpElevation, + validateStepUpElevation, + isFreshFactorMethod, + FRESH_FACTOR_METHODS, STEP_UP_ELEVATION_TTL_SECONDS, } = await import("@/lib/auth/step-up"); -const { MFA_STEP_UP_MAX_AGE_SECONDS } = await import("@/lib/api-handler"); +const { MFA_STEP_UP_MAX_AGE_SECONDS, STEP_UP_ELEVATION_HEADER } = + await import("@/lib/api-handler"); +const { hashToken } = await import("@/lib/auth/hmac"); + +beforeEach(() => { + createMock.mockClear(); +}); + +async function mint() { + return mintStepUpElevation({ + userId: "u1", + apiTokenId: "t1", + method: "password", + }); +} + +/** The `data` object the mint handed to Prisma. */ +function lastCreateData(): { tokenHash: string; expiresAt: Date } { + return createMock.mock.calls.at(-1)![0].data; +} describe("the two step-up windows stay pinned together", () => { it("matches the cookie path's freshness window exactly", () => { @@ -40,70 +72,114 @@ describe("the two step-up windows stay pinned together", () => { }); describe("the minted value", () => { - it("is a prefixed 32-byte CSPRNG secret, and the plaintext never reaches the row", async () => { - const { prisma } = await import("@/lib/db"); - const { token } = await mintStepUpElevation({ - userId: "u1", - apiTokenId: "t1", - method: "password", - }); - + it("is a prefixed 32-byte CSPRNG secret", async () => { + const { token } = await mint(); expect(token).toMatch(/^hle_[0-9a-f]{64}$/); + }); - const created = vi.mocked(prisma.stepUpElevation.create).mock.calls[0][0]; - const stored = (created.data as { tokenHash: string }).tokenHash; - expect(stored).not.toContain(token); - expect(stored).toMatch(/^[0-9a-f]{64}$/); + it("stores the HMAC and nothing derived from the plaintext", async () => { + const { token } = await mint(); + const { tokenHash } = lastCreateData(); + + // Asserted as an EQUALITY against the expected hash. The previous + // `not.toContain` comparison was vacuous — the stored value is 64 chars and + // the token is 68, so a 64-char string can never contain it, and storing + // `token.slice(4)` in cleartext would have passed. + expect(tokenHash).toBe(hashToken(token)); + expect(tokenHash).not.toBe(token); + expect(tokenHash).not.toBe(token.slice(4)); }); it("differs on every mint", async () => { - const a = await mintStepUpElevation({ - userId: "u1", - apiTokenId: "t1", - method: "password", - }); - const b = await mintStepUpElevation({ - userId: "u1", - apiTokenId: "t1", - method: "password", - }); + const a = await mint(); + const b = await mint(); expect(a.token).not.toBe(b.token); }); }); -describe("redemption rejects a wrong-shaped value before touching the database", () => { - it("refuses a value carrying another credential's prefix", async () => { - const { prisma } = await import("@/lib/db"); - vi.mocked(prisma.stepUpElevation.updateMany).mockClear(); +describe("the expiry that is actually written", () => { + it("lands five minutes ahead, not five minutes' worth of milliseconds", async () => { + // The returned `expiresInSeconds` is an echo of a constant and proves + // nothing about the row. Dropping the `* 1000` from the mint yields a + // 300-MILLISECOND elevation with every constant still reading 300, so the + // computed timestamp is what has to be asserted. + const before = Date.now(); + const { expiresAt } = await mint(); + const after = Date.now(); - // `hls_` is the session cookie secret and `hlk_` an access token. Neither - // is an elevation, and neither should cost a query to find that out. - for (const value of [ - `hls_${"a".repeat(64)}`, - `hlk_${"a".repeat(64)}`, - "", - ]) { - const result = await redeemStepUpElevation({ + const written = lastCreateData().expiresAt; + expect(written.getTime()).toBe(expiresAt.getTime()); + + const target = STEP_UP_ELEVATION_TTL_SECONDS * 1000; + expect(written.getTime() - before).toBeGreaterThanOrEqual(target - 1_000); + expect(written.getTime() - after).toBeLessThanOrEqual(target + 1_000); + // And unambiguously far from the millisecond-scale mistake. + expect(written.getTime() - before).toBeGreaterThan(60_000); + }); +}); + +describe("the fresh-factor rule", () => { + it("admits only the methods the web stamps a session for", () => { + expect([...FRESH_FACTOR_METHODS].sort()).toEqual([ + "passkey", + "totp", + "webauthn", + ]); + expect(isFreshFactorMethod("totp")).toBe(true); + expect(isFreshFactorMethod("webauthn")).toBe(true); + expect(isFreshFactorMethod("passkey")).toBe(true); + // The B1 invariant: a password is not a second factor. + expect(isFreshFactorMethod("password")).toBe(false); + }); +}); + +describe("a wrong-shaped value is refused before touching the database", () => { + it.each([ + ["session secret", `hls_${"a".repeat(64)}`], + ["access token", `hlk_${"a".repeat(64)}`], + ["empty", ""], + ])("refuses a %s without a query", async (_label, value) => { + const { prisma } = await import("@/lib/db"); + vi.mocked(prisma.$queryRaw).mockClear(); + vi.mocked(prisma.stepUpElevation.findUnique).mockClear(); + + for (const fn of [claimStepUpElevation, validateStepUpElevation]) { + const result = await fn({ rawToken: value, userId: "u1", apiTokenId: "t1", + requireFreshFactor: false, }); expect(result).toEqual({ ok: false, reason: "malformed" }); } - expect(prisma.stepUpElevation.updateMany).not.toHaveBeenCalled(); + expect(prisma.$queryRaw).not.toHaveBeenCalled(); + expect(prisma.stepUpElevation.findUnique).not.toHaveBeenCalled(); }); }); describe("the elevation header name is stable", () => { - it("is the lowercase form the gate reads", async () => { - const { STEP_UP_ELEVATION_HEADER } = await import("@/lib/api-handler"); - // Header lookups in the gate go through `headers().get()`, which is - // case-insensitive; the constant is lowercase so a direct Map-backed test - // jar matches it too. + it("is the lowercase form the gate reads", () => { expect(STEP_UP_ELEVATION_HEADER).toBe("x-step-up"); expect(STEP_UP_ELEVATION_HEADER).toBe( STEP_UP_ELEVATION_HEADER.toLowerCase(), ); }); }); + +describe("the credential prefix lists do not drift apart", () => { + it("the idempotency cache and the log redactor both know `hle_`", async () => { + // Two parallel denylists guard the same class of value. They were already + // one prefix out of step (`hls_` was in one and not the other), which is + // how the next one gets missed. + const idempotency = readFileSync("src/lib/idempotency.ts", "utf8"); + expect(idempotency).toContain("hle_"); + + const { redactSecrets } = await import("@/lib/logging/redact"); + const secret = `hle_${"a".repeat(64)}`; + expect(redactSecrets(`elevation=${secret}`)).not.toContain(secret); + expect(redactSecrets(`session=hls_${"b".repeat(64)}`)).toContain( + "[REDACTED]", + ); + }); +}); diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index 4cb7839b2..cdc22a74a 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -368,6 +368,16 @@ export async function destroyAllSessions(userId: string): Promise { ]); } +/** + * How the caller of a "revoke everything else" action authenticated. + * + * Exists so the caller's own credential can be spared without any code having + * to guess what kind of id it is holding — see `destroyOtherSessions`. + */ +export type CurrentCredential = + | { kind: "session"; sessionId: string } + | { kind: "accessToken"; accessTokenHash: string }; + /** * v1.23 — "sign out everywhere" for the user-facing active-session surface * (issue #64). Distinct from `destroyAllSessions`: this keeps the caller's @@ -382,20 +392,41 @@ export async function destroyAllSessions(userId: string): Promise { */ export async function destroyOtherSessions( userId: string, - currentSessionId: string, + current: CurrentCredential, ): Promise<{ sessionsRevoked: number }> { + // Which rows describe the CALLER depends on how the caller authenticated, and + // the two shapes are not interchangeable. A cookie caller is a `Session` row. + // A Bearer caller is an `ApiToken` whose device login is the `RefreshToken` + // carrying the matching `accessTokenHash` — it has no session row at all, so + // every session goes. Passing an ApiToken id in as a session id (which an + // earlier revision did) silently excluded nothing and revoked the caller's own + // refresh token, logging the calling device out at its next rotation. + const keptSessionId = + current.kind === "session" ? current.sessionId : undefined; + const keptRefreshWhere = + current.kind === "accessToken" + ? { accessTokenHash: { not: current.accessTokenHash } } + : {}; + const [deleted] = await prisma.$transaction([ prisma.session.deleteMany({ - where: { userId, id: { not: currentSessionId } }, + where: { + userId, + ...(keptSessionId ? { id: { not: keptSessionId } } : {}), + }, }), prisma.refreshToken.updateMany({ - where: { userId, revokedAt: null }, + where: { userId, revokedAt: null, ...keptRefreshWhere }, data: { revokedAt: new Date() }, }), // v1.23 — "sign out everywhere" also drops every trusted device so a // remembered browser can no longer skip the second factor (§1.7: a device // cookie is killed on sign-out-everywhere). prisma.trustedDevice.deleteMany({ where: { userId } }), + // v1.30.34 — and every step-up elevation. A user who signs out everywhere + // expects nothing to remain that could act on the account; a live elevation + // is exactly that. + prisma.stepUpElevation.deleteMany({ where: { userId } }), ]); return { sessionsRevoked: deleted.count }; } diff --git a/src/lib/auth/step-up.ts b/src/lib/auth/step-up.ts index 734c214c1..d3aa78834 100644 --- a/src/lib/auth/step-up.ts +++ b/src/lib/auth/step-up.ts @@ -16,36 +16,41 @@ * session; a token client has no session to stamp. An elevation is that proof, * made explicit and made disposable. * - * THE PROPERTIES, and why each one is load-bearing: + * WHICH FACTOR WAS PROVED IS PART OF THE PROOF. `method` is an authorisation + * input, not telemetry. The web stamps `mfaVerifiedAt` only for a completed + * SECOND factor (TOTP, security key) or a primary passkey login — never for a + * password. If a password-proved elevation satisfied the fresh-factor routes, + * a stolen token plus the account password would rotate the recovery codes + * (which take no factor in the body) and then spend one of them to disable the + * second factor outright. So the fresh-factor routes accept only + * `FRESH_FACTOR_METHODS`, and the password arm reaches exactly what a plain + * cookie session reaches — no more. + * + * THE OTHER PROPERTIES, and why each is load-bearing: * * Minted only against a re-proved factor. Presenting the token alone yields - * nothing — that is the entire point. A stolen token gains no more reach than - * it had before this module existed. + * nothing. A stolen token gains no more reach than it had before. * * Bound to one token (`apiTokenId`). An elevation minted by the phone cannot - * be redeemed by a second token, not even another of the same user's. Theft of - * the elevation in transit is therefore useless without also holding the exact - * token it was minted for. + * be redeemed by a second token, not even another of the same user's. * - * Single-use. Redemption is a conditional UPDATE (see `redeemStepUpElevation`) - * so two concurrent redemptions cannot both succeed. One management action per - * proof, by construction rather than by convention. + * Single-use. The claim is one conditional UPDATE, so two concurrent + * redemptions cannot both succeed. * - * Short-lived. `STEP_UP_ELEVATION_TTL_SECONDS` mirrors the cookie path's - * `MFA_STEP_UP_MAX_AGE_SECONDS` exactly, so both transports carry the same - * freshness promise and neither is the softer way in. + * Short-lived. `STEP_UP_ELEVATION_TTL_SECONDS` equals the cookie path's + * `MFA_STEP_UP_MAX_AGE_SECONDS`, so neither transport is the softer way in. * * Stored hashed. Only `hashToken(secret)` reaches the database — the same - * posture as ApiToken, RefreshToken, and the session secret. A table dump - * yields nothing redeemable, and the raw value exists only in the mint - * response and the client's memory. + * posture as ApiToken, RefreshToken, and the session secret. * - * INVALIDATION comes from two directions. A password change calls - * `revokeStepUpElevations` outright. A revoked or deleted token strands its - * elevations without any code here running: redemption sits behind Bearer - * resolution, which refuses a revoked token, and the FK cascades on delete. + * INVALIDATION comes from three directions. A password change (self-service or + * operator-forced) and a sign-out-everywhere both call + * `revokeStepUpElevations`. A revoked or deleted token strands its elevations + * without any code here running: redemption sits behind Bearer resolution, + * which refuses a revoked token, and the FK cascades on delete. */ import { randomBytes } from "node:crypto"; +import { Prisma } from "@/generated/prisma/client"; import { prisma } from "@/lib/db"; import { hashToken } from "@/lib/auth/hmac"; @@ -65,16 +70,48 @@ const ELEVATION_SECRET_BYTES = 32; * guarding the same actions should not offer two different freshness promises — * if they diverged, the looser one would simply become the way in. Five minutes * also sits at the tight end of the 5–15 minute band OWASP recommends for - * re-authentication, which is where destructive actions belong; it is long - * enough to type a code from an authenticator app and short enough that an - * elevation captured off a screen or a proxy log is stale before it is useful. + * re-authentication, which is where destructive actions belong. * * A test pins the two constants together so a future edit to one is caught. */ export const STEP_UP_ELEVATION_TTL_SECONDS = 5 * 60; -/** Which factor was re-proved. Recorded for audit; never an authorisation input. */ -export type StepUpMethod = "password" | "passkey"; +/** + * Which factor was re-proved. This is an AUTHORISATION INPUT — see + * `FRESH_FACTOR_METHODS` and the file header. + * + * password — the account password. Reaches what a plain cookie session + * reaches, and no further. + * totp — a current code from the enrolled authenticator. + * webauthn — an assertion from a registered second-factor security key. + * passkey — an assertion from a primary passkey. At parity with + * `/api/auth/passkey/login-verify`, which stamps `mfaVerifiedAt` + * on the web for the very same ceremony. + */ +export type StepUpMethod = "password" | "totp" | "webauthn" | "passkey"; + +/** + * The methods that satisfy a fresh-factor gate — exactly the set for which the + * web writes `mfaVerifiedAt`. `password` is deliberately absent: a password + * login has never satisfied step-up on the web and must not start here. + * + * A recovery code is also absent, and that IS a narrowing relative to the web, + * where `/api/auth/mfa/verify` accepts one and does stamp the session. The + * consequence is that an account which has lost its authenticator cannot + * disable its second factor from the app — only from the web. That asymmetry is + * deliberate and flagged rather than quietly closed, because admitting a + * recovery code here means spending one break-glass credential to authorise + * spending another. + */ +export const FRESH_FACTOR_METHODS: ReadonlySet = new Set([ + "totp", + "webauthn", + "passkey", +]); + +export function isFreshFactorMethod(method: StepUpMethod): boolean { + return FRESH_FACTOR_METHODS.has(method); +} export interface MintedElevation { /** The raw `hle_<64 hex>` value. Returned once; never persisted, never logged. */ @@ -89,10 +126,12 @@ export interface MintedElevation { * not check anything, it records a decision already made. Keep the two adjacent * so that stays obvious. * - * Any earlier unconsumed elevation for the same token is deleted first. A fresh - * proof supersedes an older one, so there is never more than one redeemable - * elevation per token; that both bounds the table and shrinks the window in - * which a captured elevation is worth anything. + * The delete-then-create runs in ONE transaction. Without it two concurrent + * mints can interleave so the second's delete removes the first's row after the + * first has already handed its value to the client, leaving a caller holding an + * elevation that can never redeem. The partial unique index from migration 0260 + * is the backstop: if two transactions still collide, one loses on the + * constraint and retries rather than both rows surviving. */ export async function mintStepUpElevation(opts: { userId: string; @@ -102,60 +141,135 @@ export async function mintStepUpElevation(opts: { const raw = `${ELEVATION_PREFIX}${randomBytes(ELEVATION_SECRET_BYTES).toString("hex")}`; const expiresAt = new Date(Date.now() + STEP_UP_ELEVATION_TTL_SECONDS * 1000); - await prisma.stepUpElevation.deleteMany({ - where: { apiTokenId: opts.apiTokenId, consumedAt: null }, - }); + const write = async () => + prisma.$transaction(async (tx) => { + // A fresh proof supersedes an older one, so there is never more than one + // redeemable elevation per token. That bounds the table and shrinks the + // window in which a captured elevation is worth anything. + await tx.stepUpElevation.deleteMany({ + where: { apiTokenId: opts.apiTokenId, consumedAt: null }, + }); + await tx.stepUpElevation.create({ + data: { + userId: opts.userId, + apiTokenId: opts.apiTokenId, + tokenHash: hashToken(raw), + method: opts.method, + expiresAt, + }, + select: { id: true }, + }); + }); - await prisma.stepUpElevation.create({ - data: { - userId: opts.userId, - apiTokenId: opts.apiTokenId, - tokenHash: hashToken(raw), - method: opts.method, - expiresAt, - }, - select: { id: true }, - }); - - // Opportunistic housekeeping — cheap, indexed, and keeps the table from - // accumulating spent rows without needing a scheduled job for a table that - // only ever sees a handful of rows per user per year. - prisma.stepUpElevation - .deleteMany({ where: { expiresAt: { lt: new Date() } } }) - .catch(() => {}); + try { + await write(); + } catch (err) { + // P2002 on the partial index: a concurrent mint won the race between our + // delete and our insert. One retry (whose delete removes that winner in + // turn) resolves it deterministically. + if ( + err instanceof Prisma.PrismaClientKnownRequestError && + err.code === "P2002" + ) { + await write(); + } else { + throw err; + } + } return { token: raw, expiresAt }; } -/** Why a redemption was refused. Audit detail only — never surfaced to a caller. */ -export type StepUpRedeemFailure = - "malformed" | "unknown" | "wrong_token" | "consumed" | "expired"; +/** Why an elevation was refused. Audit detail only — never surfaced to a caller. */ +export type StepUpRejection = + | "malformed" + | "unknown" + | "wrong_token" + | "consumed" + | "expired" + | "insufficient_factor"; -export type StepUpRedeemResult = - | { ok: true; method: StepUpMethod } - | { ok: false; reason: StepUpRedeemFailure }; +export type StepUpOutcome = + { ok: true; method: StepUpMethod } | { ok: false; reason: StepUpRejection }; + +/** + * Check an elevation WITHOUT consuming it. + * + * Split from the claim so a route can refuse a bad request — a 429, a malformed + * body, a wrong TOTP code — without burning the caller's elevation. Burning one + * on a validation failure was a self-inflicted lockout: the mint allows five + * attempts per fifteen minutes, so five fat-fingered codes would lock a user out + * of their own second-factor settings. + * + * This is NOT an authorisation decision on its own, and nothing may mutate on + * the strength of it. `claimStepUpElevation` still has to win the race before + * any effect happens; this only decides whether the request is worth processing. + */ +export async function validateStepUpElevation(opts: { + rawToken: string; + userId: string; + apiTokenId: string; + requireFreshFactor: boolean; +}): Promise { + if (!opts.rawToken.startsWith(ELEVATION_PREFIX)) { + return { ok: false, reason: "malformed" }; + } + + const row = await prisma.stepUpElevation.findUnique({ + where: { tokenHash: hashToken(opts.rawToken) }, + select: { + userId: true, + apiTokenId: true, + consumedAt: true, + expiresAt: true, + method: true, + }, + }); + + if (!row) return { ok: false, reason: "unknown" }; + if (row.userId !== opts.userId || row.apiTokenId !== opts.apiTokenId) { + return { ok: false, reason: "wrong_token" }; + } + if (row.consumedAt) return { ok: false, reason: "consumed" }; + if (row.expiresAt <= new Date()) return { ok: false, reason: "expired" }; + + const method = row.method as StepUpMethod; + if (opts.requireFreshFactor && !isFreshFactorMethod(method)) { + return { ok: false, reason: "insufficient_factor" }; + } + + return { ok: true, method }; +} /** * Consume an elevation, or refuse. * - * SINGLE-USE ATOMICITY. The claim is one conditional `UPDATE ... WHERE + * SINGLE-USE ATOMICITY. The claim is one conditional `UPDATE … WHERE * token_hash = $1 AND user_id = $2 AND api_token_id = $3 AND consumed_at IS NULL - * AND expires_at > $4` (Prisma's `updateMany`), and the caller proceeds only on - * `count === 1`. Postgres under READ COMMITTED serialises the two writers on the - * row lock: the loser re-evaluates the predicate against the winner's committed - * version, finds `consumed_at` no longer null, and matches zero rows. There is - * no read-then-write window to lose, which is exactly why the check-then-update + * AND expires_at > $4 RETURNING method`, and the caller proceeds only on exactly + * one returned row. Postgres under READ COMMITTED serialises the two writers on + * the row lock: the loser re-evaluates the predicate against the winner's + * committed version, finds `consumed_at` no longer null, and returns nothing. + * There is no read-then-write window to lose, which is why the check-then-update * shape was not used. * - * Both binding fields are in the predicate rather than checked afterwards, so a - * mismatched token cannot be claimed and then rejected — it is never claimed at - * all, and the legitimate holder's elevation survives someone else's attempt. + * `RETURNING method` rather than a follow-up SELECT: a separate read can miss, + * and any fallback for that miss would fabricate an authorisation-relevant + * value — logging a passkey-proved elevation as password-proved, or worse, + * deciding a gate on the invented one. The statement that decides is the + * statement that reports. + * + * The fresh-factor rule is enforced here, on the returned row, not only in + * `validateStepUpElevation`. That validation is advisory; this is the gate. When + * the method is insufficient the row stays consumed — it was a real elevation + * presented in a real redemption, so spending it is right. */ -export async function redeemStepUpElevation(opts: { +export async function claimStepUpElevation(opts: { rawToken: string; userId: string; apiTokenId: string; -}): Promise { + requireFreshFactor: boolean; +}): Promise { if (!opts.rawToken.startsWith(ELEVATION_PREFIX)) { return { ok: false, reason: "malformed" }; } @@ -163,56 +277,69 @@ export async function redeemStepUpElevation(opts: { const tokenHash = hashToken(opts.rawToken); const now = new Date(); - const claimed = await prisma.stepUpElevation.updateMany({ - where: { - tokenHash, - userId: opts.userId, - apiTokenId: opts.apiTokenId, - consumedAt: null, - expiresAt: { gt: now }, - }, - data: { consumedAt: now }, - }); + const claimed = await prisma.$queryRaw<{ method: string }[]>` + UPDATE step_up_elevations + SET consumed_at = ${now} + WHERE token_hash = ${tokenHash} + AND user_id = ${opts.userId} + AND api_token_id = ${opts.apiTokenId} + AND consumed_at IS NULL + AND expires_at > ${now} + RETURNING method + `; - if (claimed.count === 1) { - const row = await prisma.stepUpElevation.findUnique({ + if (claimed.length !== 1) { + // Classify for the audit row only. The caller returns one generic response + // for every branch, so this tells an operator what happened without telling + // a prober anything. + const existing = await prisma.stepUpElevation.findUnique({ where: { tokenHash }, - select: { method: true }, + select: { userId: true, apiTokenId: true, consumedAt: true }, }); - return { ok: true, method: (row?.method as StepUpMethod) ?? "password" }; + if (!existing) return { ok: false, reason: "unknown" }; + if ( + existing.userId !== opts.userId || + existing.apiTokenId !== opts.apiTokenId + ) { + return { ok: false, reason: "wrong_token" }; + } + if (existing.consumedAt) return { ok: false, reason: "consumed" }; + return { ok: false, reason: "expired" }; } - // Classify the refusal for the audit row only. The caller returns one generic - // response for every branch below, so this read tells an operator what - // happened without telling a prober anything. - const existing = await prisma.stepUpElevation.findUnique({ - where: { tokenHash }, - select: { - apiTokenId: true, - userId: true, - consumedAt: true, - expiresAt: true, - }, - }); - if (!existing) return { ok: false, reason: "unknown" }; - if ( - existing.userId !== opts.userId || - existing.apiTokenId !== opts.apiTokenId - ) { - return { ok: false, reason: "wrong_token" }; + const method = claimed[0].method as StepUpMethod; + if (opts.requireFreshFactor && !isFreshFactorMethod(method)) { + return { ok: false, reason: "insufficient_factor" }; } - if (existing.consumedAt) return { ok: false, reason: "consumed" }; - return { ok: false, reason: "expired" }; + + return { ok: true, method }; } /** * Drop every elevation for a user. * - * Called when the account password changes: an elevation is a statement about a - * credential that no longer exists, so it must not outlive it. Deliberately + * Called when the account password changes (self-service or operator-forced) + * and on sign-out-everywhere. An elevation is a statement about a credential and + * a moment, and neither survives either of those events. Deliberately * unconditional over the user's rows — consumed ones go too, since there is no * reason to keep them once the anchor moved. */ export async function revokeStepUpElevations(userId: string): Promise { await prisma.stepUpElevation.deleteMany({ where: { userId } }); } + +/** + * Delete every elevation whose window has closed. + * + * Runs on the queue, not on a request. The earlier fire-and-forget sweep on the + * mint path was a cross-tenant unbounded DELETE competing with the request for a + * pool connection, and recurring work belongs on pg-boss. An expired row is + * already unredeemable — the predicate in `claimStepUpElevation` refuses it — so + * this is hygiene, not enforcement, and a missed run breaks nothing. + */ +export async function pruneExpiredStepUpElevations(): Promise { + const { count } = await prisma.stepUpElevation.deleteMany({ + where: { expiresAt: { lt: new Date() } }, + }); + return count; +} diff --git a/src/lib/idempotency.ts b/src/lib/idempotency.ts index a10155bc2..ad0ba614a 100644 --- a/src/lib/idempotency.ts +++ b/src/lib/idempotency.ts @@ -426,7 +426,7 @@ export function withIdempotency< // or any other word containing `sk-` would otherwise // silently break idempotency for benign retries). const SECRET_PATTERN = - /(?:\b(?:hlk_|hlr_|hls_|hlv_)[A-Za-z0-9_-]+|\bsk-(?:ant-)?[A-Za-z0-9_-]{8,})/; + /(?:\b(?:hlk_|hlr_|hls_|hlv_|hle_)[A-Za-z0-9_-]+|\bsk-(?:ant-)?[A-Za-z0-9_-]{8,})/; const cloned = response.clone(); const text = await cloned.text(); if (!SECRET_PATTERN.test(text)) { diff --git a/src/lib/jobs/reminder/cleanup-handlers.ts b/src/lib/jobs/reminder/cleanup-handlers.ts index 105367bdb..bdbe007be 100644 --- a/src/lib/jobs/reminder/cleanup-handlers.ts +++ b/src/lib/jobs/reminder/cleanup-handlers.ts @@ -40,6 +40,10 @@ const PUSH_ATTEMPT_RETENTION_DAYS = 90; // the two never drift. Slots at 03:40 between push-attempt cleanup (03:35) // and the drain (03:45) inside the existing 03:xx maintenance window. +export interface StepUpElevationCleanupPayload { + _?: never; +} + export interface RateLimitCleanupPayload { triggeredAt: string; } @@ -193,6 +197,32 @@ export async function handleIdempotencyCleanup( }); } +/** + * v1.30.34 — sweep expired step-up elevations. + * + * Hygiene, not enforcement: an expired row is already unredeemable because + * `claimStepUpElevation`'s predicate refuses it, so a missed run costs nothing + * but table space. It lives here rather than fire-and-forget on the mint request + * because a cross-tenant unbounded DELETE has no business competing with a + * user's request for a pool connection. + */ +export async function handleStepUpElevationCleanup( + jobs: Job[], +) { + void jobs; + await withBackgroundEvent("job.step_up_elevation_cleanup", async (evt) => { + const p = getWorkerPrisma(); + try { + const { count } = await p.stepUpElevation.deleteMany({ + where: { expiresAt: { lt: new Date() } }, + }); + evt.addMeta("step_up_elevation_cleanup_deleted", count); + } catch (err) { + evt.addWarning(`step-up-elevation-cleanup failed: ${err}`); + } + }); +} + export async function handleAuditLogCleanup( jobs: Job[], ) { diff --git a/src/lib/jobs/reminder/register-maintenance.ts b/src/lib/jobs/reminder/register-maintenance.ts index c258a79ca..397ea802d 100644 --- a/src/lib/jobs/reminder/register-maintenance.ts +++ b/src/lib/jobs/reminder/register-maintenance.ts @@ -134,6 +134,7 @@ import { } from "./backup-handlers"; import { RateLimitCleanupPayload, + StepUpElevationCleanupPayload, IdempotencyCleanupPayload, AuditLogCleanupPayload, MoodReminderCleanupPayload, @@ -145,6 +146,7 @@ import { handleRateLimitCleanup, handleIdempotencyCleanup, handleAuditLogCleanup, + handleStepUpElevationCleanup, CoachMessageCleanupPayload, handleCoachMessageCleanup, McpTokenCleanupPayload, @@ -184,6 +186,12 @@ const IDEMPOTENCY_CLEANUP_QUEUE = "idempotency-cleanup"; const IDEMPOTENCY_CLEANUP_CRON = "0 3 * * *"; // daily at 03:00 (Europe/Berlin) +const STEP_UP_ELEVATION_CLEANUP_QUEUE = "step-up-elevation-cleanup"; + +// Every 15 minutes. An elevation lives five, so nothing lingers long; the +// cadence only has to be short enough that the table stays small. +const STEP_UP_ELEVATION_CLEANUP_CRON = "*/15 * * * *"; + const AUDIT_LOG_CLEANUP_QUEUE = "audit-log-cleanup"; const AUDIT_LOG_CLEANUP_CRON = "15 3 * * *"; // daily at 03:15 (Europe/Berlin) @@ -280,6 +288,7 @@ const allQueues = [ RATE_LIMIT_CLEANUP_QUEUE, IDEMPOTENCY_CLEANUP_QUEUE, AUDIT_LOG_CLEANUP_QUEUE, + STEP_UP_ELEVATION_CLEANUP_QUEUE, OFFHOST_BACKUP_QUEUE, RESTORE_DRILL_QUEUE, HOST_METRIC_QUEUE, @@ -380,6 +389,7 @@ const schedules: ScheduleEntry[] = [ [RATE_LIMIT_CLEANUP_QUEUE, RATE_LIMIT_CLEANUP_CRON], [IDEMPOTENCY_CLEANUP_QUEUE, IDEMPOTENCY_CLEANUP_CRON], [AUDIT_LOG_CLEANUP_QUEUE, AUDIT_LOG_CLEANUP_CRON], + [STEP_UP_ELEVATION_CLEANUP_QUEUE, STEP_UP_ELEVATION_CLEANUP_CRON], [OFFHOST_BACKUP_QUEUE, OFFHOST_BACKUP_CRON], [RESTORE_DRILL_QUEUE, RESTORE_DRILL_CRON], [HOST_METRIC_QUEUE, HOST_METRIC_CRON], @@ -554,6 +564,11 @@ export async function registerMaintenanceQueues( { localConcurrency: 1 }, handleAuditLogCleanup, ); + await boss.work( + STEP_UP_ELEVATION_CLEANUP_QUEUE, + { localConcurrency: 1 }, + handleStepUpElevationCleanup, + ); await boss.work( OFFHOST_BACKUP_QUEUE, { localConcurrency: 1 }, diff --git a/src/lib/validations/step-up.ts b/src/lib/validations/step-up.ts index c167bab42..96028cd6e 100644 --- a/src/lib/validations/step-up.ts +++ b/src/lib/validations/step-up.ts @@ -7,14 +7,20 @@ import { z } from "zod/v4"; import { webauthnCredentialSchema } from "@/lib/validations/mfa"; /** - * Re-prove a factor to mint an elevation. + * Re-prove a factor to mint an elevation. Four accepted proofs, discriminated on + * `method`, and the choice is an AUTHORISATION input rather than a preference: * - * Two accepted proofs, discriminated on `method`: - * - `password` — the account password. Unavailable on an account that signed - * in through OIDC SSO and never set one; such an account uses the passkey - * arm, or manages its second factor on the web. - * - `passkey` — an assertion against a primary passkey the device already - * holds, begun at `/api/auth/step-up/options`. + * - `password` — the account password. Reaches exactly what a plain cookie + * session reaches. Unavailable on an account provisioned through OIDC SSO + * that never set one. + * - `totp` — a current code from the enrolled authenticator. + * - `webauthn` — an assertion from a registered second-factor security key, + * begun at `/api/auth/step-up/options` with `method: "webauthn"`. + * - `passkey` — an assertion from a primary passkey, begun at the same + * endpoint with `method: "passkey"`. + * + * Only the last three satisfy the fresh-factor routes, mirroring the web, where + * a password login never stamps a session second-factor-verified. * * The password is length-bounded so a body cap is not the only thing standing * between a caller and an unbounded Argon2id verify. @@ -25,6 +31,18 @@ export const stepUpMintSchema = z method: z.literal("password"), password: z.string().min(1).max(512), }), + z.object({ + method: z.literal("totp"), + code: z + .string() + .trim() + .regex(/^\d{6}$/, "Enter the 6-digit code from your authenticator app"), + }), + z.object({ + method: z.literal("webauthn"), + challengeId: z.string().min(1), + credential: webauthnCredentialSchema, + }), z.object({ method: z.literal("passkey"), challengeId: z.string().min(1), @@ -34,5 +52,16 @@ export const stepUpMintSchema = z .meta({ id: "StepUpMintRequest", description: - "Re-prove the account password or a primary passkey to mint a single-use, token-bound step-up elevation.", + "Re-prove a factor to mint a single-use, token-bound step-up elevation. `password` reaches the same routes a plain cookie session reaches; `totp`, `webauthn`, and `passkey` additionally satisfy the fresh-factor routes (disable, recovery-code rotation, security-key removal), matching exactly the ceremonies for which the web stamps a session second-factor-verified.", }); + +/** Which assertion ceremony to begin at `POST /api/auth/step-up/options`. */ +export const stepUpOptionsSchema = z + .object({ + method: z + .enum(["passkey", "webauthn"]) + .describe( + "`passkey` begins an assertion against the account's primary passkeys; `webauthn` against its registered second-factor security keys.", + ), + }) + .meta({ id: "StepUpOptionsRequest" }); diff --git a/tests/integration/sessions-and-known-devices.test.ts b/tests/integration/sessions-and-known-devices.test.ts index fb9cbc7c5..c54e97f91 100644 --- a/tests/integration/sessions-and-known-devices.test.ts +++ b/tests/integration/sessions-and-known-devices.test.ts @@ -58,7 +58,10 @@ describe("destroyOtherSessions", () => { }, }); - const result = await destroyOtherSessions(user.id, current.id); + const result = await destroyOtherSessions(user.id, { + kind: "session", + sessionId: current.id, + }); expect(result.sessionsRevoked).toBe(2); const remaining = await prisma.session.findMany({ diff --git a/tests/integration/step-up-elevation.test.ts b/tests/integration/step-up-elevation.test.ts index f85eacd37..d62ac379e 100644 --- a/tests/integration/step-up-elevation.test.ts +++ b/tests/integration/step-up-elevation.test.ts @@ -15,6 +15,7 @@ */ import { beforeEach, describe, expect, it, vi } from "vitest"; import { NextRequest } from "next/server"; +import * as OTPAuth from "otpauth"; import { cookieJar, headerJar } from "./mock-next-headers"; import { getPrismaClient, truncateAllTables } from "./setup"; @@ -118,15 +119,79 @@ function req(path: string, method = "GET", body?: unknown): NextRequest { return new NextRequest(`https://health.example${path}`, init as never); } +/** Drive the mint endpoint with an arbitrary proof body. */ +async function mintWith( + body: Record, +): Promise<{ status: number; body: Record }> { + const { POST } = await import("@/app/api/auth/step-up/route"); + const res = await POST(req("/api/auth/step-up", "POST", body)); + return { status: res.status, body: (await res.json()) as never }; +} + /** Drive the mint endpoint with a password proof. */ async function mintElevation( password: string, ): Promise<{ status: number; body: Record }> { - const { POST } = await import("@/app/api/auth/step-up/route"); - const res = await POST( - req("/api/auth/step-up", "POST", { method: "password", password }), - ); - return { status: res.status, body: (await res.json()) as never }; + return mintWith({ method: "password", password }); +} + +/** + * Mint a TOTP-proved elevation the way a real client would: enrol a secret, + * then present a live code for it. This is the only fresh-factor arm reachable + * without a WebAuthn authenticator, so it carries the fresh-factor coverage. + */ +async function enrolTotp(): Promise { + const { generateTotpSecret } = await import("@/lib/auth/mfa/totp"); + const { encrypt } = await import("@/lib/crypto"); + const secret = generateTotpSecret(); + await getPrismaClient().user.update({ + where: { id: USER_ID }, + data: { + totpSecretEncrypted: encrypt(secret), + totpConfirmedAt: new Date(), + totpLastStep: null, + }, + }); + return secret; +} + +/** Mirrors the server's TOTP parameters — there is no code generator to import. */ +function currentTotpCode(secretBase32: string): string { + const totp = new OTPAuth.TOTP({ + issuer: "HealthLog", + label: "HealthLog", + algorithm: "SHA1", + digits: 6, + period: 30, + secret: OTPAuth.Secret.fromBase32(secretBase32), + }); + return totp.generate({ timestamp: Date.now() }); +} + +async function mintTotpElevation( + secret: string, +): Promise<{ status: number; body: Record }> { + return mintWith({ method: "totp", code: currentTotpCode(secret) }); +} + +/** + * Seed a known recovery-code batch and return the plaintext. + * + * The disable route wants a live factor in its BODY on top of the step-up. A + * TOTP code spent minting the elevation cannot be reused there — the shared + * verifier burns its time-step, by design — so within one 30-second window the + * body factor has to be something else. That is exactly how the web behaves + * too. + */ +async function seedRecoveryCodes(): Promise { + const { regenerateRecoveryCodes } = + await import("@/lib/auth/mfa/recovery-codes"); + return regenerateRecoveryCodes(USER_ID); +} + +/** Pull the raw elevation out of a successful mint. */ +function elevationOf(result: { body: Record }): string { + return (result.body.data as { elevation: string }).elevation; } /** @@ -136,7 +201,19 @@ async function mintElevation( */ type RouteFn = (request: NextRequest) => Promise; -/** GET /api/auth/me/mfa — the cheapest route in the elevation-accepting set. */ +/** + * POST /api/auth/me/mfa/totp/setup — the cheapest MUTATION in the + * elevation-accepting set, and the probe every redemption case drives. + * + * NOT `GET /api/auth/me/mfa`: that read is plain `requireAuth()` now, so using + * it here would have quietly stopped exercising the elevation path at all. + */ +async function callMfaSetup(): Promise { + const { POST } = await import("@/app/api/auth/me/mfa/totp/setup/route"); + return await POST(req("/api/auth/me/mfa/totp/setup", "POST")); +} + +/** The status read — plain Bearer, no elevation. */ async function callMfaStatus(): Promise { const { GET } = await import("@/app/api/auth/me/mfa/route"); return await (GET as unknown as RouteFn)(req("/api/auth/me/mfa")); @@ -160,7 +237,7 @@ describe("E1 — the token alone is not a step-up proof", () => { useToken(raw); useElevation(null); - const res = await callMfaStatus(); + const res = await callMfaSetup(); // 401 with the machine code the client branches on to launch a re-proof — // not a 200, and not the 403 an unauthorised token would get. @@ -183,7 +260,7 @@ describe("E1 — the token alone is not a step-up proof", () => { useToken(raw); useElevation(`hle_${"f".repeat(64)}`); - expect((await callMfaStatus()).status).toBe(401); + expect((await callMfaSetup()).status).toBe(401); await new Promise((r) => setTimeout(r, 100)); expect(await auditReasons("auth.stepup.elevation.rejected")).toContain( "unknown", @@ -207,7 +284,7 @@ describe("E2 — an elevation is not portable between tokens", () => { // thing standing between these two, which is exactly the point. useToken(b.raw); useElevation(elevation); - const res = await callMfaStatus(); + const res = await callMfaSetup(); expect(res.status).toBe(401); await new Promise((r) => setTimeout(r, 100)); @@ -221,7 +298,7 @@ describe("E2 — an elevation is not portable between tokens", () => { useToken(a.raw); useElevation(elevation); - expect((await callMfaStatus()).status).toBe(200); + expect((await callMfaSetup()).status).toBe(200); }); it("refuses redemption by another user's token", async () => { @@ -236,7 +313,7 @@ describe("E2 — an elevation is not portable between tokens", () => { useToken(theirs.raw); useElevation(elevation); - expect((await callMfaStatus()).status).toBe(401); + expect((await callMfaSetup()).status).toBe(401); }); }); @@ -251,8 +328,8 @@ describe("E3 — an elevation is consumed exactly once", () => { ).elevation; useElevation(elevation); - expect((await callMfaStatus()).status).toBe(200); - expect((await callMfaStatus()).status).toBe(401); + expect((await callMfaSetup()).status).toBe(200); + expect((await callMfaSetup()).status).toBe(401); await new Promise((r) => setTimeout(r, 100)); expect(await auditReasons("auth.stepup.elevation.rejected")).toContain( @@ -271,15 +348,16 @@ describe("E3 — an elevation is consumed exactly once", () => { // conditional UPDATEs on one row rather than between two HTTP handlers that // happen to interleave. A check-then-update implementation passes the // sequential case above and fails here. - const { redeemStepUpElevation } = await import("@/lib/auth/step-up"); + const { claimStepUpElevation } = await import("@/lib/auth/step-up"); const attempt = () => - redeemStepUpElevation({ + claimStepUpElevation({ rawToken: elevation, userId: USER_ID, apiTokenId: id, + requireFreshFactor: false, }); - const results = await Promise.all([ + const results: Awaited>[] = await Promise.all([ attempt(), attempt(), attempt(), @@ -309,7 +387,7 @@ describe("E4 — an expired elevation is refused", () => { }); useElevation(elevation); - expect((await callMfaStatus()).status).toBe(401); + expect((await callMfaSetup()).status).toBe(401); await new Promise((r) => setTimeout(r, 100)); expect(await auditReasons("auth.stepup.elevation.rejected")).toContain( @@ -434,9 +512,9 @@ describe("E7 — the cookie flow is unchanged", () => { return session.id; } - it("serves an MFA-management read on a plain cookie session", async () => { + it("serves an MFA-management mutation on a plain cookie session", async () => { await useCookieSession(); - expect((await callMfaStatus()).status).toBe(200); + expect((await callMfaSetup()).status).toBe(200); }); it("still refuses a step-up-gated route on a cookie with no fresh factor", async () => { @@ -479,7 +557,7 @@ describe("E7 — the cookie flow is unchanged", () => { await useCookieSession(); useElevation(elevation); - expect((await callMfaStatus()).status).toBe(200); + expect((await callMfaSetup()).status).toBe(200); const row = await getPrismaClient().stepUpElevation.findFirst({ where: { apiTokenId: id }, @@ -491,14 +569,85 @@ describe("E7 — the cookie flow is unchanged", () => { // ── E8 — invalidation on credential rotation ───────────────────────── describe("E8 — an elevation does not outlive its anchor", () => { - it("is dropped when the account password changes", async () => { + it("is dropped by a real password change, through the route", async () => { + // Driven through POST /api/auth/password rather than by calling the + // revocation helper. Calling the helper directly proved only that the helper + // deletes rows — deleting the call from BOTH password routes left the old + // version of this test green, so the invalidation story was uncovered. const { raw } = await mintToken("e8a"); useToken(raw); + const elevation = elevationOf(await mintElevation(PASSWORD)); + expect(await getPrismaClient().stepUpElevation.count()).toBe(1); + + const NEW_PASSWORD = "Zt7#qvbLm2xR!e9Wd4Kp"; + const { POST } = await import("@/app/api/auth/password/route"); + const res = await POST( + req("/api/auth/password", "POST", { + currentPassword: PASSWORD, + newPassword: NEW_PASSWORD, + confirmPassword: NEW_PASSWORD, + }), + ); + expect(res.status).toBe(200); + + expect(await getPrismaClient().stepUpElevation.count()).toBe(0); + + // And the value the client still holds is dead. The password route mints a + // replacement cookie session, which the shared jar would otherwise pick up + // and satisfy the gate on the cookie arm — clear it so this probes the + // Bearer arm, which is the one under test. + cookieJar.clear(); + useElevation(elevation); + expect((await callMfaSetup()).status).toBe(401); + }); + + it("is dropped by an operator-forced reset, through the admin route", async () => { + const { raw } = await mintToken("e8a2"); + useToken(raw); await mintElevation(PASSWORD); expect(await getPrismaClient().stepUpElevation.count()).toBe(1); - const { revokeStepUpElevations } = await import("@/lib/auth/step-up"); - await revokeStepUpElevations(USER_ID); + // The admin route is cookie-only, so the reset arrives on an ADMIN session + // while the elevation belongs to the target's token. + await getPrismaClient().user.update({ + where: { id: OTHER_USER_ID }, + data: { role: "ADMIN" }, + }); + const adminSession = await getPrismaClient().session.create({ + data: { + userId: OTHER_USER_ID, + expiresAt: new Date(Date.now() + 3_600_000), + }, + }); + cookieJar.set("healthlog_session", adminSession.id); + headerJar.delete("authorization"); + + const { POST } = + await import("@/app/api/admin/users/[id]/reset-password/route"); + const res = await POST( + req(`/api/admin/users/${USER_ID}/reset-password`, "POST", { + password: "operator chosen long passphrase 77", + }), + { params: Promise.resolve({ id: USER_ID }) }, + ); + expect(res.status).toBe(200); + + expect(await getPrismaClient().stepUpElevation.count()).toBe(0); + }); + + it("is dropped by sign-out-everywhere", async () => { + const { raw } = await mintToken("e8a3"); + useToken(raw); + await mintElevation(PASSWORD); + + const session = await getPrismaClient().session.create({ + data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + const { destroyOtherSessions } = await import("@/lib/auth/session"); + await destroyOtherSessions(USER_ID, { + kind: "session", + sessionId: session.id, + }); expect(await getPrismaClient().stepUpElevation.count()).toBe(0); }); @@ -518,7 +667,7 @@ describe("E8 — an elevation does not outlive its anchor", () => { useElevation(elevation); // Refused at Bearer resolution, before the elevation is even looked at — // which is why no code in the revoke path needs to know about elevations. - expect((await callMfaStatus()).status).toBe(401); + expect((await callMfaSetup()).status).toBe(401); const row = await getPrismaClient().stepUpElevation.findFirst(); expect(row?.consumedAt).toBeNull(); @@ -563,7 +712,7 @@ describe("E9 — a native client can manage its second factor", () => { }); expect(user?.totpSecretEncrypted).toBeTruthy(); expect(user?.totpConfirmedAt).toBeNull(); - expect((await callMfaStatus()).status).toBe(401); + expect((await callMfaSetup()).status).toBe(401); }); it("refuses the mint surface to a cookie session", async () => { @@ -585,3 +734,361 @@ describe("E9 — a native client can manage its second factor", () => { expect(await getPrismaClient().stepUpElevation.count()).toBe(0); }); }); + +// ── E10 — the fresh-factor rule (B1) ───────────────────────────────── + +describe("E10 — a password proof cannot reach the destructive routes", () => { + /** + * The attack this closes, in the order it ran before the fix: + * 1. mint an elevation with the account password (a stolen token plus a + * known password is the whole prerequisite); + * 2. POST recovery-codes/regenerate — which takes NO factor in its body — + * and receive ten plaintext recovery codes; + * 3. POST disable with one of them, satisfying the body check. + * The second factor is gone. On the web that chain is impossible because a + * password login never stamps `mfaVerifiedAt`. + */ + async function regenerate(): Promise { + const { POST } = + await import("@/app/api/auth/me/mfa/recovery-codes/regenerate/route"); + return POST(req("/api/auth/me/mfa/recovery-codes/regenerate", "POST")); + } + + async function disable(code: string, method = "recovery"): Promise { + const { POST } = await import("@/app/api/auth/me/mfa/disable/route"); + return POST(req("/api/auth/me/mfa/disable", "POST", { code, method })); + } + + async function removeKey(id: string): Promise { + const { DELETE } = + await import("@/app/api/auth/me/mfa/webauthn/[id]/route"); + return DELETE(req(`/api/auth/me/mfa/webauthn/${id}`, "DELETE"), { + params: Promise.resolve({ id }), + }); + } + + it("refuses recovery-code regeneration on a password-proved elevation", async () => { + await enrolTotp(); + const { raw } = await mintToken("e10a"); + useToken(raw); + useElevation(elevationOf(await mintElevation(PASSWORD))); + + const res = await regenerate(); + + expect(res.status).toBe(401); + // No codes were minted, so step 2 of the chain yields nothing. + expect(await getPrismaClient().mfaRecoveryCode.count()).toBe(0); + await new Promise((r) => setTimeout(r, 100)); + expect(await auditReasons("auth.stepup.elevation.rejected")).toContain( + "insufficient_factor", + ); + }); + + it("refuses disable on a password-proved elevation", async () => { + const secret = await enrolTotp(); + const { raw } = await mintToken("e10b"); + useToken(raw); + useElevation(elevationOf(await mintElevation(PASSWORD))); + + const res = await disable(currentTotpCode(secret), "totp"); + + expect(res.status).toBe(401); + const user = await getPrismaClient().user.findUnique({ + where: { id: USER_ID }, + }); + expect(user?.totpConfirmedAt).not.toBeNull(); + }); + + it("refuses security-key removal on a password-proved elevation", async () => { + const key = await getPrismaClient().webauthnMfaCredential.create({ + data: { + userId: USER_ID, + name: "key", + credentialId: "cred-e10c", + credentialPublicKey: Buffer.from([1, 2, 3]), + transports: ["internal"], + }, + }); + const { raw } = await mintToken("e10c"); + useToken(raw); + useElevation(elevationOf(await mintElevation(PASSWORD))); + + expect((await removeKey(key.id)).status).toBe(401); + expect(await getPrismaClient().webauthnMfaCredential.count()).toBe(1); + }); + + it("does NOT burn the elevation when the factor is too weak", async () => { + // The refusal is about reach, not validity. The same elevation must still + // work on the routes it was always entitled to. + await enrolTotp(); + const { raw } = await mintToken("e10d"); + useToken(raw); + const elevation = elevationOf(await mintElevation(PASSWORD)); + + useElevation(elevation); + expect((await regenerate()).status).toBe(401); + + const row = await getPrismaClient().stepUpElevation.findFirst(); + expect(row?.consumedAt).toBeNull(); + }); + + it("admits the SAME routes a plain cookie session reaches", async () => { + // The password arm is not useless — it is exactly cookie-equivalent. + const { raw } = await mintToken("e10e"); + useToken(raw); + useElevation(elevationOf(await mintElevation(PASSWORD))); + + expect((await callMfaSetup()).status).toBe(200); + }); + + it("admits the destructive routes on a TOTP-proved elevation", async () => { + const secret = await enrolTotp(); + const { raw } = await mintToken("e10f"); + useToken(raw); + + const minted = await mintTotpElevation(secret); + expect(minted.status).toBe(200); + expect( + (minted.body.data as { satisfiesFreshFactor: boolean }) + .satisfiesFreshFactor, + ).toBe(true); + + useElevation(elevationOf(minted)); + const res = await regenerate(); + + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { recoveryCodes: string[] }; + }; + expect(body.data.recoveryCodes.length).toBeGreaterThan(0); + }); + + it("marks a password-proved elevation as not fresh-factor in the mint response", async () => { + const { raw } = await mintToken("e10g"); + useToken(raw); + const minted = await mintElevation(PASSWORD); + const data = minted.body.data as { + method: string; + satisfiesFreshFactor: boolean; + }; + expect(data.method).toBe("password"); + expect(data.satisfiesFreshFactor).toBe(false); + }); + + it("refuses a stale TOTP code and does not mint", async () => { + const secret = await enrolTotp(); + const { raw } = await mintToken("e10h"); + useToken(raw); + + // Spend the code through the shared verifier first; the replay guard must + // then refuse the same code here, exactly as it does at login. + const code = currentTotpCode(secret); + const { verifyMfaFactor } = await import("@/lib/auth/mfa/verify-factor"); + const user = await getPrismaClient().user.findUnique({ + where: { id: USER_ID }, + }); + await verifyMfaFactor(user!, "totp", code); + + const res = await mintWith({ method: "totp", code }); + + expect(res.status).toBe(401); + expect(await getPrismaClient().stepUpElevation.count()).toBe(0); + }); +}); + +// ── E11 — the disable route keeps the caller signed in (B2) ────────── + +describe("E11 — disabling MFA over Bearer spares the calling device", () => { + it("revokes every OTHER refresh token but not the caller's own", async () => { + const secret = await enrolTotp(); + const { raw, id } = await mintToken("e11a"); + + const { hashToken } = await import("@/lib/auth/hmac"); + // The caller's own device login: the RefreshToken row cross-referencing the + // access token being presented. + const mine = await getPrismaClient().refreshToken.create({ + data: { + userId: USER_ID, + tokenHash: hashToken(`hlr_mine${"0".repeat(56)}`), + accessTokenHash: hashToken(raw), + expiresAt: new Date(Date.now() + 30 * 24 * 3_600_000), + }, + }); + // Another device. + const theirs = await getPrismaClient().refreshToken.create({ + data: { + userId: USER_ID, + tokenHash: hashToken(`hlr_theirs${"0".repeat(54)}`), + accessTokenHash: hashToken(`hlk_other${"0".repeat(55)}`), + expiresAt: new Date(Date.now() + 30 * 24 * 3_600_000), + }, + }); + // And a browser session, which has no "current" on the Bearer path. + const webSession = await getPrismaClient().session.create({ + data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + + const codes = await seedRecoveryCodes(); + useToken(raw); + useElevation(elevationOf(await mintTotpElevation(secret))); + + const { POST } = await import("@/app/api/auth/me/mfa/disable/route"); + const res = await POST( + req("/api/auth/me/mfa/disable", "POST", { + code: codes[0], + method: "recovery", + }), + ); + expect(res.status).toBe(200); + + // The defect: passing the ApiToken id where a Session id was expected made + // the "keep the current one" exclusion match nothing, so the caller's own + // refresh token was revoked and the app logged itself out at next rotation. + const mineAfter = await getPrismaClient().refreshToken.findUnique({ + where: { id: mine.id }, + }); + const theirsAfter = await getPrismaClient().refreshToken.findUnique({ + where: { id: theirs.id }, + }); + expect(mineAfter?.revokedAt).toBeNull(); + expect(theirsAfter?.revokedAt).not.toBeNull(); + + // Web sessions all go — none of them is the caller. + expect( + await getPrismaClient().session.findUnique({ + where: { id: webSession.id }, + }), + ).toBeNull(); + + // And the calling token itself is untouched, so the next request works. + const token = await getPrismaClient().apiToken.findUnique({ + where: { id }, + }); + expect(token?.revoked).toBe(false); + }); + + it("keeps the caller's own session on the cookie path, as before", async () => { + const secret = await enrolTotp(); + const current = await getPrismaClient().session.create({ + data: { + userId: USER_ID, + expiresAt: new Date(Date.now() + 3_600_000), + mfaVerifiedAt: new Date(), + }, + }); + const other = await getPrismaClient().session.create({ + data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + cookieJar.set("healthlog_session", current.id); + headerJar.delete("authorization"); + + const { POST } = await import("@/app/api/auth/me/mfa/disable/route"); + const res = await POST( + req("/api/auth/me/mfa/disable", "POST", { + code: currentTotpCode(secret), + method: "totp", + }), + ); + expect(res.status).toBe(200); + + expect( + await getPrismaClient().session.findUnique({ where: { id: current.id } }), + ).not.toBeNull(); + expect( + await getPrismaClient().session.findUnique({ where: { id: other.id } }), + ).toBeNull(); + }); +}); + +// ── E12 — validation failures do not burn the proof ────────────────── + +describe("E12 — a rejected request keeps the elevation spendable", () => { + it("survives a wrong TOTP code on disable", async () => { + const secret = await enrolTotp(); + const codes = await seedRecoveryCodes(); + const { raw } = await mintToken("e12a"); + useToken(raw); + const elevation = elevationOf(await mintTotpElevation(secret)); + + useElevation(elevation); + const { POST } = await import("@/app/api/auth/me/mfa/disable/route"); + const bad = await POST( + req("/api/auth/me/mfa/disable", "POST", { + code: "000000", + method: "totp", + }), + ); + expect(bad.status).toBe(401); + + // Unspent — otherwise five fat-fingered codes would exhaust the 5-per-15-min + // mint ceiling and lock the user out of their own security settings. + const row = await getPrismaClient().stepUpElevation.findFirst(); + expect(row?.consumedAt).toBeNull(); + + // And the retry goes through on the SAME elevation. + const good = await POST( + req("/api/auth/me/mfa/disable", "POST", { + code: codes[0], + method: "recovery", + }), + ); + expect(good.status).toBe(200); + }); + + it("survives a malformed body", async () => { + const { raw } = await mintToken("e12b"); + useToken(raw); + useElevation(elevationOf(await mintElevation(PASSWORD))); + + const { POST } = + await import("@/app/api/auth/me/mfa/webauthn/register/verify/route"); + const res = await POST( + req("/api/auth/me/mfa/webauthn/register/verify", "POST", { nope: 1 }), + ); + expect(res.status).toBe(422); + + const row = await getPrismaClient().stepUpElevation.findFirst(); + expect(row?.consumedAt).toBeNull(); + }); +}); + +// ── E13 — the status read is plain Bearer ──────────────────────────── + +describe("E13 — the second-factor status read needs no elevation", () => { + it("serves a token with no elevation header at all", async () => { + const { raw } = await mintToken("e13a"); + useToken(raw); + useElevation(null); + + const res = await callMfaStatus(); + + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { totp: { enabled: boolean }; webauthn: unknown[] }; + }; + expect(body.data.totp).toBeDefined(); + expect(Array.isArray(body.data.webauthn)).toBe(true); + }); + + it("carries no credential material", async () => { + await getPrismaClient().webauthnMfaCredential.create({ + data: { + userId: USER_ID, + name: "key", + credentialId: "cred-e13b-secret", + credentialPublicKey: Buffer.from([9, 9, 9]), + transports: ["internal"], + }, + }); + const { raw } = await mintToken("e13b"); + useToken(raw); + + const text = await (await callMfaStatus()).text(); + + // The justification for relaxing this route is that the payload is inert. + // Pin it: no credential id, no public key, no codes. + expect(text).not.toContain("cred-e13b-secret"); + expect(text).not.toContain("credentialPublicKey"); + expect(text).not.toContain("credentialId"); + }); +});