docs(06.2): capture phase context + fix admin user creation 500

- Phase 6.2 CONTEXT.md: cloud-delete propagation, SHARE-03/05, audit log CSV export fix, daily export UI, user handle display - Fix: admin create_user missing session.flush() before write_audit_log caused FK violation on PostgreSQL (silent on SQLite) - Regression test: test_create_user_writes_audit_log in test_admin_api.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-31 11:00:45 +02:00
parent 3825f670a1
commit 7be48266ae
6 changed files with 252 additions and 1 deletions
@@ -318,6 +318,26 @@ Before any phase is marked complete, all three gates must pass:
 ---
 ### Phase 6.2: Close v1 sharing + cloud-delete + CSV export gaps
 **Goal**: Close remaining v1 gaps — sharing edge cases, cloud document deletion propagation to the remote backend, and CSV export for the admin audit log.
 **Mode:** mvp
 **Depends on**: Phase 6.1
 **Requirements**: TBD
 **Success Criteria** (what must be TRUE):
 1. TBD
 **Plans**: TBD
 **Phase gates (must pass before Phase 6.2 is complete):**
 - [ ] `pytest -v` — zero failures
 - [ ] Security agent: bandit + pip audit + npm audit all clean
 ---
 ## Progress Table
 | Phase | Plans Complete | Status | Completed |
@@ -329,3 +349,4 @@ Before any phase is marked complete, all three gates must pass:
 | 5. Cloud Storage Backends | 12/12 | Complete | 2026-05-30 |
 | 6. Performance & Production Hardening | 0/TBD | Not started | — |
 | 6.1. Close v1.0 audit gaps | 2/2 | Complete | 2026-05-30 |
 | 6.2. Close v1 sharing + cloud-delete + CSV export gaps | 0/TBD | Not started | — |
@@ -31,6 +31,7 @@ progress:
 | 5 | Cloud Storage Backends | ✓ Complete (12/12 plans, UAT 5/6 passed, 3 gaps closed by 05-12) |
 | 6 | Performance & Production Hardening | Not started |
 | 6.1 | Close v1.0 audit gaps: SHARE-02/STORE-06/ADMIN-06 | ✓ Complete (2/2 plans) |
 | 6.2 | Close v1 sharing + cloud-delete + CSV export gaps | Not started |
 ## Current Position
@@ -141,6 +142,7 @@ progress:
 - Phase 6 added: Performance & Production Hardening (2026-05-30)
 - Phase 6.1 inserted: Close v1.0 audit gaps — SHARE-02/STORE-06/ADMIN-06 (2026-05-30)
 - Phase 6.2 inserted: Close v1 sharing + cloud-delete + CSV export gaps (2026-05-31)
 ### Open Questions
@@ -0,0 +1,148 @@
 # Phase 6.2: Close v1 sharing + cloud-delete + CSV export gaps - Context
 **Gathered:** 2026-05-31
 **Status:** Ready for planning
 <domain>
 ## Phase Boundary
 Close three categories of v1 gaps discovered during manual UAT:
 1. **Admin user creation 500** (fixed during discussion — regression test added): `create_user` called `write_audit_log` before flushing the new User to the DB, causing a FK violation on PostgreSQL. Fix: `await session.flush()` added before `write_audit_log` in `backend/api/admin.py`.
 2. **Sharing**: SHARE-05 "shared" badge mismatch (frontend checks `doc.share_count` but backend sends `doc.is_shared`); SHARE-03 permission level control missing (backend hardcodes `permission="view"`; no UI or PATCH endpoint to change it).
 3. **Cloud-delete**: `delete_document()` in `services/storage.py` always calls MinIO `delete_object()` regardless of `doc.storage_backend`. Cloud-stored documents (Google Drive, OneDrive, Nextcloud, WebDAV) are removed from the DB but the file is never deleted from the provider.
 4. **Audit log / CSV export**: Export button uses `window.location.href` which cannot carry the in-memory access token → 401. Audit log table shows raw UUIDs instead of user handles. User filter accepts any string but backend expects a UUID (422 silently swallowed → empty results). Daily Celery exports land in MinIO with no UI to list or download them.
 No new user-facing features. All changes close existing v1 requirement gaps.
 </domain>
 <decisions>
 ## Implementation Decisions
 ### Bug Fixed During Discussion (pre-phase)
 - **D-00:** `backend/api/admin.py` `create_user` — added `await session.flush()` after `session.add(quota)` and before `write_audit_log`. Mirrors `auth.py:177` pattern. Regression test `test_create_user_writes_audit_log` added to `tests/test_admin_api.py`. This fix is already committed.
 ### Cloud-Delete Propagation
 - **D-01:** Default delete (existing delete button) propagates to the cloud provider. `delete_document()` in `services/storage.py` must check `doc.storage_backend` and, for cloud docs, call `get_storage_backend_for_document(doc)` to get the correct cloud backend, then call `cloud_backend.delete_object(doc.object_key)`.
 - **D-02:** "Remove from app" is a separate, distinct action — removes the DocuVault DB record only; the file on the cloud provider is preserved. This requires a new API endpoint (e.g., `DELETE /api/documents/{id}?remove_only=true` or a separate `POST /api/documents/{id}/remove-local`). Claude decides the cleanest route.
 - **D-03:** Cloud provider delete failure handling: show a warning modal to the user ("Cloud delete failed. Remove from app anyway?"). User chooses. If user confirms removal, delete the DB record without deleting the cloud file (best-effort). The delete endpoint must return a structured error response that the frontend can distinguish from a hard failure.
 - **D-04:** Cloud documents do NOT affect MinIO quota — unchanged from existing design. Cloud uploads already skip the quota UPDATE; deletes should also skip it.
 - **D-05 (DEFERRED):** Persistent local Celery cache in MinIO for cloud docs — not part of this phase. When Celery analyses a cloud doc, the temp file is discarded as today; no persistent local copy is created or quota-tracked.
 ### Sharing — SHARE-05 Badge Fix
 - **D-06:** `frontend/src/components/documents/DocumentCard.vue:31` — change `v-if="doc.share_count > 0"` to `v-if="doc.is_shared"`. The backend already returns `is_shared: bool` in the document list response. No backend change needed.
 ### Sharing — SHARE-03 Permission Level Control
 - **D-07:** Permission levels: `view` (default) and `edit`. The `Share.permission` column already exists. No migration needed.
 - **D-08:** Permission set at share creation time: add a `view` / `edit` dropdown to `ShareModal.vue` before submitting the share. The existing `POST /api/shares` endpoint already accepts a `permission` field.
 - **D-09:** Permission changeable after creation: add a View/Edit toggle per share row in the existing shares list inside `ShareModal.vue`. Calls a new `PATCH /api/shares/{id}` endpoint with `{ permission: "view" | "edit" }`. Backend must enforce ownership (share owner only; 404 for wrong owner).
 - **D-10:** The backend `permission` field is already stored but the POST handler hardcodes `permission="view"` (line 97 of `shares.py`). Fix: read `permission` from the request body (add it to the request Pydantic model).
 ### Audit Log — User Handles
 - **D-11:** `_audit_to_dict()` in `backend/api/audit.py` currently returns `user_id` and `actor_id` as UUID strings. Extend it to also return `user_handle` and `actor_handle` by joining the `User` table. The frontend already tries to render `entry.user_handle || entry.user_id || '—'`.
 - **D-12:** The `user_id` filter in `GET /api/admin/audit-log` currently expects a UUID. Change it to accept a handle string: look up `User.handle == handle`, resolve to UUID, then apply the filter. If no user with that handle exists, return empty results (not an error).
 ### Audit Log — CSV Export Fix
 - **D-13:** Replace `window.location.href = ...` in `AuditLogTab.vue:exportCsv()` with a `fetch()` + Blob URL pattern. The access token lives in Pinia memory and must be sent as an `Authorization` header — a browser navigation cannot do this.
 - **D-14:** Add `adminExportAuditLogCsv(params)` to `frontend/src/api/client.js`. This function must NOT call `res.json()` — it must call `res.text()` (or `res.blob()`) to receive CSV content. Then create an object URL and trigger an `<a>` click to download.
 ### Audit Log — Daily Export UI
 - **D-15:** Add `GET /api/admin/audit-log/daily-exports` endpoint: list available daily export files from the MinIO `audit-logs` bucket. Returns `[{ date: "2026-05-30", key: "audit-logs/2026-05-30.csv" }]` sorted descending.
 - **D-16:** Add `GET /api/admin/audit-log/daily-exports/{date}` endpoint: serve a specific daily export file from MinIO as a streaming text/csv response. Uses the same auth gate (`get_current_admin`). The filename key pattern is `audit-logs/{date}.csv` (as written by `audit_tasks.py:79`).
 - **D-17:** Frontend `AuditLogTab.vue`: add a searchable date dropdown populated from `adminListDailyExports()` API call. A "Download" button fetches the selected date via `fetch()` + Blob URL (same pattern as D-13/D-14).
 ### Claude's Discretion
 - Exact API shape for "remove from app only" vs "delete from provider" — Claude picks the cleanest route (`?cloud_only=false` query param vs separate endpoint).
 - Whether `PATCH /api/shares/{id}` accepts the full share body or just `{ permission: "view"|"edit" }` — minimal body preferred.
 - Exact error response shape for cloud delete failure — must be distinguishable from a hard 4xx/5xx by the frontend.
 - MinIO `list_objects` pagination — Claude handles if the audit-logs bucket has more than 1000 files.
 </decisions>
 <canonical_refs>
 ## Canonical References
 **Downstream agents MUST read these before planning or implementing.**
 ### Phase Goal and Requirements
 - `.planning/ROADMAP.md` §"Phase 6.2" — Goal and success criteria (TBD; use decisions above as the source of truth).
 - `.planning/REQUIREMENTS.md` §SHARE-01..05 — Sharing requirements (SHARE-03, SHARE-05 are the open ones).
 - `.planning/REQUIREMENTS.md` §ADMIN-06 — Admin audit log with filters and export.
 - `.planning/phases/06.1-close-v1-audit-gaps/06.1-VERIFICATION.md` — Gap #10 (filter behavioral test) and Gap #11 (STORE-06 integration gate).
 ### Security Mandates
 - `CLAUDE.md` §"Key Architectural Rules" — JWT in Pinia memory only (never localStorage); admin never returns document content; atomic quota UPDATE pattern.
 - `CLAUDE.md` §"Security Protocol" — bandit/pip audit/npm audit gates; admin endpoint whitelist.
 ### Existing Implementation — Backend
 - `backend/api/shares.py` — current share API (POST/GET/DELETE); `permission="view"` hardcoded at line 97; PATCH endpoint missing.
 - `backend/api/audit.py` — `_audit_to_dict()`, `_build_filtered_query()`, both endpoints; CSV StreamingResponse pattern.
 - `backend/services/storage.py:143` — `delete_document()`: MinIO-only delete path; cloud backend routing missing.
 - `backend/tasks/audit_tasks.py` — Celery daily export; key pattern `audit-logs/{yesterday.isoformat()}.csv`; bucket `audit-logs`.
 - `backend/db/models.py` — `Share.permission` column (line 256); `AuditLog` model (line 267).
 ### Existing Implementation — Frontend
 - `frontend/src/components/documents/DocumentCard.vue:31` — `share_count > 0` bug; fix to `is_shared`.
 - `frontend/src/components/sharing/ShareModal.vue` — share creation form and existing shares list with Revoke button.
 - `frontend/src/components/admin/AuditLogTab.vue` — filter UI, `exportCsv()` with `window.location.href` (broken); `fetchLog()`.
 - `frontend/src/api/client.js` — `request()` function (always calls `res.json()`); `adminListAuditLog()`.
 - `frontend/src/views/SharedView.vue` — "Shared with me" view (SHARE-02).
 ### Cloud Backend Patterns
 - `backend/storage/__init__.py` — `get_storage_backend_for_document()` factory; use this for cloud-aware delete routing.
 - `backend/api/admin.py:481` — `delete_user()` cloud cleanup pattern: iterates cloud connections, gets backend, calls `delete_user_files()` — reference for how to call cloud backends.
 - `backend/storage/cloud_utils.py` — `decrypt_credentials()` HKDF pattern used when constructing cloud backends.
 </canonical_refs>
 <code_context>
 ## Existing Code Insights
 ### Reusable Assets
 - `backend/services/storage.py:delete_document()` — extend this function; it already handles MinIO delete + quota decrement. Add cloud routing before the MinIO branch.
 - `backend/storage/__init__.py:get_storage_backend_for_document()` — already resolves the correct backend given a Document ORM object. Use it directly in delete_document().
 - `backend/api/audit.py:_build_filtered_query()` — shared filter logic already works; only the handle→UUID resolution is missing.
 - `backend/db/models.py:Share.permission` — column exists, default "view". No migration needed.
 - `frontend/src/components/sharing/ShareModal.vue` — shares list with Revoke button already rendered; add View/Edit toggle to each row.
 ### Established Patterns
 - `backend/api/admin.py:delete_user()` — cloud cleanup: get_storage_backend_for_document() + backend.delete_user_files() pattern to follow for per-document cloud delete.
 - `backend/api/auth.py:177` — `await session.flush()` before audit log write — the fix already applied to admin.py follows this pattern.
 - Cloud document content proxy in `backend/api/documents.py` — uses `fetch()` + streaming via `get_storage_backend_for_document()`; similar pattern for cloud delete.
 - `backend/tasks/audit_tasks.py:put_object_raw()` — how daily exports write to MinIO; reverse: use `get_object()` or presigned GET URL for the download endpoint.
 ### Integration Points
 - `backend/services/storage.py:delete_document()` — add cloud routing here; the caller (`api/documents.py:delete_document`) doesn't need to change.
 - `backend/api/audit.py` — add two new GET endpoints for daily export listing and download.
 - `backend/api/shares.py` — add PATCH `/api/shares/{id}` endpoint + fix permission field on POST.
 - `frontend/src/api/client.js` — add `adminExportAuditLogCsv()` and `adminListDailyExports()` + `adminDownloadDailyExport()` functions (returning text/blob, not JSON).
 </code_context>
 <specifics>
 ## Specific Ideas
 - **Cloud delete failure UX**: Warning modal with two options — "Delete from app only" and "Cancel". This mirrors the existing `UserDeleteConfirm` pattern in Phase 5.
 - **Daily export dropdown**: Searchable `<select>` or combobox, populated on tab open. Sorted newest-first. Date format: `YYYY-MM-DD`. If bucket is empty, show "No daily exports yet".
 - **Audit log user display**: Backend returns `user_handle` and `actor_handle` alongside the UUID fields. Frontend table shows handle; UUID shown as tooltip or hidden. Filter input is a plain text field labeled "User handle".
 - **PATCH /api/shares/{id}**: Minimal body `{ "permission": "view" | "edit" }`. Owner-only; 404 for wrong owner (same IDOR pattern as DELETE).
 </specifics>
 <deferred>
 ## Deferred Ideas
 - **Persistent local Celery cache for cloud docs with quota tracking** — user wants cloud doc analysis to create a persistent MinIO copy that counts against quota, removable via "Remove download". Requires architectural changes to the Celery task and quota system. Future phase.
 - **Celery local cache "Remove download" button** — depends on the deferred item above.
 - **SHARE-03 edit permission beyond view/edit** — if more granular permissions are needed (e.g., comment, reshare), that's a future phase.
 </deferred>
 ---
 *Phase: 6.2-Close v1 sharing + cloud-delete + CSV export gaps*
 *Context gathered: 2026-05-31*
@@ -0,0 +1,60 @@
 # Phase 6.2 Discussion Log
 **Date:** 2026-05-31
 **Areas discussed:** Cloud-delete propagation, Sharing gaps scope, CSV export gap
 ---
 ## Area 1: Cloud-delete propagation
 **Q:** Should delete propagate to the cloud provider?
 **A:** Yes, default delete deletes from provider. A separate "Remove from app" action keeps the cloud file.
 **Q:** How should the two delete actions be surfaced?
 **A:** Default delete button = delete from provider. New "Remove download" button = removes app record only.
 **Q:** If cloud provider delete fails, what should happen?
 **A:** Warn the user with a modal. Let them decide whether to remove from app anyway.
 **Q:** Should cloud docs decrement MinIO quota on delete?
 **A:** No, cloud docs don't touch MinIO quota. But user noted future desire for quota tracking if cloud docs are cached locally — deferred.
 ---
 ## Area 2: Sharing gaps scope
 **Context discovered:** Admin user creation was returning HTTP 500. Root cause: `write_audit_log` flushed the AuditLog INSERT before the new User was in the DB, causing FK violation on PostgreSQL (silent on SQLite). Fixed by adding `await session.flush()` before `write_audit_log` in `admin.py:create_user`. Regression test added.
 **User context:** Could not test sharing manually because admin create-user was broken.
 **Q:** What share behaviors should Phase 6.2 address?
 **A:** Both the `is_shared` badge fix (SHARE-05) and permission level control (SHARE-03).
 **Q:** Should permission be set at share time or editable after?
 **A:** Both — dropdown in ShareModal at creation AND View/Edit toggle per share row after creation (requires new PATCH endpoint).
 ---
 ## Area 3: CSV export gap
 **User reported issues:**
 1. Export button redirects to URL → 401 "Not authenticated" (access token is in Pinia memory, not sent on browser navigation)
 2. Applying filters shows nothing (user_id filter accepts any text; backend expects UUID; 422 silently swallowed)
 3. Daily exports not accessible from UI (they go to MinIO audit-logs bucket)
 4. Audit log shows raw UUIDs instead of user handles
 **Q:** How should admins filter by user?
 **A:** Admin sees users in the Users tab with handles. Audit log should show handles, not UUIDs. Filter by handle (backend resolves to UUID).
 **Q:** Daily export UI?
 **A:** Add a searchable dropdown in the audit tab to select which daily export to download, plus a download button.
 ---
 ## Deferred Ideas
 - Persistent Celery local cache in MinIO for cloud docs with quota tracking — requires architectural changes; future phase.
 ---
 *Claude's discretion items: exact API shape for "remove from app" endpoint; PATCH /api/shares/{id} body shape; cloud delete error response format; MinIO list_objects pagination.*
@@ -244,6 +244,7 @@ async def create_user(
        used_bytes=0,
    )
    session.add(quota)
    await session.flush()  # persist User + Quota before audit_log FK references them
    # D-13: admin user created event
    _ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None)
    await write_audit_log(
@@ -21,7 +21,8 @@ import pytest_asyncio
 from httpx import ASGITransport, AsyncClient
 from sqlalchemy.ext.asyncio import AsyncSession
-from db.models import Quota, User
+from db.models import AuditLog, Quota, User
 from sqlalchemy import select
 from deps.auth import get_current_admin
 from deps.db import get_db
 from services.auth import hash_password
@@ -140,6 +141,24 @@ async def test_create_user_sets_password_must_change(admin_client):
    assert user.password_must_change is True
@pytest.mark.asyncio
 async def test_create_user_writes_audit_log(admin_client):
    """POST /api/admin/users → 201 and audit_log row created (regression: FK ordering bug)."""
    client, _admin, session = admin_client
    body = {
        "handle": "auditcheck_user",
        "email": "auditcheck@example.com",
        "password": "AuditCheck1@Pass",
        "role": "user",
    }
    resp = await client.post("/api/admin/users", json=body)
    assert resp.status_code == 201, f"expected 201, got {resp.status_code}: {resp.text}"
    result = await session.execute(
        select(AuditLog).where(AuditLog.event_type == "admin.user_created")
    )
    assert result.scalars().first() is not None, "audit log entry not created after user creation"
@pytest.mark.asyncio
 async def test_create_user_weak_password(admin_client):
    """POST /api/admin/users with weak password → 422."""