chore: merge executor worktree (05-11 admin hard-delete)

2026-05-30 11:40:40 +02:00
parent bd3b637d30 3180e759de
commit 02ef11c432
5 changed files with 264 additions and 1 deletions
@@ -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
@@ -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")
@@ -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
@@ -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) {
@@ -171,6 +171,43 @@
                </div>
              </div>

+              <!-- Inline delete confirmation panel -->
+              <div v-else-if="confirmDelete === user.id" class="space-y-2">
+                <p class="text-xs text-red-700 font-semibold">
+                  Permanently delete <span class="font-bold">{{ user.email }}</span>?
+                  This will erase all their documents, cloud connections, and quota data. This cannot be undone.
+                </p>
+                <div>
+                  <label class="block text-xs text-gray-700 mb-1 font-semibold">Your admin password to confirm</label>
+                  <input
+                    v-model="deletePassword"
+                    type="password"
+                    autocomplete="current-password"
+                    placeholder="Admin password"
+                    class="block w-full rounded-lg px-2 py-1.5 text-xs border border-red-300 focus:outline-none focus:ring-2 focus:ring-red-500 focus:border-red-500 transition-colors"
+                    @keydown.enter.prevent="confirmDoDelete(user.id)"
+                  />
+                </div>
+                <p v-if="deleteError" class="text-xs text-red-600">{{ deleteError }}</p>
+                <div class="flex items-center gap-2">
+                  <button
+                    @click="confirmDoDelete(user.id)"
+                    :disabled="pendingAction[user.id] || !deletePassword"
+                    class="text-red-700 hover:text-red-800 text-sm font-semibold disabled:opacity-50"
+                  >
+                    <span v-if="pendingAction[user.id]" class="flex items-center gap-1">
+                      <span class="animate-spin rounded-full border-2 border-current border-t-transparent w-3 h-3"></span>
+                      Deleting…
+                    </span>
+                    <span v-else>Delete permanently</span>
+                  </button>
+                  <span class="text-gray-300">·</span>
+                  <button @click="cancelDelete" class="text-gray-500 hover:text-gray-700 text-sm">
+                    Cancel
+                  </button>
+                </div>
+              </div>
+
              <!-- Normal actions -->
              <div v-else class="flex items-center gap-2">
                <span v-if="pendingAction[user.id]" class="flex items-center gap-1 text-gray-400 text-sm">
@@ -191,6 +228,13 @@
                  >
                    Deactivate
                  </button>
+                  <span class="text-gray-300">·</span>
+                  <button
+                    @click="startDelete(user.id)"
+                    class="text-red-800 hover:text-red-900 text-sm font-semibold"
+                  >
+                    Delete
+                  </button>
                </template>

                <template v-else>
@@ -200,6 +244,13 @@
                  >
                    Reactivate
                  </button>
+                  <span class="text-gray-300">·</span>
+                  <button
+                    @click="startDelete(user.id)"
+                    class="text-red-800 hover:text-red-900 text-sm font-semibold"
+                  >
+                    Delete
+                  </button>
                </template>
              </div>
            </td>
@@ -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) {