diff --git a/src/lib/auth/__tests__/passkey.test.ts b/src/lib/auth/__tests__/passkey.test.ts index d7edcda31..e4877a0e9 100644 --- a/src/lib/auth/__tests__/passkey.test.ts +++ b/src/lib/auth/__tests__/passkey.test.ts @@ -14,6 +14,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/db", () => ({ prisma: { + $queryRaw: vi.fn(), authChallenge: { findUnique: vi.fn(), deleteMany: vi.fn(), @@ -57,6 +58,9 @@ const VALID_BODY = { }; beforeEach(() => { + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { challenge: "challenge", user_id: null }, + ] as never); vi.mocked(prisma.authChallenge.findUnique).mockResolvedValue({ id: "ch-1", challenge: "challenge", diff --git a/src/lib/auth/__tests__/webauthn-library-contract.test.ts b/src/lib/auth/__tests__/webauthn-library-contract.test.ts index a20e474a9..cfe07e234 100644 --- a/src/lib/auth/__tests__/webauthn-library-contract.test.ts +++ b/src/lib/auth/__tests__/webauthn-library-contract.test.ts @@ -63,6 +63,22 @@ vi.mock("@/lib/db", () => { ) => [...table.values()].find((r) => r.credentialId === credentialId) ?? null; return { prisma: { + // `claimAuthChallenge`'s guarded DELETE ... RETURNING, over the map: + // values are (id, type) or (id, type, userId). + $queryRaw: async (_sql: TemplateStringsArray, ...values: unknown[]) => { + const [id, type, userId] = values as [string, string, string?]; + const row = store.authChallenge.get(id); + if ( + !row || + row.type !== type || + (userId !== undefined && row.userId !== userId) || + !((row.expiresAt as Date) > new Date()) + ) { + return []; + } + store.authChallenge.delete(id); + return [{ challenge: row.challenge, user_id: row.userId ?? null }]; + }, authChallenge: { deleteMany: async () => ({ count: 0 }), create: async ({ data }: { data: Record }) => { diff --git a/src/lib/auth/mfa/webauthn.ts b/src/lib/auth/mfa/webauthn.ts index 24c6b9ac2..691a5bd90 100644 --- a/src/lib/auth/mfa/webauthn.ts +++ b/src/lib/auth/mfa/webauthn.ts @@ -31,6 +31,7 @@ import { } from "@simplewebauthn/server"; import { z } from "zod/v4"; import { prisma } from "@/lib/db"; +import { claimAuthChallenge } from "@/lib/auth/passkey"; import { RP_NAME as rpName, getRpId, @@ -146,38 +147,30 @@ export async function verifyMfaRegistration( userId: string, response: unknown, ): Promise { - const challenge = await prisma.authChallenge.findUnique({ - where: { id: challengeId }, - }); - // The challenge must be live, of the registration type, and bound to the // calling user — a challenge minted for another account can never be reused. - if ( - !challenge || - challenge.expiresAt < new Date() || - challenge.type !== "mfa_registration" || - challenge.userId !== userId - ) { + // Claimed (deleted) in the same statement that checks it, so two concurrent + // requests cannot both use it. + const challenge = await claimAuthChallenge({ + challengeId, + type: "mfa_registration", + userId, + }); + if (!challenge) { throw new Error("Challenge expired or not found"); } - try { - const parsed = registrationResponseSchema.safeParse(response); - if (!parsed.success) { - throw new Error("Registration response shape invalid"); - } - return await verifyRegistrationResponse({ - response: parsed.data as RegistrationResponseJSON, - expectedChallenge: challenge.challenge, - expectedOrigin: getExpectedOrigin(), - expectedRPID: getRpId(), - supportedAlgorithmIDs: REGISTRATION_ACCEPTED_ALGORITHM_IDS, - }); - } finally { - await prisma.authChallenge - .delete({ where: { id: challengeId } }) - .catch(() => {}); + const parsed = registrationResponseSchema.safeParse(response); + if (!parsed.success) { + throw new Error("Registration response shape invalid"); } + return verifyRegistrationResponse({ + response: parsed.data as RegistrationResponseJSON, + expectedChallenge: challenge.challenge, + expectedOrigin: getExpectedOrigin(), + expectedRPID: getRpId(), + supportedAlgorithmIDs: REGISTRATION_ACCEPTED_ALGORITHM_IDS, + }); } // ── Authentication (mid-login second factor) ───────────────────────── @@ -232,16 +225,14 @@ export async function verifyMfaAuthentication( userId: string, response: unknown, ): Promise { - const challenge = await prisma.authChallenge.findUnique({ - where: { id: challengeId }, + // Claimed (deleted) in the same statement that checks it: one assertion, + // one verification, even when two requests carry it at once. + const challenge = await claimAuthChallenge({ + challengeId, + type: "mfa_authentication", + userId, }); - - if ( - !challenge || - challenge.expiresAt < new Date() || - challenge.type !== "mfa_authentication" || - challenge.userId !== userId - ) { + if (!challenge) { throw new Error("Challenge expired or not found"); } @@ -251,43 +242,37 @@ export async function verifyMfaAuthentication( } const typedResponse = parsed.data as unknown as AuthenticationResponseJSON; - try { - // The presented credential MUST belong to the password-identified user — - // look it up scoped to `userId` so an assertion against another account's - // key can never satisfy this account's second factor. - const credential = await prisma.webauthnMfaCredential.findFirst({ - where: { credentialId: typedResponse.id, userId }, + // The presented credential MUST belong to the password-identified user — + // look it up scoped to `userId` so an assertion against another account's + // key can never satisfy this account's second factor. + const credential = await prisma.webauthnMfaCredential.findFirst({ + where: { credentialId: typedResponse.id, userId }, + }); + if (!credential) return false; + + const verification: VerifiedAuthenticationResponse = + await verifyAuthenticationResponse({ + response: typedResponse, + expectedChallenge: challenge.challenge, + expectedOrigin: getExpectedOrigin(), + expectedRPID: getRpId(), + credential: { + id: credential.credentialId, + publicKey: credential.credentialPublicKey, + counter: Number(credential.counter), + transports: credential.transports as Transport[], + }, }); - if (!credential) return false; - const verification: VerifiedAuthenticationResponse = - await verifyAuthenticationResponse({ - response: typedResponse, - expectedChallenge: challenge.challenge, - expectedOrigin: getExpectedOrigin(), - expectedRPID: getRpId(), - credential: { - id: credential.credentialId, - publicKey: credential.credentialPublicKey, - counter: Number(credential.counter), - transports: credential.transports as Transport[], - }, - }); - - if (verification.verified) { - await prisma.webauthnMfaCredential.update({ - where: { id: credential.id }, - data: { - counter: BigInt(verification.authenticationInfo.newCounter), - lastUsedAt: new Date(), - }, - }); - } - - return verification.verified; - } finally { - await prisma.authChallenge - .delete({ where: { id: challengeId } }) - .catch(() => {}); + if (verification.verified) { + await prisma.webauthnMfaCredential.update({ + where: { id: credential.id }, + data: { + counter: BigInt(verification.authenticationInfo.newCounter), + lastUsedAt: new Date(), + }, + }); } + + return verification.verified; } diff --git a/src/lib/auth/passkey.ts b/src/lib/auth/passkey.ts index d32ee3ad6..8598beb0d 100644 --- a/src/lib/auth/passkey.ts +++ b/src/lib/auth/passkey.ts @@ -201,6 +201,46 @@ export async function consumeRegistrationChallenge(input: { }; } +/** + * Claim a WebAuthn challenge: delete it and return its material in one + * statement, or return null when it is unknown, expired, of another ceremony, + * or (when `userId` is given) minted for another account. + * + * Every ceremony verifier goes through here. They used to read the challenge, + * verify, and delete it in a `finally`, so two requests carrying the same + * assertion could both read it before either deleted it and both be + * accepted. The guarded DELETE ... RETURNING gives exactly one of them the + * challenge; the other finds nothing. + */ +export async function claimAuthChallenge(input: { + challengeId: string; + type: string; + userId?: string; +}): Promise<{ challenge: string; userId: string | null } | null> { + const rows = input.userId + ? await prisma.$queryRaw< + Array<{ challenge: string; user_id: string | null }> + >` + DELETE FROM auth_challenges + WHERE id = ${input.challengeId} + AND type = ${input.type} + AND user_id = ${input.userId} + AND expires_at > NOW() + RETURNING challenge, user_id + ` + : await prisma.$queryRaw< + Array<{ challenge: string; user_id: string | null }> + >` + DELETE FROM auth_challenges + WHERE id = ${input.challengeId} + AND type = ${input.type} + AND expires_at > NOW() + RETURNING challenge, user_id + `; + if (rows.length !== 1) return null; + return { challenge: rows[0].challenge, userId: rows[0].user_id }; +} + export async function verifyRegistration( expectedChallenge: string, response: unknown, @@ -271,11 +311,13 @@ export async function verifyAuthentication( verification: VerifiedAuthenticationResponse; passkey: { userId: string }; }> { - const challenge = await prisma.authChallenge.findUnique({ - where: { id: challengeId }, + // Claimed before anything else: a challenge is good for one verification, + // successful or not, and a concurrent second request finds nothing. + const challenge = await claimAuthChallenge({ + challengeId, + type: "authentication", }); - - if (!challenge || challenge.expiresAt < new Date()) { + if (!challenge) { throw new Error("Challenge expired or not found"); } @@ -301,37 +343,36 @@ export async function verifyAuthentication( throw new Error("Passkey not found"); } - try { - const verification = await verifyAuthenticationResponse({ - response: typedResponse, - expectedChallenge: challenge.challenge, - expectedOrigin: getExpectedOrigin(), - expectedRPID: getRpId(), - credential: { - id: passkey.credentialId, - publicKey: passkey.credentialPublicKey, - counter: Number(passkey.counter), - transports: passkey.transports as Transport[], + // A challenge begun for one account (the re-proof arm names the account) + // is answered only by that account's passkey. + if (challenge.userId !== null && challenge.userId !== passkey.userId) { + throw new Error("Challenge expired or not found"); + } + + const verification = await verifyAuthenticationResponse({ + response: typedResponse, + expectedChallenge: challenge.challenge, + expectedOrigin: getExpectedOrigin(), + expectedRPID: getRpId(), + credential: { + id: passkey.credentialId, + publicKey: passkey.credentialPublicKey, + counter: Number(passkey.counter), + transports: passkey.transports as Transport[], + }, + }); + + if (verification.verified) { + // Update counter + stamp last-used so the management UI can surface + // when each passkey was last exercised (v1.23 passkey QoL). + await prisma.passkey.update({ + where: { id: passkey.id }, + data: { + counter: BigInt(verification.authenticationInfo.newCounter), + lastUsedAt: new Date(), }, }); - - if (verification.verified) { - // Update counter + stamp last-used so the management UI can surface - // when each passkey was last exercised (v1.23 passkey QoL). - await prisma.passkey.update({ - where: { id: passkey.id }, - data: { - counter: BigInt(verification.authenticationInfo.newCounter), - lastUsedAt: new Date(), - }, - }); - } - - return { verification, passkey: { userId: passkey.userId } }; - } finally { - // Invalidate challenge after first verification attempt (success or failure) - await prisma.authChallenge - .delete({ where: { id: challengeId } }) - .catch(() => {}); } + + return { verification, passkey: { userId: passkey.userId } }; } diff --git a/tests/integration/webauthn-challenge-claim.test.ts b/tests/integration/webauthn-challenge-claim.test.ts new file mode 100644 index 000000000..e3a31bb3e --- /dev/null +++ b/tests/integration/webauthn-challenge-claim.test.ts @@ -0,0 +1,98 @@ +/** + * A WebAuthn challenge is good for exactly one verification. + * + * The ceremony verifiers used to read the challenge, verify, and delete it + * afterwards, so two requests carrying the same assertion could both read it + * and both pass. `claimAuthChallenge` deletes and returns it in one guarded + * statement; this pins that against real Postgres. + */ +import { beforeEach, describe, expect, it } from "vitest"; + +import { getPrismaClient, truncateAllTables } from "./setup"; + +const { claimAuthChallenge } = await import("@/lib/auth/passkey"); + +const USER_ID = "user-challenge-claim"; + +async function challenge( + type: string, + opts: { userId?: string | null; expiresInMs?: number } = {}, +): Promise { + const row = await getPrismaClient().authChallenge.create({ + data: { + userId: opts.userId === undefined ? USER_ID : opts.userId, + challenge: `material-${type}`, + type, + expiresAt: new Date(Date.now() + (opts.expiresInMs ?? 60_000)), + }, + }); + return row.id; +} + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + await getPrismaClient().user.create({ + data: { + id: USER_ID, + username: "claim-user", + email: "claim@example.test", + }, + }); +}); + +describe("claimAuthChallenge", () => { + it("hands the challenge to exactly one of two concurrent claims", async () => { + const id = await challenge("authentication"); + const results = await Promise.all( + Array.from({ length: 8 }, () => + claimAuthChallenge({ challengeId: id, type: "authentication" }), + ), + ); + expect(results.filter((r) => r !== null)).toHaveLength(1); + expect(await getPrismaClient().authChallenge.count({ where: { id } })).toBe( + 0, + ); + }); + + it("returns the material and the account it was begun for", async () => { + const id = await challenge("mfa_authentication"); + await expect( + claimAuthChallenge({ + challengeId: id, + type: "mfa_authentication", + userId: USER_ID, + }), + ).resolves.toEqual({ + challenge: "material-mfa_authentication", + userId: USER_ID, + }); + }); + + it("refuses another ceremony's challenge and leaves it in place", async () => { + const id = await challenge("mfa_authentication"); + await expect( + claimAuthChallenge({ challengeId: id, type: "authentication" }), + ).resolves.toBeNull(); + expect(await getPrismaClient().authChallenge.count({ where: { id } })).toBe( + 1, + ); + }); + + it("refuses a challenge begun for another account", async () => { + const id = await challenge("mfa_registration"); + await expect( + claimAuthChallenge({ + challengeId: id, + type: "mfa_registration", + userId: "someone-else", + }), + ).resolves.toBeNull(); + }); + + it("refuses an expired challenge", async () => { + const id = await challenge("authentication", { expiresInMs: -1_000 }); + await expect( + claimAuthChallenge({ challengeId: id, type: "authentication" }), + ).resolves.toBeNull(); + }); +});