Skip to content

Commit f8a69a6

Browse files
author
mini0n-ai
committed
fix(auth): verify affectedRows on atomic delete and assert predicate in tests
1 parent 06cbdbb commit f8a69a6

2 files changed

Lines changed: 76 additions & 56 deletions

File tree

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

Lines changed: 57 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type { SQL } from "drizzle-orm";
2+
import { MySqlDialect } from "drizzle-orm/mysql-core";
13
import type { MySql2Database } from "drizzle-orm/mysql2";
24
import { describe, expect, it } from "vitest";
35
import { DrizzleAdapter } from "../../../../packages/database/auth/drizzle-adapter";
@@ -8,36 +10,46 @@ interface VerificationTokenRow {
810
expires: Date;
911
}
1012

13+
const dialect = new MySqlDialect();
14+
1115
function createMockDb(initialRows: VerificationTokenRow[]) {
1216
let table = [...initialRows];
13-
let deletePredicate: unknown = null;
17+
let lastDeleteQuery: { sql: string; params: unknown[] } | null = null;
1418

1519
const db = {
1620
select: () => ({
1721
from: () => ({
18-
where: () => ({
19-
limit: async () => table.slice(0, 1),
20-
}),
22+
where: (pred: unknown) => {
23+
const query = dialect.sqlToQuery(pred as SQL);
24+
const identifierParam = String(query.params[0] ?? "").toLowerCase();
25+
return {
26+
limit: async () =>
27+
table
28+
.filter(
29+
(row) => row.identifier.toLowerCase() === identifierParam,
30+
)
31+
.slice(0, 1),
32+
};
33+
},
2134
}),
2235
}),
2336
delete: () => ({
2437
where: (pred: unknown) => {
25-
deletePredicate = pred;
38+
const query = dialect.sqlToQuery(pred as SQL);
39+
lastDeleteQuery = query;
40+
const [identifierParam, tokenParam] = query.params;
2641
const initialCount = table.length;
2742
table = table.filter(
2843
(row) =>
29-
!(
30-
row.identifier.toLowerCase() === "user@example.com" &&
31-
row.token === "123456"
32-
),
44+
!(row.identifier === identifierParam && row.token === tokenParam),
3345
);
34-
const rowsAffected = initialCount - table.length;
35-
return Promise.resolve({ rowsAffected });
46+
const affectedRows = initialCount - table.length;
47+
return Promise.resolve([{ affectedRows }]);
3648
},
3749
}),
3850
transaction: async (cb: (tx: unknown) => Promise<unknown>) => cb(db),
3951
getTable: () => table,
40-
getDeletePredicate: () => deletePredicate,
52+
getLastDeleteQuery: () => lastDeleteQuery,
4153
};
4254

4355
return db;
@@ -60,7 +72,13 @@ describe("useVerificationToken", () => {
6072
});
6173

6274
expect(result).toBeNull();
63-
expect(mockDb.getDeletePredicate()).not.toBeNull();
75+
const deleteQuery = mockDb.getLastDeleteQuery();
76+
expect(deleteQuery).not.toBeNull();
77+
expect(deleteQuery?.sql).toContain(
78+
"`verification_tokens`.`identifier` = ?",
79+
);
80+
expect(deleteQuery?.sql).toContain("`verification_tokens`.`token` = ?");
81+
expect(deleteQuery?.params).toEqual(["user@example.com", "123456"]);
6482
expect(mockDb.getTable()).toHaveLength(0);
6583
});
6684

@@ -82,7 +100,13 @@ describe("useVerificationToken", () => {
82100
expect(result).not.toBeNull();
83101
expect(result?.identifier).toBe("user@example.com");
84102
expect(result?.token).toBe("123456");
85-
expect(mockDb.getDeletePredicate()).not.toBeNull();
103+
const deleteQuery = mockDb.getLastDeleteQuery();
104+
expect(deleteQuery).not.toBeNull();
105+
expect(deleteQuery?.sql).toContain(
106+
"`verification_tokens`.`identifier` = ?",
107+
);
108+
expect(deleteQuery?.sql).toContain("`verification_tokens`.`token` = ?");
109+
expect(deleteQuery?.params).toEqual(["user@example.com", "123456"]);
86110
expect(mockDb.getTable()).toHaveLength(0);
87111
});
88112

@@ -96,10 +120,10 @@ describe("useVerificationToken", () => {
96120
});
97121

98122
expect(result).toBeNull();
99-
expect(mockDb.getDeletePredicate()).toBeNull();
123+
expect(mockDb.getLastDeleteQuery()).toBeNull();
100124
});
101125

102-
it("prevents race condition by checking rowsAffected on token consumption", async () => {
126+
it("prevents race condition by checking affectedRows on token consumption", async () => {
103127
let table = [
104128
{
105129
identifier: "user@example.com",
@@ -108,6 +132,7 @@ describe("useVerificationToken", () => {
108132
},
109133
];
110134

135+
let firstDeleteDone = false;
111136
const mockDb = {
112137
select: () => ({
113138
from: () => ({
@@ -117,11 +142,16 @@ describe("useVerificationToken", () => {
117142
}),
118143
}),
119144
delete: () => ({
120-
where: () => {
121-
const initialCount = table.length;
122-
table = [];
123-
const rowsAffected = initialCount;
124-
return Promise.resolve({ rowsAffected });
145+
where: (pred: unknown) => {
146+
const query = dialect.sqlToQuery(pred as SQL);
147+
expect(query.sql).toContain("`verification_tokens`.`identifier` = ?");
148+
expect(query.sql).toContain("`verification_tokens`.`token` = ?");
149+
if (!firstDeleteDone) {
150+
firstDeleteDone = true;
151+
table = [];
152+
return Promise.resolve([{ affectedRows: 1 }]);
153+
}
154+
return Promise.resolve([{ affectedRows: 0 }]);
125155
},
126156
}),
127157
transaction: async (cb: (tx: unknown) => Promise<unknown>) => cb(mockDb),
@@ -146,7 +176,7 @@ describe("useVerificationToken", () => {
146176
});
147177

148178
it("deletes only the selected token instance and preserves replacement tokens for the same user", async () => {
149-
let table = [
179+
const mockDb = createMockDb([
150180
{
151181
identifier: "user@example.com",
152182
token: "123456",
@@ -157,40 +187,17 @@ describe("useVerificationToken", () => {
157187
token: "replacement_token",
158188
expires: new Date(Date.now() + 600000),
159189
},
160-
];
161-
162-
const mockDb = {
163-
select: () => ({
164-
from: () => ({
165-
where: () => ({
166-
limit: async () => [table[0]],
167-
}),
168-
}),
169-
}),
170-
delete: () => ({
171-
where: () => {
172-
const initialCount = table.length;
173-
table = table.filter(
174-
(row) =>
175-
!(
176-
row.identifier === "user@example.com" && row.token === "123456"
177-
),
178-
);
179-
const rowsAffected = initialCount - table.length;
180-
return Promise.resolve({ rowsAffected });
181-
},
182-
}),
183-
transaction: async (cb: (tx: unknown) => Promise<unknown>) => cb(mockDb),
184-
} as unknown as MySql2Database;
190+
]);
185191

186-
const adapter = DrizzleAdapter(mockDb);
192+
const adapter = DrizzleAdapter(mockDb as unknown as MySql2Database);
187193
const result = await adapter.useVerificationToken?.({
188194
identifier: "USER@example.com",
189195
token: "999999",
190196
});
191197

192198
expect(result).toBeNull();
193-
expect(table.some((r) => r.token === "123456")).toBe(false);
194-
expect(table.some((r) => r.token === "replacement_token")).toBe(true);
199+
const remaining = mockDb.getTable();
200+
expect(remaining.some((r) => r.token === "123456")).toBe(false);
201+
expect(remaining.some((r) => r.token === "replacement_token")).toBe(true);
195202
});
196203
});

packages/database/auth/drizzle-adapter.ts

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,21 @@ async function hasLinkedAccount(db: MySql2Database, userId: User.UserId) {
7070
return !!linkedAccount;
7171
}
7272

73+
function getAffectedRows(result: unknown): number {
74+
if (Array.isArray(result)) {
75+
return (
76+
(result[0] as { affectedRows?: number } | undefined)?.affectedRows ?? 0
77+
);
78+
}
79+
return (
80+
(result as { affectedRows?: number; rowsAffected?: number } | undefined)
81+
?.affectedRows ??
82+
(result as { affectedRows?: number; rowsAffected?: number } | undefined)
83+
?.rowsAffected ??
84+
0
85+
);
86+
}
87+
7388
export function DrizzleAdapter(
7489
db: MySql2Database,
7590
options?: { getSsoIdentity: () => ValidatedSsoIdentity | null },
@@ -525,8 +540,6 @@ export function DrizzleAdapter(
525540
}
526541
const storedIdentifier = row.identifier?.toLowerCase() ?? "";
527542

528-
// Invalidate the specific token instance that was selected. This burns wrong guesses
529-
// while scoping deletion to both identifier AND row.token to protect newly issued replacement tokens.
530543
const result = await tx
531544
.delete(verificationTokens)
532545
.where(
@@ -536,9 +549,7 @@ export function DrizzleAdapter(
536549
),
537550
);
538551

539-
// If database reports 0 rows affected, token was consumed or rotated concurrently
540-
const rowsAffected = (result as { rowsAffected?: number })?.rowsAffected;
541-
if (rowsAffected === 0) {
552+
if (getAffectedRows(result) === 0) {
542553
console.warn(
543554
"[useVerificationToken] Token already consumed or invalid during deletion.",
544555
);
@@ -554,7 +565,9 @@ export function DrizzleAdapter(
554565
};
555566

556567
if (typeof db.transaction === "function") {
557-
return await db.transaction(async (tx) => execute(tx as unknown as typeof db));
568+
return await db.transaction(async (tx) =>
569+
execute(tx as unknown as typeof db),
570+
);
558571
}
559572
return await execute(db);
560573
},

0 commit comments

Comments
 (0)