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 <noreply@anthropic.com>
This commit is contained in:
parent
c56830ddb0
commit
e7cb6de7d5
104
backend/alembic/versions/036_add_cascade_to_transitive_fks.py
Normal file
104
backend/alembic/versions/036_add_cascade_to_transitive_fks.py
Normal file
@ -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"])
|
||||||
@ -15,10 +15,10 @@ class CalendarEvent(Base):
|
|||||||
end_datetime: Mapped[datetime] = mapped_column(nullable=False)
|
end_datetime: Mapped[datetime] = mapped_column(nullable=False)
|
||||||
all_day: Mapped[bool] = mapped_column(Boolean, default=False)
|
all_day: Mapped[bool] = mapped_column(Boolean, default=False)
|
||||||
color: Mapped[Optional[str]] = mapped_column(String(20), nullable=True)
|
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)
|
recurrence_rule: Mapped[Optional[str]] = mapped_column(String(255), nullable=True)
|
||||||
is_starred: Mapped[bool] = mapped_column(Boolean, default=False, server_default="false")
|
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
|
# Recurrence fields
|
||||||
# parent_event_id: set on child events; NULL on the parent template row
|
# parent_event_id: set on child events; NULL on the parent template row
|
||||||
|
|||||||
@ -9,7 +9,7 @@ class ProjectTask(Base):
|
|||||||
__tablename__ = "project_tasks"
|
__tablename__ = "project_tasks"
|
||||||
|
|
||||||
id: Mapped[int] = mapped_column(primary_key=True, index=True)
|
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(
|
parent_task_id: Mapped[Optional[int]] = mapped_column(
|
||||||
Integer, ForeignKey("project_tasks.id", ondelete="CASCADE"), nullable=True
|
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")
|
status: Mapped[str] = mapped_column(String(20), default="pending")
|
||||||
priority: Mapped[str] = mapped_column(String(20), default="medium")
|
priority: Mapped[str] = mapped_column(String(20), default="medium")
|
||||||
due_date: Mapped[Optional[date]] = mapped_column(Date, nullable=True)
|
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)
|
sort_order: Mapped[int] = mapped_column(Integer, default=0)
|
||||||
created_at: Mapped[datetime] = mapped_column(default=func.now())
|
created_at: Mapped[datetime] = mapped_column(default=func.now())
|
||||||
updated_at: Mapped[datetime] = mapped_column(default=func.now(), onupdate=func.now())
|
updated_at: Mapped[datetime] = mapped_column(default=func.now(), onupdate=func.now())
|
||||||
|
|||||||
@ -23,7 +23,7 @@ class Todo(Base):
|
|||||||
recurrence_rule: Mapped[Optional[str]] = mapped_column(String(255), nullable=True)
|
recurrence_rule: Mapped[Optional[str]] = mapped_column(String(255), nullable=True)
|
||||||
reset_at: Mapped[Optional[datetime]] = mapped_column(nullable=True, index=True)
|
reset_at: Mapped[Optional[datetime]] = mapped_column(nullable=True, index=True)
|
||||||
next_due_date: Mapped[Optional[date]] = mapped_column(Date, nullable=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())
|
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())
|
updated_at: Mapped[datetime] = mapped_column(default=func.now(), onupdate=func.now(), server_default=func.now())
|
||||||
|
|
||||||
|
|||||||
@ -34,6 +34,7 @@ from app.schemas.admin import (
|
|||||||
AuditLogEntry,
|
AuditLogEntry,
|
||||||
AuditLogResponse,
|
AuditLogResponse,
|
||||||
CreateUserRequest,
|
CreateUserRequest,
|
||||||
|
DeleteUserResponse,
|
||||||
ResetPasswordResponse,
|
ResetPasswordResponse,
|
||||||
SystemConfigResponse,
|
SystemConfigResponse,
|
||||||
SystemConfigUpdate,
|
SystemConfigUpdate,
|
||||||
@ -459,6 +460,65 @@ async def revoke_user_sessions(
|
|||||||
return {"message": f"{revoked} session(s) revoked."}
|
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
|
# GET /users/{user_id}/sessions
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@ -135,6 +135,11 @@ class ResetPasswordResponse(BaseModel):
|
|||||||
temporary_password: str
|
temporary_password: str
|
||||||
|
|
||||||
|
|
||||||
|
class DeleteUserResponse(BaseModel):
|
||||||
|
message: str
|
||||||
|
deleted_username: str
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Audit log
|
# Audit log
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@ -10,6 +10,7 @@ import {
|
|||||||
Smartphone,
|
Smartphone,
|
||||||
ChevronRight,
|
ChevronRight,
|
||||||
Loader2,
|
Loader2,
|
||||||
|
Trash2,
|
||||||
} from 'lucide-react';
|
} from 'lucide-react';
|
||||||
import { Button } from '@/components/ui/button';
|
import { Button } from '@/components/ui/button';
|
||||||
import { useConfirmAction } from '@/hooks/useConfirmAction';
|
import { useConfirmAction } from '@/hooks/useConfirmAction';
|
||||||
@ -21,6 +22,7 @@ import {
|
|||||||
useRemoveMfaEnforcement,
|
useRemoveMfaEnforcement,
|
||||||
useToggleUserActive,
|
useToggleUserActive,
|
||||||
useRevokeSessions,
|
useRevokeSessions,
|
||||||
|
useDeleteUser,
|
||||||
getErrorMessage,
|
getErrorMessage,
|
||||||
} from '@/hooks/useAdmin';
|
} from '@/hooks/useAdmin';
|
||||||
import type { AdminUserDetail, UserRole } from '@/types';
|
import type { AdminUserDetail, UserRole } from '@/types';
|
||||||
@ -49,6 +51,7 @@ export default function UserActionsMenu({ user }: UserActionsMenuProps) {
|
|||||||
const removeMfaEnforcement = useRemoveMfaEnforcement();
|
const removeMfaEnforcement = useRemoveMfaEnforcement();
|
||||||
const toggleActive = useToggleUserActive();
|
const toggleActive = useToggleUserActive();
|
||||||
const revokeSessions = useRevokeSessions();
|
const revokeSessions = useRevokeSessions();
|
||||||
|
const deleteUser = useDeleteUser();
|
||||||
|
|
||||||
// Close on outside click
|
// Close on outside click
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
@ -88,6 +91,10 @@ export default function UserActionsMenu({ user }: UserActionsMenuProps) {
|
|||||||
handleAction(() => revokeSessions.mutateAsync(user.id), 'Sessions revoked');
|
handleAction(() => revokeSessions.mutateAsync(user.id), 'Sessions revoked');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const deleteUserConfirm = useConfirmAction(() => {
|
||||||
|
handleAction(() => deleteUser.mutateAsync(user.id), 'User permanently deleted');
|
||||||
|
});
|
||||||
|
|
||||||
const isLoading =
|
const isLoading =
|
||||||
updateRole.isPending ||
|
updateRole.isPending ||
|
||||||
resetPassword.isPending ||
|
resetPassword.isPending ||
|
||||||
@ -95,7 +102,8 @@ export default function UserActionsMenu({ user }: UserActionsMenuProps) {
|
|||||||
enforceMfa.isPending ||
|
enforceMfa.isPending ||
|
||||||
removeMfaEnforcement.isPending ||
|
removeMfaEnforcement.isPending ||
|
||||||
toggleActive.isPending ||
|
toggleActive.isPending ||
|
||||||
revokeSessions.isPending;
|
revokeSessions.isPending ||
|
||||||
|
deleteUser.isPending;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div ref={menuRef} className="relative">
|
<div ref={menuRef} className="relative">
|
||||||
@ -274,6 +282,22 @@ export default function UserActionsMenu({ user }: UserActionsMenuProps) {
|
|||||||
<LogOut className="h-4 w-4" />
|
<LogOut className="h-4 w-4" />
|
||||||
{revokeSessionsConfirm.confirming ? 'Sure? Click to confirm' : 'Revoke All Sessions'}
|
{revokeSessionsConfirm.confirming ? 'Sure? Click to confirm' : 'Revoke All Sessions'}
|
||||||
</button>
|
</button>
|
||||||
|
|
||||||
|
<div className="my-1 border-t border-border" />
|
||||||
|
|
||||||
|
{/* Delete User — destructive, red two-click confirm */}
|
||||||
|
<button
|
||||||
|
className={cn(
|
||||||
|
'flex w-full items-center gap-2 px-3 py-2 text-sm transition-colors',
|
||||||
|
deleteUserConfirm.confirming
|
||||||
|
? 'text-red-400 bg-red-500/10 hover:bg-red-500/15'
|
||||||
|
: 'text-red-400 hover:bg-card-elevated'
|
||||||
|
)}
|
||||||
|
onClick={deleteUserConfirm.handleClick}
|
||||||
|
>
|
||||||
|
<Trash2 className="h-4 w-4" />
|
||||||
|
{deleteUserConfirm.confirming ? 'Sure? This is permanent' : 'Delete User'}
|
||||||
|
</button>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@ -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() {
|
export function useUpdateConfig() {
|
||||||
return useAdminMutation(async (config: Partial<SystemConfig>) => {
|
return useAdminMutation(async (config: Partial<SystemConfig>) => {
|
||||||
const { data } = await api.put('/admin/config', config);
|
const { data } = await api.put('/admin/config', config);
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user