docs(06.2): create 4-plan phase covering SHARE-03, SHARE-05, cloud-delete, ADMIN-06
Wave 0: 11 xfail stubs across test_shares/test_documents/test_audit Wave 1 (parallel): SHARE-05 badge + SHARE-03 permission control; cloud-delete propagation Wave 2: audit handle enrichment, user_handle filter, CSV fetch+Blob, daily-export UI Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
curo1305
@@ -0,0 +1,204 @@
|
||||
---
|
||||
phase: "06.2"
|
||||
plan: "01"
|
||||
type: execute
|
||||
wave: 0
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- backend/tests/test_shares.py
|
||||
- backend/tests/test_documents.py
|
||||
- backend/tests/test_audit.py
|
||||
autonomous: true
|
||||
requirements:
|
||||
- SHARE-03
|
||||
- SHARE-05
|
||||
- ADMIN-06
|
||||
must_haves:
|
||||
truths:
|
||||
- "pytest exits 0 after adding 11 xfail stubs — no new failures"
|
||||
- "Each stub is reachable by name so Wave 1 and 2 plans can promote them individually"
|
||||
- "Stubs use strict=False so they report as xfail, not xpass, while implementation is absent"
|
||||
artifacts:
|
||||
- path: "backend/tests/test_shares.py"
|
||||
provides: "xfail stubs for test_share_create_with_permission, test_share_patch_permission, test_share_patch_idor"
|
||||
contains: "pytest.xfail"
|
||||
- path: "backend/tests/test_documents.py"
|
||||
provides: "xfail stubs for test_delete_cloud_document_propagates, test_delete_cloud_document_failure, test_delete_cloud_remove_only"
|
||||
contains: "pytest.xfail"
|
||||
- path: "backend/tests/test_audit.py"
|
||||
provides: "xfail stubs for 5 audit gap tests"
|
||||
contains: "pytest.xfail"
|
||||
key_links:
|
||||
- from: "backend/tests/test_shares.py"
|
||||
to: "Wave 1 Plan 06.2-02"
|
||||
via: "test function names (must match exactly)"
|
||||
pattern: "test_share_create_with_permission|test_share_patch_permission|test_share_patch_idor"
|
||||
- from: "backend/tests/test_documents.py"
|
||||
to: "Wave 1 Plan 06.2-03"
|
||||
via: "test function names (must match exactly)"
|
||||
pattern: "test_delete_cloud_document_propagates|test_delete_cloud_document_failure|test_delete_cloud_remove_only"
|
||||
- from: "backend/tests/test_audit.py"
|
||||
to: "Wave 2 Plan 06.2-04"
|
||||
via: "test function names (must match exactly)"
|
||||
pattern: "test_audit_log_includes_user_handle|test_audit_log_filter_by_handle|test_audit_log_filter_unknown_handle|test_daily_exports_list|test_daily_export_download"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Add 11 xfail test stubs — one per Wave 0 gap identified in VALIDATION.md — across three test files. These stubs establish the Nyquist contract: each gap has a named, runnable test before any implementation begins. Wave 1 and Wave 2 plans promote individual stubs to real tests.
|
||||
|
||||
Purpose: Nyquist compliance — no task in Waves 1 or 2 can complete without a matching automated test. Stubs guarantee the test function names exist before any executor tries to promote them.
|
||||
|
||||
Output: 11 new test functions (3 in test_shares.py, 3 in test_documents.py, 5 in test_audit.py), all marked xfail(strict=False).
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-VALIDATION.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/ROADMAP.md
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-CONTEXT.md
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Add xfail stubs to test_shares.py (SHARE-03)</name>
|
||||
<files>backend/tests/test_shares.py</files>
|
||||
<read_first>
|
||||
- backend/tests/test_shares.py — read the full file to understand the existing async_client/auth_user/second_auth_user/db_session fixture pattern and function naming conventions before appending
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-VALIDATION.md — Wave 0 requirements section, exact test names
|
||||
</read_first>
|
||||
<action>
|
||||
Append three new async test functions to the end of backend/tests/test_shares.py. Each uses the `pytest.xfail("not implemented yet")` call immediately as its first statement (no imports, no fixtures consumed). Use `@pytest.mark.xfail(strict=False, reason="Phase 6.2 — not implemented yet")` decorator OR inline `pytest.xfail(...)` at function start — inline is preferred to match the existing xfail pattern in test_documents.py (which uses the inline call, not the decorator).
|
||||
|
||||
The three function signatures to add are:
|
||||
|
||||
1. `async def test_share_create_with_permission(async_client, auth_user, second_auth_user, db_session):`
|
||||
- Docstring: "POST /api/shares respects permission field from request body (SHARE-03, D-08, D-10)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
2. `async def test_share_patch_permission(async_client, auth_user, second_auth_user, db_session):`
|
||||
- Docstring: "PATCH /api/shares/{id} changes permission to edit (SHARE-03, D-09)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
3. `async def test_share_patch_idor(async_client, auth_user, second_auth_user, db_session):`
|
||||
- Docstring: "PATCH /api/shares/{id} by non-owner returns 404 — IDOR protection (SHARE-03, D-09, T-IDOR)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
Do NOT add any imports — `pytest` is already imported at the top of the file. Do NOT implement any logic beyond the xfail call.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_shares.py::test_share_create_with_permission tests/test_shares.py::test_share_patch_permission tests/test_shares.py::test_share_patch_idor -v 2>&1 | grep -E "xfail|XFAIL|passed|failed" | head -20</automated>
|
||||
</verify>
|
||||
<done>All three new tests collected and reported as XFAIL (not ERROR, not FAILED); `pytest tests/test_shares.py -x -q` exits 0</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Add xfail stubs to test_documents.py (cloud-delete)</name>
|
||||
<files>backend/tests/test_documents.py</files>
|
||||
<read_first>
|
||||
- backend/tests/test_documents.py — read the full file to confirm import structure, existing xfail pattern (inline pytest.xfail call), and where to append new functions
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-VALIDATION.md — Wave 0 requirements section
|
||||
</read_first>
|
||||
<action>
|
||||
Append three new async test functions to the end of backend/tests/test_documents.py. Use inline `pytest.xfail("Phase 6.2 — not implemented yet")` as the first statement — matching the existing pattern in the file (which uses `@pytest.mark.xfail(strict=False, ...)` decorator on legacy tests at the top, but newer additions in this file use the inline call pattern from VALIDATION.md guidance).
|
||||
|
||||
The three function signatures to add are:
|
||||
|
||||
1. `async def test_delete_cloud_document_propagates(async_client, auth_user, db_session):`
|
||||
- Docstring: "DELETE /api/documents/{id} for a cloud doc calls cloud backend delete_object (D-01)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
2. `async def test_delete_cloud_document_failure(async_client, auth_user, db_session):`
|
||||
- Docstring: "DELETE /api/documents/{id} returns cloud_delete_failed=True when provider raises (D-03)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
3. `async def test_delete_cloud_remove_only(async_client, auth_user, db_session):`
|
||||
- Docstring: "DELETE /api/documents/{id}?remove_only=true skips cloud delete, removes DB row only (D-02)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
All three stubs must have `pytestmark = pytest.mark.asyncio` coverage — confirm this is already at the top of the file or add it if missing. Do not implement any logic beyond xfail.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_documents.py::test_delete_cloud_document_propagates tests/test_documents.py::test_delete_cloud_document_failure tests/test_documents.py::test_delete_cloud_remove_only -v 2>&1 | grep -E "xfail|XFAIL|passed|failed" | head -20</automated>
|
||||
</verify>
|
||||
<done>All three new tests collected and reported as XFAIL; `pytest tests/test_documents.py -x -q` exits 0</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Add xfail stubs to test_audit.py (ADMIN-06 gaps)</name>
|
||||
<files>backend/tests/test_audit.py</files>
|
||||
<read_first>
|
||||
- backend/tests/test_audit.py — read the full file to see existing helpers (_seed_audit), fixture usage (async_client, admin_user, db_session), and where to append
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-VALIDATION.md — Wave 0 requirements section
|
||||
</read_first>
|
||||
<action>
|
||||
Append five new async test functions to the end of backend/tests/test_audit.py. Use inline `pytest.xfail("Phase 6.2 — not implemented yet")` as the first statement in each body.
|
||||
|
||||
The five function signatures to add are:
|
||||
|
||||
1. `async def test_audit_log_includes_user_handle(async_client, admin_user, db_session):`
|
||||
- Docstring: "Audit log items include user_handle and actor_handle strings (D-11)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
2. `async def test_audit_log_filter_by_handle(async_client, admin_user, db_session):`
|
||||
- Docstring: "GET /api/admin/audit-log?user_handle=X filters to matching entries (D-12)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
3. `async def test_audit_log_filter_unknown_handle(async_client, admin_user, db_session):`
|
||||
- Docstring: "GET /api/admin/audit-log?user_handle=unknown returns empty items list, not 422 (D-12)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
4. `async def test_daily_exports_list(async_client, admin_user):`
|
||||
- Docstring: "GET /api/admin/audit-log/daily-exports returns {items: [...]} (D-15)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
5. `async def test_daily_export_download(async_client, admin_user):`
|
||||
- Docstring: "GET /api/admin/audit-log/daily-exports/{date} returns CSV bytes with Content-Disposition (D-16)"
|
||||
- Body: `pytest.xfail("Phase 6.2 — not implemented yet")`
|
||||
|
||||
Do not add any new imports beyond what is already at the top of the file. Do not implement any logic.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_audit.py::test_audit_log_includes_user_handle tests/test_audit.py::test_audit_log_filter_by_handle tests/test_audit.py::test_audit_log_filter_unknown_handle tests/test_audit.py::test_daily_exports_list tests/test_audit.py::test_daily_export_download -v 2>&1 | grep -E "xfail|XFAIL|passed|failed" | head -20</automated>
|
||||
</verify>
|
||||
<done>All five new tests collected and reported as XFAIL; `pytest tests/test_audit.py -x -q` exits 0; total xfail count in test_audit.py increases by 5</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| test runner → test files | xfail stubs must not execute any production code paths |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-06.2-01-01 | Tampering | xfail stubs accidentally implementing logic | accept | Stubs contain only `pytest.xfail(...)` — no imports, no API calls, no fixtures consumed beyond signature |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
After all three tasks complete:
|
||||
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_shares.py tests/test_audit.py tests/test_documents.py -x -q
|
||||
```
|
||||
|
||||
Expected: exits 0, all 11 new stubs reported as xfail. Pre-existing 310 passing tests must remain passing. Pre-existing `test_extract_docx` failure is allowed.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 11 new xfail stubs collected across the three test files
|
||||
- `pytest tests/test_shares.py tests/test_audit.py tests/test_documents.py -x -q` exits 0
|
||||
- Every stub matches the exact function name from VALIDATION.md Wave 0 Requirements
|
||||
- No existing passing tests are broken
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-01-SUMMARY.md` when done.
|
||||
</output>
|
||||
@@ -0,0 +1,262 @@
|
||||
---
|
||||
phase: "06.2"
|
||||
plan: "02"
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on:
|
||||
- "06.2-01"
|
||||
files_modified:
|
||||
- backend/api/shares.py
|
||||
- frontend/src/components/documents/DocumentCard.vue
|
||||
- frontend/src/components/sharing/ShareModal.vue
|
||||
- frontend/src/stores/documents.js
|
||||
- backend/tests/test_shares.py
|
||||
autonomous: true
|
||||
requirements:
|
||||
- SHARE-03
|
||||
- SHARE-05
|
||||
must_haves:
|
||||
truths:
|
||||
- "Documents shared with others display a 'Shared' pill in DocumentCard (reads doc.is_shared, not doc.share_count)"
|
||||
- "Owner can set permission to 'view' or 'edit' when creating a share"
|
||||
- "Owner can toggle permission per share row after creation"
|
||||
- "PATCH /api/shares/{id} by the wrong owner returns 404 (IDOR protection)"
|
||||
- "POST /api/shares respects the permission field from the request body"
|
||||
artifacts:
|
||||
- path: "backend/api/shares.py"
|
||||
provides: "ShareCreate model with permission field; PATCH /{share_id} endpoint"
|
||||
contains: "class SharePermissionPatch"
|
||||
- path: "frontend/src/components/documents/DocumentCard.vue"
|
||||
provides: "Corrected is_shared guard on Shared pill"
|
||||
contains: "v-if=\"doc.is_shared\""
|
||||
- path: "frontend/src/components/sharing/ShareModal.vue"
|
||||
provides: "Permission dropdown in creation row; View/Edit toggle per share row"
|
||||
contains: "Permission level"
|
||||
key_links:
|
||||
- from: "frontend/src/components/sharing/ShareModal.vue"
|
||||
to: "PATCH /api/shares/{id}"
|
||||
via: "docsStore.updateSharePermission(shareId, permission)"
|
||||
pattern: "updateSharePermission"
|
||||
- from: "backend/api/shares.py PATCH"
|
||||
to: "Share.owner_id"
|
||||
via: "IDOR check — 404 on mismatch"
|
||||
pattern: "share.owner_id != current_user.id"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Close SHARE-05 (badge uses wrong field) and SHARE-03 (no permission control) in a single vertical slice. Delivers: corrected is_shared badge, permission dropdown at share creation, View/Edit toggle per share row, and the backing PATCH endpoint with IDOR protection.
|
||||
|
||||
Purpose: Users can now see which documents they've shared (correct badge), set the permission level when sharing, and change it afterward. Closes two open v1 requirements.
|
||||
|
||||
Output: Modified shares.py (new ShareCreate.permission field + PATCH endpoint), modified DocumentCard.vue (badge fix), modified ShareModal.vue (dropdown + toggle UI), modified documents store (updateSharePermission action), three promoted test stubs.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/ROADMAP.md
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-CONTEXT.md
|
||||
|
||||
<interfaces>
|
||||
<!-- Key types and contracts the executor needs. Extracted from codebase. -->
|
||||
|
||||
From backend/api/shares.py (current state):
|
||||
class ShareCreate(BaseModel):
|
||||
document_id: str
|
||||
recipient_handle: str
|
||||
# permission="view" hardcoded at line 97 in grant_share()
|
||||
|
||||
@router.delete("/{share_id}", status_code=204)
|
||||
async def revoke_share(share_id: str, ...) -> None:
|
||||
sid = uuid.UUID(share_id) # 404 on ValueError
|
||||
share = await session.get(Share, sid)
|
||||
if share is None or share.owner_id != current_user.id:
|
||||
raise HTTPException(404, "Share not found") # IDOR pattern to mirror
|
||||
|
||||
# Route ordering: GET /received defined BEFORE DELETE /{share_id}
|
||||
|
||||
From backend/db/models.py (Share model — key fields):
|
||||
Share.id: UUID
|
||||
Share.document_id: UUID
|
||||
Share.owner_id: UUID (FK to User)
|
||||
Share.recipient_id: UUID (FK to User)
|
||||
Share.permission: str # column exists, default "view" — no migration needed
|
||||
|
||||
From frontend/src/components/documents/DocumentCard.vue (line 31, buggy):
|
||||
v-if="doc.share_count > 0" # BUG — backend sends is_shared: bool, not share_count
|
||||
|
||||
From frontend/src/components/sharing/ShareModal.vue (shares list row, line 75):
|
||||
<span class="text-xs bg-gray-100 text-gray-600 px-2 py-1 rounded-full font-medium">view</span>
|
||||
# This static "view" span must be replaced with the View/Edit toggle (C-2)
|
||||
|
||||
From frontend/src/stores/documents.js — existing share methods to reference:
|
||||
shareDocument(docId, recipientHandle) — calls POST /api/shares
|
||||
revokeShare(shareId) — calls DELETE /api/shares/{id}
|
||||
listShares(docId) — calls GET /api/shares?document_id={docId}
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Backend — ShareCreate permission field + PATCH endpoint</name>
|
||||
<files>backend/api/shares.py, backend/tests/test_shares.py</files>
|
||||
<read_first>
|
||||
- backend/api/shares.py — read the full file; understand ShareCreate model (line 38), grant_share handler (hardcoded permission="view" at line 97), revoke_share IDOR pattern (lines 239-265), route ordering comments
|
||||
- backend/tests/test_shares.py — read the full file; understand async_client/auth_user/second_auth_user/db_session fixture pattern and _make_doc helper
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md — Pattern 2 (PATCH IDOR-safe pattern) and Anti-Patterns section (route ordering)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- test_share_create_with_permission: POST /api/shares with {"permission": "edit"} returns 201 and body["permission"] == "edit"; POST with no permission field defaults to "view"
|
||||
- test_share_patch_permission: PATCH /api/shares/{valid_id} with {"permission": "edit"} returns 200 and {"permission": "edit"}; a second PATCH with {"permission": "view"} returns 200 and {"permission": "view"}
|
||||
- test_share_patch_idor: PATCH /api/shares/{id_owned_by_user_A} authenticated as user_B returns 404 (not 403, not 401)
|
||||
</behavior>
|
||||
<action>
|
||||
Make two changes to backend/api/shares.py:
|
||||
|
||||
CHANGE 1 — ShareCreate model (add permission field):
|
||||
Add `permission: str = "view"` to the ShareCreate model. Add a field_validator named `validate_permission` that checks the value is in `{"view", "edit"}` and raises ValueError otherwise. Import `field_validator` from pydantic if not already imported.
|
||||
|
||||
In the `grant_share` handler, change the hardcoded `permission="view"` in the Share(...) constructor (line 97) to `permission=body.permission`.
|
||||
|
||||
CHANGE 2 — Add SharePermissionPatch model and PATCH endpoint:
|
||||
Add a new Pydantic model class `SharePermissionPatch(BaseModel)` with a single field `permission: str` and a `field_validator("permission")` classmethod that validates `v in {"view", "edit"}` (same pattern as above).
|
||||
|
||||
Add the PATCH endpoint `@router.patch("/{share_id}", status_code=200)` as `async def update_share_permission(...)`. Place it BEFORE the existing `@router.delete("/{share_id}", ...)` in the file (style consistency; method discrimination makes ordering safe, but before DELETE is conventional). The handler body:
|
||||
- Parse `share_id` as `uuid.UUID(share_id)`, raising HTTPException(404) on ValueError
|
||||
- `share = await session.get(Share, sid)` — 404 if None
|
||||
- IDOR check: `if share is None or share.owner_id != current_user.id: raise HTTPException(404, "Share not found")` — mirrors revoke_share exactly (T-04-04-02)
|
||||
- `share.permission = body.permission`
|
||||
- `await session.commit()`
|
||||
- Return `{"id": str(share.id), "permission": share.permission}`
|
||||
|
||||
Then in backend/tests/test_shares.py, promote the three xfail stubs added in Plan 06.2-01 to real tests. Replace the `pytest.xfail(...)` body with actual test logic following the _make_doc helper pattern and async_client fixture conventions already in the file.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_shares.py::test_share_create_with_permission tests/test_shares.py::test_share_patch_permission tests/test_shares.py::test_share_patch_idor -x -v 2>&1 | tail -20</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- `pytest tests/test_shares.py -x -q` exits 0 — all 10 tests pass (7 pre-existing + 3 promoted)
|
||||
- `grep "class SharePermissionPatch" backend/api/shares.py` returns a match
|
||||
- `grep "share.owner_id != current_user.id" backend/api/shares.py` returns at least 2 matches (one in revoke_share, one in update_share_permission)
|
||||
- PATCH /api/shares/{id} with wrong owner returns 404 (confirmed by test_share_patch_idor)
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Frontend — is_shared badge fix + permission dropdown + View/Edit toggle</name>
|
||||
<files>frontend/src/components/documents/DocumentCard.vue, frontend/src/components/sharing/ShareModal.vue, frontend/src/stores/documents.js</files>
|
||||
<read_first>
|
||||
- frontend/src/components/documents/DocumentCard.vue — read lines 25-40 to see the share_count bug at line 31 and surrounding template context
|
||||
- frontend/src/components/sharing/ShareModal.vue — read the full file; understand the flex gap-2 creation row (lines 31-50), the static "view" span in the recipient list row (line 75), and how handleRevoke uses docsStore
|
||||
- frontend/src/stores/documents.js — find shareDocument(), revokeShare(), and listShares() methods; understand the request() wrapper used by these methods so updateSharePermission follows the same pattern
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md — Component Contracts C-1 (permission dropdown markup), C-2 (View/Edit toggle markup), Copywriting Contract (label copy)
|
||||
</read_first>
|
||||
<action>
|
||||
Make three frontend changes:
|
||||
|
||||
CHANGE 1 — DocumentCard.vue line 31 (per D-06):
|
||||
Change `v-if="doc.share_count > 0"` to `v-if="doc.is_shared"`. This is a one-word change. No other modifications to DocumentCard.vue.
|
||||
|
||||
CHANGE 2 — ShareModal.vue — permission dropdown in creation row (per D-08, C-1 from UI-SPEC):
|
||||
Add a `permission` reactive ref defaulting to `"view"` in the script setup section.
|
||||
|
||||
In the template, inside the `<div class="flex gap-2">` creation row (the row containing the handle input and the "Share document" button), insert a `<select>` element BETWEEN the handle `<input>` and the submit `<button>`. The select uses `v-model="permission"`, `aria-label="Permission level"`, and Tailwind classes: `border border-gray-300 rounded-lg px-3 py-2 text-sm bg-white focus:outline-none focus:ring-2 focus:ring-indigo-500 shrink-0`. Two options: `<option value="view">Can view</option>` and `<option value="edit">Can edit</option>`.
|
||||
|
||||
Pass `permission: permission.value` into the `docsStore.shareDocument(props.doc.id, trimmed, permission.value)` call (the store method needs updating — see below). Reset `permission.value = "view"` on successful submit.
|
||||
|
||||
CHANGE 3 — ShareModal.vue — View/Edit toggle per share row (per D-09, C-2 from UI-SPEC) and in-flight error state:
|
||||
Add a reactive `permissionError` ref (string, null default) and a `updatingPermission` ref (Set or Object tracking in-flight share IDs) in script setup.
|
||||
|
||||
Replace the static `<span class="text-xs bg-gray-100 text-gray-600 px-2 py-1 rounded-full font-medium">view</span>` in each recipient list row with a View/Edit toggle group. The toggle group is a `<div>` with `role="group"` `aria-label="Permission"` containing two `<button>` elements ("View" and "Edit"). Each button:
|
||||
- Active state classes: `bg-indigo-50 text-indigo-600 font-medium`
|
||||
- Inactive state classes: `bg-gray-100 text-gray-600`
|
||||
- Common classes: `text-xs px-2 py-1 rounded-full font-medium transition-colors`
|
||||
- `aria-pressed` attribute reflecting whether the button's value matches `share.permission`
|
||||
- `aria-label` pattern: "Change permission for {share.recipient_handle}"
|
||||
- Disabled (opacity-50 pointer-events-none) when `updatingPermission.has(share.id)`
|
||||
- On click of the inactive button: call `handlePermissionChange(share.id, 'view'|'edit')`
|
||||
|
||||
Add `handlePermissionChange(shareId, newPermission)` function:
|
||||
- Optimistic: find the share in `shares.value`, set `share.permission = newPermission` immediately
|
||||
- Mark in-flight: `updatingPermission.value.add(shareId)` (use `ref(new Set())`)
|
||||
- Call `await docsStore.updateSharePermission(shareId, newPermission)`
|
||||
- On error: revert `share.permission` to the old value, set `permissionError.value = "Failed to update permission."`
|
||||
- Finally: `updatingPermission.value.delete(shareId)`
|
||||
|
||||
Show `permissionError` below the list (same `text-xs text-red-600 mt-2` pattern as the existing `error` display).
|
||||
|
||||
CHANGE 4 — documents.js store — add updateSharePermission action and update shareDocument signature:
|
||||
Add `updateSharePermission(shareId, permission)` action that calls `PATCH /api/shares/${shareId}` with body `{ permission }` via the existing `request()` wrapper.
|
||||
|
||||
Update `shareDocument(docId, recipientHandle, permission = 'view')` to pass `{ document_id: docId, recipient_handle: recipientHandle, permission }` in the POST body (previously only `document_id` and `recipient_handle`).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/frontend && grep -n "doc.is_shared" src/components/documents/DocumentCard.vue | head -5</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- `grep "doc.is_shared" frontend/src/components/documents/DocumentCard.vue` returns a match (not share_count)
|
||||
- `grep "Permission level" frontend/src/components/sharing/ShareModal.vue` returns a match
|
||||
- `grep "handlePermissionChange" frontend/src/components/sharing/ShareModal.vue` returns a match
|
||||
- `grep "updateSharePermission" frontend/src/stores/documents.js` returns a match
|
||||
- `grep "share_count" frontend/src/components/documents/DocumentCard.vue` returns no match (old bug removed)
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| browser → PATCH /api/shares/{id} | User-supplied share_id and permission value cross the API boundary |
|
||||
| ShareModal → documents store | permission value must be one of the two literals before reaching the backend |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-06.2-02-01 | Elevation of Privilege | PATCH /api/shares/{id} | mitigate | `share.owner_id != current_user.id` → HTTPException(404) — mirrors existing revoke_share IDOR pattern; returns 404 not 403 to prevent share ID enumeration |
|
||||
| T-06.2-02-02 | Tampering | SharePermissionPatch model | mitigate | `field_validator("permission")` checks value in `{"view", "edit"}` — no arbitrary string passthrough from request body to DB |
|
||||
| T-06.2-02-03 | Tampering | ShareCreate.permission field | mitigate | Same field_validator as SharePermissionPatch — "view" is the server-enforced default if client omits the field |
|
||||
| T-06.2-SC | Tampering | npm/pip/cargo installs | accept | No new packages installed in this plan |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
After both tasks complete:
|
||||
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_shares.py -x -q
|
||||
```
|
||||
|
||||
Expected: 10 passed (7 pre-existing + 3 promoted). No xfail in test_shares.py.
|
||||
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest -v 2>&1 | tail -20
|
||||
```
|
||||
|
||||
Expected: zero failures (pre-existing `test_extract_docx` xfail is allowed).
|
||||
|
||||
Frontend spot-checks (manual or via `grep`):
|
||||
- DocumentCard.vue contains `v-if="doc.is_shared"` and NOT `share_count`
|
||||
- ShareModal.vue contains `aria-label="Permission level"` and `handlePermissionChange`
|
||||
- documents.js contains `updateSharePermission`
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- POST /api/shares with permission="edit" stores "edit" in DB — confirmed by test_share_create_with_permission
|
||||
- PATCH /api/shares/{id} changes permission — confirmed by test_share_patch_permission
|
||||
- PATCH /api/shares/{id} by wrong owner returns 404 — confirmed by test_share_patch_idor
|
||||
- DocumentCard shows "Shared" pill based on doc.is_shared (not doc.share_count)
|
||||
- ShareModal creation row has permission dropdown defaulting to "Can view"
|
||||
- ShareModal share rows show View/Edit toggle instead of static "view" text
|
||||
- All 10 test_shares.py tests pass
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-02-SUMMARY.md` when done.
|
||||
</output>
|
||||
@@ -0,0 +1,290 @@
|
||||
---
|
||||
phase: "06.2"
|
||||
plan: "03"
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on:
|
||||
- "06.2-01"
|
||||
files_modified:
|
||||
- backend/api/documents.py
|
||||
- backend/services/storage.py
|
||||
- frontend/src/views/DocumentView.vue
|
||||
- frontend/src/api/client.js
|
||||
- backend/tests/test_documents.py
|
||||
autonomous: true
|
||||
requirements:
|
||||
- SHARE-03
|
||||
- SHARE-05
|
||||
- ADMIN-06
|
||||
must_haves:
|
||||
truths:
|
||||
- "Deleting a cloud document calls the cloud provider's delete_object, not MinIO's"
|
||||
- "Cloud delete failure returns HTTP 200 with cloud_delete_failed: true in the response body (not a hard 4xx/5xx)"
|
||||
- "remove_only=true deletes only the DB row, leaving the cloud file intact, and skips quota decrement"
|
||||
- "Cloud document deletes do NOT decrement the user's quota (cloud docs never charged quota at upload)"
|
||||
- "Frontend shows CloudDeleteWarningModal when cloud_delete_failed response is received"
|
||||
- "User can confirm 'Remove from app' which calls DELETE ?remove_only=true and navigates away"
|
||||
artifacts:
|
||||
- path: "backend/api/documents.py"
|
||||
provides: "cloud-aware delete_document endpoint with remove_only query param"
|
||||
contains: "remove_only"
|
||||
- path: "backend/services/storage.py"
|
||||
provides: "skip_quota guard in delete_document service function"
|
||||
contains: "skip_quota"
|
||||
- path: "frontend/src/views/DocumentView.vue"
|
||||
provides: "CloudDeleteWarningModal inline block; remove_only confirm path"
|
||||
contains: "showCloudDeleteWarning"
|
||||
key_links:
|
||||
- from: "backend/api/documents.py"
|
||||
to: "storage.get_storage_backend_for_document()"
|
||||
via: "cloud routing before MinIO path"
|
||||
pattern: "get_storage_backend_for_document"
|
||||
- from: "backend/api/documents.py"
|
||||
to: "services/storage.delete_document(skip_quota=True)"
|
||||
via: "skip_quota parameter for cloud docs"
|
||||
pattern: "skip_quota"
|
||||
- from: "frontend/src/views/DocumentView.vue"
|
||||
to: "DELETE /api/documents/{id}?remove_only=true"
|
||||
via: "confirmRemoveOnly() handler called from modal CTA"
|
||||
pattern: "remove_only=true"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Close the cloud-delete propagation gap in a single vertical slice. The default delete button now propagates to the cloud provider. A structured error response (cloud_delete_failed: true) triggers a warning modal in the frontend. The "Remove from app" path uses ?remove_only=true to delete only the DB record. Cloud docs skip quota decrement.
|
||||
|
||||
Purpose: Users who delete cloud-stored documents no longer create orphaned files on the provider. This is a correctness fix: the app claimed to delete documents but only removed the DB row.
|
||||
|
||||
Output: Modified api/documents.py (cloud routing + remove_only param), modified services/storage.py (skip_quota guard), modified DocumentView.vue (warning modal + remove_only path), new client.js deleteDocument function variant, three promoted test stubs.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/ROADMAP.md
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-CONTEXT.md
|
||||
|
||||
<interfaces>
|
||||
<!-- Key types and contracts the executor needs. Extracted from codebase. -->
|
||||
|
||||
From backend/services/storage.py:delete_document (current signature):
|
||||
async def delete_document(session: AsyncSession, doc_id: str) -> bool:
|
||||
# Always calls _backend().delete_object() (MinIO singleton)
|
||||
# Always runs quota decrement UPDATE
|
||||
# Returns False if doc not found, True on success
|
||||
|
||||
From backend/storage/__init__.py:
|
||||
async def get_storage_backend_for_document(
|
||||
document: Document,
|
||||
user: User,
|
||||
session: AsyncSession,
|
||||
) -> StorageBackend:
|
||||
# Returns MinIOBackend for storage_backend == "minio"
|
||||
# For cloud docs: loads CloudConnection, decrypts HKDF creds, returns cloud backend
|
||||
# Raises HTTPException(503) if connection not found/inactive
|
||||
|
||||
From backend/api/admin.py lines 527-539 (canonical cloud delete pattern):
|
||||
for doc in cloud_docs:
|
||||
try:
|
||||
backend = await get_storage_backend_for_document(doc, user, session)
|
||||
await backend.delete_object(doc.object_key)
|
||||
except Exception:
|
||||
pass # best-effort
|
||||
|
||||
From backend/api/documents.py (existing delete endpoint stub — find the @router.delete("/{doc_id}") handler):
|
||||
# Existing handler: calls await storage.delete_document(session, doc_id)
|
||||
# Must be extended with remove_only query param and cloud routing
|
||||
|
||||
From frontend/src/api/client.js (existing deleteDocument function — search for "deleteDocument"):
|
||||
# Current implementation calls DELETE /api/documents/{id} via request() wrapper
|
||||
# request() calls res.json() — this is correct for 200 responses
|
||||
# New behavior: parse response body for cloud_delete_failed flag
|
||||
|
||||
From frontend/src/views/DocumentView.vue (existing confirmDelete pattern — search for "confirmDelete" or "handleDelete"):
|
||||
# Existing: window.confirm() then deleteDocument() then router.push('/')
|
||||
# New: after API call, check response.cloud_delete_failed — if true, show modal
|
||||
|
||||
From .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md C-3:
|
||||
Cloud Delete Warning Modal — inline in DocumentView.vue:
|
||||
Fixed overlay: bg-black/40 flex items-center justify-center z-50
|
||||
Panel: bg-white rounded-2xl shadow-xl p-6 max-w-sm w-full mx-4
|
||||
Heading: "Cloud delete failed" (text-lg font-semibold text-gray-900 mb-2)
|
||||
Body: "The file could not be deleted from {provider}. Remove it from DocuVault anyway? The file will remain on {provider}."
|
||||
Warning icon: Heroicons ExclamationTriangleIcon inline SVG, w-5 h-5 text-amber-500
|
||||
Primary CTA: "Remove from app" — bg-red-600 hover:bg-red-700 text-white text-sm px-4 py-2 rounded-lg
|
||||
Secondary: "Cancel" — border border-gray-300 text-gray-700 text-sm px-4 py-2 rounded-lg hover:bg-gray-50
|
||||
role="dialog" aria-modal="true" aria-labelledby="cloud-delete-modal-title"
|
||||
@click.self closes modal; Cancel abandons delete; "Remove from app" calls DELETE ?remove_only=true
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Backend — cloud-aware delete routing + skip_quota + remove_only param</name>
|
||||
<files>backend/api/documents.py, backend/services/storage.py, backend/tests/test_documents.py</files>
|
||||
<read_first>
|
||||
- backend/services/storage.py — read lines 143-179 (full delete_document function) to understand current MinIO path and quota decrement logic that must be preserved for MinIO docs
|
||||
- backend/api/documents.py — find and read the existing @router.delete("/{doc_id}") handler (search for "router.delete" or "delete_document") to see its current signature and body
|
||||
- backend/storage/__init__.py — read get_storage_backend_for_document signature (lines 53-132) to confirm it takes (document: Document, user: User, session: AsyncSession)
|
||||
- backend/api/admin.py — read lines 520-545 to see the canonical cloud delete pattern (get_storage_backend_for_document + backend.delete_object in try/except)
|
||||
- backend/tests/test_documents.py — read the full file to understand conftest fixtures (async_client, auth_user, db_session) and _make_doc or equivalent helper patterns
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md — Pattern 1 (cloud routing preferred in API layer), Pitfall 1 (skip_quota), Pitfall 2 (MinIO no-op on missing keys)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- test_delete_cloud_document_propagates: Create a Document with storage_backend="google_drive". Mock get_storage_backend_for_document to return a mock backend. DELETE /api/documents/{id}. Assert the mock backend's delete_object was called once with the document's object_key. Assert quota UPDATE was NOT executed (cloud docs never charged quota).
|
||||
- test_delete_cloud_document_failure: Create a Document with storage_backend="google_drive". Mock get_storage_backend_for_document to return a mock backend whose delete_object raises Exception("provider error"). DELETE /api/documents/{id}. Assert HTTP 200. Assert response body has cloud_delete_failed=True and success=False. Assert the DB row is NOT deleted (doc still exists after the call).
|
||||
- test_delete_cloud_remove_only: Create a Document with storage_backend="google_drive". DELETE /api/documents/{id}?remove_only=true WITHOUT mocking the cloud backend. Assert HTTP 200. Assert DB row is deleted. Assert quota UPDATE was NOT executed.
|
||||
</behavior>
|
||||
<action>
|
||||
Make two file changes:
|
||||
|
||||
CHANGE 1 — backend/services/storage.py: add skip_quota parameter to delete_document():
|
||||
Change the function signature to `async def delete_document(session: AsyncSession, doc_id: str, skip_quota: bool = False) -> bool:`.
|
||||
Wrap the existing quota decrement block in `if not skip_quota:` so it only runs when skip_quota is False (i.e., for MinIO documents). The MinIO `_backend().delete_object(doc.object_key)` call stays where it is — it is only reached for MinIO docs once the API layer routing is correct (the API layer will handle cloud routing before calling this function).
|
||||
|
||||
CHANGE 2 — backend/api/documents.py: add remove_only param + cloud routing to the delete endpoint:
|
||||
Add `remove_only: bool = Query(default=False)` to the existing delete_document endpoint handler signature.
|
||||
|
||||
In the handler body, BEFORE the call to `storage.delete_document()`, add cloud routing logic:
|
||||
|
||||
If `doc.storage_backend != "minio"` and `not remove_only`:
|
||||
- Try: call `cloud_backend = await get_storage_backend_for_document(doc, current_user, session)`; then `await cloud_backend.delete_object(doc.object_key)`
|
||||
- Except Exception: return JSONResponse(status_code=200, content={"success": False, "cloud_delete_failed": True, "detail": "Cloud provider delete failed. You can remove from app only."})
|
||||
- (If cloud delete succeeds, fall through to DB delete with skip_quota=True)
|
||||
|
||||
If `doc.storage_backend != "minio"` (regardless of remove_only): call `storage.delete_document(session, str(doc.id), skip_quota=True)`
|
||||
If `doc.storage_backend == "minio"`: call `storage.delete_document(session, str(doc.id), skip_quota=False)` (existing behavior)
|
||||
|
||||
The import for `get_storage_backend_for_document` must be added at the top of documents.py (or lazily inside the handler body following the lazy-import pattern already in the file). Also import `JSONResponse` from `fastapi.responses` if not already imported. Add `from fastapi import Query` if not already imported.
|
||||
|
||||
CRITICAL: The cloud delete exception handler must NOT include the exception message `str(exc)` in the response body. The generic detail string is sufficient. Log the exception to stderr internally if desired: `print(f"[cloud-delete] provider error: {exc}", file=sys.stderr)`.
|
||||
|
||||
Then in backend/tests/test_documents.py: promote the three xfail stubs to real tests using unittest.mock.patch and the async_client fixture pattern established in the existing file. Mock `api.documents.get_storage_backend_for_document` (the import path used in documents.py) or `storage.get_storage_backend_for_document` depending on how the import appears in the delete handler. Use `AsyncMock` for the mock backend's delete_object method.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_documents.py::test_delete_cloud_document_propagates tests/test_documents.py::test_delete_cloud_document_failure tests/test_documents.py::test_delete_cloud_remove_only -x -v 2>&1 | tail -20</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- All three promoted tests pass
|
||||
- `grep "skip_quota" backend/services/storage.py` returns a match
|
||||
- `grep "remove_only" backend/api/documents.py` returns a match
|
||||
- `grep "cloud_delete_failed" backend/api/documents.py` returns a match
|
||||
- `grep "get_storage_backend_for_document" backend/api/documents.py` returns a match
|
||||
- `pytest tests/test_documents.py -x -q` exits 0
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Frontend — CloudDeleteWarningModal + remove_only path in DocumentView</name>
|
||||
<files>frontend/src/views/DocumentView.vue, frontend/src/api/client.js</files>
|
||||
<read_first>
|
||||
- frontend/src/views/DocumentView.vue — read the full file to find: existing confirmDelete (or equivalent delete handler) function, existing router.push('/') navigation, how the document object is loaded, and the template structure where the modal should be inserted
|
||||
- frontend/src/api/client.js — search for "deleteDocument" or the function that calls DELETE /api/documents/{id} to understand the current implementation; also read lines 399-428 (fetchDocumentContent) to understand the raw fetch pattern used for authenticated non-JSON responses
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md — C-3 component contract for the CloudDeleteWarningModal, Copywriting Contract for exact copy, State Inventory for states required
|
||||
</read_first>
|
||||
<action>
|
||||
Make two file changes:
|
||||
|
||||
CHANGE 1 — frontend/src/api/client.js:
|
||||
Find the existing deleteDocument function. Modify it to accept an optional `removeOnly = false` parameter. The delete endpoint call should append `?remove_only=true` to the URL when removeOnly is true: `/api/documents/${docId}?remove_only=true`. The response is always JSON (HTTP 200 for both success and cloud_delete_failed), so keep `res.json()` via the existing `request()` wrapper. The function should return the parsed JSON body (not throw on success) so callers can inspect `cloud_delete_failed`.
|
||||
|
||||
If the existing deleteDocument function uses `request()` and it throws on non-2xx, wrap accordingly: HTTP 200 with cloud_delete_failed body is a valid 2xx response so `request()` will return it normally.
|
||||
|
||||
Add a second function `deleteDocumentRemoveOnly(docId)` that calls `deleteDocument(docId, true)` — a convenience wrapper for the remove_only path called from the modal CTA.
|
||||
|
||||
CHANGE 2 — frontend/src/views/DocumentView.vue:
|
||||
Add two new reactive refs to the script section:
|
||||
- `showCloudDeleteWarning` (boolean, default false)
|
||||
- `cloudProviderName` (string, default 'your cloud storage')
|
||||
|
||||
Modify the existing delete handler (confirmDelete or equivalent). After the delete API call, instead of always navigating to '/', check the response:
|
||||
- If `response.cloud_delete_failed === true`: set `cloudProviderName.value` from document's storage_backend (map "google_drive" → "Google Drive", "onedrive" → "OneDrive", "nextcloud" → "Nextcloud", "webdav" → "WebDAV", fallback to "your cloud storage"); set `showCloudDeleteWarning.value = true`; do NOT navigate
|
||||
- Otherwise (success): navigate to `/` as before
|
||||
|
||||
Add a `confirmRemoveOnly()` async function:
|
||||
- Call `await api.deleteDocumentRemoveOnly(props.docId)` (or however the document ID is referenced in DocumentView)
|
||||
- On success: `showCloudDeleteWarning.value = false`; navigate to `/`
|
||||
- On error: show an inline error message within the modal (reuse existing error display pattern)
|
||||
|
||||
Add `cancelCloudDeleteWarning()` function: set `showCloudDeleteWarning.value = false`; abort — document is NOT deleted.
|
||||
|
||||
In the template, add the cloud delete warning modal as an inline conditional block (`v-if="showCloudDeleteWarning"`) following the C-3 contract from UI-SPEC:
|
||||
- Fixed overlay with `@click.self="cancelCloudDeleteWarning"`
|
||||
- Panel with `role="dialog"` `aria-modal="true"` `aria-labelledby="cloud-delete-modal-title"`
|
||||
- Heading id="cloud-delete-modal-title": "Cloud delete failed"
|
||||
- ExclamationTriangleIcon inline SVG (w-5 h-5 text-amber-500): use the Heroicons stroke SVG path for the triangle: `<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z" />`
|
||||
- Body text using cloudProviderName: `The file could not be deleted from {{ cloudProviderName }}. Remove it from DocuVault anyway? The file will remain on {{ cloudProviderName }}.`
|
||||
- Buttons: "Remove from app" (@click="confirmRemoveOnly") and "Cancel" (@click="cancelCloudDeleteWarning")
|
||||
- Exact Tailwind classes from UI-SPEC C-3 (bg-red-600, bg-white rounded-2xl, etc.)
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/frontend && grep -n "showCloudDeleteWarning\|cloud_delete_failed\|removeOnly\|remove_only" src/views/DocumentView.vue src/api/client.js | head -20</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- `grep "showCloudDeleteWarning" frontend/src/views/DocumentView.vue` returns at least 2 matches (ref + template v-if)
|
||||
- `grep "cloud-delete-modal-title" frontend/src/views/DocumentView.vue` returns a match
|
||||
- `grep "remove_only" frontend/src/api/client.js` returns a match
|
||||
- `grep "Remove from app" frontend/src/views/DocumentView.vue` returns a match
|
||||
- No console errors when building: `cd frontend && npm run build 2>&1 | grep -i error | head -10` returns empty (or pre-existing errors only)
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| browser → DELETE /api/documents/{id} | remove_only query param is user-supplied; must not bypass ownership checks |
|
||||
| api/documents.py → cloud backend | cloud credentials must not appear in the error response returned to the browser |
|
||||
| api/documents.py → services/storage.py | skip_quota flag must be set correctly to prevent quota underflow |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-06.2-03-01 | Tampering | Quota underflow on cloud delete | mitigate | `skip_quota=True` passed to `delete_document()` for all non-minio documents; cloud docs never had quota charged at upload |
|
||||
| T-06.2-03-02 | Information Disclosure | Cloud credential exposure in error response | mitigate | Exception caught as generic `except Exception`; only a fixed string "Cloud provider delete failed." returned to client — `str(exc)` is logged to stderr only, never serialized to JSON response |
|
||||
| T-06.2-03-03 | Elevation of Privilege | remove_only param bypasses ownership | accept | Ownership assertion (`doc.user_id != current_user.id → 404`) occurs BEFORE the remove_only branch — authenticated user must own the document regardless of query param value |
|
||||
| T-06.2-03-04 | Spoofing | Silent MinIO no-op for cloud docs | mitigate | Cloud routing happens before any MinIO call for non-minio documents — `get_storage_backend_for_document()` returns the cloud backend, not the MinIO singleton (Pitfall 2 from RESEARCH.md) |
|
||||
| T-06.2-SC | Tampering | npm/pip/cargo installs | accept | No new packages installed in this plan |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
After both tasks complete:
|
||||
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_documents.py -x -q
|
||||
```
|
||||
|
||||
Expected: exits 0, 3 promoted cloud-delete tests pass, all pre-existing tests still pass.
|
||||
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest -v 2>&1 | tail -20
|
||||
```
|
||||
|
||||
Expected: zero failures.
|
||||
|
||||
Frontend build check:
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/frontend && npm run build 2>&1 | grep -c "error" || echo "0 errors"
|
||||
```
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- DELETE /api/documents/{id} for a cloud doc calls cloud backend delete_object — confirmed by test_delete_cloud_document_propagates
|
||||
- Cloud delete failure returns HTTP 200 with cloud_delete_failed=True — confirmed by test_delete_cloud_document_failure
|
||||
- remove_only=true skips cloud, removes DB row, skips quota decrement — confirmed by test_delete_cloud_remove_only
|
||||
- Cloud doc deletes never decrement quota (skip_quota=True path)
|
||||
- DocumentView.vue shows CloudDeleteWarningModal when cloud_delete_failed is received
|
||||
- "Remove from app" calls DELETE ?remove_only=true and navigates to /
|
||||
- All test_documents.py tests pass
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-03-SUMMARY.md` when done.
|
||||
</output>
|
||||
@@ -0,0 +1,393 @@
|
||||
---
|
||||
phase: "06.2"
|
||||
plan: "04"
|
||||
type: execute
|
||||
wave: 2
|
||||
depends_on:
|
||||
- "06.2-02"
|
||||
- "06.2-03"
|
||||
files_modified:
|
||||
- backend/api/audit.py
|
||||
- frontend/src/components/admin/AuditLogTab.vue
|
||||
- frontend/src/api/client.js
|
||||
- backend/tests/test_audit.py
|
||||
autonomous: true
|
||||
requirements:
|
||||
- ADMIN-06
|
||||
must_haves:
|
||||
truths:
|
||||
- "Audit log JSON viewer returns user_handle and actor_handle alongside user_id and actor_id"
|
||||
- "GET /api/admin/audit-log?user_handle=X filters to entries for that user"
|
||||
- "GET /api/admin/audit-log?user_handle=nonexistent returns empty items list, not 422"
|
||||
- "CSV export button in AuditLogTab downloads a file via fetch+Blob (not window.location.href)"
|
||||
- "GET /api/admin/audit-log/daily-exports returns sorted list of available export dates"
|
||||
- "GET /api/admin/audit-log/daily-exports/{date} streams the CSV for that date"
|
||||
- "Daily exports section in AuditLogTab shows date dropdown + Download button"
|
||||
- "Date path parameter validated against YYYY-MM-DD regex before MinIO key construction"
|
||||
artifacts:
|
||||
- path: "backend/api/audit.py"
|
||||
provides: "handle-enriched query; user_handle filter; two daily-export endpoints"
|
||||
contains: "_audit_to_dict_with_handles"
|
||||
- path: "frontend/src/api/client.js"
|
||||
provides: "adminExportAuditLogCsv(), adminListDailyExports(), adminDownloadDailyExport()"
|
||||
contains: "adminExportAuditLogCsv"
|
||||
- path: "frontend/src/components/admin/AuditLogTab.vue"
|
||||
provides: "fixed exportCsv(), daily exports section, user_handle filter label"
|
||||
contains: "Daily exports"
|
||||
key_links:
|
||||
- from: "backend/api/audit.py list_audit_log"
|
||||
to: "User table (aliased twice)"
|
||||
via: "outerjoin on user_id and actor_id FKs"
|
||||
pattern: "outerjoin.*UserSubject|outerjoin.*UserActor"
|
||||
- from: "backend/api/audit.py list_daily_exports"
|
||||
to: "MinIO audit-logs bucket"
|
||||
via: "asyncio.to_thread(_list)"
|
||||
pattern: "asyncio.to_thread"
|
||||
- from: "frontend/src/components/admin/AuditLogTab.vue:exportCsv"
|
||||
to: "adminExportAuditLogCsv() in client.js"
|
||||
via: "fetch() + Blob URL — no window.location.href"
|
||||
pattern: "adminExportAuditLogCsv"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Close the ADMIN-06 gaps in a single vertical slice: user handles in audit log responses, handle-based filter, fixed CSV export download, and a new daily-export listing + download UI.
|
||||
|
||||
Purpose: Admins can now see who performed actions by name (not UUID), filter by handle without 422 errors, download exports that actually arrive (not a 401 from window.location.href), and access the Celery-generated daily export files from the admin panel.
|
||||
|
||||
Output: Modified audit.py (handle JOIN, user_handle filter, two new endpoints), modified AuditLogTab.vue (filter label, fetch+Blob exportCsv, daily-export section), new client.js functions, five promoted test stubs.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/ROADMAP.md
|
||||
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-CONTEXT.md
|
||||
|
||||
<interfaces>
|
||||
<!-- Key types and contracts the executor needs. Extracted from codebase. -->
|
||||
|
||||
From backend/api/audit.py (current state):
|
||||
def _audit_to_dict(entry: AuditLog) -> dict:
|
||||
# Returns: id, event_type, user_id, actor_id, resource_id, ip_address, metadata_, created_at
|
||||
# Does NOT return user_handle or actor_handle
|
||||
|
||||
def _build_filtered_query(start, end, user_id: Optional[uuid.UUID], event_type):
|
||||
# Accepts user_id as UUID type — FastAPI validates this via Query(Optional[uuid.UUID])
|
||||
# This type annotation causes FastAPI to 422 on non-UUID strings
|
||||
|
||||
@router.get("/audit-log")
|
||||
async def list_audit_log(
|
||||
user_id: Optional[uuid.UUID] = Query(default=None), # BUG: must change to Optional[str]
|
||||
...
|
||||
)
|
||||
|
||||
@router.get("/audit-log/export")
|
||||
async def export_audit_log(
|
||||
user_id: Optional[uuid.UUID] = Query(default=None), # BUG: same fix needed
|
||||
...
|
||||
)
|
||||
# Both endpoints must be updated to accept user_handle: Optional[str]
|
||||
|
||||
From backend/db/models.py (User model — key fields):
|
||||
User.id: UUID
|
||||
User.handle: str (unique, indexed)
|
||||
|
||||
From backend/tasks/audit_tasks.py line 79:
|
||||
key = f"audit-logs/{yesterday.isoformat()}.csv"
|
||||
# MinIO bucket: "audit-logs"
|
||||
# Key pattern: "audit-logs/YYYY-MM-DD.csv"
|
||||
|
||||
From backend/storage/__init__.py:
|
||||
def get_storage_backend() -> StorageBackend:
|
||||
# Returns MinIOBackend; has ._client attribute (Minio SDK instance)
|
||||
|
||||
From backend/storage/minio_backend.py:
|
||||
# _client: Minio SDK instance
|
||||
# _client.list_objects(bucket, prefix, recursive) → synchronous iterator
|
||||
# _client.get_object(bucket, key) → response with .read() and .release_conn()
|
||||
|
||||
From frontend/src/api/client.js (existing patterns):
|
||||
# request() wrapper: always calls res.json() — NOT for CSV responses
|
||||
# fetchDocumentContent() at lines 399-428: raw fetch() pattern with Authorization header
|
||||
# export async function fetchDocumentContent(docId, options = {}) { ... }
|
||||
|
||||
From frontend/src/components/admin/AuditLogTab.vue (current state):
|
||||
# filters reactive object: { start, end, user_id, event_type }
|
||||
# exportCsv() at lines 185-192: uses window.location.href (broken)
|
||||
# fetchLog() sends user_id: filters.user_id to adminListAuditLog()
|
||||
# Table renders: entry.user_handle || entry.user_id || '—' (line 89 — already expects handle)
|
||||
|
||||
From .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md:
|
||||
C-4: Daily Exports Section — below pagination block, border-t separator
|
||||
C-5: User filter label change from "User" to "User handle"
|
||||
Copywriting: section label "Daily exports", dropdown label "Select date", button "Download"
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Backend — handle enrichment, user_handle filter, two daily-export endpoints</name>
|
||||
<files>backend/api/audit.py, backend/tests/test_audit.py</files>
|
||||
<read_first>
|
||||
- backend/api/audit.py — read the full file; understand _audit_to_dict(), _build_filtered_query(), both existing endpoints and their exact Query parameter signatures; understand how both endpoints share _build_filtered_query
|
||||
- backend/db/models.py — search for "class User" and "class AuditLog" to confirm handle field and user_id/actor_id FK field names
|
||||
- backend/storage/__init__.py — read lines 32-50 (get_storage_backend factory) to understand how to get the MinIOBackend instance for the daily-export endpoints; confirm _client attribute
|
||||
- backend/tasks/audit_tasks.py — read lines 78-86 to confirm the MinIO bucket name ("audit-logs") and key pattern ("audit-logs/YYYY-MM-DD.csv")
|
||||
- backend/tests/test_audit.py — read the full file to understand _seed_audit helper, admin_user fixture, and existing test patterns before promoting stubs
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md — Pattern 3 (aliased double-JOIN), Pattern 4 (handle-to-UUID resolution), Pattern 6 (list_objects), Pattern 7 (daily export streaming), Pitfall 4 (COUNT query breaks after JOIN), Pitfall 6 (date regex), Pitfall 7 (both endpoints must use enriched function)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- test_audit_log_includes_user_handle: Seed an audit entry for admin_user. GET /api/admin/audit-log. Assert each item in items has keys "user_handle" and "actor_handle". Assert the first item's user_handle matches admin_user["user"].handle (not None for a seeded entry).
|
||||
- test_audit_log_filter_by_handle: Seed one entry for admin_user. Seed one entry for a second distinct user. GET /api/admin/audit-log?user_handle={admin_user.handle}. Assert items contains only entries matching admin_user (user_handle == admin_user.handle). Seeded second entry must not appear.
|
||||
- test_audit_log_filter_unknown_handle: GET /api/admin/audit-log?user_handle=definitely_does_not_exist. Assert status 200. Assert response body items == []. Assert total == 0. Assert no 422 error.
|
||||
- test_daily_exports_list: Mock MinIOBackend._client.list_objects to return fake objects (or patch get_storage_backend and its _client). GET /api/admin/audit-log/daily-exports. Assert status 200. Assert response has "items" key. Items sorted descending by date.
|
||||
- test_daily_export_download: Mock MinIOBackend._client.get_object to return fake CSV bytes. GET /api/admin/audit-log/daily-exports/2026-05-30. Assert status 200. Assert Content-Type: text/csv. Assert Content-Disposition header contains "2026-05-30". Also test GET /api/admin/audit-log/daily-exports/invalid-date returns 404.
|
||||
</behavior>
|
||||
<action>
|
||||
Make these changes to backend/api/audit.py:
|
||||
|
||||
CHANGE 1 — Add SQLAlchemy aliased imports and User import check:
|
||||
Add `from sqlalchemy.orm import aliased` to the imports if not already present. Confirm `User` is already imported from `db.models`.
|
||||
|
||||
CHANGE 2 — New helper _audit_to_dict_with_handles():
|
||||
Add a new function `_audit_to_dict_with_handles(entry: AuditLog, user_handle: Optional[str], actor_handle: Optional[str]) -> dict` that returns the same dict as `_audit_to_dict(entry)` PLUS two additional keys: `"user_handle": user_handle or None` and `"actor_handle": actor_handle or None`. Do NOT remove or rename `_audit_to_dict` — preserve it as a fallback.
|
||||
|
||||
CHANGE 3 — New query builder _build_filtered_query_with_handles():
|
||||
Add function `_build_filtered_query_with_handles(start, end, user_uuid, event_type)` that builds a multi-column select:
|
||||
```
|
||||
UserSubject = aliased(User)
|
||||
UserActor = aliased(User)
|
||||
stmt = (
|
||||
select(AuditLog, UserSubject.handle.label("user_handle"), UserActor.handle.label("actor_handle"))
|
||||
.outerjoin(UserSubject, UserSubject.id == AuditLog.user_id)
|
||||
.outerjoin(UserActor, UserActor.id == AuditLog.actor_id)
|
||||
.order_by(AuditLog.created_at.desc())
|
||||
)
|
||||
```
|
||||
Apply the same start/end/user_uuid/event_type filters as the original `_build_filtered_query`. Return the statement. This is a standalone function, NOT replacing `_build_filtered_query` (the old function stays for the count query — see Pitfall 4).
|
||||
|
||||
CHANGE 4 — Update list_audit_log endpoint:
|
||||
Change `user_id: Optional[uuid.UUID] = Query(default=None)` to `user_handle: Optional[str] = Query(default=None)`.
|
||||
|
||||
Add handle-to-UUID resolution logic before executing the main query (Pattern 4 from RESEARCH.md):
|
||||
```python
|
||||
user_uuid: Optional[uuid.UUID] = None
|
||||
if user_handle:
|
||||
result = await session.execute(select(User.id).where(User.handle == user_handle))
|
||||
uid = result.scalar_one_or_none()
|
||||
if uid is None:
|
||||
return {"items": [], "total": 0, "page": page, "per_page": per_page}
|
||||
user_uuid = uid
|
||||
```
|
||||
|
||||
For the count query, use the ORIGINAL `_build_filtered_query(start, end, user_uuid, event_type)` to avoid the COUNT subquery problem (Pitfall 4). Count query is unchanged.
|
||||
|
||||
For the data query, use `_build_filtered_query_with_handles(start, end, user_uuid, event_type)`. Add `.limit(per_page).offset((page - 1) * per_page)`. Execute. Iterate `result.all()` as tuples: `for row in rows: entry, user_handle_val, actor_handle_val = row[0], row[1], row[2]`. Build each item with `_audit_to_dict_with_handles(entry, user_handle_val, actor_handle_val)`.
|
||||
|
||||
CHANGE 5 — Update export_audit_log endpoint:
|
||||
Apply the same user_handle→user_uuid resolution (identical block as above). Use `_build_filtered_query_with_handles` for the data query. Iterate rows as tuples. Use `_audit_to_dict_with_handles` for CSV serialization. Add `"user_handle"` and `"actor_handle"` to the `fields` list for the CSV DictWriter. This satisfies Pitfall 7 (both endpoints must use enriched function).
|
||||
|
||||
CHANGE 6 — Add two new endpoints for daily exports:
|
||||
Before the existing endpoints, add necessary imports: `import asyncio`, `import re`. The `StreamingResponse` import should already be present.
|
||||
|
||||
Add endpoint `@router.get("/audit-log/daily-exports")`:
|
||||
- Auth: `_admin: User = Depends(get_current_admin)`
|
||||
- No session param needed (MinIO call only)
|
||||
- Body: get the MinIO backend via `from storage import get_storage_backend; from storage.minio_backend import MinIOBackend; backend = get_storage_backend()`. If not MinIOBackend, return `{"items": []}`.
|
||||
- Define inner `_list() -> list[dict]` function (synchronous) that calls `backend._client.list_objects("audit-logs", prefix="audit-logs/", recursive=False)`, iterates objects, filters `.endswith(".csv")`, extracts date from `obj.object_name.removeprefix("audit-logs/").removesuffix(".csv")`, builds `{"date": date_str, "key": obj.object_name}`, sorts by date descending.
|
||||
- Execute: `items = await asyncio.to_thread(_list)`
|
||||
- Return `{"items": items}`
|
||||
|
||||
Add endpoint `@router.get("/audit-log/daily-exports/{date}")`:
|
||||
- Auth: `_admin: User = Depends(get_current_admin)`
|
||||
- Path param: `date: str`
|
||||
- Date validation (Pitfall 6 / D-16): `if not re.fullmatch(r"\d{4}-\d{2}-\d{2}", date): raise HTTPException(404, "Invalid date format")`
|
||||
- Get backend, construct key = `f"audit-logs/{date}.csv"`
|
||||
- Define inner `_get() -> bytes` (synchronous): `response = backend._client.get_object("audit-logs", key); try: return response.read(); finally: response.close(); response.release_conn()`
|
||||
- Execute: wrap in `try: csv_bytes = await asyncio.to_thread(_get); except Exception: raise HTTPException(404, "Export not found")`
|
||||
- Return `StreamingResponse(iter([csv_bytes]), media_type="text/csv", headers={"Content-Disposition": f'attachment; filename="audit-{date}.csv"'})`
|
||||
|
||||
CRITICAL: The two new endpoints must be placed BEFORE the existing `@router.get("/audit-log/export")` and `@router.get("/audit-log")` in the file, because FastAPI routes are matched in registration order. The path `/audit-log/daily-exports` is more specific than `/audit-log` and must be registered first. Or, at minimum, place them before the `@router.get("/audit-log")` GET handler.
|
||||
|
||||
Then in backend/tests/test_audit.py: promote all five xfail stubs. Use `unittest.mock.patch` to mock `storage.get_storage_backend` for the daily-export endpoint tests, returning a mock MinIOBackend with a `_client` mock.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_audit.py::test_audit_log_includes_user_handle tests/test_audit.py::test_audit_log_filter_by_handle tests/test_audit.py::test_audit_log_filter_unknown_handle tests/test_audit.py::test_daily_exports_list tests/test_audit.py::test_daily_export_download -x -v 2>&1 | tail -25</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- All five promoted tests pass
|
||||
- `grep "_audit_to_dict_with_handles" backend/api/audit.py` returns at least 2 matches (definition + both endpoint usages — Pitfall 7)
|
||||
- `grep "user_handle" backend/api/audit.py` returns at least 4 matches
|
||||
- `grep "daily-exports" backend/api/audit.py` returns 2 matches (two new endpoints)
|
||||
- `grep "fullmatch" backend/api/audit.py` returns a match (date regex validation)
|
||||
- `pytest tests/test_audit.py -x -q` exits 0
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Frontend — user_handle filter, fetch+Blob export, daily-export section</name>
|
||||
<files>frontend/src/components/admin/AuditLogTab.vue, frontend/src/api/client.js</files>
|
||||
<read_first>
|
||||
- frontend/src/components/admin/AuditLogTab.vue — read the full file; understand filters reactive object (filters.user_id must become filters.user_handle), fetchLog() which passes params to adminListAuditLog(), exportCsv() (broken window.location.href on lines 185-192), pagination block location (where to add the new daily-export section below it)
|
||||
- frontend/src/api/client.js — read lines 395-435 (fetchDocumentContent — the fetch+Blob reference pattern); search for "adminListAuditLog" to find its current implementation; note that request() wrapper always calls res.json() and must NOT be used for CSV responses
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md — Pattern 5 (fetch+Blob URL for CSV), Pattern 6 (adminListDailyExports signature), Pattern 7 (adminDownloadDailyExport)
|
||||
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md — C-4 (daily exports section markup), C-5 (user filter label), Copywriting Contract (section copy), State Inventory (loading/empty/populated states)
|
||||
</read_first>
|
||||
<action>
|
||||
Make two file changes:
|
||||
|
||||
CHANGE 1 — frontend/src/api/client.js: add three new functions
|
||||
Follow the exact fetch+Blob pattern from fetchDocumentContent (lines 399-428) — NOT using the request() wrapper.
|
||||
|
||||
Add `adminExportAuditLogCsv(params = {})`:
|
||||
- Import useAuthStore lazily (same pattern as fetchDocumentContent)
|
||||
- Build URLSearchParams with format=csv; add start, end, event_type if provided; add user_handle if provided (NOT user_id — the backend param is now user_handle)
|
||||
- Raw fetch to `/api/admin/audit-log/export?${searchParams}` with Authorization Bearer header and credentials: 'include'
|
||||
- On !res.ok: throw Error(`Export failed: ${res.status}`)
|
||||
- `const text = await res.text()` (NOT res.json())
|
||||
- Create Blob([text], { type: 'text/csv' }), URL.createObjectURL, create `<a>` element, set href + download='audit-export.csv', click, URL.revokeObjectURL
|
||||
|
||||
Add `adminListDailyExports()`:
|
||||
- Raw fetch to `/api/admin/audit-log/daily-exports` with Authorization Bearer header
|
||||
- On !res.ok: throw Error
|
||||
- Return `await res.json()` — this endpoint returns JSON
|
||||
|
||||
Add `adminDownloadDailyExport(date)`:
|
||||
- Raw fetch to `/api/admin/audit-log/daily-exports/${date}` with Authorization Bearer header and credentials: 'include'
|
||||
- On !res.ok: throw Error(`Download failed: ${res.status}`)
|
||||
- `const text = await res.text()`
|
||||
- Blob + URL.createObjectURL + `<a>` click with download=`audit-${date}.csv` + revokeObjectURL
|
||||
|
||||
CHANGE 2 — frontend/src/components/admin/AuditLogTab.vue: three UI changes
|
||||
|
||||
CHANGE 2a — User filter label and binding (per D-12, C-5):
|
||||
In the filters reactive object, rename `user_id: ''` to `user_handle: ''`.
|
||||
In the fetchLog() function, change `user_id: filters.user_id || undefined` to `user_handle: filters.user_handle || undefined`.
|
||||
In the template filter bar, change the label text from "User" to "User handle". Change `v-model="filters.user_id"` to `v-model="filters.user_handle"`.
|
||||
Update adminListAuditLog() call to pass `user_handle` not `user_id` (check the existing call signature in fetchLog).
|
||||
|
||||
CHANGE 2b — Fix exportCsv() (per D-13):
|
||||
Replace the entire body of `function exportCsv()` with an async call to `api.adminExportAuditLogCsv({...})`. Change the function declaration to `async function exportCsv()`. Pass current filter values: `start: filters.start || undefined, end: filters.end || undefined, user_handle: filters.user_handle || undefined, event_type: filters.event_type || undefined`. Add a ref `exportingCsv` (boolean, default false) and set it true/false around the call. On error: show an alert or set an error ref with "Export failed. Please try again."
|
||||
|
||||
CHANGE 2c — Add daily exports section (per D-17, C-4 from UI-SPEC):
|
||||
Add new reactive state in script setup:
|
||||
- `dailyExports` ref (Array, default [])
|
||||
- `loadingExports` ref (boolean, default false)
|
||||
- `selectedExportDate` ref (string, default '')
|
||||
- `downloadingExport` ref (boolean, default false)
|
||||
- `exportsError` ref (string, default null)
|
||||
|
||||
Add `loadDailyExports()` async function that calls `await api.adminListDailyExports()` and populates `dailyExports.value` from `data.items`. Set `loadingExports` accordingly. Call `loadDailyExports()` inside `onMounted()` alongside the existing `fetchLog()` call.
|
||||
|
||||
Add `downloadDailyExport()` async function that calls `await api.adminDownloadDailyExport(selectedExportDate.value)`. Set `downloadingExport` true/false. On error: set `exportsError.value = "Download failed. Please try again."`.
|
||||
|
||||
In the template, add the daily-export section below the pagination block, following C-4 markup from UI-SPEC:
|
||||
- Section separator: `<div class="border-t border-gray-100 mt-6 pt-6">`
|
||||
- Section label: `<h3 class="text-sm font-semibold text-gray-700 mb-3">Daily exports</h3>`
|
||||
- Loading state: `<p v-if="loadingExports" class="text-sm text-gray-400">Loading exports…</p>`
|
||||
- Empty state: `<p v-else-if="dailyExports.length === 0" class="text-sm text-gray-400 italic">No daily exports available.</p>`
|
||||
- Controls row (v-else): `<div class="flex items-end gap-3">`
|
||||
- `<select v-model="selectedExportDate" class="text-sm border border-gray-300 rounded-lg px-3 py-2 focus:outline-none focus:ring-2 focus:ring-indigo-500 bg-white">`
|
||||
- `<option value="" disabled>Choose a date</option>`
|
||||
- `<option v-for="exp in dailyExports" :key="exp.date" :value="exp.date">{{ exp.date }}</option>`
|
||||
- `<button @click="downloadDailyExport" :disabled="!selectedExportDate || downloadingExport" class="bg-indigo-600 hover:bg-indigo-700 text-white text-sm px-4 py-2 rounded-lg disabled:opacity-50 transition-colors">`
|
||||
- Loading spinner inline when downloadingExport (same animate-spin pattern as ShareModal)
|
||||
- "Download" text otherwise
|
||||
- Error display: `<p v-if="exportsError" class="text-xs text-red-600 mt-2">{{ exportsError }}</p>`
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/frontend && grep -n "adminExportAuditLogCsv\|adminListDailyExports\|adminDownloadDailyExport" src/api/client.js | head -10</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- `grep "adminExportAuditLogCsv" frontend/src/api/client.js` returns a match
|
||||
- `grep "adminListDailyExports" frontend/src/api/client.js` returns a match
|
||||
- `grep "adminDownloadDailyExport" frontend/src/api/client.js` returns a match
|
||||
- `grep "window.location.href" frontend/src/components/admin/AuditLogTab.vue` returns NO match (broken export removed)
|
||||
- `grep "Daily exports" frontend/src/components/admin/AuditLogTab.vue` returns a match
|
||||
- `grep "User handle" frontend/src/components/admin/AuditLogTab.vue` returns a match
|
||||
- `grep "user_handle" frontend/src/components/admin/AuditLogTab.vue` returns at least 2 matches (filter binding + fetchLog param)
|
||||
- No build errors: `cd /Users/nik/Documents/Progamming/document_scanner/frontend && npm run build 2>&1 | grep -i "error" | grep -v "^>" | head -10` returns empty
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| browser → GET /api/admin/audit-log/daily-exports/{date} | date path param is user-supplied; must not allow MinIO key injection |
|
||||
| api/audit.py → MinIO | asyncio.to_thread isolates sync SDK from the async event loop |
|
||||
| AuditLogTab → /api/admin/audit-log/export | fetch() must carry Bearer header; window.location.href cannot |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-06.2-04-01 | Tampering | Date path parameter injection | mitigate | `re.fullmatch(r"\d{4}-\d{2}-\d{2}", date)` validates before `f"audit-logs/{date}.csv"` key construction — rejects any non-date string including path traversal sequences (Pitfall 6 from RESEARCH.md) |
|
||||
| T-06.2-04-02 | Elevation of Privilege | Unauthenticated daily-export access | mitigate | Both new endpoints use `_admin: User = Depends(get_current_admin)` — regular users receive 403, unauthenticated receive 401 |
|
||||
| T-06.2-04-03| Information Disclosure | Audit log CSV token bypass via window.location.href | mitigate | exportCsv() replaced with fetch()+Blob pattern that sends Authorization Bearer header — no unauthenticated CSV download possible |
|
||||
| T-06.2-04-04 | Information Disclosure | user_handle in audit response leaks PII | accept | handle is already public within the platform (users are identified by handle in sharing UI); admin view of handles is consistent with existing admin privileges |
|
||||
| T-06.2-04-05 | Denial of Service | list_objects blocking event loop | mitigate | `asyncio.to_thread(_list)` wraps synchronous Minio iterator — event loop is not blocked |
|
||||
| T-06.2-SC | Tampering | npm/pip/cargo installs | accept | No new packages installed in this plan |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
After both tasks complete:
|
||||
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_audit.py -x -q
|
||||
```
|
||||
|
||||
Expected: exits 0, all 9 tests pass (4 pre-existing + 5 promoted).
|
||||
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest -v 2>&1 | tail -20
|
||||
```
|
||||
|
||||
Expected: zero failures.
|
||||
|
||||
Phase gate — full suite:
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest -v 2>&1 | grep -E "passed|failed|error" | tail -5
|
||||
```
|
||||
|
||||
Frontend:
|
||||
```
|
||||
cd /Users/nik/Documents/Progamming/document_scanner/frontend && npm run build 2>&1 | grep -i "error" | grep -v "^>" | head -10
|
||||
```
|
||||
|
||||
Security spot-checks:
|
||||
```
|
||||
grep "window.location.href" /Users/nik/Documents/Progamming/document_scanner/frontend/src/components/admin/AuditLogTab.vue
|
||||
# Expected: no output (bug removed)
|
||||
|
||||
grep "fullmatch" /Users/nik/Documents/Progamming/document_scanner/backend/api/audit.py
|
||||
# Expected: matches the date regex line
|
||||
|
||||
grep "get_current_admin" /Users/nik/Documents/Progamming/document_scanner/backend/api/audit.py
|
||||
# Expected: 4 matches (2 existing endpoints + 2 new endpoints)
|
||||
```
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Audit log JSON response includes user_handle and actor_handle — confirmed by test_audit_log_includes_user_handle
|
||||
- user_handle filter returns correct filtered results — confirmed by test_audit_log_filter_by_handle
|
||||
- Unknown handle returns empty (not 422) — confirmed by test_audit_log_filter_unknown_handle
|
||||
- Daily export list endpoint returns sorted items — confirmed by test_daily_exports_list
|
||||
- Daily export download streams CSV with regex-validated date — confirmed by test_daily_export_download
|
||||
- AuditLogTab exportCsv() uses fetch+Blob (window.location.href removed)
|
||||
- AuditLogTab user filter labeled "User handle"
|
||||
- AuditLogTab has Daily exports section with date dropdown and Download button
|
||||
- All 9 test_audit.py tests pass
|
||||
- Full pytest suite exits 0
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-04-SUMMARY.md` when done.
|
||||
</output>
|
||||
Reference in New Issue
Block a user