diff --git a/apps/web/__tests__/integration/sso-database.test.ts b/apps/web/__tests__/integration/sso-database.test.ts index 5d4c6ce485..55c2b5a68f 100644 --- a/apps/web/__tests__/integration/sso-database.test.ts +++ b/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 adapter = DrizzleAdapter(database()); + if (!adapter.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 }, () => + adapter.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 adapter = DrizzleAdapter(database()); + if (!adapter.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) => + adapter.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); + }); }, ); diff --git a/apps/web/__tests__/unit/verification-token.test.ts b/apps/web/__tests__/unit/verification-token.test.ts new file mode 100644 index 0000000000..ea7c963578 --- /dev/null +++ b/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); + }); +}); diff --git a/packages/database/auth/drizzle-adapter.ts b/packages/database/auth/drizzle-adapter.ts index 16f7e10453..882fd950b9 100644 --- a/packages/database/auth/drizzle-adapter.ts +++ b/packages/database/auth/drizzle-adapter.ts @@ -17,6 +17,15 @@ import { } from "../schema.ts"; import type { ValidatedSsoIdentity } from "./sso.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>[0]; type LinkAccountData = Parameters>[0]; type UnlinkAccountData = Parameters>[0]; @@ -510,31 +519,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 }; }, }; }