kite/.planning/phases/04-folders-sharing-quotas-document-ux/04-07-PLAN.md

---
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>