mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(auth): claim a WebAuthn challenge in the statement that checks it
The passkey sign-in, security-key sign-in and security-key registration verifiers read the challenge, verified the response, and deleted the challenge afterwards. Two requests carrying the same assertion could both read it before either deleted it, and both be accepted. A challenge is now claimed with one guarded DELETE ... RETURNING that also checks its ceremony, its account and its expiry, before anything is verified; a concurrent second request finds nothing. A passkey sign-in also refuses a challenge that was begun for a different account than the passkey belongs to.
This commit is contained in:
1 parent
ece5b1d624
commit
bf63f6bdba
5 files changed
+249
-105
No files matched your search
@@ -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",
|
||||
|
||||
@@ -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<string, unknown> }) => {
|
||||
|
||||
@@ -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<VerifiedRegistrationResponse> {
|
||||
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<boolean> {
|
||||
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;
|
||||
}
|
||||
+75
-34
@@ -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 } };
|
||||
}
|
||||
@@ -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<string> {
|
||||
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();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user