From ba45a25cb0e54b1c64bcfe7ef22f46958c92d135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 19 Jul 2026 13:14:38 +0200 Subject: [PATCH] test(auth): retarget the MFA route mocks at the shared gate The three unit suites that stubbed `requireCookieAuth` / `requireFreshMfa` directly now stub `requireMfaManagementAuth`, and the password-change suite stubs the elevation revocation the route gained. Adds the unit checks that need no database: the elevation TTL is pinned to the cookie path's step-up window so the two cannot drift apart, the minted value is a prefixed CSPRNG secret whose plaintext never reaches the row, and a value carrying another credential's prefix is refused without costing a query. --- .../me/mfa/disable/__tests__/route.test.ts | 6 +- .../webauthn/register/__tests__/route.test.ts | 8 +- .../api/auth/password/__tests__/route.test.ts | 6 + src/lib/auth/__tests__/step-up.test.ts | 109 ++++++++++++++++++ tests/integration/step-up-elevation.test.ts | 11 +- 5 files changed, 132 insertions(+), 8 deletions(-) create mode 100644 src/lib/auth/__tests__/step-up.test.ts diff --git a/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts b/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts index dfd88440c..422ed90d9 100644 --- a/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts +++ b/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts @@ -7,7 +7,7 @@ vi.mock("@/lib/api-handler", async () => { ); return { ...actual, - requireFreshMfa: vi.fn(), + requireMfaManagementAuth: vi.fn(), }; }); @@ -39,7 +39,7 @@ vi.mock("@/lib/auth/mfa/verify-factor", () => ({ vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); import { POST } from "../route"; -import { requireFreshMfa } from "@/lib/api-handler"; +import { requireMfaManagementAuth } from "@/lib/api-handler"; import { prisma } from "@/lib/db"; import { destroyOtherSessions } from "@/lib/auth/session"; import { verifyMfaFactor } from "@/lib/auth/mfa/verify-factor"; @@ -54,7 +54,7 @@ function req() { beforeEach(() => { vi.clearAllMocks(); - vi.mocked(requireFreshMfa).mockResolvedValue({ + vi.mocked(requireMfaManagementAuth).mockResolvedValue({ user: { id: "u1" }, session: { id: "sess-current" }, } as never); diff --git a/src/app/api/auth/me/mfa/webauthn/register/__tests__/route.test.ts b/src/app/api/auth/me/mfa/webauthn/register/__tests__/route.test.ts index 37551d36d..30655630f 100644 --- a/src/app/api/auth/me/mfa/webauthn/register/__tests__/route.test.ts +++ b/src/app/api/auth/me/mfa/webauthn/register/__tests__/route.test.ts @@ -8,7 +8,7 @@ vi.mock("@/lib/auth/mfa-enrollment", () => ({ vi.mock("@/lib/api-handler", () => ({ apiHandler: (fn: unknown) => fn, - requireCookieAuth: vi.fn(), + requireMfaManagementAuth: vi.fn(), })); vi.mock("@/lib/db", () => ({ @@ -33,7 +33,7 @@ vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); import { POST as REGISTER_OPTIONS } from "../options/route"; import { POST as REGISTER_VERIFY } from "../verify/route"; -import { requireCookieAuth } from "@/lib/api-handler"; +import { requireMfaManagementAuth } from "@/lib/api-handler"; import { prisma } from "@/lib/db"; import { checkRateLimit } from "@/lib/rate-limit"; import { @@ -56,7 +56,9 @@ function verifyRequest(body: Record): NextRequest { beforeEach(() => { vi.resetAllMocks(); - vi.mocked(requireCookieAuth).mockResolvedValue({ user: USER } as never); + vi.mocked(requireMfaManagementAuth).mockResolvedValue({ + user: USER, + } as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true } as never); }); diff --git a/src/app/api/auth/password/__tests__/route.test.ts b/src/app/api/auth/password/__tests__/route.test.ts index f5fe53763..e13ecff29 100644 --- a/src/app/api/auth/password/__tests__/route.test.ts +++ b/src/app/api/auth/password/__tests__/route.test.ts @@ -24,6 +24,12 @@ vi.mock("@/lib/db", () => ({ }, })); +// v1.30.34 — the route drops the account's step-up elevations alongside its +// sessions, so the rotation cannot leave a proof of the old credential standing. +vi.mock("@/lib/auth/step-up", () => ({ + revokeStepUpElevations: vi.fn().mockResolvedValue(undefined), +})); + vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn(), destroyAllSessions: vi.fn().mockResolvedValue(undefined), diff --git a/src/lib/auth/__tests__/step-up.test.ts b/src/lib/auth/__tests__/step-up.test.ts new file mode 100644 index 000000000..70a46d2b0 --- /dev/null +++ b/src/lib/auth/__tests__/step-up.test.ts @@ -0,0 +1,109 @@ +/** + * 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. + */ +import { describe, it, expect, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + 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), + }, + }, +})); + +process.env.API_TOKEN_HMAC_KEY ??= + "test-hmac-key-step-up-unit-32-bytes-minimum-abcdefghijklmnop"; + +const { + mintStepUpElevation, + redeemStepUpElevation, + STEP_UP_ELEVATION_TTL_SECONDS, +} = await import("@/lib/auth/step-up"); +const { MFA_STEP_UP_MAX_AGE_SECONDS } = await import("@/lib/api-handler"); + +describe("the two step-up windows stay pinned together", () => { + it("matches the cookie path's freshness window exactly", () => { + // If these ever diverge, the looser transport quietly becomes the way in. + // Changing one is a decision; changing one by accident is a hole. + expect(STEP_UP_ELEVATION_TTL_SECONDS).toBe(MFA_STEP_UP_MAX_AGE_SECONDS); + }); +}); + +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", + }); + + 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("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", + }); + 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(); + + // `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({ + rawToken: value, + userId: "u1", + apiTokenId: "t1", + }); + expect(result).toEqual({ ok: false, reason: "malformed" }); + } + + expect(prisma.stepUpElevation.updateMany).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. + expect(STEP_UP_ELEVATION_HEADER).toBe("x-step-up"); + expect(STEP_UP_ELEVATION_HEADER).toBe( + STEP_UP_ELEVATION_HEADER.toLowerCase(), + ); + }); +}); diff --git a/tests/integration/step-up-elevation.test.ts b/tests/integration/step-up-elevation.test.ts index 4bfa41372..f85eacd37 100644 --- a/tests/integration/step-up-elevation.test.ts +++ b/tests/integration/step-up-elevation.test.ts @@ -129,10 +129,17 @@ async function mintElevation( return { status: res.status, body: (await res.json()) as never }; } +/** + * Some handlers in the set declare no request parameter (`apiHandler(async () => + * …)`), so their wrapped type takes none. The runtime always passes one; this + * keeps the call sites honest without loosening the handlers themselves. + */ +type RouteFn = (request: NextRequest) => Promise; + /** GET /api/auth/me/mfa — the cheapest route in the elevation-accepting set. */ async function callMfaStatus(): Promise { const { GET } = await import("@/app/api/auth/me/mfa/route"); - return await GET(req("/api/auth/me/mfa")); + return await (GET as unknown as RouteFn)(req("/api/auth/me/mfa")); } async function auditReasons(action: string): Promise { @@ -338,7 +345,7 @@ describe("E5 — an elevation does not satisfy requireAdmin", () => { useElevation(elevation); const { GET } = await import("@/app/api/admin/users/route"); - const res = await GET(req("/api/admin/users")); + const res = await (GET as unknown as RouteFn)(req("/api/admin/users")); // Cookie-only, and the elevation is not a cookie. The account being a // genuine admin is what makes this test worth having: the refusal is the