Skip to content

Commit 600a0d6

Browse files
N2D4N2D4
authored
fix: handle race condition in recordExternalDbSyncDeletion (#1466)
1 parent 03e7b61 commit 600a0d6

2 files changed

Lines changed: 94 additions & 61 deletions

File tree

‎apps/backend/src/lib/external-db-sync.ts‎

Lines changed: 10 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export async function recordExternalDbSyncDeletion(
126126

127127
if (target.tableName === "ProjectUser") {
128128
assertUuid(target.projectUserId, "projectUserId");
129-
const insertedCount = await tx.$executeRaw(Prisma.sql`
129+
await tx.$executeRaw(Prisma.sql`
130130
INSERT INTO "DeletedRow" (
131131
"id",
132132
"tenancyId",
@@ -150,18 +150,13 @@ export async function recordExternalDbSyncDeletion(
150150
FOR UPDATE
151151
`);
152152

153-
if (insertedCount !== 1) {
154-
throw new StackAssertionError(
155-
`Expected to insert 1 DeletedRow entry for ProjectUser, got ${insertedCount}.`
156-
);
157-
}
158153
return;
159154
}
160155

161156
if (target.tableName === "ContactChannel") {
162157
assertUuid(target.projectUserId, "projectUserId");
163158
assertUuid(target.contactChannelId, "contactChannelId");
164-
const insertedCount = await tx.$executeRaw(Prisma.sql`
159+
await tx.$executeRaw(Prisma.sql`
165160
INSERT INTO "DeletedRow" (
166161
"id",
167162
"tenancyId",
@@ -193,17 +188,12 @@ export async function recordExternalDbSyncDeletion(
193188
FOR UPDATE
194189
`);
195190

196-
if (insertedCount !== 1) {
197-
throw new StackAssertionError(
198-
`Expected to insert 1 DeletedRow entry for ContactChannel, got ${insertedCount}.`
199-
);
200-
}
201191
return;
202192
}
203193

204194
if (target.tableName === "Team") {
205195
assertUuid(target.teamId, "teamId");
206-
const insertedCount = await tx.$executeRaw(Prisma.sql`
196+
await tx.$executeRaw(Prisma.sql`
207197
INSERT INTO "DeletedRow" (
208198
"id",
209199
"tenancyId",
@@ -227,18 +217,13 @@ export async function recordExternalDbSyncDeletion(
227217
FOR UPDATE
228218
`);
229219

230-
if (insertedCount !== 1) {
231-
throw new StackAssertionError(
232-
`Expected to insert 1 DeletedRow entry for Team, got ${insertedCount}.`
233-
);
234-
}
235220
return;
236221
}
237222

238223
if (target.tableName === "TeamMember") {
239224
assertUuid(target.projectUserId, "projectUserId");
240225
assertUuid(target.teamId, "teamId");
241-
const insertedCount = await tx.$executeRaw(Prisma.sql`
226+
await tx.$executeRaw(Prisma.sql`
242227
INSERT INTO "DeletedRow" (
243228
"id",
244229
"tenancyId",
@@ -263,17 +248,12 @@ export async function recordExternalDbSyncDeletion(
263248
FOR UPDATE
264249
`);
265250

266-
if (insertedCount !== 1) {
267-
throw new StackAssertionError(
268-
`Expected to insert 1 DeletedRow entry for TeamMember, got ${insertedCount}.`
269-
);
270-
}
271251
return;
272252
}
273253

274254
if (target.tableName === "TeamMemberDirectPermission") {
275255
assertUuid(target.permissionDbId, "permissionDbId");
276-
const insertedCount = await tx.$executeRaw(Prisma.sql`
256+
await tx.$executeRaw(Prisma.sql`
277257
INSERT INTO "DeletedRow" (
278258
"id",
279259
"tenancyId",
@@ -302,17 +282,12 @@ export async function recordExternalDbSyncDeletion(
302282
FOR UPDATE
303283
`);
304284

305-
if (insertedCount !== 1) {
306-
throw new StackAssertionError(
307-
`Expected to insert 1 DeletedRow entry for TeamMemberDirectPermission, got ${insertedCount}.`
308-
);
309-
}
310285
return;
311286
}
312287

313288
if (target.tableName === "ProjectUserDirectPermission") {
314289
assertUuid(target.permissionDbId, "permissionDbId");
315-
const insertedCount = await tx.$executeRaw(Prisma.sql`
290+
await tx.$executeRaw(Prisma.sql`
316291
INSERT INTO "DeletedRow" (
317292
"id",
318293
"tenancyId",
@@ -340,17 +315,12 @@ export async function recordExternalDbSyncDeletion(
340315
FOR UPDATE
341316
`);
342317

343-
if (insertedCount !== 1) {
344-
throw new StackAssertionError(
345-
`Expected to insert 1 DeletedRow entry for ProjectUserDirectPermission, got ${insertedCount}.`
346-
);
347-
}
348318
return;
349319
}
350320

351321
if (target.tableName === "UserNotificationPreference") {
352322
assertUuid(target.notificationPreferenceId, "notificationPreferenceId");
353-
const insertedCount = await tx.$executeRaw(Prisma.sql`
323+
await tx.$executeRaw(Prisma.sql`
354324
INSERT INTO "DeletedRow" (
355325
"id",
356326
"tenancyId",
@@ -377,17 +347,12 @@ export async function recordExternalDbSyncDeletion(
377347
FOR UPDATE
378348
`);
379349

380-
if (insertedCount !== 1) {
381-
throw new StackAssertionError(
382-
`Expected to insert 1 DeletedRow entry for UserNotificationPreference, got ${insertedCount}.`
383-
);
384-
}
385350
return;
386351
}
387352

388353
if (target.tableName === "ProjectUserRefreshToken") {
389354
assertUuid(target.refreshTokenId, "refreshTokenId");
390-
const insertedCount = await tx.$executeRaw(Prisma.sql`
355+
await tx.$executeRaw(Prisma.sql`
391356
INSERT INTO "DeletedRow" (
392357
"id",
393358
"tenancyId",
@@ -411,17 +376,12 @@ export async function recordExternalDbSyncDeletion(
411376
FOR UPDATE
412377
`);
413378

414-
if (insertedCount !== 1) {
415-
throw new StackAssertionError(
416-
`Expected to insert 1 DeletedRow entry for ProjectUserRefreshToken, got ${insertedCount}.`
417-
);
418-
}
419379
return;
420380
}
421381

422382
if (target.tableName === "ProjectUserOAuthAccount") {
423383
assertUuid(target.oauthAccountId, "oauthAccountId");
424-
const insertedCount = await tx.$executeRaw(Prisma.sql`
384+
await tx.$executeRaw(Prisma.sql`
425385
INSERT INTO "DeletedRow" (
426386
"id",
427387
"tenancyId",
@@ -445,11 +405,6 @@ export async function recordExternalDbSyncDeletion(
445405
FOR UPDATE
446406
`);
447407

448-
if (insertedCount !== 1) {
449-
throw new StackAssertionError(
450-
`Expected to insert 1 DeletedRow entry for ProjectUserOAuthAccount, got ${insertedCount}.`
451-
);
452-
}
453408
return;
454409
}
455410

@@ -458,7 +413,7 @@ export async function recordExternalDbSyncDeletion(
458413
assertNonEmptyString(target.verificationCodeProjectId, "verificationCodeProjectId");
459414
assertNonEmptyString(target.verificationCodeBranchId, "verificationCodeBranchId");
460415
assertUuid(target.verificationCodeId, "verificationCodeId");
461-
const insertedCount = await tx.$executeRaw(Prisma.sql`
416+
await tx.$executeRaw(Prisma.sql`
462417
INSERT INTO "DeletedRow" (
463418
"id",
464419
"tenancyId",
@@ -487,11 +442,6 @@ export async function recordExternalDbSyncDeletion(
487442
FOR UPDATE OF "VerificationCode"
488443
`);
489444

490-
if (insertedCount !== 1) {
491-
throw new StackAssertionError(
492-
`Expected to insert 1 DeletedRow entry for VerificationCode_TEAM_INVITATION, got ${insertedCount}.`
493-
);
494-
}
495445
return;
496446
}
497447
}

‎apps/e2e/tests/backend/endpoints/api/v1/auth/sessions/current/index.test.ts‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,88 @@
11
import { it } from "../../../../../../../helpers";
2-
import { Auth, niceBackendFetch } from "../../../../../../backend-helpers";
2+
import { Auth, backendContext, niceBackendFetch } from "../../../../../../backend-helpers";
3+
4+
it("should not crash when signing out a session that was already deleted by a bulk operation", async ({ expect }) => {
5+
// Reproduce: sign up, then admin-delete all refresh tokens (simulating a
6+
// concurrent password change), then attempt sign-out with the stale access token.
7+
// Before fix: 500 assertion error in recordExternalDbSyncDeletion.
8+
// After fix: 401 REFRESH_TOKEN_NOT_FOUND_OR_EXPIRED.
9+
const signUpRes = await Auth.Password.signUpWithEmail({ noWaitForEmail: true });
10+
const savedAuth = backendContext.value.userAuth ?? undefined;
11+
12+
// Admin updates the user's password, which bulk-deletes all refresh tokens
13+
await niceBackendFetch(`/api/v1/users/${signUpRes.userId}`, {
14+
accessType: "admin",
15+
method: "PATCH",
16+
body: { password: "completely-new-password-12345" },
17+
});
18+
19+
// Try to sign out using the original access token (which still references the
20+
// now-deleted refresh token). This should NOT throw a 500 assertion error.
21+
const response = await niceBackendFetch("/api/v1/auth/sessions/current", {
22+
method: "DELETE",
23+
accessType: "client",
24+
userAuth: savedAuth,
25+
});
26+
expect(response.status).not.toBe(500);
27+
expect(response).toMatchInlineSnapshot(`
28+
NiceResponse {
29+
"status": 401,
30+
"body": {
31+
"code": "REFRESH_TOKEN_NOT_FOUND_OR_EXPIRED",
32+
"error": "Refresh token not found for this project, or the session has expired/been revoked.",
33+
},
34+
"headers": Headers {
35+
"x-stack-known-error": "REFRESH_TOKEN_NOT_FOUND_OR_EXPIRED",
36+
<some fields may have been hidden>,
37+
},
38+
}
39+
`);
40+
});
41+
42+
it("should not crash when deleting a session that was already deleted by a bulk operation", async ({ expect }) => {
43+
// Same race condition but via the sessions CRUD DELETE endpoint
44+
const signUpRes = await Auth.Password.signUpWithEmail({ noWaitForEmail: true });
45+
46+
// Create a second session
47+
const newSessionRes = await niceBackendFetch("/api/v1/auth/sessions", {
48+
accessType: "server",
49+
method: "POST",
50+
body: { user_id: signUpRes.userId },
51+
});
52+
expect(newSessionRes.status).toBe(200);
53+
54+
// List sessions to get the second session's ID
55+
const listRes = await niceBackendFetch("/api/v1/auth/sessions", {
56+
accessType: "client",
57+
method: "GET",
58+
query: { user_id: signUpRes.userId },
59+
});
60+
expect(listRes.status).toBe(200);
61+
const nonCurrentSession = listRes.body.items.find((s: any) => !s.is_current_session);
62+
expect(nonCurrentSession).toBeDefined();
63+
64+
// Admin-update user password → bulk-deletes all refresh tokens
65+
await niceBackendFetch(`/api/v1/users/${signUpRes.userId}`, {
66+
accessType: "admin",
67+
method: "PATCH",
68+
body: { password: "another-new-password-12345" },
69+
});
70+
71+
// Try to delete the (now-deleted) session via CRUD endpoint
72+
const deleteRes = await niceBackendFetch(`/api/v1/auth/sessions/${nonCurrentSession.id}`, {
73+
accessType: "client",
74+
method: "DELETE",
75+
query: { user_id: signUpRes.userId },
76+
});
77+
expect(deleteRes.status).not.toBe(500);
78+
expect(deleteRes).toMatchInlineSnapshot(`
79+
NiceResponse {
80+
"status": 404,
81+
"body": "Session not found.",
82+
"headers": Headers { <some fields may have been hidden> },
83+
}
84+
`);
85+
});
386

487
it("should sign out users", async ({ expect }) => {
588
await Auth.Password.signUpWithEmail();

0 commit comments

Comments
 (0)