From e7cb6de7d59e75d87ad62bc30c5789531bd88feb Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Fri, 27 Feb 2026 19:20:47 +0800 Subject: [PATCH] Add admin delete-user with full cascade cleanup Migration 036 adds ondelete rules to 5 transitive FKs that would otherwise block user deletion (calendar_events via calendars, project_tasks via projects, todos via projects, etc.). DELETE /api/admin/users/{user_id} with self-action guard, last-admin guard, session revocation, and audit logging. Frontend gets a red two-click confirm button in the IAM actions menu. Co-Authored-By: Claude Opus 4.6 --- .../036_add_cascade_to_transitive_fks.py | 104 ++++++++++++++++++ backend/app/models/calendar_event.py | 4 +- backend/app/models/project_task.py | 4 +- backend/app/models/todo.py | 2 +- backend/app/routers/admin.py | 60 ++++++++++ backend/app/schemas/admin.py | 5 + .../src/components/admin/UserActionsMenu.tsx | 26 ++++- frontend/src/hooks/useAdmin.ts | 7 ++ 8 files changed, 206 insertions(+), 6 deletions(-) create mode 100644 backend/alembic/versions/036_add_cascade_to_transitive_fks.py diff --git a/backend/alembic/versions/036_add_cascade_to_transitive_fks.py b/backend/alembic/versions/036_add_cascade_to_transitive_fks.py new file mode 100644 index 0000000..a0e6b22 --- /dev/null +++ b/backend/alembic/versions/036_add_cascade_to_transitive_fks.py @@ -0,0 +1,104 @@ +"""Add ondelete to transitive FK constraints. + +Without these, deleting a user would fail because the DB-level CASCADE +only reaches first-level children (calendars, projects, people, locations). +Second-level children (calendar_events via calendar_id, project_tasks via +project_id, etc.) need their own ondelete rules to allow the full cascade. + +FK changes: + calendar_events.calendar_id → CASCADE (events die with calendar) + calendar_events.location_id → SET NULL (optional ref, just unlink) + project_tasks.project_id → CASCADE (tasks die with project) + project_tasks.person_id → SET NULL (optional assignee, just unlink) + todos.project_id → SET NULL (optional ref, just unlink) + +Revision ID: 036 +Revises: 035 +Create Date: 2026-02-27 +""" +from alembic import op + +revision = "036" +down_revision = "035" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # calendar_events.calendar_id → CASCADE + op.drop_constraint( + "fk_calendar_events_calendar_id", "calendar_events", type_="foreignkey" + ) + op.create_foreign_key( + "fk_calendar_events_calendar_id", + "calendar_events", + "calendars", + ["calendar_id"], + ["id"], + ondelete="CASCADE", + ) + + # calendar_events.location_id → SET NULL + op.drop_constraint( + "calendar_events_location_id_fkey", "calendar_events", type_="foreignkey" + ) + op.create_foreign_key( + "calendar_events_location_id_fkey", + "calendar_events", + "locations", + ["location_id"], + ["id"], + ondelete="SET NULL", + ) + + # project_tasks.project_id → CASCADE + op.drop_constraint( + "project_tasks_project_id_fkey", "project_tasks", type_="foreignkey" + ) + op.create_foreign_key( + "project_tasks_project_id_fkey", + "project_tasks", + "projects", + ["project_id"], + ["id"], + ondelete="CASCADE", + ) + + # project_tasks.person_id → SET NULL + op.drop_constraint( + "project_tasks_person_id_fkey", "project_tasks", type_="foreignkey" + ) + op.create_foreign_key( + "project_tasks_person_id_fkey", + "project_tasks", + "people", + ["person_id"], + ["id"], + ondelete="SET NULL", + ) + + # todos.project_id → SET NULL + op.drop_constraint( + "todos_project_id_fkey", "todos", type_="foreignkey" + ) + op.create_foreign_key( + "todos_project_id_fkey", + "todos", + "projects", + ["project_id"], + ["id"], + ondelete="SET NULL", + ) + + +def downgrade() -> None: + # Reverse: remove ondelete by re-creating without it + for table, col, ref_table, constraint in [ + ("todos", "project_id", "projects", "todos_project_id_fkey"), + ("project_tasks", "person_id", "people", "project_tasks_person_id_fkey"), + ("project_tasks", "project_id", "projects", "project_tasks_project_id_fkey"), + ("calendar_events", "location_id", "locations", "calendar_events_location_id_fkey"), + ("calendar_events", "calendar_id", "calendars", "fk_calendar_events_calendar_id"), + ]: + op.drop_constraint(constraint, table, type_="foreignkey") + op.create_foreign_key(constraint, table, ref_table, [col], ["id"]) diff --git a/backend/app/models/calendar_event.py b/backend/app/models/calendar_event.py index 725d3eb..c85dfc7 100644 --- a/backend/app/models/calendar_event.py +++ b/backend/app/models/calendar_event.py @@ -15,10 +15,10 @@ class CalendarEvent(Base): end_datetime: Mapped[datetime] = mapped_column(nullable=False) all_day: Mapped[bool] = mapped_column(Boolean, default=False) color: Mapped[Optional[str]] = mapped_column(String(20), nullable=True) - location_id: Mapped[Optional[int]] = mapped_column(Integer, ForeignKey("locations.id"), nullable=True) + location_id: Mapped[Optional[int]] = mapped_column(Integer, ForeignKey("locations.id", ondelete="SET NULL"), nullable=True) recurrence_rule: Mapped[Optional[str]] = mapped_column(String(255), nullable=True) is_starred: Mapped[bool] = mapped_column(Boolean, default=False, server_default="false") - calendar_id: Mapped[int] = mapped_column(Integer, ForeignKey("calendars.id"), nullable=False) + calendar_id: Mapped[int] = mapped_column(Integer, ForeignKey("calendars.id", ondelete="CASCADE"), nullable=False) # Recurrence fields # parent_event_id: set on child events; NULL on the parent template row diff --git a/backend/app/models/project_task.py b/backend/app/models/project_task.py index 6d52be5..94a41d1 100644 --- a/backend/app/models/project_task.py +++ b/backend/app/models/project_task.py @@ -9,7 +9,7 @@ class ProjectTask(Base): __tablename__ = "project_tasks" id: Mapped[int] = mapped_column(primary_key=True, index=True) - project_id: Mapped[int] = mapped_column(Integer, ForeignKey("projects.id"), nullable=False) + project_id: Mapped[int] = mapped_column(Integer, ForeignKey("projects.id", ondelete="CASCADE"), nullable=False) parent_task_id: Mapped[Optional[int]] = mapped_column( Integer, ForeignKey("project_tasks.id", ondelete="CASCADE"), nullable=True ) @@ -18,7 +18,7 @@ class ProjectTask(Base): status: Mapped[str] = mapped_column(String(20), default="pending") priority: Mapped[str] = mapped_column(String(20), default="medium") due_date: Mapped[Optional[date]] = mapped_column(Date, nullable=True) - person_id: Mapped[Optional[int]] = mapped_column(Integer, ForeignKey("people.id"), nullable=True) + person_id: Mapped[Optional[int]] = mapped_column(Integer, ForeignKey("people.id", ondelete="SET NULL"), nullable=True) sort_order: Mapped[int] = mapped_column(Integer, default=0) created_at: Mapped[datetime] = mapped_column(default=func.now()) updated_at: Mapped[datetime] = mapped_column(default=func.now(), onupdate=func.now()) diff --git a/backend/app/models/todo.py b/backend/app/models/todo.py index 23850ed..8934179 100644 --- a/backend/app/models/todo.py +++ b/backend/app/models/todo.py @@ -23,7 +23,7 @@ class Todo(Base): recurrence_rule: Mapped[Optional[str]] = mapped_column(String(255), nullable=True) reset_at: Mapped[Optional[datetime]] = mapped_column(nullable=True, index=True) next_due_date: Mapped[Optional[date]] = mapped_column(Date, nullable=True) - project_id: Mapped[Optional[int]] = mapped_column(Integer, ForeignKey("projects.id"), nullable=True) + project_id: Mapped[Optional[int]] = mapped_column(Integer, ForeignKey("projects.id", ondelete="SET NULL"), nullable=True) created_at: Mapped[datetime] = mapped_column(default=func.now(), server_default=func.now()) updated_at: Mapped[datetime] = mapped_column(default=func.now(), onupdate=func.now(), server_default=func.now()) diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py index 4e29e01..963fe4a 100644 --- a/backend/app/routers/admin.py +++ b/backend/app/routers/admin.py @@ -34,6 +34,7 @@ from app.schemas.admin import ( AuditLogEntry, AuditLogResponse, CreateUserRequest, + DeleteUserResponse, ResetPasswordResponse, SystemConfigResponse, SystemConfigUpdate, @@ -459,6 +460,65 @@ async def revoke_user_sessions( return {"message": f"{revoked} session(s) revoked."} +# --------------------------------------------------------------------------- +# DELETE /users/{user_id} — hard delete user + all data +# --------------------------------------------------------------------------- + +@router.delete("/users/{user_id}", response_model=DeleteUserResponse) +async def delete_user( + user_id: int = Path(ge=1, le=2147483647), + request: Request = ..., + db: AsyncSession = Depends(get_db), + actor: User = Depends(get_current_user), +): + """ + Permanently delete a user and all their data. + DB CASCADE rules handle child row cleanup. + """ + _guard_self_action(actor, user_id, "delete") + + result = await db.execute(sa.select(User).where(User.id == user_id)) + target = result.scalar_one_or_none() + if not target: + raise HTTPException(status_code=404, detail="User not found") + + # Prevent deleting the last admin + if target.role == "admin": + admin_count = await db.scalar( + sa.select(sa.func.count()).select_from(User).where(User.role == "admin") + ) + if admin_count <= 1: + raise HTTPException( + status_code=409, + detail="Cannot delete the last admin account", + ) + + deleted_username = target.username + + # Belt-and-suspenders: explicitly revoke sessions before delete + await _revoke_all_sessions(db, user_id) + + await log_audit_event( + db, + action="admin.user_deleted", + actor_id=actor.id, + target_id=user_id, + detail={"user_id": user_id, "username": deleted_username}, + ip=request.client.host if request.client else None, + ) + # Flush audit + session revocation within the same transaction + await db.flush() + + # DB CASCADE handles all child rows; SET NULL fires on audit_log.target_user_id + await db.delete(target) + await db.commit() + + return DeleteUserResponse( + message=f"User '{deleted_username}' permanently deleted.", + deleted_username=deleted_username, + ) + + # --------------------------------------------------------------------------- # GET /users/{user_id}/sessions # --------------------------------------------------------------------------- diff --git a/backend/app/schemas/admin.py b/backend/app/schemas/admin.py index e1af013..ea03df2 100644 --- a/backend/app/schemas/admin.py +++ b/backend/app/schemas/admin.py @@ -135,6 +135,11 @@ class ResetPasswordResponse(BaseModel): temporary_password: str +class DeleteUserResponse(BaseModel): + message: str + deleted_username: str + + # --------------------------------------------------------------------------- # Audit log # --------------------------------------------------------------------------- diff --git a/frontend/src/components/admin/UserActionsMenu.tsx b/frontend/src/components/admin/UserActionsMenu.tsx index edc65f9..c2d8c43 100644 --- a/frontend/src/components/admin/UserActionsMenu.tsx +++ b/frontend/src/components/admin/UserActionsMenu.tsx @@ -10,6 +10,7 @@ import { Smartphone, ChevronRight, Loader2, + Trash2, } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { useConfirmAction } from '@/hooks/useConfirmAction'; @@ -21,6 +22,7 @@ import { useRemoveMfaEnforcement, useToggleUserActive, useRevokeSessions, + useDeleteUser, getErrorMessage, } from '@/hooks/useAdmin'; import type { AdminUserDetail, UserRole } from '@/types'; @@ -49,6 +51,7 @@ export default function UserActionsMenu({ user }: UserActionsMenuProps) { const removeMfaEnforcement = useRemoveMfaEnforcement(); const toggleActive = useToggleUserActive(); const revokeSessions = useRevokeSessions(); + const deleteUser = useDeleteUser(); // Close on outside click useEffect(() => { @@ -88,6 +91,10 @@ export default function UserActionsMenu({ user }: UserActionsMenuProps) { handleAction(() => revokeSessions.mutateAsync(user.id), 'Sessions revoked'); }); + const deleteUserConfirm = useConfirmAction(() => { + handleAction(() => deleteUser.mutateAsync(user.id), 'User permanently deleted'); + }); + const isLoading = updateRole.isPending || resetPassword.isPending || @@ -95,7 +102,8 @@ export default function UserActionsMenu({ user }: UserActionsMenuProps) { enforceMfa.isPending || removeMfaEnforcement.isPending || toggleActive.isPending || - revokeSessions.isPending; + revokeSessions.isPending || + deleteUser.isPending; return (
@@ -274,6 +282,22 @@ export default function UserActionsMenu({ user }: UserActionsMenuProps) { {revokeSessionsConfirm.confirming ? 'Sure? Click to confirm' : 'Revoke All Sessions'} + +
+ + {/* Delete User — destructive, red two-click confirm */} +
)}
diff --git a/frontend/src/hooks/useAdmin.ts b/frontend/src/hooks/useAdmin.ts index 5d73c33..58d2521 100644 --- a/frontend/src/hooks/useAdmin.ts +++ b/frontend/src/hooks/useAdmin.ts @@ -156,6 +156,13 @@ export function useRevokeSessions() { }); } +export function useDeleteUser() { + return useAdminMutation(async (userId: number) => { + const { data } = await api.delete(`/admin/users/${userId}`); + return data; + }); +} + export function useUpdateConfig() { return useAdminMutation(async (config: Partial) => { const { data } = await api.put('/admin/config', config);