docs(04): create phase 4 plan (9 plans, 7 waves)
Folders, Sharing, Quotas & Document UX — plans verified (0 blockers, 2 non-blocking warnings). Covers FOLD-01..05, SHARE-01..05, SEC-08/09, ADMIN-06, DOC-01/02. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
curo1305
@@ -0,0 +1,271 @@
|
||||
---
|
||||
phase: 04-folders-sharing-quotas-document-ux
|
||||
plan: 07
|
||||
type: execute
|
||||
wave: 5
|
||||
depends_on:
|
||||
- "04-05"
|
||||
- "04-06"
|
||||
files_modified:
|
||||
- backend/api/auth.py
|
||||
- backend/api/admin.py
|
||||
- backend/api/documents.py
|
||||
autonomous: true
|
||||
requirements:
|
||||
- SEC-08
|
||||
- SEC-09
|
||||
- ADMIN-06
|
||||
- DOC-01
|
||||
- DOC-02
|
||||
- SHARE-01
|
||||
- SHARE-02
|
||||
- SHARE-03
|
||||
- SHARE-04
|
||||
- SHARE-05
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "Auth events (login, logout, password change, TOTP, sign-out-all) are written to the audit log"
|
||||
- "Admin events (user create, deactivate, quota change, AI provider assign) are written to the audit log"
|
||||
- "credentials_enc field is absent from every serialized response across the entire API"
|
||||
- "Admin delete-user triggers delete_user_files() — MinIO objects deleted before DB records removed"
|
||||
- "Document upload and delete events are written to the audit log"
|
||||
artifacts:
|
||||
- path: "backend/api/auth.py"
|
||||
provides: "write_audit_log() calls backfilled into login, logout, password change, TOTP, sign-out-all handlers"
|
||||
- path: "backend/api/admin.py"
|
||||
provides: "write_audit_log() calls in user create/deactivate/quota/AI-config handlers; delete-user cleanup; CloudConnectionOut Pydantic model for SEC-08"
|
||||
- path: "backend/api/documents.py"
|
||||
provides: "write_audit_log() calls in upload confirm and document delete handlers"
|
||||
key_links:
|
||||
- from: "backend/api/auth.py"
|
||||
to: "backend/services/audit.py"
|
||||
via: "write_audit_log called after successful auth events"
|
||||
pattern: "write_audit_log"
|
||||
- from: "backend/api/admin.py"
|
||||
to: "backend/db/models.py"
|
||||
via: "delete_user_files() iterates Document records and calls storage.delete_object"
|
||||
pattern: "delete_user_files\|delete_object"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Back-fill audit log writes into auth and admin handlers (D-13), implement SEC-08
|
||||
(credentials_enc exclusion) and SEC-09 (delete-user file cleanup). These are security
|
||||
hardening tasks — they modify existing handlers without changing their primary behavior.
|
||||
|
||||
Purpose: Complete the audit trail (ADMIN-06) and close the two security requirements
|
||||
(SEC-08, SEC-09) that are in scope for Phase 4.
|
||||
Output: Audit writes in auth.py, admin.py, documents.py + CloudConnectionOut Pydantic model
|
||||
+ delete_user_files() cleanup in admin.py.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@$HOME/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/04-folders-sharing-quotas-document-ux/04-CONTEXT.md
|
||||
@.planning/phases/04-folders-sharing-quotas-document-ux/04-PATTERNS.md
|
||||
@backend/api/auth.py
|
||||
@backend/api/admin.py
|
||||
@backend/api/documents.py
|
||||
@backend/services/audit.py
|
||||
@backend/db/models.py
|
||||
</context>
|
||||
|
||||
<interfaces>
|
||||
<!-- Key interfaces the executor needs. Extracted from codebase. -->
|
||||
|
||||
<!-- write_audit_log signature (from backend/services/audit.py, created in plan 04-03):
|
||||
async def write_audit_log(
|
||||
session: AsyncSession,
|
||||
event_type: str,
|
||||
user_id: Optional[uuid.UUID],
|
||||
actor_id: Optional[uuid.UUID],
|
||||
resource_id: Optional[uuid.UUID],
|
||||
ip_address: Optional[str],
|
||||
metadata_: Optional[dict] = None,
|
||||
) -> None
|
||||
-->
|
||||
|
||||
<!-- From backend/db/models.py — CloudConnection model (read actual file to confirm columns):
|
||||
class CloudConnection(Base):
|
||||
__tablename__ = "cloud_connections"
|
||||
id: ...
|
||||
user_id: ...
|
||||
provider: Mapped[str]
|
||||
display_name: Mapped[str]
|
||||
credentials_enc: Mapped[bytes] # THIS MUST NEVER APPEAR IN API RESPONSES (SEC-08)
|
||||
status: Mapped[str]
|
||||
connected_at: Mapped[datetime]
|
||||
-->
|
||||
|
||||
<!-- SEC-08 response model (per D-18, RESEARCH.md Pattern 9):
|
||||
class CloudConnectionOut(BaseModel):
|
||||
id: str
|
||||
provider: str
|
||||
display_name: str
|
||||
status: str
|
||||
connected_at: datetime
|
||||
# credentials_enc is deliberately absent — SEC-08
|
||||
model_config = {"from_attributes": True}
|
||||
-->
|
||||
|
||||
<!-- IP extraction pattern (established in folders.py and shares.py):
|
||||
ip_address = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None)
|
||||
-->
|
||||
</interfaces>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Back-fill audit log writes into auth.py and documents.py handlers</name>
|
||||
<files>backend/api/auth.py, backend/api/documents.py</files>
|
||||
<read_first>
|
||||
backend/api/auth.py — read the entire file; identify all state-changing endpoints: login (POST /api/auth/login), logout (POST /api/auth/logout), password change (POST /api/auth/change-password), TOTP enroll (POST /api/auth/totp/enroll), TOTP verify/enable, backup code use, sign-out-all (POST /api/auth/sign-out-all); note which already inject Request and which need it added
|
||||
backend/api/documents.py — read the entire file; identify the confirm endpoint (POST /api/documents/{id}/confirm or similar) and the delete endpoint (DELETE /api/documents/{id}); note existing imports
|
||||
backend/services/audit.py — confirm write_audit_log is importable
|
||||
</read_first>
|
||||
<behavior>
|
||||
Auth events to write (D-13 — all 4 categories required):
|
||||
|
||||
1. login success → event_type="auth.login", user_id=user.id, actor_id=user.id, resource_id=None, metadata_={"totp_used": bool}
|
||||
2. login failure → event_type="auth.login_failed", user_id=None, actor_id=None, resource_id=None, metadata_={"email_hash": hmac of email for correlation without PII exposure — or simply omit metadata_}
|
||||
NOTE: Do NOT log the email or password in metadata_. Log only the IP and event type.
|
||||
3. logout → event_type="auth.logout", user_id=current_user.id
|
||||
4. password change → event_type="auth.password_changed", user_id=current_user.id
|
||||
5. TOTP enrolled → event_type="auth.totp_enrolled", user_id=current_user.id
|
||||
6. TOTP revoked (if endpoint exists) → event_type="auth.totp_revoked"
|
||||
7. backup code used → event_type="auth.backup_code_used" (on login with backup code)
|
||||
8. sign-out-all → event_type="auth.sign_out_all", user_id=current_user.id
|
||||
|
||||
Document events (D-13):
|
||||
9. document uploaded (on confirm) → event_type="document.uploaded", user_id=current_user.id, resource_id=doc.id, metadata_={"size_bytes": doc.size_bytes, "storage_backend": "minio"} — MUST NOT include filename or extracted_text
|
||||
10. document deleted → event_type="document.deleted", user_id=current_user.id, resource_id=doc.id, metadata_={"size_bytes": doc.size_bytes}
|
||||
|
||||
write_audit_log is called AFTER the successful operation. If the operation fails (exception raised before commit), write_audit_log is not called. Call it just before the return statement.
|
||||
</behavior>
|
||||
<action>
|
||||
Modify backend/api/auth.py:
|
||||
- Add import: `from services.audit import write_audit_log`
|
||||
- For handlers that do not already inject `request: Request`, add it as a parameter
|
||||
- After each successful auth operation (commit or token issuance), call write_audit_log with appropriate event_type
|
||||
- IP extraction: `ip_address = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None)`
|
||||
- CRITICAL: login_failed event must NOT log email or password in metadata_ — log only IP
|
||||
|
||||
Modify backend/api/documents.py:
|
||||
- Add import: `from services.audit import write_audit_log` if not already present
|
||||
- In the confirm/finalize endpoint (where document upload completes): call write_audit_log with event_type="document.uploaded"; metadata_ contains size_bytes and storage_backend only — NOT filename, NOT extracted_text
|
||||
- In the delete endpoint: call write_audit_log with event_type="document.deleted"; metadata_ contains size_bytes only
|
||||
|
||||
Do NOT change any existing endpoint return values, status codes, or response schemas.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && python -m pytest tests/test_auth_api.py tests/test_documents.py -x -v --no-header 2>&1 | tail -30</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `from services.audit import write_audit_log` appears in both auth.py and documents.py
|
||||
- write_audit_log called in at least 6 distinct places across auth.py (grep: `write_audit_log` count >= 6 in auth.py)
|
||||
- write_audit_log called in at least 2 distinct places in documents.py (confirm + delete)
|
||||
- auth.py audit calls for document.uploaded and document.deleted do NOT include filename or extracted_text in metadata_ (grep these strings absent from write_audit_log call sites in documents.py)
|
||||
- Existing auth tests still pass: `pytest tests/test_auth_api.py -v --no-header 2>&1 | grep -E "^FAILED"` returns nothing
|
||||
- Full suite: `pytest tests/ -x --no-header 2>&1 | grep -E "^FAILED"` returns nothing
|
||||
</acceptance_criteria>
|
||||
<done>Audit writes backfilled into auth and documents handlers; no sensitive data in metadata_; existing tests pass.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: SEC-08 / SEC-09 hardening + admin event audit writes in admin.py</name>
|
||||
<files>backend/api/admin.py</files>
|
||||
<read_first>
|
||||
backend/api/admin.py — read the entire file; identify all state-changing admin endpoints: create user, deactivate user, activate user, reset password (admin-triggered), change quota, assign AI provider; find the delete-user endpoint if it exists (SEC-09 requires file cleanup before DB deletion); identify the _user_to_dict() whitelist pattern; find imports
|
||||
backend/db/models.py — read the CloudConnection model class fully to confirm credentials_enc column name; read the Document model to identify columns used for cleanup
|
||||
backend/services/audit.py — confirm write_audit_log signature
|
||||
</read_first>
|
||||
<behavior>
|
||||
Admin event audit writes (D-13 category 4):
|
||||
- user created by admin → event_type="admin.user_created", user_id=new_user.id, actor_id=admin.id
|
||||
- user deactivated → event_type="admin.user_deactivated", user_id=target_user.id, actor_id=admin.id
|
||||
- user activated → event_type="admin.user_activated", user_id=target_user.id, actor_id=admin.id
|
||||
- quota changed → event_type="admin.quota_changed", user_id=target_user.id, actor_id=admin.id, metadata_={"old_bytes": old_limit, "new_bytes": new_limit}
|
||||
- AI provider assigned → event_type="admin.ai_provider_assigned", user_id=target_user.id, actor_id=admin.id, metadata_={"provider": new_provider, "model": new_model}
|
||||
|
||||
SEC-08 — credentials_enc exclusion (D-18):
|
||||
- Define CloudConnectionOut Pydantic model: id (str), provider (str), display_name (str), status (str), connected_at (datetime) — credentials_enc is ABSENT; model_config = {"from_attributes": True}
|
||||
- If any admin endpoint currently returns CloudConnection ORM objects as JSON: add response_model=CloudConnectionOut or use explicit dict serialization that excludes credentials_enc
|
||||
- Phase 4 note: no cloud connection admin endpoints exist yet (Phase 5). Define the model now so Phase 5 cannot accidentally expose credentials_enc. Place it in admin.py schemas section.
|
||||
|
||||
SEC-09 — delete-user file cleanup (D-19):
|
||||
- If DELETE /api/admin/users/{id} endpoint exists: before deleting DB records, collect all documents for the user via query; call get_storage_backend().delete_object(doc.object_key) for each (best-effort, try/except); decrement quota for deleted files (or simply rely on CASCADE DELETE of quota row since user is deleted); then delete the user DB record
|
||||
- If DELETE endpoint does not exist: create it with the cleanup logic
|
||||
- Endpoint: DELETE /api/admin/users/{user_id} — auth: get_current_admin; assert user is not admin (cannot delete admin accounts); cleanup user files; delete user record; write audit log event_type="admin.user_deleted"
|
||||
</behavior>
|
||||
<action>
|
||||
Modify backend/api/admin.py:
|
||||
|
||||
1. Add imports: `from services.audit import write_audit_log`, `from datetime import datetime` (if not present), `from storage import get_storage_backend`, `from sqlalchemy import select` (if not present), `from db.models import Document` (if not already imported).
|
||||
|
||||
2. Define CloudConnectionOut Pydantic model near the top of the file (after existing request models): fields id, provider, display_name, status, connected_at; add docstring "SEC-08: credentials_enc deliberately excluded".
|
||||
|
||||
3. Add write_audit_log calls after each successful admin state change as specified in the behavior block. ip_address from request.headers or request.client.
|
||||
|
||||
4. Implement or extend DELETE /api/admin/users/{user_id}: read all user documents, delete MinIO objects best-effort, then call `await session.delete(user)` and `await session.commit()`. Write audit log event_type="admin.user_deleted".
|
||||
|
||||
5. Do NOT change _user_to_dict() whitelist — it already excludes credentials_enc and password_hash (per Phase 3 implementation). Only add the new CloudConnectionOut model.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && python -m pytest tests/test_admin_api.py tests/test_security.py -x -v --no-header 2>&1 | tail -30</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- CloudConnectionOut Pydantic model defined in admin.py with no credentials_enc field (grep: `CloudConnectionOut` in admin.py; grep: `credentials_enc` absent from CloudConnectionOut class body)
|
||||
- write_audit_log called in at least 4 admin state-changing endpoints (grep: `write_audit_log` count >= 4 in admin.py)
|
||||
- DELETE /api/admin/users/{id} endpoint exists and calls get_storage_backend().delete_object (grep: `delete_object` in admin.py delete-user handler)
|
||||
- test_credentials_enc_not_in_response turns green or remains xfail — not FAILED
|
||||
- test_delete_user_cleans_files turns green or remains xfail — not FAILED
|
||||
- `cd backend && python -m pytest tests/ -x --no-header 2>&1 | grep -E "^FAILED"` returns nothing
|
||||
- `cd backend && python -m pytest tests/test_admin_api.py -v --no-header 2>&1 | grep -E "^FAILED"` returns nothing
|
||||
</acceptance_criteria>
|
||||
<done>Audit writes backfilled; CloudConnectionOut model defined without credentials_enc; delete-user cleanup implemented.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| Audit write ← auth handler | write_audit_log must not receive PII (email, password) in metadata_ |
|
||||
| Admin → DELETE /api/admin/users | Must not delete admin accounts; must clean MinIO before DB |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-04-07-01 | Sensitive Data Exposure | auth.login_failed logs email or password | mitigate | login_failed event metadata_ MUST be empty or contain only non-PII correlation data; email never logged |
|
||||
| T-04-07-02 | Sensitive Data Exposure | document.uploaded audit event contains filename or extracted_text | mitigate | metadata_ for document.uploaded contains only size_bytes and storage_backend; filename and extracted_text explicitly excluded |
|
||||
| T-04-07-03 | Sensitive Data Exposure | credentials_enc in API response (SEC-08) | mitigate | CloudConnectionOut Pydantic model excludes credentials_enc; safe-by-default (whitelist not blacklist) |
|
||||
| T-04-07-04 | Tampering | Admin deletes their own account | mitigate | DELETE /api/admin/users/{id} asserts target user role != admin before proceeding |
|
||||
| T-04-07-05 | Information Disclosure | Orphaned MinIO objects after user deletion (SEC-09) | mitigate | delete_user_files logic collects all Document.object_key values and calls delete_object before DB delete |
|
||||
| T-04-07-06 | Repudiation | Auth events not logged | mitigate | All 8 auth event types logged per D-13; audit log is append-only (no DELETE on audit_log table) |
|
||||
| T-04-SC | Tampering | npm/pip/cargo installs | accept | No new packages installed in this plan |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
1. SEC-08 test: `cd backend && python -m pytest tests/test_security.py::test_credentials_enc_not_in_response -v`
|
||||
2. SEC-09 test: `cd backend && python -m pytest tests/test_admin_api.py::test_delete_user_cleans_files -v`
|
||||
3. PII check on audit writes: `grep -A5 "login_failed\|auth.login_failed" backend/api/auth.py | grep "email\|password"` — expect no match
|
||||
4. Full suite: `cd backend && python -m pytest tests/ -v --no-header 2>&1 | grep -E "FAILED|ERROR"`
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Audit log has entries for all D-13 event categories (auth, document, folder/share, admin)
|
||||
- credentials_enc never appears in any serialized response — CloudConnectionOut model enforces this
|
||||
- delete-user endpoint cleans MinIO objects before removing DB records
|
||||
- Full pytest suite green — all existing tests pass
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/04-folders-sharing-quotas-document-ux/04-07-SUMMARY.md` when done.
|
||||
</output>
|
||||
Reference in New Issue
Block a user