mirror of
https://github.com/screentinker/screentinker.git
synced 2026-06-15 10:43:36 -06:00
DELETE /api/auth/users/:id ran a bare `DELETE FROM users`, but 23 columns
reference users(id) and only 4 cascade, so with foreign_keys=ON the delete
fails the moment the user is referenced anywhere - and a real user always is
(owns an org, created a workspace, has login activity). Reproduces on a fresh
DB, exactly as reported.
The schema also lacks cascades from workspaces -> tenant resources, so the DB
can't clean up on its own. New lib/user-deletion.js resolves every reference in
one transaction (defer_foreign_keys=ON for forgiving order; table-existence
guard for resilience):
- Refuse (409) if the user OWNS an organization that has other members -
don't nuke a shared tenant; transfer ownership first.
- Hard-delete the organizations they SOLELY own (workspaces + all contents).
- In orgs they don't own, PRESERVE resources: SET NULL the nullable
creator/inviter columns, and reassign the NOT NULL legacy creator user_id to
the resource's org owner (fallback: the acting admin).
- Memberships (organization_members/workspace_members/team_members/
content_folders) cascade on the user delete; pending invites they sent and
legacy teams they own are removed.
The handler now 404s an unknown id and 409s the shared-org case.
Tests (node:test): reproduces the FK failure, then verifies provisioned-member
delete (resources preserved + unlinked/reassigned), solo-org-owner cascade,
shared-org refusal (409), self-delete 400, non-superadmin 403, unknown 404.
Full suite 22/22. Verified end-to-end on a copy of a real DB: deleted a user
owning 2 solo orgs, foreign_key_check clean.
Closes #18.
150 lines
7.5 KiB
JavaScript
150 lines
7.5 KiB
JavaScript
'use strict';
|
|
|
|
// Issue #18: deleting a user 500'd with "FOREIGN KEY constraint failed".
|
|
//
|
|
// 23 columns reference users(id) and only 4 (the *_members join tables +
|
|
// content_folders) carry ON DELETE CASCADE, so a bare `DELETE FROM users`
|
|
// fails the moment the user is referenced anywhere - and a real user always is
|
|
// (owns an org, created a workspace, has login activity). The schema also lacks
|
|
// cascades from workspaces -> tenant resources, so we cannot rely on the DB to
|
|
// clean up; we do it explicitly here in one transaction.
|
|
//
|
|
// Policy (chosen for #18):
|
|
// - Refuse (OrgHasOtherMembersError -> 409) if the user OWNS an organization
|
|
// that has any other member: don't nuke a shared tenant; transfer first.
|
|
// - Otherwise hard-delete the organizations they solely own (and everything
|
|
// inside), and for orgs they DON'T own, preserve the resources - just unlink
|
|
// the user (SET NULL where the column is nullable, or reassign the legacy
|
|
// creator user_id to the resource's org owner where it is NOT NULL).
|
|
//
|
|
// defer_foreign_keys=ON makes intra-transaction delete ORDER forgiving (FKs are
|
|
// validated once at COMMIT); we still clear every reference so COMMIT is clean.
|
|
// A table-existence guard keeps this resilient to partial/older schemas (and
|
|
// makes it unit-testable without standing up all ~25 tables).
|
|
|
|
class OrgHasOtherMembersError extends Error {
|
|
constructor(message, sharedOrgCount) {
|
|
super(message);
|
|
this.name = 'OrgHasOtherMembersError';
|
|
this.sharedOrgCount = sharedOrgCount;
|
|
}
|
|
}
|
|
|
|
// Workspace-scoped tables whose rows must be deleted before their workspace
|
|
// (workspace_id is NO ACTION). CASCADE child tables (playlist_items, telemetry,
|
|
// assignments, layout_zones, *_devices, *_members) clean themselves up.
|
|
const WORKSPACE_SCOPED = [
|
|
'playlists', 'schedules', 'video_walls', 'device_groups', 'devices',
|
|
'content', 'layouts', 'widgets', 'content_folders', 'kiosk_pages',
|
|
'white_labels', 'alert_configs',
|
|
];
|
|
// Logs that carry a device_id but NO foreign key (so they don't block, but we
|
|
// clean them to avoid dangling rows).
|
|
const DEVICE_LOG_TABLES = ['device_status_log', 'player_debug_logs'];
|
|
// Nullable creator/inviter columns -> SET NULL (preserve the resource).
|
|
const NULLABLE_USER_REFS = [
|
|
['content', 'user_id'], ['devices', 'user_id'], ['layouts', 'user_id'], ['widgets', 'user_id'],
|
|
['workspaces', 'created_by'], ['organization_members', 'invited_by'], ['workspace_members', 'invited_by'],
|
|
['team_members', 'invited_by'], ['device_fingerprints', 'user_id'],
|
|
['activity_log', 'user_id'], ['activity_log', 'acting_user_id'],
|
|
];
|
|
// NOT NULL legacy creator columns on workspace-scoped resources -> reassign to
|
|
// the resource's org owner (fallback: the acting admin) so the row survives.
|
|
const REASSIGN_USER_TABLES = [
|
|
'playlists', 'schedules', 'video_walls', 'device_groups', 'kiosk_pages', 'white_labels', 'alert_configs',
|
|
];
|
|
|
|
function listOwnedOrgsWithSharing(db, userId) {
|
|
let orgs = [];
|
|
try { orgs = db.prepare('SELECT id FROM organizations WHERE owner_user_id = ?').all(userId); }
|
|
catch { return []; } // no organizations table (legacy) -> nothing owned
|
|
return orgs.map(o => {
|
|
const otherOrgMembers = db.prepare(
|
|
'SELECT COUNT(*) AS c FROM organization_members WHERE organization_id = ? AND user_id != ?'
|
|
).get(o.id, userId).c;
|
|
const otherWsMembers = db.prepare(`
|
|
SELECT COUNT(*) AS c FROM workspace_members wm
|
|
JOIN workspaces w ON w.id = wm.workspace_id
|
|
WHERE w.organization_id = ? AND wm.user_id != ?
|
|
`).get(o.id, userId).c;
|
|
return { id: o.id, shared: (otherOrgMembers + otherWsMembers) > 0 };
|
|
});
|
|
}
|
|
|
|
// Throws OrgHasOtherMembersError if the user owns a shared org. Otherwise
|
|
// deletes the user and resolves every reference in one transaction.
|
|
function deleteUserCascade(db, { targetId, actingAdminId }) {
|
|
const owned = listOwnedOrgsWithSharing(db, targetId);
|
|
const shared = owned.filter(o => o.shared);
|
|
if (shared.length > 0) {
|
|
throw new OrgHasOtherMembersError(
|
|
'User owns an organization with other members - reassign ownership before deleting',
|
|
shared.length
|
|
);
|
|
}
|
|
const soloOrgIds = owned.map(o => o.id);
|
|
|
|
const have = new Set(db.prepare("SELECT name FROM sqlite_master WHERE type='table'").all().map(r => r.name));
|
|
const inClause = n => Array.from({ length: n }, () => '?').join(',');
|
|
|
|
const run = db.transaction(() => {
|
|
// FK checks deferred to COMMIT: order of our deletes no longer matters, only
|
|
// that no dangling reference remains at the end.
|
|
db.pragma('defer_foreign_keys = ON');
|
|
|
|
// 1) Hard-delete the orgs the user solely owns (and everything inside).
|
|
if (soloOrgIds.length) {
|
|
const wsIds = db.prepare(
|
|
`SELECT id FROM workspaces WHERE organization_id IN (${inClause(soloOrgIds.length)})`
|
|
).all(...soloOrgIds).map(r => r.id);
|
|
|
|
if (wsIds.length) {
|
|
const wph = inClause(wsIds.length);
|
|
if (have.has('devices')) {
|
|
const devIds = db.prepare(`SELECT id FROM devices WHERE workspace_id IN (${wph})`).all(...wsIds).map(r => r.id);
|
|
if (devIds.length) {
|
|
const dph = inClause(devIds.length);
|
|
for (const lt of DEVICE_LOG_TABLES) if (have.has(lt)) db.prepare(`DELETE FROM ${lt} WHERE device_id IN (${dph})`).run(...devIds);
|
|
}
|
|
}
|
|
for (const t of WORKSPACE_SCOPED) if (have.has(t)) db.prepare(`DELETE FROM ${t} WHERE workspace_id IN (${wph})`).run(...wsIds);
|
|
if (have.has('activity_log')) db.prepare(`UPDATE activity_log SET workspace_id = NULL WHERE workspace_id IN (${wph})`).run(...wsIds);
|
|
db.prepare(`DELETE FROM workspaces WHERE id IN (${wph})`).run(...wsIds); // cascades workspace_members/invites
|
|
}
|
|
|
|
const oph = inClause(soloOrgIds.length);
|
|
if (have.has('activity_log')) db.prepare(`UPDATE activity_log SET organization_id = NULL WHERE organization_id IN (${oph})`).run(...soloOrgIds);
|
|
db.prepare(`DELETE FROM organizations WHERE id IN (${oph})`).run(...soloOrgIds); // cascades organization_members
|
|
}
|
|
|
|
// 2) Unlink the user's footprint in orgs they DON'T own (rows still present).
|
|
// 2a) nullable creator/inviter columns -> SET NULL.
|
|
for (const [t, c] of NULLABLE_USER_REFS) if (have.has(t)) db.prepare(`UPDATE ${t} SET ${c} = NULL WHERE ${c} = ?`).run(targetId);
|
|
|
|
// 2b) NOT NULL legacy creator columns -> reassign to the resource's org owner
|
|
// (fallback acting admin), preserving the resource under a valid owner.
|
|
for (const t of REASSIGN_USER_TABLES) {
|
|
if (!have.has(t)) continue;
|
|
db.prepare(`
|
|
UPDATE ${t} SET user_id = COALESCE(
|
|
(SELECT o.owner_user_id FROM workspaces w JOIN organizations o ON o.id = w.organization_id WHERE w.id = ${t}.workspace_id),
|
|
?
|
|
) WHERE user_id = ?
|
|
`).run(actingAdminId, targetId);
|
|
}
|
|
|
|
// 2c) Legacy teams + NOT NULL invite rows the user owns / sent.
|
|
if (have.has('teams')) db.prepare('DELETE FROM teams WHERE owner_id = ?').run(targetId); // cascades team_members/invites
|
|
if (have.has('team_invites')) db.prepare('DELETE FROM team_invites WHERE invited_by = ?').run(targetId);
|
|
if (have.has('workspace_invites')) db.prepare('DELETE FROM workspace_invites WHERE invited_by = ?').run(targetId);
|
|
|
|
// 3) Finally the user. Their own memberships (organization_members,
|
|
// workspace_members, team_members, content_folders) CASCADE on this delete.
|
|
db.prepare('DELETE FROM users WHERE id = ?').run(targetId);
|
|
});
|
|
|
|
run();
|
|
}
|
|
|
|
module.exports = { deleteUserCascade, OrgHasOtherMembersError, listOwnedOrgsWithSharing };
|