curo1305
fdc32d431d
5 plans across 5 sequential waves covering: Alembic migration 0003 (null-user cleanup, NOT NULL constraint, quota reconciliation), presigned MinIO PUT upload flow with atomic quota enforcement, auth guards on all document/topic endpoints, flat-file settings retirement + per-user AI classification, and frontend quota bar with 3-step XHR upload progress. Verification passed across all 12 dimensions. All 8 phase requirements covered (STORE-03/04/05/06, SEC-04, DOC-03/04/05). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
26 KiB
26 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 03-document-migration-multi-user-isolation | 03 | execute | 3 |
|
|
true |
|
|
Purpose: Closes Phase 2 D-07 / Phase 3 SC4 — admins return 403, cross-user returns 404. The atomic quota path from Plan 03-02 finally runs for real authenticated traffic. Output: 6 file modifications. After this plan, the backend enforces full multi-user isolation for documents and topics; only Plan 03-04 (settings retirement + per-user AI) remains for the backend.
<execution_context> @/Users/nik/.claude/get-shit-done/workflows/execute-plan.md @/Users/nik/.claude/get-shit-done/templates/summary.md </execution_context>
@.planning/ROADMAP.md @.planning/STATE.md @.planning/phases/03-document-migration-multi-user-isolation/03-CONTEXT.md @.planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md @.planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md @.planning/phases/03-document-migration-multi-user-isolation/03-VALIDATION.md @.planning/phases/03-document-migration-multi-user-isolation/03-02-SUMMARY.md @CLAUDE.md@backend/deps/auth.py @backend/api/documents.py @backend/api/topics.py @backend/api/admin.py @backend/services/storage.py @backend/services/classifier.py
From backend/deps/auth.py (Phase 2): async def get_current_user(credentials, session) -> User # 401 on invalid/expired async def get_current_admin(user = Depends(get_current_user)) -> User # 403 on non-admin
This plan adds: async def get_regular_user(user = Depends(get_current_user)) -> User # 403 on admin
From backend/db/models.py: Document.user_id # NOT NULL after Plan 03-01 migration Topic.user_id # nullable: NULL = system topic; UUID = per-user Topic UniqueConstraint("user_id", "name", name="uq_topics_user_name")
From backend/services/storage.py (current — Plan 03-02 has removed save_upload): async def create_topic(session, name, description="", color="#6366f1") -> dict async def load_topics(session) -> list[dict] async def list_metadata(session, topic=None) -> list[dict] async def get_metadata(session, doc_id) -> Optional[dict] async def delete_document(session, doc_id) -> bool # already decrements quota for non-None user_id
This plan changes: create_topic gains user_id: uuid.UUID | None = None parameter (deduplication scoped to user namespace) NEW: load_topics_for_user(session, user_id) -> list[dict]
From backend/services/classifier.py (current — Plan 03-04 changes signature for AI provider; this plan only changes topic plumbing): async def classify_document(session, doc_id, topic_names=None) -> list[str] — uses storage.load_topics(session) for the topic list and storage.create_topic(session, name) for new suggestions async def suggest_topics_for_document(session, doc_id) -> list[str]
<threat_model>
Trust Boundaries
| Boundary | Description |
|---|---|
| browser → /api/documents/* | Authenticated user supplies doc_id path param; ownership assertion gates every read/write |
| browser → /api/topics/* | Authenticated user supplies topic ops; namespace filter prevents cross-user topic enumeration |
| admin token → /api/documents/* | Admin role is explicitly rejected with 403 — admins cannot read document content per CLAUDE.md |
STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|---|---|---|---|---|
| T-03-11 | Information Disclosure | Cross-user document access via doc_id manipulation | mitigate | Every handler taking doc_id asserts doc is None or doc.user_id != current_user.id and raises 404 (not 403) per D-16 / SEC-04 — attacker cannot distinguish "not found" from "wrong owner" |
| T-03-12 | Elevation of Privilege | Admin reading user document content | mitigate | get_regular_user dependency rejects admin with 403; applied to every /api/documents/* handler; PATTERNS.md "Auth Guard Pattern" |
| T-03-13 | Information Disclosure | Cross-user topic enumeration | mitigate | All topic queries filter by or_(Topic.user_id == current_user.id, Topic.user_id.is_(None)); create_topic dedup scoped by user_id (RESEARCH.md Finding 6 Risk 5) |
| T-03-14 | Elevation of Privilege | Regular user creating system topic | mitigate | POST /api/admin/topics requires Depends(get_current_admin); regular POST /api/topics forces user_id=current_user.id |
| T-03-15 | Tampering | object_key forged with another user's UUID prefix | mitigate | object_key is computed server-side using str(current_user.id) — never accepts user-supplied prefix; CLAUDE.md "Every document/folder endpoint asserts resource.user_id == current_user.id" |
| T-03-16 | Spoofing | Anonymous traffic to /api/documents/* after auth wiring | mitigate | HTTPBearer auto_error=True raises 403 when no Authorization header present; get_current_user raises 401 on invalid/expired token |
| T-03-SC | Tampering | pip installs | mitigate | No new package installs |
| </threat_model> |
Admin accounts cannot access document content (CLAUDE.md + SEC-04).
Returns 403 (not 404) — the admin knows document endpoints exist.
"""
if user.role == "admin":
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Admin accounts cannot access document content",
)
return user
```
Modify `backend/api/documents.py`:
- Add imports `from db.models import User` and `from deps.auth import get_regular_user`.
- In POST `/upload-url`: add `current_user: User = Depends(get_regular_user)` parameter; change `object_key = f"null-user/{doc_id}/...` to `object_key = f"{current_user.id}/{doc_id}/{uuid.uuid4()}{suffix}"`; change Document insert to `user_id=current_user.id`.
- In POST `/{doc_id}/confirm`: add `current_user: User = Depends(get_regular_user)` parameter. After loading doc by id, add `if doc is None or doc.user_id != current_user.id: raise HTTPException(status_code=404, detail="Document not found")` (this replaces the simpler 404 check). Remove the `if doc.user_id is not None:` Wave 2 branch entirely — the atomic quota update path runs unconditionally. The else-branch that returned `used_bytes=0` for None-user is deleted.
- In GET `""` (list): add `current_user: User = Depends(get_regular_user)` parameter; change `await storage.list_metadata(session, topic=topic)` to `await storage.list_metadata(session, user_id=current_user.id, topic=topic)`.
- In GET `/{doc_id}`: add `current_user: User = Depends(get_regular_user)` parameter; replace the existing `meta = await storage.get_metadata(...)` flow with `doc = await session.get(Document, uuid.UUID(doc_id))`; check `if doc is None or doc.user_id != current_user.id: raise HTTPException(404, "Document not found")`; then call `meta = await storage.get_metadata(session, doc_id)` for the response shape (or replace with `_doc_to_dict` if more direct).
- In DELETE `/{doc_id}`: add `current_user: User = Depends(get_regular_user)` parameter; load Document via `session.get(Document, uuid.UUID(doc_id))` and assert ownership before calling `storage.delete_document(session, doc_id)`; on `doc is None or doc.user_id != current_user.id` raise 404 (do not return success).
- In POST `/{doc_id}/classify`: add `current_user: User = Depends(get_regular_user)` parameter; assert ownership via `doc = await session.get(Document, uuid.UUID(doc_id)); if doc is None or doc.user_id != current_user.id: raise HTTPException(404, "Document not found")` before calling classifier.
Modify `backend/services/storage.py`:
- Change `list_metadata` signature from `(session, topic=None)` to `(session, user_id: uuid.UUID, topic: Optional[str] = None)`. Update the `stmt = select(Document).order_by(...)` block to insert `.where(Document.user_id == user_id)` before any topic join.
- Remove the `if doc.user_id is not None:` guard from `delete_document` added in Plan 03-02 — the quota decrement now always runs (post-migration, user_id is NOT NULL).
- GET `""` (list_topics): replace `await storage.load_topics(session)` with `await storage.load_topics_for_user(session, user_id=current_user.id)`. Existing topic_doc_counts call: the doc count must also be scoped to current user — pass `user_id=current_user.id` into a new `storage.topic_doc_counts(session, user_id=current_user.id)` (add user filter to that helper as part of this task — add `Document.user_id == user_id` join condition).
- POST `""` (create_topic): pass `user_id=current_user.id` into `storage.create_topic(...)`.
- PATCH `/{topic_id}` (update_topic): before calling `storage.update_topic`, load the Topic and assert `topic.user_id == current_user.id` (no editing of system topics or other users' topics) — else 404. Implementation: `t = await session.get(Topic, uuid.UUID(topic_id)); if t is None or t.user_id != current_user.id: raise HTTPException(404, "Topic not found")`.
- DELETE `/{topic_id}` (delete_topic): same ownership assertion before delete.
- POST `/suggest` (suggest_topics): just add the dep; classifier handles the namespace.
Modify `backend/api/admin.py`. Add a new Pydantic model `class SystemTopicCreate(BaseModel): name: str; description: str = ""; color: str = "#6366f1"`. Add a new endpoint:
```
@router.post("/topics", status_code=status.HTTP_201_CREATED)
async def create_system_topic(
body: SystemTopicCreate,
session: AsyncSession = Depends(get_db),
_admin: User = Depends(get_current_admin),
) -> dict:
"""Admin creates a system topic visible to all users (D-09).
System topics have user_id = NULL (per D-08 namespace model).
"""
from services import storage
topic = await storage.create_topic(
session, body.name, body.description, body.color, user_id=None
)
return topic
```
Import additions: add `from db.models import Topic` if not already imported (Topic is needed for any future admin topic edit/delete; this plan only adds POST).
Modify `backend/services/storage.py`:
1. Add `from sqlalchemy import or_` to the existing `from sqlalchemy import select, delete` import line.
2. Add `import uuid` if not already at top (it is — verify).
3. Modify `create_topic` signature: `async def create_topic(session, name, description="", color="#6366f1", user_id: uuid.UUID | None = None) -> dict`. Replace the dedup query with a namespace-scoped lookup. Because PostgreSQL needs `IS NOT DISTINCT FROM` semantics to compare NULLs but SQLite does not implement `IS NOT DISTINCT FROM`, use a branching approach:
```
if user_id is None:
q = await session.execute(
select(Topic).where(
sql_func.lower(Topic.name) == name.lower(),
Topic.user_id.is_(None),
)
)
else:
q = await session.execute(
select(Topic).where(
sql_func.lower(Topic.name) == name.lower(),
Topic.user_id == user_id,
)
)
```
On insert, set `Topic(name=name, description=description, color=color, user_id=user_id)`.
4. Add new function (after `load_topics`):
```
async def load_topics_for_user(session, user_id: uuid.UUID) -> list:
"""Return system topics (user_id IS NULL) + user's own topics, ordered by name (D-17, DOC-04)."""
q = await session.execute(
select(Topic).where(
or_(Topic.user_id == user_id, Topic.user_id.is_(None))
).order_by(Topic.name)
)
return [
{"id": str(t.id), "name": t.name, "description": t.description, "color": t.color}
for t in q.scalars()
]
```
Append `"load_topics_for_user"` to `__all__`.
5. Modify `topic_doc_counts` to optionally scope by user: add `user_id: uuid.UUID | None = None` param; if non-None, join Document and filter by `Document.user_id == user_id` (so a user only sees the count of their own documents tagged to each topic).
Modify `backend/services/classifier.py`:
1. In `classify_document` (line ~39): replace `all_topics = await storage.load_topics(session)` with `all_topics = await storage.load_topics_for_user(session, user_id=doc.user_id)`. To access `doc.user_id`, fetch the Document at the top of the function: add `from db.models import Document; from sqlalchemy.ext.asyncio import AsyncSession; doc = await session.get(Document, uuid.UUID(doc_id))` (`uuid` is already imported via meta path; if not, add `import uuid`).
2. Replace `await storage.create_topic(session, name.strip())` (line ~52) with `await storage.create_topic(session, name.strip(), user_id=doc.user_id)` to put AI-suggested topics in the user namespace (D-11).
3. In `suggest_topics_for_document`: also replace `storage.load_topics(session)` if called (it is not, per current code) — only `provider.suggest_topics` is invoked, no change needed.
<success_criteria>
- get_regular_user dependency defined in deps/auth.py
- Every /api/documents/* handler injects Depends(get_regular_user) and asserts ownership (404 on cross-user)
- Every /api/topics/* handler injects Depends(get_current_user); queries are namespace-scoped
- POST /api/admin/topics exists and requires admin
- services.storage.create_topic accepts user_id; dedup scoped by namespace
- services.storage.load_topics_for_user implemented; consumed by classifier
- classify_document creates AI-suggested topics in the user namespace
- Phase 2 SC5 deferred item (admin JWT → 403 on documents) is fulfilled </success_criteria>