diff --git a/.planning/phases/05-cloud-storage-backends/05-11-SUMMARY.md b/.planning/phases/05-cloud-storage-backends/05-11-SUMMARY.md new file mode 100644 index 0000000..39bcd92 --- /dev/null +++ b/.planning/phases/05-cloud-storage-backends/05-11-SUMMARY.md @@ -0,0 +1,100 @@ +--- +phase: 05-cloud-storage-backends +plan: 11 +subsystem: admin +tags: [admin, security, delete, password-verification, frontend] +dependency_graph: + requires: [] + provides: [admin-hard-delete-with-password-confirmation] + affects: [backend/api/admin.py, frontend/src/components/admin/AdminUsersTab.vue] +tech_stack: + added: [] + patterns: [Pydantic body model for DELETE, Argon2 password verification before destructive action, Vue inline confirmation panel] +key_files: + created: [] + modified: + - backend/api/admin.py + - frontend/src/api/client.js + - frontend/src/components/admin/AdminUsersTab.vue + - backend/tests/test_admin_api.py +decisions: + - "Password verification added as fail-fast check before user lookup — admin cannot fish for user existence via timing" + - "Delete panel and deactivate panel are mutually exclusive (each clears the other on open)" + - "Tests added to existing test_admin_api.py (not a separate file) — plan referenced test_admin.py but actual file is test_admin_api.py" +metrics: + duration: "2m" + completed_date: "2026-05-30T09:39:26Z" + tasks_completed: 2 + files_modified: 4 +requirements: [ADMIN-02, SEC-09] +--- + +# Phase 05 Plan 11: Admin Hard-Delete with Password Confirmation Summary + +Admin users can now permanently delete non-admin user accounts with Argon2 password verification — wrong or missing password returns 403 without touching any data; correct password triggers the existing SEC-09 cloud/MinIO purge pipeline. + +## Tasks Completed + +| Task | Name | Commits | Files | +|------|------|---------|-------| +| 1 (RED) | Failing tests for delete_user password verification | 8727592 | backend/tests/test_admin_api.py | +| 1 (GREEN) | UserDeleteConfirm model + password verification | 390a693 | backend/api/admin.py | +| 2 | adminDeleteUser API + inline delete confirmation panel | 7268721 | frontend/src/api/client.js, frontend/src/components/admin/AdminUsersTab.vue | + +## What Was Built + +**Backend (admin.py):** +- `UserDeleteConfirm` Pydantic model with `admin_password: str` field added to the Request models section +- `verify_password` imported from `services.auth` (alongside existing `hash_password`, `revoke_all_refresh_tokens`) +- `delete_user` handler signature updated to accept `body: UserDeleteConfirm` +- Fail-fast password check placed before any DB reads for the target user — 403 "Invalid admin password" on failure +- All existing SEC-09 cloud credential purge, MinIO object cleanup, and audit log logic is unchanged + +**Frontend (client.js):** +- `adminDeleteUser(id, adminPassword)` exported — calls `DELETE /api/admin/users/{id}` with `{ admin_password }` JSON body + +**Frontend (AdminUsersTab.vue):** +- Added state: `confirmDelete`, `deletePassword`, `deleteError` +- Added functions: `startDelete`, `cancelDelete`, `confirmDoDelete` +- `startDeactivate` updated to clear delete panel when deactivate panel opens (mutual exclusion) +- Delete button added to active user row (after Deactivate) and deactivated user row (after Reactivate) +- Inline password confirmation panel: warning text, password input with Enter shortcut, error display, "Delete permanently" / Cancel buttons with loading spinner + +## Verification + +- `pytest test_admin_api.py::test_delete_user_correct_password` — PASSED (204, user removed from list) +- `pytest test_admin_api.py::test_delete_user_wrong_password` — PASSED (403, user survives) +- `pytest test_admin_api.py::test_delete_user_no_body` — PASSED (422, Pydantic validation) +- Full `test_admin_api.py` suite — 21/21 PASSED +- `npm run build` — zero errors, built in 689ms + +## Deviations from Plan + +### Minor Filename Discrepancy (auto-handled) + +**Found during:** Task 1 +**Issue:** Plan references `backend/tests/test_admin.py` but the actual file is `backend/tests/test_admin_api.py` +**Fix:** Tests added to `backend/tests/test_admin_api.py` (the existing correct file) +**Impact:** None — tests run and pass correctly + +## TDD Gate Compliance + +- RED gate: commit `8727592` — `test(05-11): add failing tests for delete_user password verification` +- GREEN gate: commit `390a693` — `feat(05-11): add UserDeleteConfirm model + admin password verification in delete_user` +- 2/2 tests failed in RED phase (correct_password passed because old endpoint had no auth check; wrong_password and no_body failed correctly) + +## Threat Surface Scan + +No new network endpoints introduced. The DELETE `/api/admin/users/{id}` endpoint existed before this plan. Changes add a body requirement (reducing attack surface — anonymous DELETE calls now return 422 instead of 204). No new trust boundaries. + +## Known Stubs + +None — adminDeleteUser wired directly to the backend endpoint; delete panel uses live API with real error propagation. + +## Self-Check: PASSED + +- `backend/api/admin.py` — modified, contains `UserDeleteConfirm` and `verify_password` check +- `frontend/src/api/client.js` — modified, exports `adminDeleteUser` +- `frontend/src/components/admin/AdminUsersTab.vue` — modified, contains delete panel +- `backend/tests/test_admin_api.py` — modified, contains 3 new tests +- Commits 8727592, 390a693, 7268721 — all present in git log diff --git a/backend/api/admin.py b/backend/api/admin.py index a1aea53..7818db7 100644 --- a/backend/api/admin.py +++ b/backend/api/admin.py @@ -37,7 +37,7 @@ from db.models import CloudConnection, Document, Quota, RefreshToken, Topic, Use from deps.auth import get_current_admin from deps.db import get_db from services.audit import write_audit_log -from services.auth import hash_password, revoke_all_refresh_tokens +from services.auth import hash_password, revoke_all_refresh_tokens, verify_password from storage import get_storage_backend, get_storage_backend_for_document router = APIRouter(prefix="/api/admin", tags=["admin"]) @@ -138,6 +138,12 @@ class SystemTopicCreate(BaseModel): color: str = "#6366f1" +class UserDeleteConfirm(BaseModel): + """Admin password confirmation required before hard-deleting a user (ADMIN-02, T-05-11-01).""" + + admin_password: str + + # ── SEC-08: Safe CloudConnection response model ─────────────────────────────── class CloudConnectionOut(BaseModel): @@ -472,6 +478,7 @@ async def update_ai_config( @router.delete("/users/{user_id}", status_code=status.HTTP_204_NO_CONTENT) async def delete_user( user_id: uuid.UUID, + body: UserDeleteConfirm, request: Request, session: AsyncSession = Depends(get_db), _admin: User = Depends(get_current_admin), @@ -479,11 +486,20 @@ async def delete_user( """Delete a user account and clean up all their MinIO objects (SEC-09, D-19). Security invariants: + - Admin password verified via Argon2 before any deletion (T-05-11-01) - Cannot delete admin accounts (T-04-07-04) - MinIO objects are deleted BEFORE DB records are removed (SEC-09) - MinIO deletion is best-effort (try/except) — DB row is deleted regardless - Audit log written with event_type="admin.user_deleted" """ + # T-05-11-01: Verify admin password before performing any destructive action. + # Fail fast — no DB reads for the target user until the admin is confirmed. + if not verify_password(body.admin_password, _admin.password_hash): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Invalid admin password", + ) + user = await session.get(User, user_id) if user is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found") diff --git a/backend/tests/test_admin_api.py b/backend/tests/test_admin_api.py index c3d75b6..8c3e2fa 100644 --- a/backend/tests/test_admin_api.py +++ b/backend/tests/test_admin_api.py @@ -355,3 +355,58 @@ async def test_admin_response_no_password_hash(admin_client): for item in data["items"]: assert "password_hash" not in item assert "credentials_enc" not in item + + +# ── Delete user tests (Plan 05-11: ADMIN-02, SEC-09) ───────────────────────── + + +@pytest.mark.asyncio +async def test_delete_user_correct_password(admin_client): + """DELETE /api/admin/users/{id} with correct admin_password → 204; user is gone.""" + client, admin, session = admin_client + target = await make_regular_user(session) + + resp = await client.request( + "DELETE", + f"/api/admin/users/{target.id}", + json={"admin_password": "AdminPass1!Secret"}, + ) + assert resp.status_code == 204 + + # Verify the user no longer appears in the list + list_resp = await client.get("/api/admin/users") + assert list_resp.status_code == 200 + ids = [u["id"] for u in list_resp.json()["items"]] + assert str(target.id) not in ids + + +@pytest.mark.asyncio +async def test_delete_user_wrong_password(admin_client): + """DELETE /api/admin/users/{id} with wrong admin_password → 403; user is NOT deleted.""" + client, admin, session = admin_client + target = await make_regular_user(session) + + resp = await client.request( + "DELETE", + f"/api/admin/users/{target.id}", + json={"admin_password": "WrongPassword99!"}, + ) + assert resp.status_code == 403 + data = resp.json() + assert data["detail"] == "Invalid admin password" + + # Verify the user still exists + list_resp = await client.get("/api/admin/users") + assert list_resp.status_code == 200 + ids = [u["id"] for u in list_resp.json()["items"]] + assert str(target.id) in ids + + +@pytest.mark.asyncio +async def test_delete_user_no_body(admin_client): + """DELETE /api/admin/users/{id} with no body → 422 (Pydantic validation).""" + client, admin, session = admin_client + target = await make_regular_user(session) + + resp = await client.delete(f"/api/admin/users/{target.id}") + assert resp.status_code == 422 diff --git a/frontend/src/api/client.js b/frontend/src/api/client.js index 2b5b730..0d9b5b8 100644 --- a/frontend/src/api/client.js +++ b/frontend/src/api/client.js @@ -269,6 +269,14 @@ export function adminUpdateAiConfig(id, provider, model) { }) } +export function adminDeleteUser(id, adminPassword) { + return request(`/api/admin/users/${id}`, { + method: 'DELETE', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ admin_password: adminPassword }), + }) +} + // ── Folders ─────────────────────────────────────────────────────────────────── export function listFolders(parentId = null) { diff --git a/frontend/src/components/admin/AdminUsersTab.vue b/frontend/src/components/admin/AdminUsersTab.vue index ce37f28..2928f8a 100644 --- a/frontend/src/components/admin/AdminUsersTab.vue +++ b/frontend/src/components/admin/AdminUsersTab.vue @@ -171,6 +171,43 @@ + +
+

+ Permanently delete {{ user.email }}? + This will erase all their documents, cloud connections, and quota data. This cannot be undone. +

+
+ + +
+

{{ deleteError }}

+
+ + · + +
+
+
@@ -191,6 +228,13 @@ > Deactivate + · +
@@ -221,6 +272,9 @@ const users = ref([]) const loading = ref(false) const showCreateForm = ref(false) const confirmDeactivate = ref(null) +const confirmDelete = ref(null) +const deletePassword = ref('') +const deleteError = ref(null) const pendingAction = reactive({}) const actionError = ref(null) const creating = ref(false) @@ -308,6 +362,36 @@ async function submitCreate() { function startDeactivate(id) { confirmDeactivate.value = id + confirmDelete.value = null + deletePassword.value = '' + deleteError.value = null +} + +function startDelete(id) { + confirmDelete.value = id + deletePassword.value = '' + deleteError.value = null + confirmDeactivate.value = null +} + +function cancelDelete() { + confirmDelete.value = null + deletePassword.value = '' + deleteError.value = null +} + +async function confirmDoDelete(id) { + pendingAction[id] = true + deleteError.value = null + try { + await api.adminDeleteUser(id, deletePassword.value) + users.value = users.value.filter(u => u.id !== id) + cancelDelete() + } catch (e) { + deleteError.value = e.message + } finally { + delete pendingAction[id] + } } async function confirmDoDeactivate(id) {