Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mini0n-ai wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06cbdbb
fix(auth): invalidate email OTP on failed guess, not just a match (#2…
mini0n-ai Sep 11, 2026
f8a69a6
fix(auth): verify affectedRows on atomic delete and assert predicate …
Sep 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
203 changes: 203 additions & 0 deletions apps/web/__tests__/unit/verification-token.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
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";

interface VerificationTokenRow {
identifier: string;
token: string;
expires: Date;
}

const dialect = new MySqlDialect();

function createMockDb(initialRows: VerificationTokenRow[]) {
let table = [...initialRows];
let lastDeleteQuery: { sql: string; params: unknown[] } | null = null;

const db = {
select: () => ({
from: () => ({
where: (pred: unknown) => {
const query = dialect.sqlToQuery(pred as SQL);
const identifierParam = String(query.params[0] ?? "").toLowerCase();
return {
limit: async () =>
table
.filter(
(row) => row.identifier.toLowerCase() === identifierParam,
)
.slice(0, 1),
};
},
}),
}),
delete: () => ({
where: (pred: unknown) => {
const query = dialect.sqlToQuery(pred as SQL);
lastDeleteQuery = query;
const [identifierParam, tokenParam] = query.params;
const initialCount = table.length;
table = table.filter(
(row) =>
!(row.identifier === identifierParam && row.token === tokenParam),
);
const affectedRows = initialCount - table.length;
return Promise.resolve([{ affectedRows }]);
},
}),
transaction: async (cb: (tx: unknown) => Promise<unknown>) => cb(db),
getTable: () => table,
getLastDeleteQuery: () => lastDeleteQuery,
};

return db;
}

describe("useVerificationToken", () => {
it("burns the token on wrong guess and returns null", async () => {
const mockDb = createMockDb([
{
identifier: "user@example.com",
token: "123456",
expires: new Date(Date.now() + 600000),
},
]);

const adapter = DrizzleAdapter(mockDb as unknown as MySql2Database);
const result = await adapter.useVerificationToken?.({
identifier: "USER@example.com",
token: "999999",
});

expect(result).toBeNull();
const deleteQuery = mockDb.getLastDeleteQuery();
expect(deleteQuery).not.toBeNull();
expect(deleteQuery?.sql).toContain(
"`verification_tokens`.`identifier` = ?",
);
expect(deleteQuery?.sql).toContain("`verification_tokens`.`token` = ?");
expect(deleteQuery?.params).toEqual(["user@example.com", "123456"]);
expect(mockDb.getTable()).toHaveLength(0);
});

it("returns token and invalidates it on correct guess", async () => {
const mockDb = createMockDb([
{
identifier: "user@example.com",
token: "123456",
expires: new Date(Date.now() + 600000),
},
]);

const adapter = DrizzleAdapter(mockDb as unknown as MySql2Database);
const result = await adapter.useVerificationToken?.({
identifier: "USER@example.com",
token: "123456",
});

expect(result).not.toBeNull();
expect(result?.identifier).toBe("user@example.com");
expect(result?.token).toBe("123456");
const deleteQuery = mockDb.getLastDeleteQuery();
expect(deleteQuery).not.toBeNull();
expect(deleteQuery?.sql).toContain(
"`verification_tokens`.`identifier` = ?",
);
expect(deleteQuery?.sql).toContain("`verification_tokens`.`token` = ?");
expect(deleteQuery?.params).toEqual(["user@example.com", "123456"]);
expect(mockDb.getTable()).toHaveLength(0);
});

it("returns null if token does not exist", async () => {
const mockDb = createMockDb([]);

const adapter = DrizzleAdapter(mockDb as unknown as MySql2Database);
const result = await adapter.useVerificationToken?.({
identifier: "nonexistent@example.com",
token: "123456",
});

expect(result).toBeNull();
expect(mockDb.getLastDeleteQuery()).toBeNull();
});

it("prevents race condition by checking affectedRows on token consumption", async () => {
let table = [
{
identifier: "user@example.com",
token: "123456",
expires: new Date(Date.now() + 600000),
},
];

let firstDeleteDone = false;
const mockDb = {
select: () => ({
from: () => ({
where: () => ({
limit: async () => table.slice(0, 1),
}),
}),
}),
delete: () => ({
where: (pred: unknown) => {
const query = dialect.sqlToQuery(pred as SQL);
expect(query.sql).toContain("`verification_tokens`.`identifier` = ?");
expect(query.sql).toContain("`verification_tokens`.`token` = ?");
if (!firstDeleteDone) {
firstDeleteDone = true;
table = [];
return Promise.resolve([{ affectedRows: 1 }]);
}
return Promise.resolve([{ affectedRows: 0 }]);
},
}),
transaction: async (cb: (tx: unknown) => Promise<unknown>) => cb(mockDb),
} as unknown as MySql2Database;

const adapter = DrizzleAdapter(mockDb);

const firstResult = await adapter.useVerificationToken?.({
identifier: "USER@example.com",
token: "123456",
});

expect(firstResult).not.toBeNull();
expect(firstResult?.token).toBe("123456");

const secondResult = await adapter.useVerificationToken?.({
identifier: "USER@example.com",
token: "123456",
});

expect(secondResult).toBeNull();
});

it("deletes only the selected token instance and preserves replacement tokens for the same user", async () => {
const mockDb = createMockDb([
{
identifier: "user@example.com",
token: "123456",
expires: new Date(Date.now() + 600000),
},
{
identifier: "user@example.com",
token: "replacement_token",
expires: new Date(Date.now() + 600000),
},
]);

const adapter = DrizzleAdapter(mockDb as unknown as MySql2Database);
const result = await adapter.useVerificationToken?.({
identifier: "USER@example.com",
token: "999999",
});

expect(result).toBeNull();
const remaining = mockDb.getTable();
expect(remaining.some((r) => r.token === "123456")).toBe(false);
expect(remaining.some((r) => r.token === "replacement_token")).toBe(true);
});
});
81 changes: 58 additions & 23 deletions packages/database/auth/drizzle-adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,21 @@ async function hasLinkedAccount(db: MySql2Database, userId: User.UserId) {
return !!linkedAccount;
}

function getAffectedRows(result: unknown): number {
if (Array.isArray(result)) {
return (
(result[0] as { affectedRows?: number } | undefined)?.affectedRows ?? 0
);
}
return (
(result as { affectedRows?: number; rowsAffected?: number } | undefined)
?.affectedRows ??
(result as { affectedRows?: number; rowsAffected?: number } | undefined)
?.rowsAffected ??
0
);
}

export function DrizzleAdapter(
db: MySql2Database,
options?: { getSsoIdentity: () => ValidatedSsoIdentity | null },
Expand Down Expand Up @@ -510,31 +525,51 @@ export function DrizzleAdapter(
return row;
},
async useVerificationToken({ identifier, token }) {
const rows = await db
.select()
.from(verificationTokens)
.where(eq(verificationTokens.token, token))
.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
.delete(verificationTokens)
.where(
and(
eq(verificationTokens.token, token),
eq(verificationTokens.identifier, row.identifier),
),

const execute = async (tx: typeof db) => {
const rows = await tx
.select()
.from(verificationTokens)
.where(eq(verificationTokens.identifier, normalizedIdentifier))
.limit(1);
const row = rows[0];
if (!row) {
console.warn("[useVerificationToken] No token found");
return null;
}
const storedIdentifier = row.identifier?.toLowerCase() ?? "";

@superagent-security superagent-security Bot Sep 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The OTP deletion is scoped only by identifier and can invalidate a different token during rotation

DELETE filters only by email, so OTP rotation or concurrent requests can consume the wrong row or replay one OTP.

Atomically consume the selected token by its unique key and verify the affected-row count; add race tests.

AI prompt 
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="packages/database/auth/drizzle-adapter.ts">
<violation number="1" location="packages/database/auth/drizzle-adapter.ts:528">
<priority>P2</priority>
<title>The OTP deletion is scoped only by identifier and can invalidate a different token during rotation</title>
<evidence>After selecting one row by normalized identifier, the new deletion uses only eq(verificationTokens.identifier, row.identifier). A token generated for the same email between the SELECT and DELETE, or another concurrently stored token for that email, can therefore be deleted even though it was not the row selected for this request. The separate SELECT and DELETE also leave concurrent verification requests able to both observe and return the same valid row before either deletion is committed, so this does not provide the claimed replay/race protection.</evidence>
<recommendation>Consume the exact selected row atomically: use a transaction with row locking, or a single conditional DELETE/UPDATE that identifies the row by its unique token/key and returns the consumed row. Check the affected-row/result count before returning success, and add concurrency and token-rotation tests.</recommendation>
</violation>
</file>

const result = await tx
.delete(verificationTokens)
.where(
and(
eq(verificationTokens.identifier, row.identifier),
eq(verificationTokens.token, row.token),
),
);

if (getAffectedRows(result) === 0) {
console.warn(
"[useVerificationToken] Token already consumed or invalid during deletion.",
);
return null;
}

if (row.token !== token) {
console.warn("[useVerificationToken] Token mismatch");
return null;
}

return { ...row, identifier: storedIdentifier };
};

if (typeof db.transaction === "function") {
return await db.transaction(async (tx) =>
execute(tx as unknown as typeof db),
);
return { ...row, identifier: storedIdentifier };
}
return await execute(db);
},
};
}