docs(phase-4): complete plan 04-07 — SUMMARY.md + STATE.md update
This commit is contained in:
+10
-4
@@ -27,13 +27,13 @@ progress:
|
|||||||
| 1 | Infrastructure Foundation | ✓ Complete |
|
| 1 | Infrastructure Foundation | ✓ Complete |
|
||||||
| 2 | Users & Authentication | ✓ Complete (5/5 plans) |
|
| 2 | Users & Authentication | ✓ Complete (5/5 plans) |
|
||||||
| 3 | Document Migration & Multi-User Isolation | ✓ Complete (5/5 plans, 10/10 UAT, security gate passed) |
|
| 3 | Document Migration & Multi-User Isolation | ✓ Complete (5/5 plans, 10/10 UAT, security gate passed) |
|
||||||
| 4 | Folders, Sharing, Quotas & Document UX | In Progress (4/9 plans complete) |
|
| 4 | Folders, Sharing, Quotas & Document UX | In Progress (7/9 plans complete) |
|
||||||
| 5 | Cloud Storage Backends | Not Started |
|
| 5 | Cloud Storage Backends | Not Started |
|
||||||
|
|
||||||
## Current Position
|
## Current Position
|
||||||
|
|
||||||
**Phase:** 04-folders-sharing-quotas-document-ux — In progress
|
**Phase:** 04-folders-sharing-quotas-document-ux — In progress
|
||||||
**Plan:** 4/9 — Wave 0 scaffolds (04-01), migration 0004 + put_object_raw (04-02), Folders API + audit helper (04-03), Sharing API (04-04)
|
**Plan:** 7/9 — Wave 0 scaffolds (04-01), migration 0004 + put_object_raw (04-02), Folders API + audit helper (04-03), Sharing API (04-04), Streaming proxy + preferences (04-05), Quota enforcement (04-06), Audit log backfill + SEC-08/SEC-09 (04-07)
|
||||||
**Progress:** ██████░░░░ 60% (3/5 phases complete)
|
**Progress:** ██████░░░░ 60% (3/5 phases complete)
|
||||||
|
|
||||||
## Performance Metrics
|
## Performance Metrics
|
||||||
@@ -115,6 +115,11 @@ progress:
|
|||||||
| Share IDOR: DELETE returns 404 not 403 | Prevents share ID enumeration; attacker cannot learn which share IDs exist for other users (T-04-04-02) |
|
| Share IDOR: DELETE returns 404 not 403 | Prevents share ID enumeration; attacker cannot learn which share IDs exist for other users (T-04-04-02) |
|
||||||
| /received before /{share_id} in router | Path parameter conflict: FastAPI routes /received as /{share_id}="received" if DELETE is defined first — ordering enforced by comment |
|
| /received before /{share_id} in router | Path parameter conflict: FastAPI routes /received as /{share_id}="received" if DELETE is defined first — ordering enforced by comment |
|
||||||
| No quota touch in shares.py | Recipient's quota is never modified by share operations (T-04-04-04); sharing is metadata-only from quota's perspective |
|
| No quota touch in shares.py | Recipient's quota is never modified by share operations (T-04-04-04); sharing is metadata-only from quota's perspective |
|
||||||
|
| login_failed audit metadata_=None | No email, no hash, no PII in login failure audit events — T-04-07-01 threat mitigation |
|
||||||
|
| document audit metadata whitelist | document.uploaded contains only size_bytes and storage_backend; document.deleted contains only size_bytes — no filename, no extracted_text |
|
||||||
|
| CloudConnectionOut whitelist pattern | Pydantic model with exactly the safe fields; credentials_enc absent by omission — SEC-08 safe-by-default |
|
||||||
|
| admin.user_deleted flush before delete | audit write flushed (session.flush()) while user FK still valid; session.delete(user) follows — preserves audit FK integrity |
|
||||||
|
| test_admin_impersonation 405 acceptable | DELETE /users/{id} causes GET to return 405 not 422; both mean no GET impersonation endpoint; test updated to accept {404, 405, 422} |
|
||||||
|
|
||||||
### Open Questions
|
### Open Questions
|
||||||
|
|
||||||
@@ -155,6 +160,7 @@ _Updated at each phase transition._
|
|||||||
| Last session | 2026-05-25 — Plan 04-02 executed: migration 0004 (pdf_open_mode, GIN FTS index, audit-logs bucket) + MinIOBackend.put_object_raw(); 122 tests pass |
|
| Last session | 2026-05-25 — Plan 04-02 executed: migration 0004 (pdf_open_mode, GIN FTS index, audit-logs bucket) + MinIOBackend.put_object_raw(); 122 tests pass |
|
||||||
| Last session | 2026-05-25 — Plan 04-03 executed: write_audit_log() helper (flush-not-commit, never-raises) + FOLD-01..05 folder API + document sort/FTS/move; 122 pass, 0 new failures |
|
| Last session | 2026-05-25 — Plan 04-03 executed: write_audit_log() helper (flush-not-commit, never-raises) + FOLD-01..05 folder API + document sort/FTS/move; 122 pass, 0 new failures |
|
||||||
| Last session | 2026-05-25 — Plan 04-04 executed: Sharing API (SHARE-01..05) — grant/list/received/revoke with IDOR protection; 7 xfailed, zero new failures |
|
| Last session | 2026-05-25 — Plan 04-04 executed: Sharing API (SHARE-01..05) — grant/list/received/revoke with IDOR protection; 7 xfailed, zero new failures |
|
||||||
| Next action | Continue Wave 3 execution: run plan 04-05 (quota enforcement) |
|
| Last session | 2026-05-25 — Plan 04-07 executed: audit log backfill (D-13, 8 auth + 2 doc + 5 admin events), SEC-08 CloudConnectionOut, SEC-09 delete-user MinIO cleanup; 92 passed, 1 pre-existing failure |
|
||||||
|
| Next action | Continue execution: run plan 04-08 (frontend integration) |
|
||||||
| Pending decisions | None |
|
| Pending decisions | None |
|
||||||
| Resume file | `.planning/phases/04-folders-sharing-quotas-document-ux/04-04-SUMMARY.md` |
|
| Resume file | `.planning/phases/04-folders-sharing-quotas-document-ux/04-07-SUMMARY.md` |
|
||||||
|
|||||||
@@ -0,0 +1,156 @@
|
|||||||
|
---
|
||||||
|
phase: 04-folders-sharing-quotas-document-ux
|
||||||
|
plan: "07"
|
||||||
|
subsystem: security-hardening
|
||||||
|
tags:
|
||||||
|
- audit-log
|
||||||
|
- sec-08
|
||||||
|
- sec-09
|
||||||
|
- d-13
|
||||||
|
- admin
|
||||||
|
|
||||||
|
dependency_graph:
|
||||||
|
requires:
|
||||||
|
- "04-05"
|
||||||
|
- "04-06"
|
||||||
|
provides:
|
||||||
|
- audit-log-backfill-auth
|
||||||
|
- audit-log-backfill-documents
|
||||||
|
- audit-log-backfill-admin
|
||||||
|
- sec-08-credentials-enc-exclusion
|
||||||
|
- sec-09-delete-user-minio-cleanup
|
||||||
|
affects:
|
||||||
|
- backend/api/auth.py
|
||||||
|
- backend/api/admin.py
|
||||||
|
- backend/api/documents.py
|
||||||
|
|
||||||
|
tech_stack:
|
||||||
|
added: []
|
||||||
|
patterns:
|
||||||
|
- "write_audit_log() flush-before-commit pattern (D-14): audit entry flushed in same transaction as primary operation"
|
||||||
|
- "SEC-08 CloudConnectionOut whitelist pattern: Pydantic response model excludes credentials_enc by omission"
|
||||||
|
- "SEC-09 pre-delete MinIO cleanup: collect docs, delete_object best-effort, then session.delete(user)"
|
||||||
|
|
||||||
|
key_files:
|
||||||
|
created: []
|
||||||
|
modified:
|
||||||
|
- backend/api/auth.py
|
||||||
|
- backend/api/admin.py
|
||||||
|
- backend/api/documents.py
|
||||||
|
- backend/tests/test_admin_api.py
|
||||||
|
|
||||||
|
decisions:
|
||||||
|
- "write_audit_log for login_failed uses metadata_=None (no PII) — T-04-07-01"
|
||||||
|
- "document.uploaded and document.deleted audit metadata contains only size_bytes and storage_backend — T-04-07-02"
|
||||||
|
- "audit write for login success written just before return, inside a session.commit() call since create_refresh_token already flushed"
|
||||||
|
- "delete_document audit log written after storage.delete_document commits, in a new flush + commit — storage service owns its own commit"
|
||||||
|
- "admin.user_deleted audit written with session.flush() before session.delete(user) to keep user FK intact"
|
||||||
|
- "test_admin_impersonation_not_found updated to accept 405 in addition to 404/422 (Rule 1 auto-fix)"
|
||||||
|
|
||||||
|
metrics:
|
||||||
|
duration: "~25 minutes"
|
||||||
|
completed_at: "2026-05-25T19:51:37Z"
|
||||||
|
tasks_completed: 2
|
||||||
|
files_modified: 4
|
||||||
|
---
|
||||||
|
|
||||||
|
# Phase 4 Plan 7: Audit Log Backfill, SEC-08 CloudConnectionOut, SEC-09 Delete-User Summary
|
||||||
|
|
||||||
|
## One-liner
|
||||||
|
|
||||||
|
Audit log backfill for all D-13 event categories (auth, document, admin), SEC-08 credentials_enc exclusion via CloudConnectionOut Pydantic model, SEC-09 MinIO object cleanup in delete-user endpoint.
|
||||||
|
|
||||||
|
## What Was Built
|
||||||
|
|
||||||
|
### Task 1: Audit Log Backfill — auth.py and documents.py
|
||||||
|
|
||||||
|
Added `from services.audit import write_audit_log` to both files and backfilled write_audit_log calls into 8 auth handlers and 2 document handlers:
|
||||||
|
|
||||||
|
**auth.py (9 write_audit_log calls):**
|
||||||
|
- `auth.login_failed` — login with wrong credentials; metadata_=None (no PII, T-04-07-01)
|
||||||
|
- `auth.login` — successful login; metadata_={"totp_used": bool}
|
||||||
|
- `auth.backup_code_used` — backup code consumed during login
|
||||||
|
- `auth.logout` — refresh token revoked; user_id extracted from RefreshToken row
|
||||||
|
- `auth.sign_out_all` — all sessions revoked; metadata_={"sessions_revoked": count}
|
||||||
|
- `auth.password_changed` — password updated by user
|
||||||
|
- `auth.totp_enrolled` — TOTP enabled and backup codes generated
|
||||||
|
- `auth.totp_revoked` — TOTP disabled
|
||||||
|
|
||||||
|
Added `request: Request` parameter to `change_password` and `disable_totp` (previously missing).
|
||||||
|
|
||||||
|
**documents.py (2 write_audit_log calls):**
|
||||||
|
- `document.uploaded` — upload confirmed; metadata_={"size_bytes": N, "storage_backend": "minio"} — NO filename, NO extracted_text (T-04-07-02)
|
||||||
|
- `document.deleted` — document deleted; metadata_={"size_bytes": N}
|
||||||
|
|
||||||
|
Added `request: Request` parameter to `confirm_upload` and `delete_document`.
|
||||||
|
|
||||||
|
### Task 2: SEC-08, SEC-09, Admin Audit Writes — admin.py
|
||||||
|
|
||||||
|
**CloudConnectionOut (SEC-08):**
|
||||||
|
```python
|
||||||
|
class CloudConnectionOut(BaseModel):
|
||||||
|
"""SEC-08: credentials_enc deliberately excluded."""
|
||||||
|
id: str
|
||||||
|
provider: str
|
||||||
|
display_name: str
|
||||||
|
status: str
|
||||||
|
connected_at: datetime
|
||||||
|
model_config = {"from_attributes": True}
|
||||||
|
```
|
||||||
|
Model placed in admin.py for Phase 5 import-ready use. credentials_enc absent by whitelist.
|
||||||
|
|
||||||
|
**Admin audit log writes (5 additional write_audit_log calls):**
|
||||||
|
- `admin.user_created` — new user created by admin
|
||||||
|
- `admin.user_deactivated` — user deactivated
|
||||||
|
- `admin.user_activated` — user reactivated
|
||||||
|
- `admin.quota_changed` — metadata_={"old_bytes": N, "new_bytes": M}
|
||||||
|
- `admin.ai_provider_assigned` — metadata_={"provider": ..., "model": ...}
|
||||||
|
|
||||||
|
**DELETE /api/admin/users/{user_id} (SEC-09):**
|
||||||
|
- Auth: `get_current_admin`
|
||||||
|
- Guard: 400 if target user is admin (T-04-07-04)
|
||||||
|
- Collects all Document rows for user via `select(Document).where(Document.user_id == user_id)`
|
||||||
|
- Calls `get_storage_backend().delete_object(doc.object_key)` for each — try/except (best-effort)
|
||||||
|
- Writes `admin.user_deleted` audit log via `session.flush()` BEFORE `session.delete(user)`
|
||||||
|
- Cascades delete to quota, documents, refresh_tokens via FK ON DELETE CASCADE
|
||||||
|
- Returns 204 No Content
|
||||||
|
|
||||||
|
## Deviations from Plan
|
||||||
|
|
||||||
|
### Auto-fixed Issues
|
||||||
|
|
||||||
|
**1. [Rule 1 - Bug] test_admin_impersonation_not_found: 405 not accepted**
|
||||||
|
- **Found during:** Task 2 test run
|
||||||
|
- **Issue:** Adding `DELETE /api/admin/users/{user_id}` caused FastAPI to return 405 Method Not Allowed for GET requests to `/api/admin/users/impersonate` (previously 422 UUID parse error). The test only accepted {404, 422}.
|
||||||
|
- **Fix:** Updated assertion to accept 405 in addition to 404/422. Security invariant is still met: there is no GET impersonation endpoint. 405 means the path is handled but only via DELETE — which is the user-delete endpoint, not impersonation.
|
||||||
|
- **Files modified:** backend/tests/test_admin_api.py
|
||||||
|
- **Commit:** 8e6005c
|
||||||
|
|
||||||
|
## Threat Model Coverage
|
||||||
|
|
||||||
|
| Threat ID | Status |
|
||||||
|
|-----------|--------|
|
||||||
|
| T-04-07-01 | Mitigated: login_failed metadata_=None, no PII |
|
||||||
|
| T-04-07-02 | Mitigated: document audit contains only size_bytes and storage_backend |
|
||||||
|
| T-04-07-03 | Mitigated: CloudConnectionOut model excludes credentials_enc |
|
||||||
|
| T-04-07-04 | Mitigated: DELETE /users/{id} returns 400 for admin accounts |
|
||||||
|
| T-04-07-05 | Mitigated: MinIO objects deleted before DB record in delete-user |
|
||||||
|
| T-04-07-06 | Mitigated: 8 auth event types + 2 doc events + 5 admin events logged |
|
||||||
|
|
||||||
|
## Verification Results
|
||||||
|
|
||||||
|
- `pytest tests/test_auth_api.py tests/test_documents.py -x -v` — 43 passed, 5 xfailed
|
||||||
|
- `pytest tests/test_admin_api.py tests/test_security.py -x -v` — 18 passed, 2 xfailed
|
||||||
|
- `pytest tests/ -x` — 92 passed, 1 failed (pre-existing test_extract_docx missing python-docx), 3 skipped, 10 xfailed
|
||||||
|
- `grep -c "write_audit_log" backend/api/auth.py` → 9 (>= 6 required)
|
||||||
|
- `grep -c "write_audit_log" backend/api/documents.py` → 3 (>= 2 required)
|
||||||
|
- `grep -c "write_audit_log" backend/api/admin.py` → 6 (>= 4 required)
|
||||||
|
- No email/password in login_failed metadata confirmed by grep
|
||||||
|
- No filename/extracted_text in document audit calls confirmed by grep
|
||||||
|
|
||||||
|
## Self-Check: PASSED
|
||||||
|
|
||||||
|
- backend/api/auth.py exists and contains write_audit_log import and 9 calls
|
||||||
|
- backend/api/admin.py exists and contains CloudConnectionOut, delete_user endpoint, 6 write_audit_log calls
|
||||||
|
- backend/api/documents.py exists and contains write_audit_log import and 3 calls (import + 2 calls)
|
||||||
|
- Commits e451b16 and 8e6005c exist in git log
|
||||||
Reference in New Issue
Block a user