diff --git a/server/src/services/adminService.ts b/server/src/services/adminService.ts index 66a2d349..f17dd8ba 100644 --- a/server/src/services/adminService.ts +++ b/server/src/services/adminService.ts @@ -8,6 +8,7 @@ import { updateJwtSecret } from '../config'; import { maybe_encrypt_api_key, decrypt_api_key } from './apiKeyCrypto'; import { getAllPermissions, savePermissions as savePerms, PERMISSION_ACTIONS } from './permissions'; import { revokeUserSessions, revokeUserSessionsForClient } from '../mcp'; +import { deleteUserCompletely } from './userCleanupService'; import { validatePassword } from './passwordPolicy'; import { getPhotoProviderConfig } from './memories/helpersService'; import { send as sendNotification } from './notificationService'; @@ -170,7 +171,7 @@ export function deleteUser(id: string, currentUserId: number) { const userToDel = db.prepare('SELECT id, email FROM users WHERE id = ?').get(id) as { id: number; email: string } | undefined; if (!userToDel) return { error: 'User not found', status: 404 }; - db.prepare('DELETE FROM users WHERE id = ?').run(id); + deleteUserCompletely(userToDel.id); return { email: userToDel.email }; } diff --git a/server/src/services/authService.ts b/server/src/services/authService.ts index 3917e64b..0bbcbfc7 100644 --- a/server/src/services/authService.ts +++ b/server/src/services/authService.ts @@ -15,6 +15,7 @@ import { decrypt_api_key, maybe_encrypt_api_key, encrypt_api_key } from './apiKe import { createEphemeralToken } from './ephemeralTokens'; import { revokeUserSessions } from '../mcp'; import { startTripReminders } from '../scheduler'; +import { deleteUserCompletely } from './userCleanupService'; import { verifyJwtAndLoadUser } from '../middleware/auth'; import { User } from '../types'; import { DEMO_EMAIL_PRIMARY, isDemoEmail } from './demo'; @@ -527,7 +528,7 @@ export function deleteAccount(userId: number, userEmail: string, userRole: strin return { error: 'Cannot delete the last admin account', status: 400 }; } } - db.prepare('DELETE FROM users WHERE id = ?').run(userId); + deleteUserCompletely(userId); return { success: true }; } diff --git a/server/src/services/userCleanupService.ts b/server/src/services/userCleanupService.ts new file mode 100644 index 00000000..84239449 --- /dev/null +++ b/server/src/services/userCleanupService.ts @@ -0,0 +1,21 @@ +import { db } from '../db/database'; + +function cleanupUserReferences(userId: number): void { + db.prepare('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?').run(userId); + db.prepare('UPDATE budget_items SET paid_by_user_id = NULL WHERE paid_by_user_id = ?').run(userId); + db.prepare('DELETE FROM share_tokens WHERE created_by = ?').run(userId); + db.prepare('DELETE FROM journey_share_tokens WHERE created_by = ?').run(userId); + // Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs + db.prepare('DELETE FROM journeys WHERE user_id = ?').run(userId); + // Entries authored on other users' journeys (not covered by the cascade above) + db.prepare('DELETE FROM journey_entries WHERE author_id = ?').run(userId); + db.prepare('DELETE FROM journey_contributors WHERE user_id = ?').run(userId); +} + +export function deleteUserCompletely(userId: number): void { + const tx = db.transaction((id: number) => { + cleanupUserReferences(id); + db.prepare('DELETE FROM users WHERE id = ?').run(id); + }); + tx(userId); +} diff --git a/server/tests/integration/admin.test.ts b/server/tests/integration/admin.test.ts index 0d105c61..541b8035 100644 --- a/server/tests/integration/admin.test.ts +++ b/server/tests/integration/admin.test.ts @@ -41,7 +41,7 @@ import { createApp } from '../../src/app'; import { createTables } from '../../src/db/schema'; import { runMigrations } from '../../src/db/migrations'; import { resetTestDb } from '../helpers/test-db'; -import { createUser, createAdmin, createInviteToken } from '../helpers/factories'; +import { createUser, createAdmin, createInviteToken, createTrip, createBudgetItem, createJourney, createJourneyEntry, addJourneyContributor } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; import { loginAttempts, mfaAttempts } from '../../src/routes/auth'; @@ -148,6 +148,70 @@ describe('Admin user management', () => { expect(deleted).toBeUndefined(); }); + it('ADMIN-005b — DELETE /admin/users/:id succeeds when user has FK references', async () => { + const { user: admin } = createAdmin(testDb); + const { user: target } = createUser(testDb); + const { user: otherUser } = createUser(testDb); + const { user: thirdUser } = createUser(testDb); + + // trip_members.invited_by: target invited thirdUser to otherUser's trip + // (trip survives deletion; only invited_by should become NULL) + const otherTrip = createTrip(testDb, otherUser.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(otherTrip.id, thirdUser.id, target.id); + + // share_tokens.created_by: target created a share token for otherUser's trip + testDb.prepare("INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, 'tok-admin-test', ?)").run(otherTrip.id, target.id); + + // budget_items.paid_by_user_id: target paid for an expense on otherUser's trip + const budgetItem = createBudgetItem(testDb, otherTrip.id); + testDb.prepare('UPDATE budget_items SET paid_by_user_id = ? WHERE id = ?').run(target.id, budgetItem.id); + + // journey_contributors: target is a contributor on otherUser's journey + const otherJourney = createJourney(testDb, otherUser.id); + addJourneyContributor(testDb, otherJourney.id, target.id); + + // journey_entries: target authored an entry on otherUser's journey + createJourneyEntry(testDb, otherJourney.id, target.id); + + // journey_share_tokens: target created a share token for otherUser's journey + testDb.prepare("INSERT INTO journey_share_tokens (journey_id, token, created_by) VALUES (?, 'jst-admin-test', ?)").run(otherJourney.id, target.id); + + // notifications.sender_id (SET NULL): target sent a notification to otherUser + const sentNotif = testDb.prepare( + "INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, text_key) VALUES ('simple', 'trip', ?, ?, ?, 'k', 'k')" + ).run(otherTrip.id, target.id, otherUser.id); + // notifications.recipient_id (CASCADE): otherUser sent a notification to target + testDb.prepare( + "INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, text_key) VALUES ('simple', 'trip', ?, ?, ?, 'k', 'k')" + ).run(otherTrip.id, otherUser.id, target.id); + + // user_notice_dismissals (CASCADE): target dismissed a notice + testDb.prepare( + "INSERT INTO user_notice_dismissals (user_id, notice_id, dismissed_at) VALUES (?, 'test-notice', ?)" + ).run(target.id, Date.now()); + + const res = await request(app) + .delete(`/api/admin/users/${target.id}`) + .set('Cookie', authCookie(admin.id)); + expect(res.status).toBe(200); + expect(res.body.success).toBe(true); + + expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(target.id)).toBeUndefined(); + // trip_members row survives but invited_by is now NULL + expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(otherTrip.id, thirdUser.id) as any).invited_by).toBeNull(); + expect(testDb.prepare('SELECT id FROM share_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + expect((testDb.prepare('SELECT paid_by_user_id FROM budget_items WHERE id = ?').get(budgetItem.id) as any).paid_by_user_id).toBeNull(); + expect(testDb.prepare('SELECT user_id FROM journey_contributors WHERE journey_id = ? AND user_id = ?').get(otherJourney.id, target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_entries WHERE author_id = ?').get(target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_share_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + // sent notification survives but sender_id becomes NULL + expect((testDb.prepare('SELECT sender_id FROM notifications WHERE id = ?').get(sentNotif.lastInsertRowid) as any).sender_id).toBeNull(); + // received notification is cascade-deleted + expect(testDb.prepare('SELECT id FROM notifications WHERE recipient_id = ?').get(target.id)).toBeUndefined(); + // notice dismissals are cascade-deleted + expect(testDb.prepare("SELECT user_id FROM user_notice_dismissals WHERE user_id = ? AND notice_id = 'test-notice'").get(target.id)).toBeUndefined(); + }); + it('ADMIN-006 — admin cannot delete their own account', async () => { const { user: admin } = createAdmin(testDb); diff --git a/server/tests/integration/auth.test.ts b/server/tests/integration/auth.test.ts index deb9df5a..2ec4571d 100644 --- a/server/tests/integration/auth.test.ts +++ b/server/tests/integration/auth.test.ts @@ -52,7 +52,7 @@ import { createApp } from '../../src/app'; import { createTables } from '../../src/db/schema'; import { runMigrations } from '../../src/db/migrations'; import { resetTestDb } from '../helpers/test-db'; -import { createUser, createAdmin, createUserWithMfa, createInviteToken } from '../helpers/factories'; +import { createUser, createAdmin, createUserWithMfa, createInviteToken, createTrip, createBudgetItem, createJourney, createJourneyEntry, addJourneyContributor } from '../helpers/factories'; import { authCookie, authHeader } from '../helpers/auth'; import { loginAttempts, mfaAttempts } from '../../src/routes/auth'; @@ -509,6 +509,79 @@ describe('Extended auth scenarios', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// Account deletion +// ───────────────────────────────────────────────────────────────────────────── + +describe('Account deletion', () => { + it('AUTH-040 — DELETE /auth/me succeeds when user has FK references', async () => { + const { user: admin } = createAdmin(testDb); + const { user: target } = createUser(testDb); + const { user: otherUser } = createUser(testDb); + const { user: thirdUser } = createUser(testDb); + + // trip_members.invited_by: target invited thirdUser to otherUser's trip + // (trip survives deletion; only invited_by should become NULL) + const otherTrip = createTrip(testDb, otherUser.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(otherTrip.id, thirdUser.id, target.id); + + // share_tokens.created_by: target created a share token for otherUser's trip + testDb.prepare("INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, 'tok-auth-test', ?)").run(otherTrip.id, target.id); + + // budget_items.paid_by_user_id: target paid for an expense on otherUser's trip + const budgetItem = createBudgetItem(testDb, otherTrip.id); + testDb.prepare('UPDATE budget_items SET paid_by_user_id = ? WHERE id = ?').run(target.id, budgetItem.id); + + // journey_contributors: target is a contributor on otherUser's journey + const otherJourney = createJourney(testDb, otherUser.id); + addJourneyContributor(testDb, otherJourney.id, target.id); + + // journey_entries: target authored an entry on otherUser's journey + createJourneyEntry(testDb, otherJourney.id, target.id); + + // journey_share_tokens: target created a share token for otherUser's journey + testDb.prepare("INSERT INTO journey_share_tokens (journey_id, token, created_by) VALUES (?, 'jst-auth-test', ?)").run(otherJourney.id, target.id); + + // notifications.sender_id (SET NULL): target sent a notification to otherUser + const sentNotif = testDb.prepare( + "INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, text_key) VALUES ('simple', 'trip', ?, ?, ?, 'k', 'k')" + ).run(otherTrip.id, target.id, otherUser.id); + // notifications.recipient_id (CASCADE): otherUser sent a notification to target + testDb.prepare( + "INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, text_key) VALUES ('simple', 'trip', ?, ?, ?, 'k', 'k')" + ).run(otherTrip.id, otherUser.id, target.id); + + // user_notice_dismissals (CASCADE): target dismissed a notice + testDb.prepare( + "INSERT INTO user_notice_dismissals (user_id, notice_id, dismissed_at) VALUES (?, 'test-notice', ?)" + ).run(target.id, Date.now()); + + // admin exists to ensure target (non-admin user) passes the last-admin guard + void admin; + + const res = await request(app) + .delete('/api/auth/me') + .set('Cookie', authCookie(target.id)); + expect(res.status).toBe(200); + expect(res.body.success).toBe(true); + + expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(target.id)).toBeUndefined(); + // trip_members row survives but invited_by is now NULL + expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(otherTrip.id, thirdUser.id) as any).invited_by).toBeNull(); + expect(testDb.prepare('SELECT id FROM share_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + expect((testDb.prepare('SELECT paid_by_user_id FROM budget_items WHERE id = ?').get(budgetItem.id) as any).paid_by_user_id).toBeNull(); + expect(testDb.prepare('SELECT user_id FROM journey_contributors WHERE journey_id = ? AND user_id = ?').get(otherJourney.id, target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_entries WHERE author_id = ?').get(target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_share_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + // sent notification survives but sender_id becomes NULL + expect((testDb.prepare('SELECT sender_id FROM notifications WHERE id = ?').get(sentNotif.lastInsertRowid) as any).sender_id).toBeNull(); + // received notification is cascade-deleted + expect(testDb.prepare('SELECT id FROM notifications WHERE recipient_id = ?').get(target.id)).toBeUndefined(); + // notice dismissals are cascade-deleted + expect(testDb.prepare("SELECT user_id FROM user_notice_dismissals WHERE user_id = ? AND notice_id = 'test-notice'").get(target.id)).toBeUndefined(); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // Rate limiting (AUTH-004, AUTH-018) — placed last // ─────────────────────────────────────────────────────────────────────────────