fix(auth): invalidate email OTP on a failed guess, not just a match

apps/web/__tests__/integration/sso-database.test.ts

@@ -6,6 +6,7 @@ import {
 	organizationSso,
 	organizations,
 	users,
+	verificationTokens,
 } from "@cap/database/schema";
 import { Organisation, User } from "@cap/web-domain";
 import { and, eq } from "drizzle-orm";
@@ -344,5 +345,62 @@ describe.runIf(Boolean(databaseUrl))(
 					.where(eq(organizationMembers.userId, userId)),
 			).toHaveLength(0);
 		});
+
+		it("burns an email OTP exactly once under concurrent guesses", async () => {
+			const identifier = `${id()}@example.com`;
+			const token = "482913";
+			const { useVerificationToken } = DrizzleAdapter(database());
+			if (!useVerificationToken) {
+				throw new Error("Missing useVerificationToken adapter.");
+			}
+			await database()
+				.insert(verificationTokens)
+				.values({ identifier, token, expires: new Date(Date.now() + 600_000) });
+
+			const attempts = await Promise.all(
+				Array.from({ length: 8 }, () =>
+					useVerificationToken({ identifier, token }),
+				),
+			);
+
+			expect(attempts.filter((result) => result !== null)).toHaveLength(1);
+			expect(
+				await database()
+					.select()
+					.from(verificationTokens)
+					.where(eq(verificationTokens.identifier, identifier)),
+			).toHaveLength(0);
+		});
+
+		it("never authenticates a wrong guess when it races the correct one", async () => {
+			const identifier = `${id()}@example.com`;
+			const token = "531942";
+			const { useVerificationToken } = DrizzleAdapter(database());
+			if (!useVerificationToken) {
+				throw new Error("Missing useVerificationToken adapter.");
+			}
+			await database()
+				.insert(verificationTokens)
+				.values({ identifier, token, expires: new Date(Date.now() + 600_000) });
+
+			const guesses = ["000000", "111111", token, "222222", "333333"];
+			const attempts = await Promise.all(
+				guesses.map((guess) =>
+					useVerificationToken({ identifier, token: guess }),
+				),
+			);
+
+			const successes = attempts.filter((result) => result !== null);
+			expect(successes.length).toBeLessThanOrEqual(1);
+			for (const success of successes) {
+				expect(success).toMatchObject({ identifier, token });
+			}
+			expect(
+				await database()
+					.select()
+					.from(verificationTokens)
+					.where(eq(verificationTokens.identifier, identifier)),
+			).toHaveLength(0);
+		});
 	},
 );

apps/web/__tests__/unit/verification-token.test.ts

@@ -0,0 +1,146 @@
+import type { SQL } from "drizzle-orm";
+import { MySqlDialect } from "drizzle-orm/mysql-core";
+import type { MySql2Database } from "drizzle-orm/mysql2";
+import { describe, expect, it } from "vitest";
+import { DrizzleAdapter } from "../../../../packages/database/auth/drizzle-adapter";
+
+type TokenRow = { identifier: string; token: string; expires: Date };
+
+function matchesPredicate(condition: SQL, row: TokenRow) {
+	const query = new MySqlDialect().sqlToQuery(condition);
+	const columns = [
+		...query.sql.matchAll(
+			/`verification_tokens`\.`(identifier|token)`\s*=\s*\?/g,
+		),
+	].map((match) => match[1] as "identifier" | "token");
+	if (columns.length === 0) {
+		throw new Error(`Unexpected predicate: ${query.sql}`);
+	}
+	return columns.every((column, i) => row[column] === query.params[i]);
+}
+
+function fakeDatabase(
+	initialRow: TokenRow,
+	options?: { afterSelect?: () => void },
+) {
+	let row: TokenRow | undefined = initialRow;
+	const db = {
+		select: () => ({
+			from: () => ({
+				where: (condition: SQL) => ({
+					limit: async () => {
+						const result = row && matchesPredicate(condition, row) ? [row] : [];
+						options?.afterSelect?.();
+						return result;
+					},
+				}),
+			}),
+		}),
+		delete: () => ({
+			where: async (condition: SQL) => {
+				if (row && matchesPredicate(condition, row)) {
+					row = undefined;
+					return [{ affectedRows: 1 }];
+				}
+				return [{ affectedRows: 0 }];
+			},
+		}),
+	};
+	return {
+		db: db as unknown as MySql2Database,
+		getRow: () => row,
+		setRow: (next: TokenRow) => {
+			row = next;
+		},
+	};
+}
+
+describe("useVerificationToken", () => {
+	const identifier = "person@example.com";
+	const validRow: TokenRow = {
+		identifier,
+		token: "111111",
+		expires: new Date(Date.now() + 60_000),
+	};
+
+	it("burns the code on a wrong guess instead of leaving it guessable", async () => {
+		const { db, getRow } = fakeDatabase({ ...validRow });
+		const adapter = DrizzleAdapter(db);
+
+		const wrongGuess = await adapter.useVerificationToken?.({
+			identifier,
+			token: "000000",
+		});
+
+		expect(wrongGuess).toBeNull();
+		expect(getRow()).toBeUndefined();
+
+		const correctGuessAfterward = await adapter.useVerificationToken?.({
+			identifier,
+			token: "111111",
+		});
+		expect(correctGuessAfterward).toBeNull();
+	});
+
+	it("returns the row and deletes it on a correct guess", async () => {
+		const { db, getRow } = fakeDatabase({ ...validRow });
+		const adapter = DrizzleAdapter(db);
+
+		const result = await adapter.useVerificationToken?.({
+			identifier,
+			token: "111111",
+		});
+
+		expect(result).toMatchObject({ identifier, token: "111111" });
+		expect(getRow()).toBeUndefined();
+	});
+
+	it("returns null when no code was ever requested for the identifier", async () => {
+		const { db } = fakeDatabase({ ...validRow });
+		const adapter = DrizzleAdapter(db);
+
+		const result = await adapter.useVerificationToken?.({
+			identifier: "nobody@example.com",
+			token: "111111",
+		});
+
+		expect(result).toBeNull();
+	});
+
+	it("lets only one of two concurrent correct guesses succeed", async () => {
+		const { db, getRow } = fakeDatabase({ ...validRow });
+		const adapter = DrizzleAdapter(db);
+
+		const [first, second] = await Promise.all([
+			adapter.useVerificationToken?.({ identifier, token: "111111" }),
+			adapter.useVerificationToken?.({ identifier, token: "111111" }),
+		]);
+
+		const successes = [first, second].filter((result) => result !== null);
+		expect(successes).toHaveLength(1);
+		expect(getRow()).toBeUndefined();
+	});
+
+	it("does not delete a resent code that replaced the row a guess read", async () => {
+		const resentRow: TokenRow = {
+			identifier,
+			token: "222222",
+			expires: new Date(Date.now() + 60_000),
+		};
+		const { db, getRow, setRow } = fakeDatabase(
+			{ ...validRow },
+			{
+				afterSelect: () => setRow({ ...resentRow }),
+			},
+		);
+		const adapter = DrizzleAdapter(db);
+
+		const wrongGuessAgainstStaleCode = await adapter.useVerificationToken?.({
+			identifier,
+			token: "000000",
+		});
+
+		expect(wrongGuessAgainstStaleCode).toBeNull();
+		expect(getRow()).toEqual(resentRow);
+	});
+});

packages/database/auth/drizzle-adapter.ts

@@ -18,6 +18,15 @@ import {
 import type { ValidatedSsoIdentity } from "./sso.ts";
 import { provisionStripeCustomer } from "./stripe-customer.ts";
 
+const getAffectedRows = (result: unknown) => {
+	if (Array.isArray(result)) {
+		return (
+			(result[0] as { affectedRows?: number } | undefined)?.affectedRows ?? 0
+		);
+	}
+	return (result as { affectedRows?: number } | undefined)?.affectedRows ?? 0;
+};
+
 type CreateUserData = Parameters<NonNullable<Adapter["createUser"]>>[0];
 type LinkAccountData = Parameters<NonNullable<Adapter["linkAccount"]>>[0];
 type UnlinkAccountData = Parameters<NonNullable<Adapter["unlinkAccount"]>>[0];
@@ -456,31 +465,40 @@ export function DrizzleAdapter(
 			return row;
 		},
 		async useVerificationToken({ identifier, token }) {
+			const normalizedIdentifier = identifier?.toLowerCase() ?? "";
 			const rows = await db
 				.select()
 				.from(verificationTokens)
-				.where(eq(verificationTokens.token, token))
+				.where(eq(verificationTokens.identifier, normalizedIdentifier))
 				.limit(1);
 			const row = rows[0];
 			if (!row) {
 				console.warn("[useVerificationToken] No token found");
 				return null;
 			}
-			const normalizedIdentifier = identifier?.toLowerCase() ?? "";
-			const storedIdentifier = row.identifier?.toLowerCase() ?? "";
-			if (normalizedIdentifier !== storedIdentifier) {
-				console.warn("[useVerificationToken] Identifier mismatch");
-				return null;
-			}
-			await db
+			// Claim the exact row we just read (identifier + token) with a single
+			// delete, and only proceed if we actually removed it. This makes
+			// consumption atomic: concurrent requests race on the same delete, so
+			// at most one of them can claim the row, whether the guess is right or
+			// wrong. Scoping by token too (not identifier alone) means the delete
+			// can't clobber a resend that replaced this row after we read it.
+			const result = await db
 				.delete(verificationTokens)
 				.where(
 					and(
-						eq(verificationTokens.token, token),
 						eq(verificationTokens.identifier, row.identifier),
+						eq(verificationTokens.token, row.token),
 					),
 				);
-			return { ...row, identifier: storedIdentifier };
+			if (getAffectedRows(result) !== 1) {
+				console.warn("[useVerificationToken] Token already consumed");
+				return null;
+			}
+			if (row.token !== token) {
+				console.warn("[useVerificationToken] Token mismatch");
+				return null;
+			}
+			return { ...row, identifier: normalizedIdentifier };
 		},
 	};
 }