Skip to content

Commit 97ad9a5

Browse files
committed
fix: address coderabbit review on admin user deletion
1 parent e58092f commit 97ad9a5

5 files changed

Lines changed: 37 additions & 11 deletions

File tree

apps/webapp/app/components/admin/DeleteUserDialog.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,16 @@ export function DeleteUserDialog({ user, open, onOpenChange }: DeleteUserDialogP
2626
if (!open) setConfirmText("");
2727
}, [open]);
2828

29+
// Close the modal as soon as our own submit starts. Without reloadDocument
30+
// the component stays mounted across the post-redirect soft navigation, so
31+
// `open` would otherwise stay true after the action redirects to /admin.
32+
const isSubmittingDelete =
33+
navigation.state === "submitting" &&
34+
navigation.formData?.get("intent") === "delete";
35+
useEffect(() => {
36+
if (isSubmittingDelete) onOpenChange(false);
37+
}, [isSubmittingDelete, onOpenChange]);
38+
2939
const expected = user ? `delete ${user.email}` : "";
3040
const confirmed = !!user && confirmText === expected;
3141
const isSubmitting = navigation.state !== "idle";
@@ -47,7 +57,7 @@ export function DeleteUserDialog({ user, open, onOpenChange }: DeleteUserDialogP
4757
</div>
4858
)}
4959

50-
<Form method="post" className="flex flex-col gap-3" reloadDocument>
60+
<Form method="post" className="flex flex-col gap-3">
5161
<input type="hidden" name="intent" value="delete" />
5262
<input type="hidden" name="id" value={user?.id ?? ""} />
5363

apps/webapp/app/routes/admin._index.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ export async function action({ request }: ActionFunctionArgs) {
9292
return redirect("/admin?deleted=1");
9393
}
9494

95+
// Reject any POST that set `intent` to something we don't recognise so
96+
// unknown intents don't fall through to the impersonate flow.
97+
if (typeof payload.intent === "string") {
98+
return typedjson({ error: "Unknown action." }, { status: 400 });
99+
}
100+
95101
const { id } = ImpersonateSchema.parse(payload);
96102
return redirectWithImpersonation(request, id, "/");
97103
}

apps/webapp/app/services/personalAccessToken.server.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,24 @@ export async function authenticatePersonalAccessToken(
205205
return;
206206
}
207207

208-
await prisma.personalAccessToken.update({
209-
where: {
210-
id: personalAccessToken.id,
211-
},
212-
data: {
213-
lastAccessedAt: new Date(),
214-
},
215-
});
208+
// Best-effort touch. The token can vanish between the findFirst above and
209+
// this update if a User cascade-delete happens concurrently (admin delete
210+
// flow), so swallow not-found errors rather than 500-ing the auth path.
211+
try {
212+
await prisma.personalAccessToken.update({
213+
where: {
214+
id: personalAccessToken.id,
215+
},
216+
data: {
217+
lastAccessedAt: new Date(),
218+
},
219+
});
220+
} catch (error) {
221+
logger.warn("Failed to touch PersonalAccessToken.lastAccessedAt", {
222+
personalAccessTokenId: personalAccessToken.id,
223+
error,
224+
});
225+
}
216226

217227
const decryptedToken = decryptPersonalAccessToken(personalAccessToken);
218228

internal-packages/database/prisma/migrations/20260428112409_admin_user_deletion/migration.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ CREATE TABLE "public"."UserDeletionAuditLog" (
1111
"adminEmail" TEXT NOT NULL,
1212
"targetUserId" TEXT NOT NULL,
1313
"targetEmail" TEXT NOT NULL,
14-
"softDeletedOrgIds" TEXT[],
14+
"softDeletedOrgIds" TEXT[] NOT NULL DEFAULT ARRAY[]::TEXT[],
1515
"reason" TEXT,
1616
"ipAddress" TEXT,
1717
"createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,

internal-packages/database/prisma/schema.prisma

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2602,7 +2602,7 @@ model UserDeletionAuditLog {
26022602
26032603
/// Organization IDs that were soft-deleted as part of this user delete
26042604
/// (orgs where the deleted user was the sole member).
2605-
softDeletedOrgIds String[]
2605+
softDeletedOrgIds String[] @default([])
26062606
26072607
/// Optional: ticket link / GDPR request id / free-text SE note.
26082608
reason String?

0 commit comments

Comments
 (0)