docs(03): create Phase 3 execution plan — document migration & multi-user isolation
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>
This commit is contained in:
curo1305
+25
-2
@@ -88,7 +88,30 @@ _Last updated: 2026-05-22_
|
||||
4. Requesting a document object key or presigned URL for a document owned by a different user returns 403 — no cross-user object access is possible through any request parameter manipulation; all /api/documents/* endpoints enforce get_current_user and return 403 when the requesting user's role is admin (completing SC5 from Phase 2)
|
||||
5. AI classification for each document uses the provider and model assigned to that user by the admin, not any user-supplied or default value
|
||||
|
||||
**Plans**: TBD
|
||||
**Plans**: 5 plans
|
||||
|
||||
**Wave 1** — Migration + test scaffolds
|
||||
- [ ] 03-01-PLAN.md — Wave 0 test scaffolds (auth_user/admin_user/MinIO mock fixtures + 16 xfail stubs) + Alembic migration 0003 (null-user cleanup, NOT NULL constraint, topic cleanup, quota reconciliation, ix_topics_user_id)
|
||||
|
||||
**Wave 2** *(blocked on Wave 1)*
|
||||
- [ ] 03-02-PLAN.md — Presigned upload backend: StorageBackend ABC + MinIOBackend dual client + generate_presigned_put_url/stat_object + /api/documents/upload-url + /api/documents/{id}/confirm with atomic quota UPDATE + GET /api/auth/me/quota + delete-with-quota + abandoned-upload Celery beat + docker-compose CORS/celery-beat
|
||||
|
||||
**Wave 3** *(blocked on Wave 2)*
|
||||
- [ ] 03-03-PLAN.md — Auth guards: get_regular_user dep + ownership assertions on every /api/documents/* handler (404 not 403) + admin 403 + real user_id in object_key + namespace-scoped /api/topics/* + POST /api/admin/topics + classifier topic-namespace plumbing
|
||||
|
||||
**Wave 4** *(blocked on Wave 3)*
|
||||
- [ ] 03-04-PLAN.md — Settings retirement + per-user AI: delete /api/settings + remove load_settings/save_settings + classifier accepts ai_provider/ai_model kwargs + Celery task resolves user.ai_provider via DB + frontend SettingsView placeholder + remove settings store/API
|
||||
|
||||
**Wave 5** *(blocked on Wave 4)*
|
||||
- [ ] 03-05-PLAN.md — Frontend upload flow + quota bar: 3-step upload action with XHR progress + UploadProgress.vue progress bar and quota rejection error block + QuotaBar.vue + AppSidebar embed + quota state in auth store + human checkpoint
|
||||
|
||||
**Cross-cutting constraints:**
|
||||
- Atomic quota UPDATE pattern only lives in Plan 02; never duplicate (CLAUDE.md)
|
||||
- Every /api/documents/* handler injects get_regular_user (Plan 03)
|
||||
- AI provider/model resolved only via Celery task DB lookup (Plan 04)
|
||||
- Browser XHR PUT to MinIO sends NO Authorization header (Plan 05)
|
||||
|
||||
**UI hint**: yes
|
||||
|
||||
---
|
||||
|
||||
@@ -134,6 +157,6 @@ _Last updated: 2026-05-22_
|
||||
|-------|----------------|--------|-----------|
|
||||
| 1. Infrastructure Foundation | 5/5 | Complete | 2026-05-22 |
|
||||
| 2. Users & Authentication | 5/5 | Complete | 2026-05-22 |
|
||||
| 3. Document Migration & Multi-User Isolation | 0/? | Not started | - |
|
||||
| 3. Document Migration & Multi-User Isolation | 0/5 | Not started | - |
|
||||
| 4. Folders, Sharing, Quotas & Document UX | 0/? | Not started | - |
|
||||
| 5. Cloud Storage Backends | 0/? | Not started | - |
|
||||
|
||||
+12
-13
@@ -2,13 +2,13 @@
|
||||
gsd_state_version: 1.0
|
||||
milestone: v1.0
|
||||
milestone_name: milestone
|
||||
current_phase: 2
|
||||
status: complete
|
||||
last_updated: "2026-05-22T18:30:00Z"
|
||||
current_phase: 3
|
||||
status: planned
|
||||
last_updated: "2026-05-23T00:00:00Z"
|
||||
progress:
|
||||
total_phases: 5
|
||||
completed_phases: 2
|
||||
total_plans: 10
|
||||
total_plans: 15
|
||||
completed_plans: 10
|
||||
percent: 40
|
||||
---
|
||||
@@ -16,9 +16,9 @@ progress:
|
||||
# Project State
|
||||
|
||||
**Project:** DocuVault
|
||||
**Status:** Phase 2 Complete — Phase 3 Ready
|
||||
**Current Phase:** 2
|
||||
**Last Updated:** 2026-05-22
|
||||
**Status:** Phase 3 Planned — Ready to Execute
|
||||
**Current Phase:** 3
|
||||
**Last Updated:** 2026-05-23
|
||||
|
||||
## Phase Status
|
||||
|
||||
@@ -26,7 +26,7 @@ progress:
|
||||
|---|---|---|
|
||||
| 1 | Infrastructure Foundation | ✓ Complete |
|
||||
| 2 | Users & Authentication | ✓ Complete (5/5 plans) |
|
||||
| 3 | Document Migration & Multi-User Isolation | Not Started |
|
||||
| 3 | Document Migration & Multi-User Isolation | Planned (5 plans, ready to execute) |
|
||||
| 4 | Folders, Sharing, Quotas & Document UX | Not Started |
|
||||
| 5 | Cloud Storage Backends | Not Started |
|
||||
|
||||
@@ -87,9 +87,7 @@ progress:
|
||||
|
||||
### Open Questions
|
||||
|
||||
- Celery + Redis vs pgqueuer for Phase 3 (depends on Redis availability in deployment target)
|
||||
- Verify cloud SDK minor versions on PyPI before Phase 5 pinning
|
||||
- Phase 2 SC5 (admin JWT → 403 on /api/documents/*) deferred to Phase 3 SC4 per D-07
|
||||
|
||||
### Blockers
|
||||
|
||||
@@ -101,6 +99,7 @@ _Updated at each phase transition._
|
||||
|
||||
| Field | Value |
|
||||
|---|---|
|
||||
| Last session | 2026-05-22 — Executed Phase 2 Plan 05 (Admin panel frontend: AdminView, three tab components, AppSidebar update) |
|
||||
| Next action | Run `/gsd:discuss-phase 3` or `/gsd:plan-phase 3` to begin Phase 3 (Document Migration & Multi-User Isolation) |
|
||||
| Pending decisions | See Open Questions above |
|
||||
| Last session | 2026-05-23 — Planned Phase 3 (5 plans, 5 waves; verification passed) |
|
||||
| Next action | Run `/gsd:execute-phase 3` to begin execution |
|
||||
| Pending decisions | None — all Phase 3 decisions locked in 03-CONTEXT.md |
|
||||
| Resume file | `.planning/phases/03-document-migration-multi-user-isolation/03-01-PLAN.md` |
|
||||
|
||||
@@ -0,0 +1,297 @@
|
||||
---
|
||||
phase: 03-document-migration-multi-user-isolation
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- backend/tests/test_quota.py
|
||||
- backend/tests/test_alembic.py
|
||||
- backend/tests/test_classifier.py
|
||||
- backend/tests/test_documents.py
|
||||
- backend/tests/test_topics.py
|
||||
- backend/tests/test_settings.py
|
||||
- backend/tests/conftest.py
|
||||
- backend/migrations/versions/0003_multi_user_isolation.py
|
||||
autonomous: true
|
||||
requirements:
|
||||
- STORE-03
|
||||
- STORE-04
|
||||
- STORE-05
|
||||
- STORE-06
|
||||
- SEC-04
|
||||
- DOC-03
|
||||
- DOC-04
|
||||
- DOC-05
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "All xfail(strict=False) test stubs for Phase 3 requirements exist and are collectable by pytest"
|
||||
- "Alembic migration 0003 deletes every documents row where user_id IS NULL and removes the corresponding MinIO objects before any schema change"
|
||||
- "After migration 0003 runs, documents.user_id is NOT NULL"
|
||||
- "After migration 0003 runs, the topics table is empty (all rows deleted) and ix_topics_user_id exists"
|
||||
- "After migration 0003 runs, every quotas.used_bytes equals SUM(documents.size_bytes) for that user (or 0 if no documents)"
|
||||
- "auth_user and admin_user pytest fixtures issue JWTs that get_current_user accepts and that resolve to active users with Quota rows"
|
||||
artifacts:
|
||||
- path: "backend/tests/test_quota.py"
|
||||
provides: "Wave 0 xfail stubs for STORE-03 / STORE-05 / STORE-06 / SC2"
|
||||
contains: "test_concurrent_quota_race"
|
||||
- path: "backend/tests/test_classifier.py"
|
||||
provides: "Wave 0 xfail stubs for DOC-03 / DOC-05 per-user provider resolution"
|
||||
contains: "test_per_user_provider"
|
||||
- path: "backend/tests/conftest.py"
|
||||
provides: "auth_user, admin_user, mock_minio_presigned, mock_minio_stat fixtures"
|
||||
exports:
|
||||
- "auth_user"
|
||||
- "admin_user"
|
||||
- "mock_minio_presigned"
|
||||
- "mock_minio_stat"
|
||||
- path: "backend/migrations/versions/0003_multi_user_isolation.py"
|
||||
provides: "Null-user document cleanup + topic cleanup + NOT NULL constraint + quota reconciliation + ix_topics_user_id"
|
||||
contains: "revision = \"0003\""
|
||||
key_links:
|
||||
- from: "backend/migrations/versions/0003_multi_user_isolation.py"
|
||||
to: "MinIO (synchronous SDK)"
|
||||
via: "from minio import Minio; client.remove_object(bucket, key) for each null-user object_key"
|
||||
pattern: "remove_object"
|
||||
- from: "backend/tests/conftest.py"
|
||||
to: "backend/services/auth.py"
|
||||
via: "create_access_token + User row + Quota row for fixture users"
|
||||
pattern: "create_access_token"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Lay the Wave 0 foundation for Phase 3. Two outputs:
|
||||
|
||||
1. Test scaffolds (xfail stubs) covering every requirement in the VALIDATION.md Per-Task Verification Map plus the shared fixtures (`auth_user`, `admin_user`, MinIO mocks) every later plan needs.
|
||||
2. Alembic migration `0003_multi_user_isolation.py` that performs the null-user document cleanup (D-01/D-02), all-topics cleanup (D-10), `documents.user_id` NOT NULL constraint, quota reconciliation (D-03), and the `ix_topics_user_id` index.
|
||||
|
||||
Purpose: Wave 0 tests must exist before any later plan can run them. The migration is the precondition for adding `get_current_user` to `/api/documents/*` — once `documents.user_id` is NOT NULL, every Document has an owner and ownership assertions become meaningful.
|
||||
Output: 6 test files (5 stubs + 1 fixture file), 1 Alembic migration file.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/nik/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<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
|
||||
@CLAUDE.md
|
||||
|
||||
@backend/migrations/versions/0002_add_backup_codes_and_password_must_change.py
|
||||
@backend/db/models.py
|
||||
@backend/tests/conftest.py
|
||||
@backend/tests/test_alembic.py
|
||||
@backend/services/auth.py
|
||||
|
||||
<interfaces>
|
||||
<!-- Contracts the executor needs without re-reading the codebase. -->
|
||||
|
||||
From backend/db/models.py (Phase 1):
|
||||
```python
|
||||
class Document(Base):
|
||||
__tablename__ = "documents"
|
||||
id: Mapped[uuid.UUID] = mapped_column(UUID(as_uuid=True), primary_key=True)
|
||||
user_id: Mapped[Optional[uuid.UUID]] # nullable in Phase 1 (D-03) — Phase 3 will flip to NOT NULL
|
||||
object_key: Mapped[str]
|
||||
size_bytes: Mapped[int]
|
||||
status: Mapped[str] # 'pending' | 'uploaded' | 'classified' | 'classification_failed'
|
||||
created_at: Mapped[datetime]
|
||||
|
||||
class Quota(Base):
|
||||
__tablename__ = "quotas"
|
||||
user_id: Mapped[uuid.UUID] # PK; FK -> users.id
|
||||
limit_bytes: Mapped[int] # default 104857600 (100 MB)
|
||||
used_bytes: Mapped[int]
|
||||
|
||||
class Topic(Base):
|
||||
__tablename__ = "topics"
|
||||
id: Mapped[uuid.UUID]
|
||||
user_id: Mapped[Optional[uuid.UUID]] # NULL = system topic, UUID = per-user
|
||||
name: Mapped[str]
|
||||
# UniqueConstraint("user_id", "name", name="uq_topics_user_name")
|
||||
```
|
||||
|
||||
From backend/services/auth.py (Phase 2):
|
||||
```python
|
||||
def hash_password(password: str) -> str
|
||||
def create_access_token(user_id: str, role: str) -> str
|
||||
```
|
||||
|
||||
From backend/tests/conftest.py (existing):
|
||||
```python
|
||||
@pytest_asyncio.fixture
|
||||
async def db_session() -> AsyncSession # in-memory SQLite
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def async_client(db_session) -> AsyncClient # ASGI httpx client with get_db override
|
||||
```
|
||||
|
||||
Migration 0002 reference style (revision/down_revision header + numbered # ── 1. comments + op.execute("GRANT ...") + downgrade()).
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| migration runtime → MinIO | DDL transaction holds DB rows; MinIO deletes happen outside any DB transaction |
|
||||
| test fixtures → backend code | Fixtures fabricate JWTs that hit every guarded endpoint — must not leak across tests |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-03-01 | Tampering | Alembic migration 0003 upgrade() | mitigate | Collect object_keys via SELECT before DELETE; wrap MinIO remove_object in try/except so MinIO partial failure cannot leave DB+MinIO in worse state than orphans (RESEARCH.md Finding 1 + Risk 3) |
|
||||
| T-03-02 | Denial of Service | Alembic migration when MinIO unreachable | accept | Migration is run only by docuvault_migrate after docker-compose health checks per RESEARCH.md Risk 3; retry on next deploy. Document in upgrade() docstring. |
|
||||
| T-03-03 | Information Disclosure | xfail test fixtures (auth_user JWT) | mitigate | Fixture-issued JWTs use short-lived test secret_key; fixtures clean up via async_client teardown; never log token values |
|
||||
| T-03-SC | Tampering | pip installs | mitigate | No new package-manager installs in this plan — pytest, pytest-asyncio, alembic, sqlalchemy, minio already present in requirements.txt from Phase 1/2 |
|
||||
</threat_model>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Create Wave 0 test scaffolds and shared fixtures</name>
|
||||
<files>backend/tests/test_quota.py, backend/tests/test_alembic.py, backend/tests/test_classifier.py, backend/tests/test_documents.py, backend/tests/test_topics.py, backend/tests/test_settings.py, backend/tests/conftest.py</files>
|
||||
<read_first>
|
||||
- backend/tests/conftest.py — current async_client + db_session fixture definitions; must not break existing fixtures
|
||||
- backend/tests/test_alembic.py — existing xfail style for migration tests (revision 0001/0002)
|
||||
- backend/tests/test_documents.py — existing async test shape used by later plans
|
||||
- backend/tests/test_topics.py — existing test shape
|
||||
- backend/tests/test_settings.py — existing tests (will become obsolete after D-12, but kept here for the stub `test_settings_endpoint_removed`)
|
||||
- backend/tests/test_classifier.py — existing mock-provider tests; new tests live in same file
|
||||
- backend/services/auth.py — `hash_password`, `create_access_token` signatures used by new fixtures
|
||||
- backend/db/models.py — `User`, `Quota`, `Document`, `Topic` columns referenced by fixtures
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-VALIDATION.md — Per-Task Verification Map names every test stub required
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-CONTEXT.md — D-01..D-17 referenced by stub docstrings
|
||||
</read_first>
|
||||
<action>
|
||||
Add new pytest fixtures to `backend/tests/conftest.py` (append, do not rewrite existing fixtures). Each fixture is `@pytest_asyncio.fixture`:
|
||||
|
||||
1. `auth_user(db_session)` — creates a User row (role="user", is_active=True, password_must_change=False, password_hash via `services.auth.hash_password("Testpassword123!")`), creates a Quota row (limit_bytes=104857600, used_bytes=0), issues an access token via `services.auth.create_access_token(str(user.id), "user")`, returns dict `{"user": user, "token": token, "headers": {"Authorization": f"Bearer {token}"}}`. Persist with `db_session.commit()`.
|
||||
2. `admin_user(db_session)` — same as above but `role="admin"` and token created with `"admin"`. Returns identical dict shape.
|
||||
3. `mock_minio_presigned(monkeypatch)` — uses `monkeypatch.setattr` to replace `storage.minio_backend.MinIOBackend.generate_presigned_put_url` (method-level patch with `AsyncMock`) returning the literal string `"http://localhost:9000/docuvault/test-presigned-url"`. The fixture yields the AsyncMock so tests can assert call counts/args.
|
||||
4. `mock_minio_stat(monkeypatch)` — same pattern for `MinIOBackend.stat_object`, AsyncMock returning a configurable integer (default 1024). Yields the mock for per-test customization (e.g. `mock_minio_stat.return_value = 50_000_000`).
|
||||
|
||||
The patched methods do not exist yet (added in Plan 03-02) — using `monkeypatch.setattr` with `raising=False` ensures the patch installs before the attribute exists. Document this with an inline comment referencing Plan 03-02.
|
||||
|
||||
Create the following xfail test stub files. Every test is marked `@pytest.mark.xfail(strict=False, reason="implemented in plan 03-XX")` and asserts a single placeholder (`assert True` or `pytest.skip("scaffold")`). Stub file headers explain which requirement(s) they cover and which plan implements them.
|
||||
|
||||
`backend/tests/test_quota.py` (NEW) — async tests under Plan 03-02:
|
||||
- `test_quota_increment_atomic(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — STORE-03; after one /confirm with stat=50_000_000, GET /api/auth/me/quota returns used_bytes == 50_000_000
|
||||
- `test_concurrent_quota_race(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — STORE-03 SC2; calls asyncio.gather on two /confirm POSTs targeting two documents totaling 110 MB against a 100 MB quota; asserts statuses sorted == [200, 413]
|
||||
- `test_quota_exceeded_response(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — STORE-05; /confirm returns 413 and body shape `{"detail": {"used_bytes", "limit_bytes", "rejected_bytes"}}`
|
||||
- `test_delete_decrements_quota(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — STORE-06; upload + confirm then DELETE; GET /api/auth/me/quota returns used_bytes == 0
|
||||
|
||||
`backend/tests/test_alembic.py` (APPEND to existing) — new sync test `test_migration_0003`:
|
||||
- Wraps Phase 1 fixture setup; runs `alembic upgrade head` against tmp SQLite; pre-seeds a `documents` row with `user_id=None` AND a row with `user_id=<some uuid>` via raw INSERT before running upgrade head past 0002, then runs `alembic upgrade head` to apply 0003 and asserts: the null-user row is gone, the populated-user row remains, PRAGMA table_info shows `documents.user_id` notnull=1, all `topics` rows are gone, `ix_topics_user_id` is in `sqlite_master`, and `quotas.used_bytes` for the populated user equals SUM(size_bytes). Mark `@pytest.mark.xfail(strict=False, reason="implemented in plan 03-01 migration step")`.
|
||||
|
||||
`backend/tests/test_classifier.py` (APPEND to existing) — new async tests under Plan 03-04:
|
||||
- `test_per_user_provider(db_session)` — DOC-03; with `user.ai_provider="openai"`, `user.ai_model="gpt-4o"`, the classifier.classify_document call resolves `_settings["active_provider"] == "openai"` (use AsyncMock for `ai.get_provider` and assert call args)
|
||||
- `test_celery_task_uses_user_provider(db_session)` — DOC-05; calling `_run(document_id)` with a Document owned by `user.ai_provider="anthropic"` calls classifier with `ai_provider="anthropic"`
|
||||
- `test_default_provider_fallback(db_session)` — D-15; when `user.ai_provider is None`, classifier receives `config.settings.default_ai_provider`
|
||||
|
||||
`backend/tests/test_documents.py` (APPEND to existing) — new async tests under Plans 03-02 / 03-03:
|
||||
- `test_upload_url_endpoint(async_client, auth_user, mock_minio_presigned)` — D-05; POST /api/documents/upload-url returns `{"upload_url": "...", "document_id": "..."}` and creates a Document row with status="pending"
|
||||
- `test_confirm_endpoint(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — D-05; POST /api/documents/{id}/confirm calls stat_object once, updates Document.size_bytes from stat return, sets status="uploaded"
|
||||
- `test_get_quota(async_client, auth_user)` — STORE-04; GET /api/auth/me/quota returns `{"used_bytes": 0, "limit_bytes": 104857600}`
|
||||
- `test_cross_user_access_404(async_client, auth_user, db_session)` — SEC-04; user A creates a Document, user B's request for `GET /api/documents/{A_doc_id}` returns 404
|
||||
- `test_admin_cannot_access_documents(async_client, admin_user)` — SEC-04 SC4; GET /api/documents using admin_user.headers returns 403
|
||||
- `test_documents_require_auth(async_client)` — anonymous GET /api/documents returns 401 or 403
|
||||
|
||||
`backend/tests/test_topics.py` (APPEND to existing) — new async tests under Plan 03-03:
|
||||
- `test_topic_namespace(async_client, auth_user, db_session)` — DOC-04; seed one system topic (user_id=NULL), one auth_user-owned topic, one different-user topic; GET /api/topics returns only the first two
|
||||
- `test_admin_create_system_topic(async_client, admin_user)` — D-09; POST /api/admin/topics returns 201 and creates a Topic with user_id=NULL
|
||||
- `test_regular_user_cannot_create_system_topic(async_client, auth_user)` — D-09; POST /api/admin/topics with auth_user.headers returns 403
|
||||
- `test_topics_require_auth(async_client)` — D-17; anonymous GET /api/topics returns 401 or 403
|
||||
|
||||
`backend/tests/test_settings.py` (APPEND new test, leave existing) — under Plan 03-04:
|
||||
- `test_settings_endpoint_removed(async_client)` — D-12; GET /api/settings returns 404
|
||||
|
||||
Every NEW test must include `@pytest.mark.xfail(strict=False, reason="implemented in plan 03-XX")` with the XX matching its target plan. None of the new tests should pass today (xfail strict=False keeps unexpected passes from breaking CI per Phase 1 D-Wave0).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && pytest tests/test_quota.py tests/test_alembic.py::test_migration_0003 tests/test_classifier.py::test_per_user_provider tests/test_classifier.py::test_celery_task_uses_user_provider tests/test_classifier.py::test_default_provider_fallback tests/test_documents.py::test_upload_url_endpoint tests/test_documents.py::test_confirm_endpoint tests/test_documents.py::test_get_quota tests/test_documents.py::test_cross_user_access_404 tests/test_documents.py::test_admin_cannot_access_documents tests/test_documents.py::test_documents_require_auth tests/test_topics.py::test_topic_namespace tests/test_topics.py::test_admin_create_system_topic tests/test_topics.py::test_regular_user_cannot_create_system_topic tests/test_topics.py::test_topics_require_auth tests/test_settings.py::test_settings_endpoint_removed --collect-only -q</automated>
|
||||
</verify>
|
||||
<done>
|
||||
All 16 new test IDs are collected by pytest with no collection errors. `grep -c "xfail" backend/tests/test_quota.py` returns >= 4. `grep -c "xfail" backend/tests/test_documents.py` >= 6. `grep -c "xfail" backend/tests/test_topics.py` >= 4. `grep -c "auth_user" backend/tests/conftest.py` returns >= 1.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Write Alembic migration 0003 (null-user cleanup + NOT NULL + topic cleanup + quota reconcile + index)</name>
|
||||
<files>backend/migrations/versions/0003_multi_user_isolation.py</files>
|
||||
<read_first>
|
||||
- backend/migrations/versions/0002_add_backup_codes_and_password_must_change.py — header style, numbered comments, GRANT pattern, downgrade structure
|
||||
- backend/migrations/versions/0001_initial_schema.py — column types, table names, baseline schema
|
||||
- backend/db/models.py — Document, Quota, Topic columns + Index/UniqueConstraint definitions
|
||||
- backend/config.py — `settings.minio_endpoint`, `settings.minio_access_key`, `settings.minio_secret_key`, `settings.minio_bucket` — but DO NOT import settings inside the migration (env vars only, see action)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 1 (sync MinIO SDK inside upgrade), Finding 4 (quota reconciliation SQL), Risk 3 (idempotency, MinIO unreachability)
|
||||
- backend/tests/test_alembic.py — `test_migration_0003` (xfail stub from Task 1) defines the expected post-state
|
||||
</read_first>
|
||||
<behavior>
|
||||
- Migration revision = "0003", down_revision = "0002", branch_labels = None, depends_on = None
|
||||
- upgrade() must, in order:
|
||||
1. SELECT id, object_key FROM documents WHERE user_id IS NULL → list of (id, object_key)
|
||||
2. DELETE FROM document_topics WHERE document_id IN (those ids) [cascade safety]
|
||||
3. DELETE FROM documents WHERE user_id IS NULL
|
||||
4. For each collected object_key: call synchronous `minio.Minio(...).remove_object(bucket, key)` wrapped in try/except (log via op-style `op.execute("-- comment")` skipped; use `print()` to stderr or silent pass per RESEARCH.md Finding 1)
|
||||
5. DELETE FROM topics (all rows, per D-10)
|
||||
6. op.alter_column("documents", "user_id", existing_type=postgresql.UUID(as_uuid=True), nullable=False)
|
||||
7. op.create_index("ix_topics_user_id", "topics", ["user_id"])
|
||||
8. op.execute(text("UPDATE quotas SET used_bytes = (SELECT COALESCE(SUM(size_bytes), 0) FROM documents WHERE documents.user_id = quotas.user_id)"))
|
||||
- downgrade() must:
|
||||
1. op.drop_index("ix_topics_user_id", table_name="topics")
|
||||
2. op.alter_column("documents", "user_id", existing_type=postgresql.UUID(as_uuid=True), nullable=True)
|
||||
3. Docstring states that deleted rows + MinIO objects are NOT restorable (one-way data cleanup per D-01)
|
||||
- MinIO connection details read from os.environ ONLY (MINIO_ENDPOINT, MINIO_ACCESS_KEY, MINIO_SECRET_KEY, MINIO_BUCKET) — never `from config import settings` (circular import + Alembic side effects)
|
||||
- test_migration_0003 (Task 1) must transition from xfail → pass once this task lands (remove or keep the xfail marker; if strict=False it XPASSes silently)
|
||||
</behavior>
|
||||
<action>
|
||||
Create `backend/migrations/versions/0003_multi_user_isolation.py` following the exact header style of `0002_add_backup_codes_and_password_must_change.py`:
|
||||
|
||||
Header: revision = "0003", down_revision = "0002", branch_labels = None, depends_on = None. Docstring lists the 8 numbered steps and references D-01, D-02, D-03, D-10. Imports: `import os; from __future__ import annotations; import sqlalchemy as sa; from sqlalchemy import text; from sqlalchemy.dialects import postgresql; from alembic import op`. Conditionally `from minio import Minio` inside upgrade() (deferred import — minio not needed in downgrade or for test envs that skip MinIO).
|
||||
|
||||
upgrade() body uses numbered section comments matching the 0002 style (`# ── 1. Collect null-user object keys ─────`). Section 1 uses `bind = op.get_bind(); result = bind.execute(text("SELECT id, object_key FROM documents WHERE user_id IS NULL"))` and materializes `null_user_objects = [(row[0], row[1]) for row in result]`. Section 2 uses `op.execute(text("DELETE FROM document_topics WHERE document_id IN (SELECT id FROM documents WHERE user_id IS NULL)"))`. Section 3 uses `op.execute(text("DELETE FROM documents WHERE user_id IS NULL"))`. Section 4 instantiates `Minio(os.environ.get("MINIO_ENDPOINT", "minio:9000"), access_key=os.environ.get("MINIO_ACCESS_KEY", ""), secret_key=os.environ.get("MINIO_SECRET_KEY", ""), secure=False)` and loops over `null_user_objects`, calling `client.remove_object(bucket, key)` inside `try/except Exception: pass` (skip silently — orphans are harmless per Finding 1). Bucket name from `os.environ.get("MINIO_BUCKET", "docuvault")`. Section 5 uses `op.execute(text("DELETE FROM topics"))`. Section 6 uses `op.alter_column("documents", "user_id", existing_type=postgresql.UUID(as_uuid=True), nullable=False)`. Section 7 uses `op.create_index("ix_topics_user_id", "topics", ["user_id"])`. Section 8 uses the quota reconciliation `UPDATE quotas SET used_bytes = (SELECT COALESCE(SUM(size_bytes), 0) FROM documents WHERE documents.user_id = quotas.user_id)` via `op.execute(text(...))`.
|
||||
|
||||
SQLite test compatibility (Task 1's `test_migration_0003` runs against SQLite): wrap the MinIO step in `if os.environ.get("MINIO_ENDPOINT"): ...` so the test env without MinIO does not import minio at migration time. Also wrap `op.alter_column` with `with op.batch_alter_table("documents") as batch_op: batch_op.alter_column("user_id", existing_type=sa.dialects.postgresql.UUID(as_uuid=True), nullable=False)` — batch mode is required for SQLite ALTER COLUMN, and is a no-op pass-through on PostgreSQL. Section 7's `op.create_index` works on both dialects without batch mode.
|
||||
|
||||
downgrade() body: section 1 `op.drop_index("ix_topics_user_id", table_name="topics")`; section 2 `with op.batch_alter_table("documents") as batch_op: batch_op.alter_column("user_id", existing_type=postgresql.UUID(as_uuid=True), nullable=True)`. Docstring on downgrade() states "WARNING: deleted null-user document rows and their MinIO objects are NOT restored — Phase 3 cleanup is one-way per CONTEXT.md D-01." NO GRANT statements are needed (no new tables per RESEARCH.md Finding 1).
|
||||
|
||||
Do NOT introduce `pytest.mark.xfail` removal in `test_migration_0003`; leave xfail(strict=False) so it XPASSes silently (consistent with Wave 0 D-Wave0 from Phase 1).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && pytest tests/test_alembic.py::test_migration_0003 -x -q 2>&1 | tail -20 && grep -c "DELETE FROM topics" backend/migrations/versions/0003_multi_user_isolation.py && grep -c "ix_topics_user_id" backend/migrations/versions/0003_multi_user_isolation.py && grep -c "SUM(size_bytes)" backend/migrations/versions/0003_multi_user_isolation.py && grep -c "nullable=False" backend/migrations/versions/0003_multi_user_isolation.py</automated>
|
||||
</verify>
|
||||
<done>
|
||||
`test_migration_0003` either passes or xpasses (no FAILED). The migration file contains: `revision = "0003"`, `down_revision = "0002"`, `DELETE FROM topics`, `ix_topics_user_id`, `SUM(size_bytes)`, `nullable=False`, and a `Minio` import inside upgrade(). `grep -c "revision = \"0003\"" backend/migrations/versions/0003_multi_user_isolation.py` returns 1.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- All 16 new test IDs collect cleanly: `cd backend && pytest tests/ --collect-only -q 2>&1 | grep -E "(test_quota|test_migration_0003|test_per_user_provider|test_upload_url|test_confirm|test_get_quota|test_cross_user|test_admin_cannot|test_topic_namespace|test_settings_endpoint_removed)" | wc -l` >= 16
|
||||
- Migration applies cleanly against a pre-seeded SQLite DB (covered by `test_migration_0003`)
|
||||
- Existing tests still pass: `cd backend && pytest tests/test_documents.py tests/test_topics.py tests/test_settings.py -x -q --ignore-glob="*test_quota*"` reports no regressions in previously-green tests
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 6 test files exist with the 16 new test IDs from VALIDATION.md
|
||||
- `backend/tests/conftest.py` exports `auth_user`, `admin_user`, `mock_minio_presigned`, `mock_minio_stat`
|
||||
- `backend/migrations/versions/0003_multi_user_isolation.py` exists with revision "0003", down_revision "0002"
|
||||
- Migration upgrade() deletes null-user documents (DB + MinIO), deletes all topics, alters documents.user_id to NOT NULL, creates ix_topics_user_id, reconciles quotas.used_bytes
|
||||
- Migration downgrade() reverses schema-level changes only (documented in docstring)
|
||||
- All Wave 0 stubs are xfail(strict=False) — none of them pass/fail unexpectedly
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/03-document-migration-multi-user-isolation/03-01-SUMMARY.md` when done — list every new test ID and the migration revision identifier; flag the SQLite-vs-PostgreSQL alter_column batch-mode choice as a key downstream constraint.
|
||||
</output>
|
||||
@@ -0,0 +1,384 @@
|
||||
---
|
||||
phase: 03-document-migration-multi-user-isolation
|
||||
plan: 02
|
||||
type: execute
|
||||
wave: 2
|
||||
depends_on:
|
||||
- 03-01
|
||||
files_modified:
|
||||
- backend/storage/base.py
|
||||
- backend/storage/minio_backend.py
|
||||
- backend/storage/__init__.py
|
||||
- backend/config.py
|
||||
- backend/api/documents.py
|
||||
- backend/api/auth.py
|
||||
- backend/services/storage.py
|
||||
- backend/tasks/document_tasks.py
|
||||
- backend/celery_app.py
|
||||
- docker-compose.yml
|
||||
autonomous: true
|
||||
requirements:
|
||||
- STORE-03
|
||||
- STORE-04
|
||||
- STORE-05
|
||||
- STORE-06
|
||||
- SEC-04
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "Frontend can request a presigned PUT URL targeting a browser-resolvable hostname (not 'minio:9000') for a newly created pending Document row"
|
||||
- "After the browser PUT, /confirm reads the authoritative file size from MinIO stat_object and atomically updates the user's quota, returning 413 with {used_bytes, limit_bytes, rejected_bytes} on overflow"
|
||||
- "Two concurrent /confirm calls that would together exceed the quota result in exactly one 200 and one 413"
|
||||
- "Document delete decrements quota using GREATEST(0, used_bytes - delta) atomically"
|
||||
- "Authenticated user can read their own quota via GET /api/auth/me/quota"
|
||||
- "Celery beat runs cleanup_abandoned_uploads every 30 minutes, deleting pending Document rows older than 1 hour and removing their MinIO objects"
|
||||
- "Browser PUT requests succeed against MinIO because MINIO_API_CORS_ALLOW_ORIGIN is set to the configured CORS origin"
|
||||
artifacts:
|
||||
- path: "backend/storage/base.py"
|
||||
provides: "StorageBackend ABC with generate_presigned_put_url and stat_object abstract methods"
|
||||
contains: "generate_presigned_put_url"
|
||||
- path: "backend/storage/minio_backend.py"
|
||||
provides: "MinIOBackend dual-client (internal + public) + generate_presigned_put_url + stat_object methods"
|
||||
contains: "self._public_client"
|
||||
- path: "backend/storage/__init__.py"
|
||||
provides: "get_storage_backend factory passes public_endpoint to MinIOBackend"
|
||||
contains: "public_endpoint"
|
||||
- path: "backend/api/documents.py"
|
||||
provides: "POST /api/documents/upload-url + POST /api/documents/{id}/confirm endpoints; old /upload removed"
|
||||
contains: "/upload-url"
|
||||
- path: "backend/api/auth.py"
|
||||
provides: "GET /api/auth/me/quota endpoint"
|
||||
contains: "/me/quota"
|
||||
- path: "backend/services/storage.py"
|
||||
provides: "delete_document decrements quota atomically; save_upload removed"
|
||||
contains: "GREATEST(0, used_bytes"
|
||||
- path: "backend/tasks/document_tasks.py"
|
||||
provides: "cleanup_abandoned_uploads Celery task + _cleanup_abandoned async body"
|
||||
contains: "cleanup_abandoned_uploads"
|
||||
- path: "backend/celery_app.py"
|
||||
provides: "beat_schedule entry for cleanup_abandoned_uploads every 30 minutes"
|
||||
contains: "beat_schedule"
|
||||
- path: "docker-compose.yml"
|
||||
provides: "MINIO_API_CORS_ALLOW_ORIGIN + MINIO_PUBLIC_ENDPOINT env vars; celery-beat service"
|
||||
contains: "MINIO_API_CORS_ALLOW_ORIGIN"
|
||||
key_links:
|
||||
- from: "backend/api/documents.py"
|
||||
to: "backend/storage/minio_backend.py"
|
||||
via: "backend.generate_presigned_put_url(object_key) and backend.stat_object(object_key)"
|
||||
pattern: "generate_presigned_put_url|stat_object"
|
||||
- from: "backend/api/documents.py"
|
||||
to: "quotas table"
|
||||
via: "atomic UPDATE quotas SET used_bytes = used_bytes + :delta WHERE (used_bytes + :delta) <= limit_bytes RETURNING used_bytes"
|
||||
pattern: "UPDATE quotas"
|
||||
- from: "backend/api/documents.py"
|
||||
to: "Celery (tasks.document_tasks.extract_and_classify)"
|
||||
via: "extract_and_classify.delay(str(doc.id)) after successful confirm"
|
||||
pattern: "extract_and_classify\\.delay"
|
||||
- from: "backend/celery_app.py"
|
||||
to: "backend/tasks/document_tasks.py"
|
||||
via: "beat_schedule: tasks.document_tasks.cleanup_abandoned_uploads every 30m"
|
||||
pattern: "cleanup_abandoned_uploads"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Implement the presigned upload backend per CONTEXT.md D-04..D-07: replace multipart POST /upload with a 3-step flow (upload-url → browser PUT direct to MinIO → confirm) using the atomic quota UPDATE pattern from CLAUDE.md. Add GET /api/auth/me/quota for the sidebar quota bar. Wire the abandoned-upload cleanup Celery beat task per D-06.
|
||||
|
||||
Purpose: This plan is the only place the atomic quota SQL lives. Plan 03-03's auth guards depend on the endpoint surface defined here. Plan 03-05's frontend depends on the API contract defined here.
|
||||
Output: 6 backend code changes + 1 docker-compose change. After this plan, an authenticated user can perform the full presigned upload/delete/quota cycle from curl; auth guards still allow anonymous access (closed in Plan 03-03).
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/nik/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<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-UI-SPEC.md
|
||||
@.planning/phases/03-document-migration-multi-user-isolation/03-VALIDATION.md
|
||||
@.planning/phases/03-document-migration-multi-user-isolation/03-01-SUMMARY.md
|
||||
@CLAUDE.md
|
||||
|
||||
@backend/storage/base.py
|
||||
@backend/storage/minio_backend.py
|
||||
@backend/storage/__init__.py
|
||||
@backend/api/documents.py
|
||||
@backend/api/auth.py
|
||||
@backend/services/storage.py
|
||||
@backend/tasks/document_tasks.py
|
||||
@backend/celery_app.py
|
||||
@backend/config.py
|
||||
@docker-compose.yml
|
||||
|
||||
<interfaces>
|
||||
<!-- Contracts the executor needs without re-reading the codebase. -->
|
||||
|
||||
From backend/storage/base.py (current — Phase 1):
|
||||
```
|
||||
class StorageBackend(ABC):
|
||||
@abstractmethod
|
||||
async def put_object(self, user_id, document_id, file_bytes, extension, content_type) -> str
|
||||
@abstractmethod
|
||||
async def get_object(self, object_key) -> bytes
|
||||
@abstractmethod
|
||||
async def delete_object(self, object_key) -> None
|
||||
@abstractmethod
|
||||
async def presigned_get_url(self, object_key, expires_minutes=60) -> str
|
||||
@abstractmethod
|
||||
async def health_check(self) -> bool
|
||||
```
|
||||
|
||||
From backend/storage/minio_backend.py (current):
|
||||
```
|
||||
class MinIOBackend(StorageBackend):
|
||||
def __init__(self, endpoint, access_key, secret_key, bucket, secure=False) -> None
|
||||
# Each method wraps the synchronous Minio SDK in asyncio.to_thread.
|
||||
```
|
||||
|
||||
From CONTEXT.md decisions (D-05):
|
||||
POST /api/documents/upload-url body: {"filename": str, "content_type": str}
|
||||
Returns: {"upload_url": str, "document_id": str}
|
||||
POST /api/documents/{id}/confirm body: empty
|
||||
Returns: {"id": str, "size_bytes": int, "used_bytes": int, "status": "uploaded"}
|
||||
Quota exceeded: HTTP 413 detail = {"used_bytes": int, "limit_bytes": int, "rejected_bytes": int}
|
||||
|
||||
From backend/services/auth.py (Phase 2):
|
||||
def create_access_token(user_id: str, role: str) -> str
|
||||
|
||||
From backend/db/models.py:
|
||||
Document.user_id — Plan 03-01 migration flips to NOT NULL
|
||||
Document.status — 'pending' | 'uploaded' | 'classified' | 'classification_failed'
|
||||
Document.object_key
|
||||
Document.size_bytes
|
||||
Quota.limit_bytes — default 104857600 (100 MB)
|
||||
Quota.used_bytes
|
||||
|
||||
From backend/celery_app.py (current):
|
||||
celery_app.conf.task_routes = {
|
||||
"tasks.document_tasks.*": {"queue": "documents"},
|
||||
"tasks.email_tasks.*": {"queue": "email"},
|
||||
}
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| browser → MinIO (direct PUT) | Untrusted browser uses time-limited presigned URL; MinIO authenticates via HMAC signature; CORS preflight must succeed |
|
||||
| browser → FastAPI /confirm | Authenticated user provides only document_id; FastAPI reads size_bytes from MinIO stat (never from client) |
|
||||
| FastAPI /confirm → quotas table | Concurrent /confirm calls race against the same Quota row; PostgreSQL row-level lock + atomic UPDATE WHERE clause prevents overflow |
|
||||
| Celery beat → DB+MinIO | Runs as docuvault_app; deletes its own rows (status='pending', age > 1h) and own MinIO objects only |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-03-04 | Spoofing | POST /api/documents/upload-url body filename/content_type | mitigate | object_key is computed server-side as {user_id}/{document_id}/{uuid4()}{ext} (STORE-02) — filename stored in DB column only; extension derived from Path(body.filename).suffix.lower() |
|
||||
| T-03-05 | Tampering | Confirm endpoint quota delta | mitigate | size_bytes always comes from backend.stat_object(doc.object_key) (MinIO authoritative) — never from client body or request param |
|
||||
| T-03-06 | Denial of Service | Concurrent /confirm uploads at quota boundary (SC2) | mitigate | Atomic SQL UPDATE quotas SET used_bytes = used_bytes + :delta WHERE (used_bytes + :delta) <= limit_bytes RETURNING used_bytes; fetchone() None → HTTP 413 (RESEARCH.md Finding 4 + Risk 2) |
|
||||
| T-03-07 | Information Disclosure | Presigned URL leakage in logs | accept | 15-min TTL, single object key; leak risk acceptable for v1; do not log full URL — log only document_id |
|
||||
| T-03-08 | Repudiation | Abandoned uploads pile up MinIO orphans | mitigate | Celery beat cleanup_abandoned_uploads every 30m deletes pending docs older than 1 hour and their MinIO objects (D-06) |
|
||||
| T-03-09 | Information Disclosure | Browser→MinIO PUT CORS misconfiguration leaks origin | mitigate | MINIO_API_CORS_ALLOW_ORIGIN env var explicitly set to ${CORS_ORIGINS} (or http://localhost:5173 default) — not wildcard |
|
||||
| T-03-10 | Tampering | Docker hostname in presigned URL (minio:9000 not browser-resolvable) | mitigate | Dual MinIO client: self._client (internal endpoint, used for stat/get/put/delete) and self._public_client (public endpoint, used for generate_presigned_put_url) per RESEARCH.md Finding 3 |
|
||||
| T-03-SC | Tampering | pip installs | mitigate | No new package installs (minio + sqlalchemy already pinned in Phase 1 requirements.txt) |
|
||||
</threat_model>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Extend StorageBackend ABC + MinIOBackend with dual client, presigned PUT, stat_object; add config knobs and docker-compose env</name>
|
||||
<files>backend/storage/base.py, backend/storage/minio_backend.py, backend/storage/__init__.py, backend/config.py, docker-compose.yml</files>
|
||||
<read_first>
|
||||
- backend/storage/base.py — existing ABC method signatures and docstring style
|
||||
- backend/storage/minio_backend.py — existing constructor, asyncio.to_thread pattern, presigned_get_url example
|
||||
- backend/storage/__init__.py — current get_storage_backend factory
|
||||
- backend/config.py — Settings class shape, SettingsConfigDict usage, existing optional-string fields
|
||||
- docker-compose.yml — current minio service environment block, current celery-worker shape (model for celery-beat)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 2 (presigned_put_object signature), Finding 3 (dual-client + CORS), Finding 5 (stat_object), Finding 10 (celery-beat service block)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- StorageBackend ABC gains two new abstract methods: generate_presigned_put_url(object_key, expires_minutes=15) -> str and stat_object(object_key) -> int
|
||||
- MinIOBackend constructor accepts optional public_endpoint: str | None = None; stores self._public_client (Minio instance pointing at public_endpoint or falling back to endpoint)
|
||||
- MinIOBackend.generate_presigned_put_url uses self._public_client.presigned_put_object via asyncio.to_thread with timedelta(minutes=expires_minutes)
|
||||
- MinIOBackend.stat_object uses self._client.stat_object via asyncio.to_thread and returns .size (int)
|
||||
- get_storage_backend() reads settings.minio_public_endpoint and passes it to MinIOBackend
|
||||
- config.py adds minio_public_endpoint: str = "" (empty falls back to minio_endpoint inside MinIOBackend)
|
||||
- docker-compose.yml minio service env adds MINIO_API_CORS_ALLOW_ORIGIN: ${CORS_ORIGINS:-http://localhost:5173}; backend service env adds MINIO_PUBLIC_ENDPOINT; new celery-beat service runs celery -A celery_app beat --loglevel=info
|
||||
</behavior>
|
||||
<action>
|
||||
Modify `backend/storage/base.py`: append two `@abstractmethod async def` blocks after `presigned_get_url`, named `generate_presigned_put_url(self, object_key: str, expires_minutes: int = 15) -> str` and `stat_object(self, object_key: str) -> int`. Docstrings cite RESEARCH.md Finding 3 (public client requirement) and Finding 5 (returns authoritative size).
|
||||
|
||||
Modify `backend/storage/minio_backend.py`: extend `__init__` signature with `public_endpoint: str | None = None`; after the existing `self._client = Minio(...)` block add `self._public_client = Minio(endpoint=(public_endpoint or endpoint), access_key=access_key, secret_key=secret_key, secure=secure)`. Append two new async methods (after `presigned_get_url`):
|
||||
- `generate_presigned_put_url(self, object_key, expires_minutes=15)` — `return await asyncio.to_thread(self._public_client.presigned_put_object, self._bucket, object_key, timedelta(minutes=expires_minutes))`
|
||||
- `stat_object(self, object_key)` — `result = await asyncio.to_thread(self._client.stat_object, self._bucket, object_key); return result.size`
|
||||
|
||||
Modify `backend/storage/__init__.py`: extend `get_storage_backend()` to pass `public_endpoint=settings.minio_public_endpoint` to the `MinIOBackend(...)` constructor.
|
||||
|
||||
Modify `backend/config.py`: add `minio_public_endpoint: str = ""` to the `Settings` class alongside the existing `minio_endpoint` field. Comment reference: `# RESEARCH.md Finding 3 — browser-resolvable hostname for presigned URLs`.
|
||||
|
||||
Modify `docker-compose.yml`:
|
||||
1. In the `minio:` service `environment:` block, add `MINIO_API_CORS_ALLOW_ORIGIN: ${CORS_ORIGINS:-http://localhost:5173}` (RESEARCH.md Finding 3, T-03-09).
|
||||
2. In the `backend:` service `environment:` block, add `- MINIO_PUBLIC_ENDPOINT=${MINIO_PUBLIC_ENDPOINT:-localhost:9000}`.
|
||||
3. Append a new `celery-beat:` service mirroring `celery-worker` structure with `command: celery -A celery_app beat --loglevel=info` and the same `depends_on`, environment block, and build context (RESEARCH.md Finding 10).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && python -c "from storage.base import StorageBackend; assert 'generate_presigned_put_url' in dir(StorageBackend); assert 'stat_object' in dir(StorageBackend); from storage.minio_backend import MinIOBackend; import inspect; sig = inspect.signature(MinIOBackend.__init__); assert 'public_endpoint' in sig.parameters; print('OK')" && grep -c "MINIO_API_CORS_ALLOW_ORIGIN" docker-compose.yml && grep -c "celery-beat:" docker-compose.yml && grep -c "minio_public_endpoint" backend/config.py</automated>
|
||||
</verify>
|
||||
<done>
|
||||
StorageBackend ABC has 7 abstract methods (5 original + 2 new). MinIOBackend `__init__` accepts `public_endpoint`. docker-compose.yml contains `MINIO_API_CORS_ALLOW_ORIGIN` and `celery-beat:`. config.py contains `minio_public_endpoint`. All grep counts >= 1.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Implement upload-url, confirm, delete-with-quota, me/quota endpoints + remove old /upload + abandoned-upload Celery beat</name>
|
||||
<files>backend/api/documents.py, backend/api/auth.py, backend/services/storage.py, backend/tasks/document_tasks.py, backend/celery_app.py</files>
|
||||
<read_first>
|
||||
- backend/api/documents.py — existing handler structure (will be replaced)
|
||||
- backend/api/auth.py — existing /me endpoint pattern, router prefix, `from db.models import BackupCode, Quota, RefreshToken, User` import line
|
||||
- backend/services/storage.py — existing save_upload, delete_document, _backend() helper, _doc_to_dict, list_metadata
|
||||
- backend/tasks/document_tasks.py — existing extract_and_classify task; pattern for sync entry + async body
|
||||
- backend/tasks/email_tasks.py — _run_send_security_alert shape for async task body
|
||||
- backend/celery_app.py — current task_routes block (model for beat_schedule append)
|
||||
- backend/deps/auth.py — get_current_user signature (this plan does NOT wire auth into documents.py — Plan 03-03 does)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 4 (atomic quota SQL), Finding 5 (stat_object error handling), Finding 9 (/me/quota), Finding 10 (Celery beat schedule)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md — atomic quota pattern, Celery task pattern
|
||||
</read_first>
|
||||
<behavior>
|
||||
- POST /api/documents/upload-url body {filename: str, content_type: str} returns {upload_url: str, document_id: str} and inserts a Document row with user_id=None (Wave 2 placeholder — Plan 03-03 replaces with current_user.id), status="pending", object_key=f"null-user/{doc_id}/{uuid4()}{ext}", size_bytes=0
|
||||
- POST /api/documents/{id}/confirm: load Document by id; call backend.stat_object(doc.object_key); catch minio.error.S3Error code="NoSuchKey" → return 422; on success set doc.size_bytes; execute atomic quota UPDATE; on row None → DELETE pending Document + remove MinIO object, return 413 {used_bytes, limit_bytes, rejected_bytes}; on success set status="uploaded", commit, enqueue extract_and_classify.delay(str(doc.id)); return {id, size_bytes, used_bytes, status}
|
||||
- GET /api/auth/me/quota: load Quota by current_user.id; return {used_bytes, limit_bytes}; if quota missing return 404
|
||||
- DELETE /api/documents/{id}: existing handler retained but services.storage.delete_document gains an atomic quota decrement (UPDATE quotas SET used_bytes = GREATEST(0, used_bytes - :delta) WHERE user_id = :uid) immediately before row delete
|
||||
- Old POST /api/documents/upload (multipart) is removed entirely; services.storage.save_upload is removed
|
||||
- tasks/document_tasks.py adds cleanup_abandoned_uploads task and async _cleanup_abandoned body that selects status="pending" AND created_at < now()-1h, deletes MinIO objects (try/except), then session.delete(doc), commits; returns {"cleaned": int}
|
||||
- celery_app.py adds beat_schedule entry "cleanup-abandoned-uploads" → "tasks.document_tasks.cleanup_abandoned_uploads" with schedule timedelta(minutes=30) and timezone "UTC"
|
||||
- test_upload_url_endpoint, test_confirm_endpoint, test_get_quota, test_quota_increment_atomic, test_concurrent_quota_race, test_quota_exceeded_response, test_delete_decrements_quota transition from xfail → pass (or xpass under strict=False)
|
||||
</behavior>
|
||||
<action>
|
||||
Rewrite `backend/api/documents.py`. Module-level imports: `from __future__ import annotations`, `import uuid`, `from pathlib import Path`, `from typing import Optional`, `from fastapi import APIRouter, Depends, HTTPException, Query, status`, `from pydantic import BaseModel`, `from sqlalchemy import text`, `from sqlalchemy.ext.asyncio import AsyncSession`, `from db.models import Document, Quota`, `from deps.db import get_db`, `from services import classifier, storage`, `from tasks.document_tasks import extract_and_classify`, `from storage import get_storage_backend`, `from minio.error import S3Error`. Router: `router = APIRouter(prefix="/api/documents", tags=["documents"])`.
|
||||
|
||||
Define request model `class UploadUrlRequest(BaseModel): filename: str; content_type: str`. No response models — return plain dicts matching the documented shape.
|
||||
|
||||
Endpoint POST `/upload-url`: handler signature `async def request_upload_url(body: UploadUrlRequest, session: AsyncSession = Depends(get_db))`. Generate `doc_id = uuid.uuid4()`; `suffix = Path(body.filename).suffix.lower()`; `object_key = f"null-user/{doc_id}/{uuid.uuid4()}{suffix}"` (Plan 03-03 replaces `"null-user"` with `str(current_user.id)`); insert `Document(id=doc_id, user_id=None, filename=body.filename, content_type=body.content_type, size_bytes=0, storage_backend="minio", status="pending", object_key=object_key)`; `session.add(doc); await session.commit()`; call `upload_url = await get_storage_backend().generate_presigned_put_url(object_key, expires_minutes=15)`; return `{"upload_url": upload_url, "document_id": str(doc_id)}`.
|
||||
|
||||
Endpoint POST `/{doc_id}/confirm`: handler signature `async def confirm_upload(doc_id: str, session: AsyncSession = Depends(get_db))`. Parse `uid = uuid.UUID(doc_id)` (catch ValueError → 404); `doc = await session.get(Document, uid)`; if None → HTTPException(404, "Document not found"). Try `size = await get_storage_backend().stat_object(doc.object_key)`; catch `S3Error` where `getattr(e, "code", "") == "NoSuchKey"` → HTTPException(422, "Upload not found — presigned URL may have expired"). Set `doc.size_bytes = size`; `await session.flush()`. Conditional atomic quota update — if `doc.user_id is not None`: execute `text("UPDATE quotas SET used_bytes = used_bytes + :delta WHERE user_id = :uid AND (used_bytes + :delta) <= limit_bytes RETURNING used_bytes, limit_bytes")` with `{"delta": size, "uid": str(doc.user_id)}`; `row = result.fetchone()`. If `row is None` (quota exceeded): execute SELECT `text("SELECT used_bytes, limit_bytes FROM quotas WHERE user_id = :uid")`; `q = quota_row.fetchone()`; delete the pending Document via `await session.delete(doc)`; best-effort `try: await get_storage_backend().delete_object(doc.object_key) except Exception: pass`; `await session.commit()`; raise `HTTPException(status_code=413, detail={"used_bytes": q.used_bytes if q else 0, "limit_bytes": q.limit_bytes if q else 0, "rejected_bytes": size})`. On success path: `doc.status = "uploaded"`; `await session.commit()`; `extract_and_classify.delay(str(doc.id))`; return `{"id": str(doc.id), "size_bytes": size, "used_bytes": (row.used_bytes if row else 0), "status": "uploaded"}`. If `doc.user_id is None` (Wave 2 only): skip quota update entirely, set `doc.status = "uploaded"`, commit, enqueue, return `{"id": str(doc.id), "size_bytes": size, "used_bytes": 0, "status": "uploaded"}`.
|
||||
|
||||
Endpoints GET `""` (list), GET `/{doc_id}` (get), DELETE `/{doc_id}` (delete), POST `/{doc_id}/classify` (reclassify): preserve the current handler bodies verbatim from existing `backend/api/documents.py` (lines 70-118) but rebound to the new router. NOTE: this plan does NOT yet add `Depends(get_current_user)` — Plan 03-03 adds auth wiring. Anonymous handlers remain in Wave 2.
|
||||
|
||||
Modify `backend/services/storage.py`:
|
||||
- Delete `save_upload(session, file_bytes, original_name, mime_type)` function entirely (lines ~86-136) and its entry in `__all__`.
|
||||
- At top imports add `from sqlalchemy import text` (current import is `from sqlalchemy import select, delete` — extend the line).
|
||||
- Modify `delete_document(session, doc_id)`: after the existing `try: await _backend().delete_object(doc.object_key) ... except` block and BEFORE `await session.delete(doc)`, insert:
|
||||
```
|
||||
if doc.user_id is not None:
|
||||
await session.execute(
|
||||
text("UPDATE quotas SET used_bytes = GREATEST(0, used_bytes - :delta) WHERE user_id = :uid"),
|
||||
{"delta": doc.size_bytes, "uid": str(doc.user_id)},
|
||||
)
|
||||
```
|
||||
(Comment: "Atomic quota decrement (STORE-06, D-07). The user_id is None guard is removed in Plan 03-03.")
|
||||
|
||||
Modify `backend/api/auth.py`: append a `GET /me/quota` handler after the existing `/me` handler. Handler:
|
||||
```
|
||||
@router.get("/me/quota")
|
||||
async def get_my_quota(
|
||||
current_user: User = Depends(get_current_user),
|
||||
session: AsyncSession = Depends(get_db),
|
||||
):
|
||||
q = await session.get(Quota, current_user.id)
|
||||
if q is None:
|
||||
raise HTTPException(status_code=404, detail="Quota not found")
|
||||
return {"used_bytes": q.used_bytes, "limit_bytes": q.limit_bytes}
|
||||
```
|
||||
`Quota` is already imported on the existing `from db.models import BackupCode, Quota, RefreshToken, User` line — verify, no new import needed.
|
||||
|
||||
Modify `backend/tasks/document_tasks.py`: append at end of file (after `_run`):
|
||||
```
|
||||
@celery_app.task(name="tasks.document_tasks.cleanup_abandoned_uploads")
|
||||
def cleanup_abandoned_uploads() -> dict:
|
||||
"""Periodic Celery beat task — deletes Document rows with status='pending'
|
||||
older than 1 hour and their MinIO objects (D-06).
|
||||
"""
|
||||
return asyncio.run(_cleanup_abandoned())
|
||||
|
||||
|
||||
async def _cleanup_abandoned() -> dict:
|
||||
from datetime import datetime, timezone, timedelta
|
||||
from sqlalchemy import select
|
||||
|
||||
from db.session import AsyncSessionLocal
|
||||
from db.models import Document
|
||||
from storage import get_storage_backend
|
||||
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(hours=1)
|
||||
async with AsyncSessionLocal() as session:
|
||||
result = await session.execute(
|
||||
select(Document).where(
|
||||
Document.status == "pending",
|
||||
Document.created_at < cutoff,
|
||||
)
|
||||
)
|
||||
docs = result.scalars().all()
|
||||
backend = get_storage_backend()
|
||||
cleaned = 0
|
||||
for doc in docs:
|
||||
try:
|
||||
if doc.object_key:
|
||||
await backend.delete_object(doc.object_key)
|
||||
except Exception:
|
||||
pass # MinIO object may not exist yet — safe to ignore
|
||||
await session.delete(doc)
|
||||
cleaned += 1
|
||||
await session.commit()
|
||||
return {"cleaned": cleaned}
|
||||
```
|
||||
|
||||
Modify `backend/celery_app.py`: at module top add `from datetime import timedelta as _timedelta`. After the existing `celery_app.conf.task_routes = {...}` block (before `autodiscover_tasks`), append:
|
||||
```
|
||||
celery_app.conf.beat_schedule = {
|
||||
"cleanup-abandoned-uploads": {
|
||||
"task": "tasks.document_tasks.cleanup_abandoned_uploads",
|
||||
"schedule": _timedelta(minutes=30),
|
||||
},
|
||||
}
|
||||
celery_app.conf.timezone = "UTC"
|
||||
```
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && pytest tests/test_documents.py::test_upload_url_endpoint tests/test_documents.py::test_confirm_endpoint tests/test_documents.py::test_get_quota tests/test_quota.py -x -q 2>&1 | tail -30 && grep -c "generate_presigned_put_url" backend/api/documents.py && grep -c "stat_object" backend/api/documents.py && grep -c "UPDATE quotas" backend/api/documents.py && grep -v '^[[:space:]]*#' backend/services/storage.py | grep -c "GREATEST(0, used_bytes" && grep -c "cleanup_abandoned_uploads" backend/tasks/document_tasks.py && grep -c "beat_schedule" backend/celery_app.py && grep -c "/me/quota" backend/api/auth.py</automated>
|
||||
</verify>
|
||||
<done>
|
||||
All 7 listed pytest IDs pass (or xpass under strict=False). `backend/api/documents.py` contains `/upload-url`, `/{doc_id}/confirm`, calls `generate_presigned_put_url` + `stat_object` + atomic `UPDATE quotas`. `backend/services/storage.py` contains `GREATEST(0, used_bytes`. `backend/api/auth.py` contains `/me/quota`. `backend/tasks/document_tasks.py` contains `cleanup_abandoned_uploads`. `backend/celery_app.py` contains `beat_schedule`. All grep counts >= 1.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- Atomic quota race test green: `cd backend && pytest tests/test_quota.py::test_concurrent_quota_race -x -q` (SC2 for Phase 3)
|
||||
- Quota rejection shape green: `cd backend && pytest tests/test_quota.py::test_quota_exceeded_response -x -q`
|
||||
- Quota decrement on delete green: `cd backend && pytest tests/test_quota.py::test_delete_decrements_quota -x -q`
|
||||
- Presigned upload endpoints green: `cd backend && pytest tests/test_documents.py::test_upload_url_endpoint tests/test_documents.py::test_confirm_endpoint -x -q`
|
||||
- Quota read endpoint green: `cd backend && pytest tests/test_documents.py::test_get_quota -x -q`
|
||||
- No legacy /upload route remains: `cd backend && grep -c '"/upload"' backend/api/documents.py` returns 0
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- StorageBackend ABC has `generate_presigned_put_url` and `stat_object` abstract methods
|
||||
- MinIOBackend has dual MinIO client instances; presigned URL uses public client
|
||||
- POST /api/documents/upload-url, POST /api/documents/{id}/confirm, GET /api/auth/me/quota exist and respond as documented
|
||||
- DELETE /api/documents/{id} decrements quota atomically when doc.user_id is not None
|
||||
- POST /api/documents/upload (multipart legacy) is removed; services.storage.save_upload is removed
|
||||
- Celery beat schedule includes cleanup-abandoned-uploads every 30 minutes
|
||||
- docker-compose.yml has MINIO_API_CORS_ALLOW_ORIGIN on minio service, MINIO_PUBLIC_ENDPOINT on backend service, and celery-beat service
|
||||
- All Plan 03-01 stubs for STORE-03 / STORE-04 / STORE-05 / STORE-06 / D-05 pass or xpass
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/03-document-migration-multi-user-isolation/03-02-SUMMARY.md` when done — include the exact request/response shapes for /upload-url and /confirm; note the temporary `doc.user_id is None` guard that Plan 03-03 must remove.
|
||||
</output>
|
||||
@@ -0,0 +1,348 @@
|
||||
---
|
||||
phase: 03-document-migration-multi-user-isolation
|
||||
plan: 03
|
||||
type: execute
|
||||
wave: 3
|
||||
depends_on:
|
||||
- 03-02
|
||||
files_modified:
|
||||
- backend/deps/auth.py
|
||||
- backend/api/documents.py
|
||||
- backend/api/topics.py
|
||||
- backend/api/admin.py
|
||||
- backend/services/storage.py
|
||||
- backend/services/classifier.py
|
||||
autonomous: true
|
||||
requirements:
|
||||
- SEC-04
|
||||
- DOC-04
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "Every /api/documents/* handler requires a valid Bearer JWT and a non-admin user role; admins receive 403 on every document endpoint"
|
||||
- "Cross-user access to a document (by manipulating the doc_id path parameter) returns 404 — never reveals that the document exists"
|
||||
- "Newly uploaded Document rows have user_id = current_user.id; object_key embeds str(current_user.id) (no more 'null-user' sentinel)"
|
||||
- "GET /api/topics returns the union of system topics (user_id IS NULL) and the requesting user's own topics — never another user's topics"
|
||||
- "POST /api/admin/topics requires admin role; creates a Topic with user_id = NULL"
|
||||
- "Regular POST /api/topics creates a Topic with user_id = current_user.id"
|
||||
- "Atomic quota path in /confirm runs unconditionally (no more None-user fallback) for every authenticated upload"
|
||||
artifacts:
|
||||
- path: "backend/deps/auth.py"
|
||||
provides: "get_regular_user dependency (current_user with admin-403)"
|
||||
exports:
|
||||
- "get_regular_user"
|
||||
- path: "backend/api/documents.py"
|
||||
provides: "All handlers wired with get_regular_user dep + ownership assertion (doc.user_id == current_user.id else 404)"
|
||||
contains: "get_regular_user"
|
||||
- path: "backend/api/topics.py"
|
||||
provides: "All handlers wired with get_current_user dep + namespace-scoped queries"
|
||||
contains: "or_"
|
||||
- path: "backend/api/admin.py"
|
||||
provides: "POST /api/admin/topics endpoint (admin-only system topic creation)"
|
||||
contains: "/admin/topics"
|
||||
- path: "backend/services/storage.py"
|
||||
provides: "load_topics_for_user(session, user_id) + create_topic accepts user_id"
|
||||
exports:
|
||||
- "load_topics_for_user"
|
||||
- path: "backend/services/classifier.py"
|
||||
provides: "classify_document passes user_id through topic lookup and create_topic calls"
|
||||
contains: "load_topics_for_user"
|
||||
key_links:
|
||||
- from: "backend/api/documents.py"
|
||||
to: "backend/deps/auth.py"
|
||||
via: "current_user: User = Depends(get_regular_user) on every handler"
|
||||
pattern: "Depends\\(get_regular_user\\)"
|
||||
- from: "backend/api/documents.py"
|
||||
to: "documents table"
|
||||
via: "ownership assertion: if doc is None or doc.user_id != current_user.id → 404"
|
||||
pattern: "doc\\.user_id != current_user\\.id"
|
||||
- from: "backend/api/topics.py"
|
||||
to: "topics table"
|
||||
via: "WHERE (Topic.user_id == current_user.id OR Topic.user_id IS NULL)"
|
||||
pattern: "or_\\(Topic\\.user_id"
|
||||
- from: "backend/services/classifier.py"
|
||||
to: "backend/services/storage.py"
|
||||
via: "create_topic(session, name, user_id=doc.user_id) to keep AI-suggested topics in the user namespace"
|
||||
pattern: "create_topic.*user_id"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Enforce per-user isolation across all `/api/documents/*` and `/api/topics/*` endpoints. Add the `get_regular_user` dependency (rejects admin), wire it into every documents handler with ownership assertions (404 on cross-user access per D-16), add `get_current_user` to topics with namespace-scoped queries (D-17), and create the admin-only `POST /api/admin/topics` endpoint (D-09). Remove the Wave 2 `null-user` placeholder from upload-url so newly created Documents carry the authenticated user_id.
|
||||
|
||||
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.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/nik/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<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
|
||||
|
||||
<interfaces>
|
||||
<!-- Contracts the executor needs without re-reading the codebase. -->
|
||||
|
||||
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]
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<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>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Add get_regular_user dependency; wire auth + ownership + real user_id into /api/documents/*</name>
|
||||
<files>backend/deps/auth.py, backend/api/documents.py, backend/services/storage.py</files>
|
||||
<read_first>
|
||||
- backend/deps/auth.py — current get_current_user, get_current_admin signatures + docstrings (model for get_regular_user)
|
||||
- backend/api/documents.py — current state after Plan 03-02 (upload-url, confirm, list, get, delete, classify); locate the `"null-user"` placeholder and the `if doc.user_id is not None:` Wave 2 fallback
|
||||
- backend/services/storage.py — current list_metadata signature (must accept user_id filter); delete_document; get_metadata
|
||||
- backend/db/models.py — Document.user_id (now NOT NULL post Plan 03-01)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-CONTEXT.md — D-16 (404 not 403, admin 403)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md — "Auth Guard Pattern", "Ownership Assertion (SEC-04)"
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 7 (404 vs 403)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- get_regular_user is a FastAPI dependency that wraps get_current_user and raises 403 if user.role == "admin"
|
||||
- Every handler in api/documents.py adds `current_user: User = Depends(get_regular_user)` parameter
|
||||
- upload-url replaces `"null-user"` in object_key with `str(current_user.id)` and sets `Document.user_id = current_user.id`
|
||||
- confirm endpoint removes the `if doc.user_id is not None:` Wave 2 fallback — the atomic quota path runs for every confirm
|
||||
- confirm endpoint adds ownership assertion before the stat_object call: `if doc is None or doc.user_id != current_user.id: raise HTTPException(404, "Document not found")`
|
||||
- list endpoint filters by user_id = current_user.id (services.storage.list_metadata accepts user_id param)
|
||||
- get endpoint asserts `doc.user_id == current_user.id` else 404
|
||||
- delete endpoint asserts ownership then calls services.storage.delete_document (which decrements quota — Plan 03-02 already wired this; remove the `if doc.user_id is not None:` guard there too)
|
||||
- classify endpoint asserts ownership
|
||||
- services.storage.list_metadata adds required `user_id: uuid.UUID` parameter and `Document.user_id == user_id` filter
|
||||
- services.storage.delete_document removes the `if doc.user_id is not None:` guard (now always non-None)
|
||||
- Tests test_cross_user_access_404, test_admin_cannot_access_documents, test_documents_require_auth transition from xfail → pass
|
||||
</behavior>
|
||||
<action>
|
||||
Modify `backend/deps/auth.py`: append a new dependency function:
|
||||
```
|
||||
async def get_regular_user(user: User = Depends(get_current_user)) -> User:
|
||||
"""Reject admin accounts on all /api/documents/* endpoints (D-16, SC4).
|
||||
|
||||
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).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && pytest tests/test_documents.py::test_cross_user_access_404 tests/test_documents.py::test_admin_cannot_access_documents tests/test_documents.py::test_documents_require_auth tests/test_documents.py::test_upload_url_endpoint tests/test_documents.py::test_confirm_endpoint tests/test_quota.py -x -q 2>&1 | tail -20 && grep -c "get_regular_user" backend/deps/auth.py && grep -c "Depends(get_regular_user)" backend/api/documents.py && grep -v '^[[:space:]]*#' backend/api/documents.py | grep -c "doc.user_id != current_user.id" && grep -c "null-user" backend/api/documents.py</automated>
|
||||
</verify>
|
||||
<done>
|
||||
`get_regular_user` defined in deps/auth.py. `Depends(get_regular_user)` present in every handler of api/documents.py (>= 6 occurrences expected: upload-url, confirm, list, get, delete, classify). `null-user` literal NOT present in api/documents.py (count == 0). Ownership assertion `doc.user_id != current_user.id` appears in at least 4 handlers. All 6 listed tests pass or xpass.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Wire get_current_user + namespace filter into /api/topics/* + add POST /api/admin/topics + propagate user_id through classifier topic ops</name>
|
||||
<files>backend/api/topics.py, backend/api/admin.py, backend/services/storage.py, backend/services/classifier.py</files>
|
||||
<read_first>
|
||||
- backend/api/topics.py — current handler structure (will be modified)
|
||||
- backend/api/admin.py — pattern for `_admin: User = Depends(get_current_admin)` and Pydantic model definitions (UserCreate, AiConfigUpdate)
|
||||
- backend/services/storage.py — current `create_topic` (lines ~328-355), `load_topics` (lines ~289-295), `update_topic`, `delete_topic` signatures
|
||||
- backend/services/classifier.py — current `classify_document` body lines 33-58; `storage.load_topics` and `storage.create_topic` call sites
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-CONTEXT.md — D-08, D-09, D-10, D-11, D-17
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md — Topic namespace query (lines ~401-409)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 6 (namespace query + dedup scoping + Risk 5)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- GET /api/topics requires get_current_user; query filters `or_(Topic.user_id == current_user.id, Topic.user_id.is_(None))` via new service `load_topics_for_user`
|
||||
- POST /api/topics requires get_current_user; creates Topic with user_id = current_user.id (never NULL)
|
||||
- PATCH /api/topics/{id} requires get_current_user; the topic must belong to the user (user_id == current_user.id) — otherwise 404 (NOT 403 — same info-leak rationale as docs)
|
||||
- DELETE /api/topics/{id} requires get_current_user; same ownership rule. System topics (user_id IS NULL) cannot be edited or deleted via /api/topics/*
|
||||
- POST /api/topics/suggest requires get_current_user; existing classifier path
|
||||
- POST /api/admin/topics (NEW in api/admin.py) requires get_current_admin; creates Topic with user_id = NULL
|
||||
- services.storage.create_topic gains `user_id: uuid.UUID | None = None` parameter; dedup query is scoped: `WHERE lower(name) = :name AND user_id IS [NOT DISTINCT FROM] :user_id`
|
||||
- services.storage adds `load_topics_for_user(session, user_id) -> list[dict]` using the or_/is_ filter
|
||||
- services.classifier.classify_document replaces `storage.load_topics(session)` with `storage.load_topics_for_user(session, doc.user_id)` and passes `user_id=doc.user_id` into `storage.create_topic` for AI-suggested topics
|
||||
- Tests test_topic_namespace, test_admin_create_system_topic, test_regular_user_cannot_create_system_topic, test_topics_require_auth transition from xfail → pass
|
||||
</behavior>
|
||||
<action>
|
||||
Modify `backend/api/topics.py`. Add imports `from db.models import User` and `from deps.auth import get_current_user`. For every existing handler add `current_user: User = Depends(get_current_user)` as a parameter:
|
||||
|
||||
- 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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && pytest tests/test_topics.py::test_topic_namespace tests/test_topics.py::test_admin_create_system_topic tests/test_topics.py::test_regular_user_cannot_create_system_topic tests/test_topics.py::test_topics_require_auth tests/test_topics.py -x -q 2>&1 | tail -20 && grep -c "load_topics_for_user" backend/services/storage.py && grep -c "load_topics_for_user" backend/services/classifier.py && grep -c "or_(Topic.user_id" backend/services/storage.py && grep -c "/topics" backend/api/admin.py && grep -c "Depends(get_current_user)" backend/api/topics.py</automated>
|
||||
</verify>
|
||||
<done>
|
||||
All 4 topic-namespace tests pass or xpass. Existing topic tests (`test_list_topics_empty`, `test_create_topic`, etc.) still pass after auth wiring (they may need fixture updates — surface as a separate concern if they regress). `load_topics_for_user` is defined in storage.py and imported in classifier.py. `Depends(get_current_user)` appears in api/topics.py >= 5 times (list, create, update, delete, suggest). `POST /api/admin/topics` exists in api/admin.py.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- Cross-user access returns 404: `cd backend && pytest tests/test_documents.py::test_cross_user_access_404 -x -q`
|
||||
- Admin gets 403 on documents: `cd backend && pytest tests/test_documents.py::test_admin_cannot_access_documents -x -q`
|
||||
- Anonymous gets rejected: `cd backend && pytest tests/test_documents.py::test_documents_require_auth -x -q`
|
||||
- Topic namespace isolation: `cd backend && pytest tests/test_topics.py::test_topic_namespace -x -q`
|
||||
- Admin-only system topic creation: `cd backend && pytest tests/test_topics.py::test_admin_create_system_topic tests/test_topics.py::test_regular_user_cannot_create_system_topic -x -q`
|
||||
- No null-user sentinel remains in api/documents.py: `cd backend && grep -c "null-user" backend/api/documents.py` returns 0
|
||||
- Atomic quota path unconditional: `cd backend && grep -v '^[[:space:]]*#' backend/api/documents.py | grep -c "if doc.user_id is not None" | head -1` returns 0
|
||||
</verification>
|
||||
|
||||
<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>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/03-document-migration-multi-user-isolation/03-03-SUMMARY.md` when done — list every endpoint now requiring auth, the ownership-assertion pattern (404 not 403), and any existing topic/document tests that needed fixture updates to handle the new auth requirement.
|
||||
</output>
|
||||
@@ -0,0 +1,377 @@
|
||||
---
|
||||
phase: 03-document-migration-multi-user-isolation
|
||||
plan: 04
|
||||
type: execute
|
||||
wave: 4
|
||||
depends_on:
|
||||
- 03-03
|
||||
files_modified:
|
||||
- backend/config.py
|
||||
- backend/services/classifier.py
|
||||
- backend/services/storage.py
|
||||
- backend/tasks/document_tasks.py
|
||||
- backend/api/settings.py
|
||||
- backend/main.py
|
||||
- backend/tests/test_settings.py
|
||||
- frontend/src/views/SettingsView.vue
|
||||
- frontend/src/stores/settings.js
|
||||
- frontend/src/api/client.js
|
||||
autonomous: true
|
||||
requirements:
|
||||
- DOC-03
|
||||
- DOC-05
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "AI provider and model are resolved per document from users.ai_provider / users.ai_model via the Celery task (no flat-file read)"
|
||||
- "Falls back to settings.default_ai_provider / settings.default_ai_model env-var defaults when user.ai_provider is None"
|
||||
- "/api/settings endpoint no longer exists; backend/main.py does not register api.settings router"
|
||||
- "services.storage no longer exposes load_settings/save_settings/mask_api_key/settings_masked; settings.json is no longer read or written"
|
||||
- "classifier.classify_document accepts ai_provider and ai_model parameters; no longer reads global config from disk"
|
||||
- "SettingsView.vue displays an admin-managed message; the form is removed; stores/settings.js and api/client.js settings calls are removed"
|
||||
artifacts:
|
||||
- path: "backend/config.py"
|
||||
provides: "SYSTEM_PROMPT + DEFAULT_AI_PROVIDER + DEFAULT_AI_MODEL settings; SETTINGS_FILE / DEFAULT_SETTINGS / DEFAULT_SYSTEM_PROMPT removed"
|
||||
contains: "default_ai_provider"
|
||||
- path: "backend/services/classifier.py"
|
||||
provides: "classify_document accepts ai_provider, ai_model kwargs; _DEFAULT_SYSTEM_PROMPT module constant"
|
||||
contains: "ai_provider"
|
||||
- path: "backend/services/storage.py"
|
||||
provides: "load_settings/save_settings/mask_api_key/settings_masked removed"
|
||||
contains: "load_topics_for_user"
|
||||
- path: "backend/tasks/document_tasks.py"
|
||||
provides: "_run resolves user.ai_provider via session.get(User, doc.user_id) and passes to classifier"
|
||||
contains: "user.ai_provider"
|
||||
- path: "backend/main.py"
|
||||
provides: "api.settings router import + include_router removed"
|
||||
contains: "settings_router"
|
||||
- path: "backend/api/settings.py"
|
||||
provides: "File deleted"
|
||||
- path: "frontend/src/views/SettingsView.vue"
|
||||
provides: "Admin-managed placeholder card; form/store/api removed"
|
||||
contains: "managed by"
|
||||
- path: "frontend/src/stores/settings.js"
|
||||
provides: "File deleted or gutted to empty placeholder"
|
||||
- path: "frontend/src/api/client.js"
|
||||
provides: "getSettings/patchSettings/testProvider/getDefaultPrompt removed"
|
||||
contains: "getMyQuota"
|
||||
key_links:
|
||||
- from: "backend/tasks/document_tasks.py"
|
||||
to: "backend/db/models.py User"
|
||||
via: "user = await session.get(User, doc.user_id); ai_provider = user.ai_provider or settings.default_ai_provider"
|
||||
pattern: "user\\.ai_provider"
|
||||
- from: "backend/services/classifier.py"
|
||||
to: "backend/ai/__init__.py get_provider"
|
||||
via: "_settings = {'active_provider': ai_provider, 'providers': {ai_provider: {'model': ai_model}}}; provider = get_provider(_settings)"
|
||||
pattern: "active_provider.*ai_provider"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Retire the flat-file settings system end-to-end (D-12, D-13) and wire per-user AI classification via DB lookup (D-14, D-15, DOC-03, DOC-05). Move system prompt + defaults to env vars in config.py. Delete /api/settings; refactor classifier to receive ai_provider/ai_model as parameters; update Celery task to look up doc.user_id → user.ai_provider/ai_model and pass through to classifier. Replace the frontend Settings view with an admin-managed placeholder; remove the settings store and client calls.
|
||||
|
||||
Purpose: Phase 3 SC5 (each user's AI classification uses the provider/model assigned to that user by the admin) cannot pass while load_settings() exists. This is the final backend plan for Phase 3.
|
||||
Output: 10 file modifications (3 backend code, 1 backend route removal, 1 backend test update, 1 backend main.py, 3 frontend code, 1 backend config). After this plan, the entire Phase 3 backend is multi-user-isolated and admin-controlled.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/nik/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<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-03-SUMMARY.md
|
||||
@CLAUDE.md
|
||||
|
||||
@backend/config.py
|
||||
@backend/services/classifier.py
|
||||
@backend/services/storage.py
|
||||
@backend/tasks/document_tasks.py
|
||||
@backend/api/settings.py
|
||||
@backend/main.py
|
||||
@backend/ai/__init__.py
|
||||
@backend/tests/test_settings.py
|
||||
@frontend/src/views/SettingsView.vue
|
||||
@frontend/src/stores/settings.js
|
||||
@frontend/src/api/client.js
|
||||
|
||||
<interfaces>
|
||||
<!-- Contracts the executor needs without re-reading the codebase. -->
|
||||
|
||||
From backend/db/models.py:
|
||||
User.ai_provider: Mapped[Optional[str]] # nullable text — set by admin via PATCH /api/admin/users/{id}/ai-config
|
||||
User.ai_model: Mapped[Optional[str]] # nullable text
|
||||
|
||||
From backend/ai/__init__.py (current — RESEARCH.md Finding 12 / A4):
|
||||
def get_provider(settings: dict) -> AIProvider
|
||||
# Accepts dict with keys: "active_provider": str, "providers": {<name>: {"model": str, "api_key"?: str, "base_url"?: str}}
|
||||
# Branches on active_provider for anthropic / openai / ollama / lmstudio
|
||||
|
||||
From backend/tasks/document_tasks.py (current):
|
||||
async def _run(document_id: str) -> dict
|
||||
— fetches Document by id
|
||||
— calls `topics = await classifier.classify_document(session, document_id)` # no ai_provider/ai_model yet
|
||||
|
||||
From backend/services/classifier.py (post Plan 03-03):
|
||||
async def classify_document(session, doc_id, topic_names=None) -> list[str]
|
||||
— currently calls `settings = storage.load_settings(); provider = get_provider(settings)`
|
||||
— must change to `provider = get_provider({"active_provider": ai_provider, "providers": {ai_provider: {"model": ai_model}}})`
|
||||
|
||||
From backend/config.py (current):
|
||||
SETTINGS_FILE = Path(settings.data_dir) / "settings.json" # to be removed
|
||||
DEFAULT_SYSTEM_PROMPT = "..." # to be removed (kept as module const in classifier.py)
|
||||
DEFAULT_SETTINGS = {...} # to be removed
|
||||
class Settings(BaseSettings): ... # extend with new fields
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| user → /api/settings | Endpoint removed; previous attack surface eliminated |
|
||||
| Celery task → users table | Task resolves AI config via doc.user_id → users row; no user-controlled provider/model input |
|
||||
| admin → PATCH /api/admin/users/{id}/ai-config | Existing Phase 2 admin endpoint is the only write path to user.ai_provider / user.ai_model |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-03-17 | Elevation of Privilege | User changing their own AI provider | mitigate | /api/settings is removed (D-12); only PATCH /api/admin/users/{id}/ai-config (admin-only) writes user.ai_provider/ai_model |
|
||||
| T-03-18 | Information Disclosure | settings.json flat file persisted to disk with API keys | mitigate | services.storage.load_settings/save_settings deleted; settings.json no longer read or written; API keys live in env vars only |
|
||||
| T-03-19 | Tampering | Celery task accepting ai_provider in task signature | mitigate | Task signature unchanged (just document_id); ai_provider/ai_model resolved INSIDE the task via DB lookup so a malicious broker message cannot inject provider |
|
||||
| T-03-20 | Information Disclosure | system_prompt env var in container logs | accept | SYSTEM_PROMPT is a static instruction string with no PII; documented as low-sensitivity |
|
||||
| T-03-21 | Repudiation | Frontend SettingsView still attempts old API calls | mitigate | Remove getSettings/patchSettings/testProvider/getDefaultPrompt from api/client.js; gut stores/settings.js; SettingsView shows static message — no API calls |
|
||||
| T-03-SC | Tampering | pip installs | mitigate | No new package installs |
|
||||
</threat_model>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Backend — retire settings flat-file; route classifier through per-user AI config</name>
|
||||
<files>backend/config.py, backend/services/classifier.py, backend/services/storage.py, backend/tasks/document_tasks.py, backend/api/settings.py, backend/main.py, backend/tests/test_settings.py</files>
|
||||
<read_first>
|
||||
- backend/config.py — Settings class fields, SETTINGS_FILE definition, DEFAULT_SYSTEM_PROMPT string, DEFAULT_SETTINGS dict
|
||||
- backend/services/storage.py — load_settings / save_settings / mask_api_key / settings_masked function bodies and __all__ entries (lines ~411-473)
|
||||
- backend/services/classifier.py — current settings load + get_provider call sites (lines 33-35 and 67-69)
|
||||
- backend/tasks/document_tasks.py — _run async body, particularly the classifier.classify_document call (line ~80)
|
||||
- backend/api/settings.py — the entire file (to be deleted)
|
||||
- backend/main.py — settings_router import + include_router call site
|
||||
- backend/ai/__init__.py — get_provider factory signature (RESEARCH.md A4 — verify factory accepts {"active_provider", "providers"} dict)
|
||||
- backend/tests/test_settings.py — existing test file with active tests (to be reduced to test_settings_endpoint_removed)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 8 (Celery refactor), Finding 11 (retirement scope), Finding 12 (get_provider factory)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md — classifier signature (lines ~452-510), config.py additions (lines ~743-752)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- config.py removes `SETTINGS_FILE`, `DEFAULT_SYSTEM_PROMPT`, `DEFAULT_SETTINGS` module-level constants
|
||||
- config.py Settings class gains `system_prompt: str = ""`, `default_ai_provider: str = "ollama"`, `default_ai_model: str = "llama3.2"`
|
||||
- services/classifier.py defines a module-level `_DEFAULT_SYSTEM_PROMPT` constant (the hardcoded string from old config.DEFAULT_SYSTEM_PROMPT). classify_document signature gains `ai_provider: str | None = None, ai_model: str | None = None` keyword params; substitutes config defaults when None; constructs `_settings = {"active_provider": ai_provider, "providers": {ai_provider: {"model": ai_model}}}` and calls `get_provider(_settings)`. NEVER calls `storage.load_settings()`
|
||||
- suggest_topics_for_document gets the same treatment (also gains ai_provider/ai_model kwargs and same fallback)
|
||||
- services/storage.py: removes `load_settings`, `save_settings`, `mask_api_key`, `settings_masked`; removes these from `__all__`; removes `from config import DEFAULT_SETTINGS, SETTINGS_FILE` and any `import json`, `import copy` lines that are no longer used; adds `from config import settings as app_settings` if not already present (Plan 03-04 may not need it, but classifier does)
|
||||
- tasks/document_tasks.py: in `_run` after fetching Document, fetch `user = await session.get(User, doc.user_id)`; compute `ai_provider = (user.ai_provider if user else None) or settings.default_ai_provider`, same for model; pass into `classifier.classify_document(session, document_id, ai_provider=ai_provider, ai_model=ai_model)`. Add `from config import settings` and `from db.models import User` deferred imports inside `_run`
|
||||
- backend/api/settings.py: file deleted
|
||||
- backend/main.py: removes `from api.settings import router as settings_router` and `app.include_router(settings_router)`
|
||||
- backend/tests/test_settings.py: replaces all existing tests with the single `test_settings_endpoint_removed` (the xfail stub from Plan 03-01) — keep file path. Mark previous tests as xfail or simply delete them; the only required test is the 404 assertion
|
||||
- GET /api/settings returns 404 (route no longer exists)
|
||||
- The Plan 03-01 stub tests test_per_user_provider, test_celery_task_uses_user_provider, test_default_provider_fallback, test_settings_endpoint_removed transition from xfail → pass
|
||||
</behavior>
|
||||
<action>
|
||||
Modify `backend/config.py`:
|
||||
1. Remove `SETTINGS_FILE = Path(settings.data_dir) / "settings.json"` (line 60).
|
||||
2. Remove the `DEFAULT_SYSTEM_PROMPT = """..."""` block (lines 62-67).
|
||||
3. Remove the `DEFAULT_SETTINGS = {...}` dict (lines 69-91).
|
||||
4. Inside the `Settings` class (alongside existing fields), add:
|
||||
```
|
||||
# AI classification defaults (Phase 3 — D-13, D-15)
|
||||
system_prompt: str = "" # SYSTEM_PROMPT env var; hardcoded fallback lives in classifier.py
|
||||
default_ai_provider: str = "ollama" # DEFAULT_AI_PROVIDER env var
|
||||
default_ai_model: str = "llama3.2" # DEFAULT_AI_MODEL env var
|
||||
```
|
||||
5. Verify no module-level imports of `Path` or anything else remain unused after the deletions (clean up `from pathlib import Path` if no other consumers).
|
||||
|
||||
Modify `backend/services/classifier.py`:
|
||||
1. At module top, add `_DEFAULT_SYSTEM_PROMPT = """You are a document classification assistant..."""` with the verbatim string from the old `config.DEFAULT_SYSTEM_PROMPT`.
|
||||
2. Add imports `import uuid` and `from db.models import Document` and `from config import settings as app_settings`.
|
||||
3. Modify `classify_document` signature:
|
||||
```
|
||||
async def classify_document(
|
||||
session: AsyncSession,
|
||||
doc_id: str,
|
||||
topic_names: list[str] | None = None,
|
||||
ai_provider: str | None = None,
|
||||
ai_model: str | None = None,
|
||||
) -> list[str]:
|
||||
```
|
||||
Replace `settings = storage.load_settings(); system_prompt = settings.get("system_prompt", ""); provider = get_provider(settings)` with:
|
||||
```
|
||||
_ai_provider = ai_provider or app_settings.default_ai_provider
|
||||
_ai_model = ai_model or app_settings.default_ai_model
|
||||
system_prompt = app_settings.system_prompt or _DEFAULT_SYSTEM_PROMPT
|
||||
_settings = {
|
||||
"active_provider": _ai_provider,
|
||||
"providers": {_ai_provider: {"model": _ai_model}},
|
||||
}
|
||||
provider = get_provider(_settings)
|
||||
```
|
||||
4. Modify `suggest_topics_for_document` with the same signature change (`ai_provider`, `ai_model` kwargs) and same `_settings` construction in place of `storage.load_settings()`.
|
||||
5. Ensure no remaining `storage.load_settings()` call exists anywhere in this file.
|
||||
|
||||
Modify `backend/services/storage.py`:
|
||||
1. Remove the import `from config import DEFAULT_SETTINGS, SETTINGS_FILE` (line 36) — replace with nothing (config import is no longer needed in this module).
|
||||
2. Remove `import copy` and `import json` from top imports (no longer needed).
|
||||
3. Remove `load_settings()`, `save_settings()`, `mask_api_key()`, `settings_masked()` function definitions (lines ~416-449).
|
||||
4. Remove these from `__all__`: `"load_settings", "save_settings", "mask_api_key", "settings_masked"`.
|
||||
|
||||
Modify `backend/tasks/document_tasks.py` `_run` function:
|
||||
1. After `doc = await session.get(Document, doc_uuid)` and the None check, insert:
|
||||
```
|
||||
from db.models import User
|
||||
from config import settings as app_settings
|
||||
user = await session.get(User, doc.user_id) if doc.user_id else None
|
||||
ai_provider = (user.ai_provider if user else None) or app_settings.default_ai_provider
|
||||
ai_model = (user.ai_model if user else None) or app_settings.default_ai_model
|
||||
```
|
||||
2. Replace `topics = await classifier.classify_document(session, document_id)` with `topics = await classifier.classify_document(session, document_id, ai_provider=ai_provider, ai_model=ai_model)`.
|
||||
|
||||
Delete `backend/api/settings.py` entirely. Use the `rm` shell equivalent or write an empty stub — preferable: delete the file. (Executor: use `git rm backend/api/settings.py` or `os.remove` then commit.)
|
||||
|
||||
Modify `backend/main.py`:
|
||||
1. Remove `from api.settings import router as settings_router` (line 18).
|
||||
2. Remove `app.include_router(settings_router)` (line 174).
|
||||
|
||||
Modify `backend/tests/test_settings.py`:
|
||||
1. Delete all existing tests (they reference the removed endpoint and storage functions).
|
||||
2. Keep only the single test from Plan 03-01:
|
||||
```
|
||||
async def test_settings_endpoint_removed(async_client):
|
||||
"""D-12: /api/settings endpoint is removed in Phase 3."""
|
||||
resp = await async_client.get("/api/settings")
|
||||
assert resp.status_code == 404
|
||||
```
|
||||
Remove the `@pytest.mark.xfail` marker — this test should now pass green.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && pytest tests/test_settings.py tests/test_classifier.py::test_per_user_provider tests/test_classifier.py::test_celery_task_uses_user_provider tests/test_classifier.py::test_default_provider_fallback -x -q 2>&1 | tail -30 && test ! -f backend/api/settings.py && echo "settings.py deleted" && ! grep -q "load_settings" backend/services/storage.py && echo "load_settings removed" && grep -c "default_ai_provider" backend/config.py && grep -c "_DEFAULT_SYSTEM_PROMPT" backend/services/classifier.py && grep -c "user.ai_provider" backend/tasks/document_tasks.py</automated>
|
||||
</verify>
|
||||
<done>
|
||||
`backend/api/settings.py` file does not exist. `backend/services/storage.py` contains no `load_settings`, `save_settings`, `mask_api_key`, `settings_masked` (grep returns 0). `backend/config.py` contains `default_ai_provider` and `default_ai_model`. `backend/services/classifier.py` contains `_DEFAULT_SYSTEM_PROMPT` and accepts `ai_provider` kwarg. `backend/tasks/document_tasks.py` contains `user.ai_provider`. test_settings_endpoint_removed and the 3 classifier tests pass.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Frontend — strip settings store/API and replace SettingsView with admin-managed placeholder</name>
|
||||
<files>frontend/src/views/SettingsView.vue, frontend/src/stores/settings.js, frontend/src/api/client.js</files>
|
||||
<read_first>
|
||||
- frontend/src/views/SettingsView.vue — current form structure; replace entirely with placeholder card
|
||||
- frontend/src/stores/settings.js — actions to remove (fetchSettings, save, testConnection, resetPrompt)
|
||||
- frontend/src/api/client.js — getSettings, patchSettings, testProvider, getDefaultPrompt functions (lines ~110-132)
|
||||
- frontend/src/router/index.js — verify /settings route still exists (do not remove — UI-SPEC Risk 6 says keep route with placeholder)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-UI-SPEC.md — Phase 3 spec inherits Phase 2 design system; no specific SettingsView spec — use minimal card matching existing card style
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 11 (frontend retirement scope), Risk 6 (UX regression mitigation)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- frontend/src/views/SettingsView.vue is replaced with a static placeholder: a heading "AI Configuration" and a card explaining "AI configuration is managed by your admin." Add a secondary link or text directing users to contact their admin. No form, no API calls, no useSettingsStore import
|
||||
- frontend/src/stores/settings.js is reduced to an empty defineStore that exports no actions, OR deleted entirely. Plan: delete the file and update any imports (SettingsView no longer imports it; check if anything else imports it — grep). If other files import it, gut the file to a minimal noop store
|
||||
- frontend/src/api/client.js removes the four functions: getSettings, patchSettings, testProvider, getDefaultPrompt (lines ~110-132 in current file). Adds a new export getMyQuota() that hits GET /api/auth/me/quota (used by QuotaBar in Plan 03-05 — pre-emptively add here so Plan 03-05 is a pure UI plan)
|
||||
- Visiting /settings in the browser shows the admin-managed placeholder without any console errors
|
||||
- SettingsView.vue uses only the Phase 2 design system (existing classes; text-sm / text-xl / bg-white / border-gray-200 / rounded-xl) — UI-SPEC inheritance
|
||||
</behavior>
|
||||
<action>
|
||||
Rewrite `frontend/src/views/SettingsView.vue` with the placeholder content. Template structure:
|
||||
```vue
|
||||
<template>
|
||||
<div class="p-8 max-w-3xl mx-auto">
|
||||
<h2 class="text-2xl font-semibold text-gray-900 mb-1">Settings</h2>
|
||||
<p class="text-sm text-gray-500 mb-8">Account-level options for your DocuVault workspace.</p>
|
||||
|
||||
<section class="bg-white border border-gray-200 rounded-xl p-6">
|
||||
<h3 class="text-xl font-semibold text-gray-800 mb-2">AI configuration</h3>
|
||||
<p class="text-sm text-gray-600">
|
||||
AI provider and model are managed by your administrator. Contact your admin
|
||||
to request changes to which AI provider is used for your documents.
|
||||
</p>
|
||||
</section>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
<script setup>
|
||||
// SettingsView is a static placeholder after Phase 3 D-12 settings retirement.
|
||||
// No store usage, no API calls — AI config is admin-only via /api/admin/users/{id}/ai-config.
|
||||
</script>
|
||||
```
|
||||
Remove any `<style>` blocks if present in the original file.
|
||||
|
||||
Delete `frontend/src/stores/settings.js`. Before deleting, grep the frontend tree for imports of `'../stores/settings.js'` or `'@/stores/settings.js'`. If any consumer exists outside SettingsView.vue, instead gut the file to:
|
||||
```js
|
||||
// stores/settings.js was retired in Phase 3 D-12 — kept as a minimal no-op to avoid breaking imports.
|
||||
// Remove this file in a future cleanup once all consumers are updated.
|
||||
import { defineStore } from 'pinia'
|
||||
export const useSettingsStore = defineStore('settings', () => {
|
||||
return {}
|
||||
})
|
||||
```
|
||||
Otherwise delete the file.
|
||||
|
||||
Modify `frontend/src/api/client.js`:
|
||||
1. Remove the four functions in the `// ── Settings ──` section: `getSettings`, `patchSettings`, `testProvider`, `getDefaultPrompt` (lines ~110-132). Remove the section header comment too.
|
||||
2. Add a new function for the quota endpoint (used by Plan 03-05's QuotaBar). Place it under a new `// ── Quota ──` section (or under `// ── Auth ──`):
|
||||
```js
|
||||
export function getMyQuota() {
|
||||
return request('/api/auth/me/quota')
|
||||
}
|
||||
```
|
||||
3. Add the upload-flow API helpers used by Plan 03-05:
|
||||
```js
|
||||
export function getUploadUrl(filename, contentType) {
|
||||
return request('/api/documents/upload-url', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ filename, content_type: contentType }),
|
||||
})
|
||||
}
|
||||
|
||||
export function confirmUpload(documentId) {
|
||||
return request(`/api/documents/${documentId}/confirm`, { method: 'POST' })
|
||||
}
|
||||
```
|
||||
Place these under the `// ── Documents ──` section near the existing `uploadDocument` function. Leave `uploadDocument` in place for now (Plan 03-05 will remove it when the documents store no longer calls it).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd frontend && grep -c "managed by your administrator" src/views/SettingsView.vue && ! grep -q "useSettingsStore" src/views/SettingsView.vue && ! grep -q "getSettings\|patchSettings\|testProvider\|getDefaultPrompt" src/api/client.js && grep -c "getMyQuota\|getUploadUrl\|confirmUpload" src/api/client.js</automated>
|
||||
</verify>
|
||||
<done>
|
||||
SettingsView.vue is the placeholder card (no form, no store). api/client.js has getMyQuota + getUploadUrl + confirmUpload exports and no getSettings/patchSettings/testProvider/getDefaultPrompt. stores/settings.js is deleted OR gutted to a no-op store. Visiting /settings in the dev server renders the card with no console errors (manual check during Plan 03-05 verification — not required here).
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- Settings endpoint removed: `cd backend && pytest tests/test_settings.py::test_settings_endpoint_removed -x -q`
|
||||
- Per-user AI classification: `cd backend && pytest tests/test_classifier.py::test_per_user_provider tests/test_classifier.py::test_celery_task_uses_user_provider tests/test_classifier.py::test_default_provider_fallback -x -q`
|
||||
- No load_settings reference remains: `cd backend && grep -rn 'load_settings\|save_settings' backend --include='*.py' | grep -v tests/` returns no hits
|
||||
- Frontend settings imports clean: `cd frontend && grep -rn 'getSettings\|patchSettings\|testProvider\|getDefaultPrompt' src/` returns no hits
|
||||
- All Phase 3 tests still green: `cd backend && pytest -x -q`
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- backend/api/settings.py deleted
|
||||
- backend/main.py no longer registers settings_router
|
||||
- backend/services/storage.py no longer contains load_settings/save_settings/mask_api_key/settings_masked
|
||||
- backend/services/classifier.py classify_document accepts ai_provider and ai_model kwargs; reads no flat file
|
||||
- backend/tasks/document_tasks.py resolves user.ai_provider via DB lookup and passes to classifier
|
||||
- backend/config.py exposes system_prompt, default_ai_provider, default_ai_model fields and no longer defines SETTINGS_FILE/DEFAULT_SETTINGS/DEFAULT_SYSTEM_PROMPT
|
||||
- frontend/src/views/SettingsView.vue shows the admin-managed placeholder
|
||||
- frontend/src/api/client.js exports getMyQuota, getUploadUrl, confirmUpload and no longer exports settings functions
|
||||
- All Plan 03-01 stub tests for DOC-03 / DOC-05 / D-12 pass
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/03-document-migration-multi-user-isolation/03-04-SUMMARY.md` when done — document the final classifier signature and the env var defaults; note that Phase 4 will keep /settings as a placeholder until a dedicated storage settings page lands.
|
||||
</output>
|
||||
@@ -0,0 +1,521 @@
|
||||
---
|
||||
phase: 03-document-migration-multi-user-isolation
|
||||
plan: 05
|
||||
type: execute
|
||||
wave: 5
|
||||
depends_on:
|
||||
- 03-04
|
||||
files_modified:
|
||||
- frontend/src/stores/documents.js
|
||||
- frontend/src/stores/auth.js
|
||||
- frontend/src/components/upload/UploadProgress.vue
|
||||
- frontend/src/components/layout/QuotaBar.vue
|
||||
- frontend/src/components/layout/AppSidebar.vue
|
||||
- frontend/src/api/client.js
|
||||
autonomous: false
|
||||
requirements:
|
||||
- STORE-03
|
||||
- STORE-04
|
||||
- STORE-05
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "Selecting a file in the existing DropZone triggers a 3-step upload: POST /api/documents/upload-url → XHR PUT to MinIO with progress events → POST /api/documents/{id}/confirm — the user sees a smooth progress bar advancing 0% → 100%"
|
||||
- "An upload that would exceed the user's quota receives an HTTP 413 from /confirm; the upload row displays the inline quota rejection error block with 'Not enough storage', file size, and 'Manage storage →' link per UI-SPEC"
|
||||
- "The sidebar shows a quota bar between the topics nav and the footer; bar fills with usage; turns amber at >=80%, red at >=95%; refreshes after every successful upload + every document delete"
|
||||
- "auth store exposes a quota: {used_bytes, limit_bytes} reactive object updated by the documents store after upload/delete"
|
||||
- "Legacy uploadDocument(file) (multipart POST) is removed from api/client.js since no consumer remains"
|
||||
artifacts:
|
||||
- path: "frontend/src/stores/documents.js"
|
||||
provides: "Three-step upload action with uploadProgress reactive map; quota refetch on success/delete"
|
||||
contains: "getUploadUrl"
|
||||
- path: "frontend/src/stores/auth.js"
|
||||
provides: "Reactive quota state + fetchQuota action"
|
||||
exports:
|
||||
- "fetchQuota"
|
||||
- path: "frontend/src/components/layout/QuotaBar.vue"
|
||||
provides: "Quota bar widget consuming useAuthStore quota state"
|
||||
contains: "role=\"progressbar\""
|
||||
- path: "frontend/src/components/layout/AppSidebar.vue"
|
||||
provides: "QuotaBar embedded between topics nav and footer"
|
||||
contains: "<QuotaBar"
|
||||
- path: "frontend/src/components/upload/UploadProgress.vue"
|
||||
provides: "Per-row progress bar, percentage label, quota rejection error block"
|
||||
contains: "role=\"progressbar\""
|
||||
- path: "frontend/src/api/client.js"
|
||||
provides: "uploadDocument legacy multipart removed; getUploadUrl/confirmUpload/getMyQuota retained from Plan 03-04"
|
||||
contains: "getMyQuota"
|
||||
key_links:
|
||||
- from: "frontend/src/stores/documents.js"
|
||||
to: "MinIO (direct PUT)"
|
||||
via: "uploadToMinIO(url, file, onProgress) XHR helper"
|
||||
pattern: "XMLHttpRequest"
|
||||
- from: "frontend/src/stores/documents.js"
|
||||
to: "frontend/src/stores/auth.js"
|
||||
via: "authStore.fetchQuota() invoked after upload and delete"
|
||||
pattern: "fetchQuota"
|
||||
- from: "frontend/src/components/layout/QuotaBar.vue"
|
||||
to: "frontend/src/stores/auth.js"
|
||||
via: "useAuthStore() reads quota.used_bytes + quota.limit_bytes; onMounted triggers fetchQuota()"
|
||||
pattern: "useAuthStore"
|
||||
- from: "frontend/src/components/upload/UploadProgress.vue"
|
||||
to: "documents store uploadProgress map"
|
||||
via: "items prop carries item.progress (0-100) and item.quotaError ({used_bytes, limit_bytes, rejected_bytes})"
|
||||
pattern: "item.progress|item.quotaError"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Wire the frontend to the Phase 3 backend API per UI-SPEC. Replace the multipart `uploadDocument` action with a 3-step flow (upload-url → XHR PUT → confirm) and surface progress via `XMLHttpRequest` upload events. Add the sidebar quota bar (STORE-04) and the inline quota rejection error block (STORE-05) per the UI-SPEC contract. Store quota state in `useAuthStore` per UI-SPEC.
|
||||
|
||||
Purpose: Phase 3 SC1-SC5 cannot be observed end-to-end without the frontend cutover; the auto-classify regression test in Plan 03-01 stops at the API but the user-visible experience needs the new flow. The plan includes one `checkpoint:human-verify` to confirm the visual progress bar and quota bar match the UI-SPEC.
|
||||
Output: 6 frontend files modified; 1 net-new component (QuotaBar.vue). After this plan, Phase 3 is feature-complete.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/nik/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/nik/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<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-UI-SPEC.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-04-SUMMARY.md
|
||||
@CLAUDE.md
|
||||
|
||||
@frontend/src/stores/documents.js
|
||||
@frontend/src/stores/auth.js
|
||||
@frontend/src/api/client.js
|
||||
@frontend/src/components/upload/UploadProgress.vue
|
||||
@frontend/src/components/layout/AppSidebar.vue
|
||||
@frontend/src/components/auth/PasswordStrengthBar.vue
|
||||
|
||||
<interfaces>
|
||||
<!-- Contracts the executor needs without re-reading the codebase. -->
|
||||
|
||||
From frontend/src/api/client.js (post Plan 03-04):
|
||||
getUploadUrl(filename, contentType) -> Promise<{upload_url: string, document_id: string}>
|
||||
confirmUpload(documentId) -> Promise<{id: string, size_bytes: number, used_bytes: number, status: "uploaded"}>
|
||||
getMyQuota() -> Promise<{used_bytes: number, limit_bytes: number}>
|
||||
listDocuments({topic?, page?, perPage?}) -> Promise<{items, total, page, per_page}>
|
||||
deleteDocument(id) -> Promise<{success: boolean}>
|
||||
getDocument(id), classifyDocument(id, topics)
|
||||
|
||||
From frontend/src/stores/auth.js (current):
|
||||
state: accessToken (ref null), user (ref null), loading, error
|
||||
actions: register, login, refresh, logout, logoutAll
|
||||
This plan adds: quota (ref({used_bytes: 0, limit_bytes: 0})), fetchQuota async action
|
||||
|
||||
API contract for /confirm (Plan 03-02):
|
||||
Success: 200 + {id, size_bytes, used_bytes, status: "uploaded"}
|
||||
Quota exceeded: 413 + {detail: {used_bytes, limit_bytes, rejected_bytes}}
|
||||
Upload not found: 422 + {detail: "Upload not found — presigned URL may have expired"}
|
||||
|
||||
API client `request()` error handling (current):
|
||||
- On HTTP !ok, throws Error(msg) where msg comes from response body `.detail` (or `HTTP {status}`)
|
||||
- For structured 413 detail (dict shape), response.json().detail is an object — current `msg = (await res.json()).detail || msg` will coerce object to "[object Object]". This plan must extend the request helper or catch 413 specifically in the upload store action.
|
||||
|
||||
UI-SPEC Upload Progress steps:
|
||||
Awaiting URL: 0% / "Preparing upload…"
|
||||
URL received: 5%
|
||||
XHR progress: 5% → 90% (linear loaded/total)
|
||||
Confirm in flight: 92% / "Processing…"
|
||||
Confirm 200: 100% / "Done — classifying…"
|
||||
|
||||
UI-SPEC Quota Bar:
|
||||
pct < 80% → bg-indigo-500 fill, text-gray-500 label
|
||||
80 ≤ pct < 95% → bg-amber-500 fill, text-amber-600 label
|
||||
pct ≥ 95% → bg-red-500 fill, text-red-600 label
|
||||
Track: bg-gray-200, h-2 rounded-full
|
||||
Label format: "{used} MB of {limit} MB" (1 decimal place)
|
||||
role="progressbar" on fill, aria-valuenow, aria-valuemin=0, aria-valuemax=100
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| browser ↔ MinIO presigned PUT | XHR sends file bytes directly to MinIO over a time-limited presigned URL; no Authorization header (URL is self-authenticating) |
|
||||
| browser → /api/documents/* | Authenticated requests carry Bearer JWT via existing request() helper |
|
||||
| browser quota display | Quota values come only from authoritative server response — never computed from local file size alone (UI-SPEC Copywriting Security) |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-03-22 | Information Disclosure | XHR PUT to MinIO with Authorization header attached | mitigate | uploadToMinIO helper uses bare XMLHttpRequest with NO setRequestHeader("Authorization", ...) — presigned URL is self-authenticating; CLAUDE.md "MinIO presigned URL flow" |
|
||||
| T-03-23 | Spoofing | Client-side quota display showing values from local file.size only | mitigate | Quota rejection error block populates `used_bytes / limit_bytes / rejected_bytes` from the 413 response body — never from `file.size` calculations |
|
||||
| T-03-24 | Denial of Service | Multiple concurrent uploads exhaust browser memory | accept | XHR-based uploads stream bytes natively (no buffering); v1 accepts user-driven concurrency |
|
||||
| T-03-25 | Tampering | Upload state corruption from race conditions in uploadProgress map | mitigate | Use file name + Date.now() composite key in uploadProgress map to avoid collisions when the same filename is uploaded twice in quick succession |
|
||||
| T-03-26 | Repudiation | Upload quota refetch silently fails | mitigate | authStore.fetchQuota wraps the API call in try/catch and resets to last-known state on error; QuotaBar hides itself on fetch error per UI-SPEC "Loading and Error States" |
|
||||
| T-03-SC | Tampering | npm installs | mitigate | No new npm dependencies — uses native XMLHttpRequest |
|
||||
</threat_model>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Refactor documents store to 3-step upload + quota state in auth store + extend UploadProgress.vue with progress bar and quota error block</name>
|
||||
<files>frontend/src/stores/documents.js, frontend/src/stores/auth.js, frontend/src/components/upload/UploadProgress.vue, frontend/src/api/client.js</files>
|
||||
<read_first>
|
||||
- frontend/src/stores/documents.js — current upload action (single multipart POST)
|
||||
- frontend/src/stores/auth.js — current state shape; pattern for adding new ref + action
|
||||
- frontend/src/api/client.js — `request()` helper and existing exports (uploadDocument, getUploadUrl, confirmUpload, getMyQuota added in Plan 03-04)
|
||||
- frontend/src/components/upload/UploadProgress.vue — current row template; item shape (name, done, error, topics)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-UI-SPEC.md — Upload Flow Interaction Contract, Quota Bar Color Logic, Error States (Quota Rejection Error Block), Accessibility, Copywriting Contract
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 9 (XHR upload progress helper)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md — XHR upload helper, Pinia Store Action Pattern
|
||||
</read_first>
|
||||
<behavior>
|
||||
- frontend/src/stores/documents.js: replace single `upload(file)` with 3-step async action; track `uploadProgress` (ref({})) keyed by unique row key (filename + timestamp); on success call `authStore.fetchQuota()`; on 413 capture `quotaError` (object {used_bytes, limit_bytes, rejected_bytes}) on the corresponding upload row and re-throw a tagged error so DropZone/parent can display it
|
||||
- On `remove(id)` success, also call `authStore.fetchQuota()`
|
||||
- frontend/src/stores/auth.js: add `quota` ref({used_bytes: 0, limit_bytes: 0}) and `fetchQuota()` async action that calls `api.getMyQuota()` and updates `quota.value`; expose in the returned store object. fetchQuota wraps in try/catch — on failure leaves last-known values intact (UI-SPEC "Hide widget entirely on fetch error" → handled in QuotaBar via a separate quotaLoadFailed ref)
|
||||
- frontend/src/api/client.js: extend `request()` to surface structured 413 detail. Detect `res.status === 413` and `typeof body.detail === "object"` then throw an Error with a `.payload` property carrying the structured detail; rest of code path unchanged. Remove `uploadDocument(file, autoClassify)` (legacy multipart) — no consumer remains after this plan
|
||||
- frontend/src/components/upload/UploadProgress.vue: extend `item` shape to include `progress` (0-100) and `quotaError` ({used_bytes, limit_bytes, rejected_bytes}); render a progress bar (h-2 bg-gray-200 → fill bg-indigo-500/green-500/red-400 per UI-SPEC) when item.progress is set; when item.quotaError is set, render the inline error block per UI-SPEC "Quota Rejection Error Block" (Not enough storage / This file (X MB) would exceed your quota / You're using Y MB of Z MB / Manage storage →)
|
||||
- All Phase 3 backend tests already cover the API contract — no new test files in this plan; manual verification via the human checkpoint in Task 2
|
||||
</behavior>
|
||||
<action>
|
||||
Modify `frontend/src/api/client.js`:
|
||||
1. Remove the `uploadDocument(file, autoClassify = true)` function entirely (legacy multipart POST; no longer called).
|
||||
2. Replace the existing `request()` body with extended error handling for structured 413 detail. Specifically, the `if (!res.ok)` block becomes:
|
||||
```js
|
||||
if (!res.ok) {
|
||||
let msg = `HTTP ${res.status}`
|
||||
let payload = null
|
||||
try {
|
||||
const body = await res.json()
|
||||
if (typeof body.detail === 'object' && body.detail !== null) {
|
||||
payload = body.detail
|
||||
msg = body.detail.message || `HTTP ${res.status}`
|
||||
} else {
|
||||
msg = body.detail || msg
|
||||
}
|
||||
} catch {}
|
||||
const err = new Error(msg)
|
||||
err.status = res.status
|
||||
if (payload) err.payload = payload
|
||||
throw err
|
||||
}
|
||||
```
|
||||
|
||||
Modify `frontend/src/stores/auth.js`:
|
||||
1. Add import-side `import * as api from '../api/client.js'` (already present).
|
||||
2. Add inside the setup function (alongside accessToken, user, loading, error):
|
||||
```js
|
||||
const quota = ref({ used_bytes: 0, limit_bytes: 0 })
|
||||
|
||||
async function fetchQuota() {
|
||||
try {
|
||||
const data = await api.getMyQuota()
|
||||
quota.value = { used_bytes: data.used_bytes, limit_bytes: data.limit_bytes }
|
||||
} catch {
|
||||
// Silently ignore — QuotaBar hides itself on fetch error (UI-SPEC)
|
||||
}
|
||||
}
|
||||
```
|
||||
3. Add `quota` and `fetchQuota` to the returned store object.
|
||||
|
||||
Rewrite `frontend/src/stores/documents.js` `upload` action. Add at module top (alongside existing imports) `import { useAuthStore } from './auth.js'`. Replace the existing `upload` function with:
|
||||
```js
|
||||
function uploadToMinIO(url, file, onProgress) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const xhr = new XMLHttpRequest()
|
||||
xhr.upload.addEventListener('progress', (e) => {
|
||||
if (e.lengthComputable) onProgress(Math.round((e.loaded / e.total) * 100))
|
||||
})
|
||||
xhr.addEventListener('load', () => {
|
||||
if (xhr.status < 400) resolve()
|
||||
else reject(new Error(`PUT failed: ${xhr.status}`))
|
||||
})
|
||||
xhr.addEventListener('error', () => reject(new Error('Network error during upload')))
|
||||
xhr.open('PUT', url)
|
||||
xhr.setRequestHeader('Content-Type', file.type || 'application/octet-stream')
|
||||
// NOTE: no Authorization header — presigned URL is self-authenticating (T-03-22)
|
||||
xhr.send(file)
|
||||
})
|
||||
}
|
||||
|
||||
const uploadProgress = ref({}) // { [rowKey]: 0-100 }
|
||||
|
||||
async function upload(file, autoClassify = true) {
|
||||
const authStore = useAuthStore()
|
||||
const rowKey = `${file.name}__${Date.now()}`
|
||||
uploadProgress.value[rowKey] = 0
|
||||
try {
|
||||
// Step 1: get presigned PUT URL (UI-SPEC 0% → 5%)
|
||||
const { upload_url, document_id } = await api.getUploadUrl(file.name, file.type || 'application/octet-stream')
|
||||
uploadProgress.value[rowKey] = 5
|
||||
|
||||
// Step 2: XHR PUT to MinIO (UI-SPEC 5% → 90%)
|
||||
await uploadToMinIO(upload_url, file, (pct) => {
|
||||
// Map XHR progress 0-100 into the 5-90 visual range
|
||||
uploadProgress.value[rowKey] = 5 + Math.round(pct * 0.85)
|
||||
})
|
||||
uploadProgress.value[rowKey] = 92
|
||||
|
||||
// Step 3: confirm (UI-SPEC 92% → 100%)
|
||||
const doc = await api.confirmUpload(document_id)
|
||||
uploadProgress.value[rowKey] = 100
|
||||
|
||||
documents.value.unshift({
|
||||
id: doc.id,
|
||||
original_name: file.name,
|
||||
filename: file.name,
|
||||
mime_type: file.type,
|
||||
size_bytes: doc.size_bytes,
|
||||
topics: [],
|
||||
created_at: new Date().toISOString(),
|
||||
classified_at: null,
|
||||
})
|
||||
total.value++
|
||||
|
||||
// Refresh quota (STORE-04)
|
||||
await authStore.fetchQuota()
|
||||
|
||||
return { rowKey, doc }
|
||||
} catch (e) {
|
||||
error.value = e.message
|
||||
// Tag the row with structured quota error from 413
|
||||
if (e.status === 413 && e.payload) {
|
||||
throw Object.assign(e, { rowKey })
|
||||
}
|
||||
throw Object.assign(e, { rowKey })
|
||||
} finally {
|
||||
// Keep the rowKey progress entry visible until parent (DropZone) clears it
|
||||
// (do NOT delete uploadProgress[rowKey] here — UploadProgress component still reads it)
|
||||
}
|
||||
}
|
||||
```
|
||||
Also modify `remove(id)`: after the existing `documents.value = documents.value.filter(...)` block, add `const authStore = useAuthStore(); await authStore.fetchQuota()`.
|
||||
|
||||
Update the store return object to expose `uploadProgress`.
|
||||
|
||||
Modify `frontend/src/components/upload/UploadProgress.vue`:
|
||||
1. Inside the `<div class="flex-1 min-w-0">` wrapper, after the status text lines (`<p v-if="item.error" ...>` etc.), add the progress bar:
|
||||
```vue
|
||||
<!-- Progress bar (visible while uploading / processing) -->
|
||||
<div
|
||||
v-if="!item.done && !item.error && !item.quotaError && item.progress !== undefined"
|
||||
class="w-full h-2 bg-gray-200 rounded-full mt-1 overflow-hidden"
|
||||
>
|
||||
<div
|
||||
role="progressbar"
|
||||
:aria-valuenow="item.progress"
|
||||
aria-valuemin="0"
|
||||
aria-valuemax="100"
|
||||
:aria-label="`Upload progress for ${item.name}`"
|
||||
class="h-full rounded-full transition-all duration-300 bg-indigo-500"
|
||||
:style="{ width: `${item.progress}%` }"
|
||||
></div>
|
||||
</div>
|
||||
<p
|
||||
v-if="!item.done && !item.error && !item.quotaError && item.progress !== undefined"
|
||||
class="text-sm text-gray-400 mt-1 text-right"
|
||||
>{{ item.progress }}%</p>
|
||||
```
|
||||
2. Replace the existing `<p v-else class="text-xs text-gray-400 mt-0.5">Uploading…</p>` line with a status-string mapper. New `<p v-else ...>` reads `item.status` (a string set by parent — defaults to "Uploading…"). The UI-SPEC step strings are "Preparing upload…", "Uploading…", "Processing…", "Done — classifying…". Use `text-sm text-gray-400 mt-0.5`.
|
||||
3. Add the inline quota rejection error block at the bottom of the row's flex-1 wrapper (after the status text):
|
||||
```vue
|
||||
<div
|
||||
v-if="item.quotaError"
|
||||
role="alert"
|
||||
class="mt-1 p-3 rounded-lg bg-red-50 border border-red-200"
|
||||
>
|
||||
<p class="text-sm font-semibold text-red-700">Not enough storage</p>
|
||||
<p class="text-sm text-red-600 mt-1">
|
||||
This file ({{ (item.quotaError.rejected_bytes / 1048576).toFixed(1) }} MB) would exceed your quota.
|
||||
</p>
|
||||
<p class="text-sm text-red-600">
|
||||
You're using {{ (item.quotaError.used_bytes / 1048576).toFixed(1) }} MB of {{ (item.quotaError.limit_bytes / 1048576).toFixed(1) }} MB.
|
||||
</p>
|
||||
<router-link to="/settings" class="text-sm text-red-600 underline hover:text-red-700 font-semibold">
|
||||
Manage storage →
|
||||
</router-link>
|
||||
</div>
|
||||
```
|
||||
Add `import { RouterLink } from 'vue-router'` to `<script setup>` if needed (alternatively use plain `<a href="/settings">` to avoid the import).
|
||||
4. No changes to `defineProps({ items: ... })` — `progress`, `status`, `quotaError` are properties on each `item` object.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd frontend && grep -c "uploadToMinIO" src/stores/documents.js && grep -c "XMLHttpRequest" src/stores/documents.js && grep -c "fetchQuota" src/stores/auth.js && grep -c "role=\"progressbar\"" src/components/upload/UploadProgress.vue && grep -c "Not enough storage" src/components/upload/UploadProgress.vue && ! grep -q "uploadDocument" src/api/client.js && node -e "const fs = require('fs'); const f = fs.readFileSync('src/stores/documents.js', 'utf8'); if (!f.includes('getUploadUrl')) process.exit(1); if (!f.includes('confirmUpload')) process.exit(1); if (!f.includes('fetchQuota')) process.exit(1); console.log('store wiring OK')"</automated>
|
||||
</verify>
|
||||
<done>
|
||||
`stores/documents.js` exposes the 3-step upload action and calls `authStore.fetchQuota()` on success + delete. `stores/auth.js` exposes a `quota` ref and `fetchQuota` action. `UploadProgress.vue` renders an aria-progressbar row and the quota rejection error block on `item.quotaError`. `api/client.js` no longer exports `uploadDocument`. `request()` attaches `.status` and `.payload` to thrown errors.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Create QuotaBar.vue and embed in AppSidebar between topics nav and footer</name>
|
||||
<files>frontend/src/components/layout/QuotaBar.vue, frontend/src/components/layout/AppSidebar.vue</files>
|
||||
<read_first>
|
||||
- frontend/src/components/layout/AppSidebar.vue — current footer block (px-3 py-4 border-t border-gray-100 section)
|
||||
- frontend/src/components/auth/PasswordStrengthBar.vue — visual style analog for a Tailwind progress bar
|
||||
- frontend/src/stores/auth.js — fetchQuota + quota state added by Task 1
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-UI-SPEC.md — Quota Usage Bar — Sidebar Contract (Placement, Structure, Loading and Error States, Data Source, Accessibility)
|
||||
- .planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md — QuotaBar.vue analog (PasswordStrengthBar visual style only)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- QuotaBar.vue is a self-contained component: onMounted calls `authStore.fetchQuota()`; computed properties `pct`, `barColor`, `labelColor`, `label` derive from `authStore.quota.used_bytes` and `authStore.quota.limit_bytes` per UI-SPEC color logic
|
||||
- Bar fill includes `role="progressbar"`, `aria-valuenow`, `aria-valuemin=0`, `aria-valuemax=100`, `aria-label="Storage usage: {used} MB of {limit} MB"`
|
||||
- Loading state: when limit_bytes === 0 AND fetch has never completed, render skeleton `bg-gray-100 animate-pulse h-2 rounded-full` with `aria-label="Loading storage usage"` and `aria-busy="true"`. After first successful fetchQuota, even used_bytes=0 renders the real bar (UI-SPEC: "do not hide the widget when used_bytes=0")
|
||||
- Error state: if fetchQuota threw, the component hides itself via v-if (no DOM)
|
||||
- Label format: "12.3 MB of 100.0 MB" (1 decimal place)
|
||||
- QuotaBar mounted inside AppSidebar.vue, placed between the topics nav `<nav>` and the settings/admin/footer `<div class="px-3 py-4 border-t border-gray-100">` block
|
||||
- AppSidebar.vue script-setup imports QuotaBar
|
||||
</behavior>
|
||||
<action>
|
||||
Create `frontend/src/components/layout/QuotaBar.vue` as a new file:
|
||||
```vue
|
||||
<template>
|
||||
<div v-if="!loadFailed" class="px-4 py-3 border-t border-gray-100">
|
||||
<div class="flex items-center justify-between mb-1">
|
||||
<span class="text-sm font-semibold text-gray-500">Storage</span>
|
||||
<span v-if="ready" class="text-sm" :class="labelColor">{{ label }}</span>
|
||||
</div>
|
||||
<div
|
||||
v-if="ready"
|
||||
class="w-full h-2 bg-gray-200 rounded-full overflow-hidden"
|
||||
>
|
||||
<div
|
||||
role="progressbar"
|
||||
:aria-valuenow="pct"
|
||||
aria-valuemin="0"
|
||||
aria-valuemax="100"
|
||||
:aria-label="`Storage usage: ${label}`"
|
||||
class="h-2 rounded-full transition-all duration-500"
|
||||
:class="barColor"
|
||||
:style="{ width: `${pct}%` }"
|
||||
></div>
|
||||
</div>
|
||||
<div
|
||||
v-else
|
||||
class="w-full h-2 bg-gray-100 rounded-full animate-pulse"
|
||||
aria-label="Loading storage usage"
|
||||
aria-busy="true"
|
||||
></div>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
<script setup>
|
||||
import { ref, computed, onMounted } from 'vue'
|
||||
import { useAuthStore } from '../../stores/auth.js'
|
||||
|
||||
const authStore = useAuthStore()
|
||||
const ready = ref(false)
|
||||
const loadFailed = ref(false)
|
||||
|
||||
const pct = computed(() => {
|
||||
const { used_bytes, limit_bytes } = authStore.quota
|
||||
if (!limit_bytes || limit_bytes <= 0) return 0
|
||||
return Math.min(100, (used_bytes / limit_bytes) * 100)
|
||||
})
|
||||
|
||||
const barColor = computed(() => {
|
||||
if (pct.value >= 95) return 'bg-red-500'
|
||||
if (pct.value >= 80) return 'bg-amber-500'
|
||||
return 'bg-indigo-500'
|
||||
})
|
||||
|
||||
const labelColor = computed(() => {
|
||||
if (pct.value >= 95) return 'text-red-600'
|
||||
if (pct.value >= 80) return 'text-amber-600'
|
||||
return 'text-gray-500'
|
||||
})
|
||||
|
||||
const label = computed(() => {
|
||||
const used = (authStore.quota.used_bytes / 1048576).toFixed(1)
|
||||
const limit = (authStore.quota.limit_bytes / 1048576).toFixed(1)
|
||||
return `${used} MB of ${limit} MB`
|
||||
})
|
||||
|
||||
onMounted(async () => {
|
||||
try {
|
||||
await authStore.fetchQuota()
|
||||
ready.value = true
|
||||
} catch {
|
||||
loadFailed.value = true
|
||||
}
|
||||
})
|
||||
</script>
|
||||
```
|
||||
|
||||
Modify `frontend/src/components/layout/AppSidebar.vue`:
|
||||
1. In `<script setup>` imports, add `import QuotaBar from './QuotaBar.vue'`.
|
||||
2. In the template, insert `<QuotaBar />` between the closing `</nav>` (end of topics section) and the `<div class="px-3 py-4 border-t border-gray-100">` block (settings/admin/footer). Per UI-SPEC the QuotaBar component renders its own `border-t border-gray-100` so no additional separator needed.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd frontend && test -f src/components/layout/QuotaBar.vue && echo "QuotaBar exists" && grep -c "role=\"progressbar\"" src/components/layout/QuotaBar.vue && grep -c "useAuthStore" src/components/layout/QuotaBar.vue && grep -c "<QuotaBar" src/components/layout/AppSidebar.vue && grep -c "QuotaBar from './QuotaBar.vue'" src/components/layout/AppSidebar.vue</automated>
|
||||
</verify>
|
||||
<done>
|
||||
`QuotaBar.vue` file exists with `role="progressbar"` and `useAuthStore` reference. `AppSidebar.vue` imports and renders `<QuotaBar />`. Color classes match UI-SPEC: `bg-indigo-500` (<80%), `bg-amber-500` (80-95%), `bg-red-500` (≥95%).
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="checkpoint:human-verify" gate="blocking">
|
||||
<name>Task 3: Human-verify Phase 3 end-to-end upload + quota UX in browser</name>
|
||||
<what-built>
|
||||
Phase 3 frontend is complete: 3-step presigned upload with XHR progress bar, sidebar quota bar with color thresholds, inline 413 error block with "Manage storage →" link, SettingsView admin-managed placeholder.
|
||||
</what-built>
|
||||
<how-to-verify>
|
||||
Prerequisites: `docker compose up` is running with healthy postgres/minio/redis/backend/celery-worker/celery-beat. Frontend dev server running on http://localhost:5173. A logged-in non-admin user with a quota row (use admin panel to create a fresh user if needed).
|
||||
|
||||
1. **Upload happy path**: Sign in as a non-admin user. Open the home page (DropZone). Drag a small PDF or text file (~1 MB) onto the drop zone. Observe the UploadProgress row:
|
||||
- Status text transitions: "Preparing upload…" → "Uploading…" → "Processing…" → "Done — classifying…"
|
||||
- Progress bar fills smoothly from 0% → 5% → ~90% (XHR phase) → 92% (confirm) → 100% (done)
|
||||
- Color is indigo-500 during upload, green-500 on completion
|
||||
- Aria attributes are present on the bar (inspect element: `role="progressbar"`, `aria-valuenow`)
|
||||
Expected: file appears in document list within ~3 seconds (Celery classification finishes shortly after; topic chips may appear with a small lag)
|
||||
|
||||
2. **Sidebar quota bar**: After upload, the sidebar quota bar (between topics nav and Settings link) updates to reflect new usage. Verify:
|
||||
- Label format "X.X MB of 100.0 MB" with 1 decimal place
|
||||
- Bar fill color is indigo-500 (since pct < 80%)
|
||||
- On admin-side: temporarily set the user's quota to a low limit (e.g. via `/api/admin/users/{id}/quota` PATCH to limit_bytes=12MB) so a fresh upload pushes past 80%. Reload sidebar — bar fill switches to amber-500 and label color to amber-600 at >=80%, red-500 + red-600 at >=95%
|
||||
|
||||
3. **Quota rejection (413)**: Upload another file that would exceed quota. The UploadProgress row replaces the progress bar with the red error block:
|
||||
- Heading "Not enough storage" in red-700
|
||||
- Body line 1: "This file (X.X MB) would exceed your quota."
|
||||
- Body line 2: "You're using Y.Y MB of Z.Z MB."
|
||||
- Link "Manage storage →" navigates to /settings
|
||||
- Sidebar quota bar does not show the failed upload (no quota increment)
|
||||
|
||||
4. **Document delete + quota decrement**: Delete a document from the list. The sidebar quota bar usage decreases. The bar color reverts (e.g. from amber to indigo) if usage drops below 80%
|
||||
|
||||
5. **/settings placeholder**: Click the Settings link in the sidebar. The view shows the placeholder card with heading "Settings" and the admin-managed message — no form, no console errors
|
||||
|
||||
6. **Admin 403**: Sign out and sign in as the admin user. Attempt to navigate to /home (document list). The list either redirects/blocks via 403 banner or returns "Failed to load documents" — confirm document content is not visible to admin. (Frontend may not have a beautiful 403 UI yet — this is Phase 4 polish; verify the network response is 403 in DevTools)
|
||||
|
||||
Browser DevTools: confirm no console errors during upload; the XHR PUT request to MinIO targets `localhost:9000` (not `minio:9000`) and returns 200; the POST /api/documents/{id}/confirm carries a Bearer token (Authorization header in request) but the MinIO PUT does NOT carry an Authorization header (presigned URL is self-authenticating)
|
||||
|
||||
All steps above must pass before marking this checkpoint complete.
|
||||
</how-to-verify>
|
||||
<resume-signal>Type "approved" if all 6 scenarios pass; otherwise describe which step failed and the observed behavior</resume-signal>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- All Phase 3 stub tests now pass: `cd backend && pytest tests/test_quota.py tests/test_documents.py tests/test_topics.py tests/test_classifier.py tests/test_settings.py tests/test_alembic.py -x -q`
|
||||
- Full backend suite green: `cd backend && pytest -v`
|
||||
- Frontend builds without errors: `cd frontend && npm run build` exits 0
|
||||
- No legacy uploadDocument reference: `cd frontend && grep -rn 'uploadDocument' src/` returns no hits
|
||||
- Human checkpoint Task 3 approved
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Documents store implements 3-step upload with XHR progress
|
||||
- Auth store exposes quota state + fetchQuota action
|
||||
- UploadProgress.vue renders the progress bar and inline quota error block per UI-SPEC
|
||||
- QuotaBar.vue renders the sidebar quota bar with correct color thresholds
|
||||
- AppSidebar.vue embeds QuotaBar between topics nav and footer
|
||||
- Browser PUT requests succeed against localhost:9000 (not minio:9000)
|
||||
- /settings renders the placeholder card without errors
|
||||
- Admin role receives 403 on document endpoints (manual verification)
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/03-document-migration-multi-user-isolation/03-05-SUMMARY.md` when done — list the 6 frontend files modified, note any UI-SPEC deviations (e.g., if router-link vs anchor tag), and surface any backend issues discovered during the human checkpoint that need follow-up.
|
||||
</output>
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,768 @@
|
||||
# Phase 3: Document Migration & Multi-User Isolation — Research
|
||||
|
||||
**Researched:** 2026-05-23
|
||||
**Domain:** FastAPI + SQLAlchemy async + MinIO presigned PUT + Celery beat + Vue 3 XHR upload
|
||||
**Confidence:** HIGH
|
||||
|
||||
---
|
||||
|
||||
<user_constraints>
|
||||
## User Constraints (from CONTEXT.md)
|
||||
|
||||
### Locked Decisions
|
||||
|
||||
**Null-User Record Cleanup**
|
||||
- D-01: All documents with `user_id=NULL` are deleted (DB rows + MinIO objects) before NOT NULL is added. Zero production data loss (dev/test data only).
|
||||
- D-02: Cleanup baked into Alembic `upgrade()` in `0003_multi_user_isolation.py`.
|
||||
- D-03: After null-user cleanup, reconcile quota: `UPDATE quotas SET used_bytes = (SELECT COALESCE(SUM(size_bytes), 0) FROM documents WHERE documents.user_id = quotas.user_id)`.
|
||||
|
||||
**Presigned Upload Flow**
|
||||
- D-04: Direct-to-MinIO presigned PUT uploads. Existing multipart POST-to-FastAPI endpoint is replaced.
|
||||
- D-05: Three-step flow: (1) `POST /api/documents/upload-url` → creates `Document` row (`status='pending'`), generates presigned URL, returns `{upload_url, document_id}`. Quota NOT reserved here. (2) Browser PUTs bytes to MinIO. (3) `POST /api/documents/{id}/confirm` → `stat_object()` for authoritative size, atomic quota UPDATE, `status='uploaded'`, enqueue Celery task.
|
||||
- D-06: Abandoned upload cleanup: Celery beat task every 30 minutes deletes `Document` rows older than 1 hour with `status='pending'` and their MinIO objects.
|
||||
- D-07: Atomic quota at confirm step. `UPDATE quotas SET used_bytes = used_bytes + $delta WHERE (used_bytes + $delta) <= limit_bytes RETURNING used_bytes`. 413 response if no rows returned. Delete decrements atomically with `GREATEST(0, used_bytes - $delta)`.
|
||||
|
||||
**Topics Isolation Model**
|
||||
- D-08: System topics (`user_id=NULL`) visible to all; per-user topics (`user_id=current_user.id`) visible only to that user.
|
||||
- D-09: Only admin can CRUD system topics via `POST /api/admin/topics`. Regular users CRUD own topics via `/api/topics/*`.
|
||||
- D-10: All existing topics deleted in Phase 3 migration. Admin seeds system topics fresh post-Phase 3.
|
||||
- D-11: AI classification receives system + user topics; new AI-suggested topics go into user namespace.
|
||||
|
||||
**Settings Flat-File Retirement**
|
||||
- D-12: `/api/settings` endpoint removed. `load_settings()`, `save_settings()`, `settings.json` deleted.
|
||||
- D-13: System prompt moves to `SYSTEM_PROMPT` env var in `config.py`. Hardcoded fallback in `classifier.py`.
|
||||
- D-14: Celery task resolves AI config via `doc.user_id → users.ai_provider + users.ai_model` (second DB lookup in same session).
|
||||
- D-15: Fallback: `DEFAULT_AI_PROVIDER` + `DEFAULT_AI_MODEL` env vars (code defaults: `"ollama"` / `"llama3.2"`).
|
||||
|
||||
**Auth Guards**
|
||||
- D-16: All `/api/documents/*` get `get_current_user`. Ownership assertion returns 404 (not 403) for cross-user access. Admin role returns 403 on all document endpoints.
|
||||
- D-17: `/api/topics/*` gets `get_current_user`. Topic queries filter by `user_id IN (current_user.id, NULL)`.
|
||||
|
||||
### Claude's Discretion
|
||||
- None specified beyond decisions above.
|
||||
|
||||
### Deferred Ideas (OUT OF SCOPE)
|
||||
- Presigned GET URLs for document downloads (Phase 4 — DOC-02 PDF preview proxied through app)
|
||||
- Per-user system prompt overrides
|
||||
- Quota reservation at upload-url initiation with client-supplied size
|
||||
- MinIO event notification webhook approach
|
||||
</user_constraints>
|
||||
|
||||
---
|
||||
|
||||
<phase_requirements>
|
||||
## Phase Requirements
|
||||
|
||||
| ID | Description | Research Support |
|
||||
|----|-------------|------------------|
|
||||
| STORE-03 | Atomic quota enforcement at upload using UPDATE...WHERE...RETURNING pattern | Finding 4: verified pattern with psycopg v3 / SQLAlchemy `text()` + `fetchone()` null check |
|
||||
| STORE-04 | Quota usage bar in sidebar (X MB of Y MB) amber at 80%, red at 95% | Finding 9: `GET /api/me/quota` endpoint; Vue reactivity via `useAuthStore` |
|
||||
| STORE-05 | Upload rejection at quota limit with usage/size/link error | Finding 4: 413 response shape `{used_bytes, limit_bytes, rejected_bytes}` |
|
||||
| STORE-06 | Atomic quota decrement on document delete | Finding 4: `GREATEST(0, used_bytes - $delta)` pattern verified |
|
||||
| SEC-04 | All file access resolved through DB lookup — no object key reconstruction from params | Finding 7: ownership assertion on every document handler using `doc.user_id == current_user.id` |
|
||||
| DOC-03 | AI provider/model from DB per user; user cannot change AI config | Finding 8: Celery task second DB lookup `doc.user_id → user.ai_provider` |
|
||||
| DOC-04 | System default topics + per-user topic overrides preserved | Finding 6: `user_id IS NULL OR user_id = :uid` filter; index on `topics.user_id` |
|
||||
| DOC-05 | AI classification uses user's assigned provider and model | Finding 8: classifier refactor to accept `ai_provider` + `ai_model` parameters |
|
||||
</phase_requirements>
|
||||
|
||||
---
|
||||
|
||||
## Research Summary
|
||||
|
||||
Phase 3 has three technical challenge clusters: (1) the Alembic migration with side effects (MinIO object deletion, quota reconciliation), (2) the two-step presigned PUT upload flow with its Docker hostname pitfall, and (3) multi-user data isolation across documents, topics, auth guards, and AI config.
|
||||
|
||||
The codebase is in excellent shape for this phase. The `get_current_user` / `get_current_admin` FastAPI dependencies are production-ready. The `Quota`, `Document`, and `Topic` ORM models already have all necessary columns. The atomic quota SQL pattern is straightforward to implement with SQLAlchemy `text()` + `session.execute()`. The MinIO SDK (`minio==7.2.20`) has `presigned_put_object()` and `stat_object()` with clean APIs. The biggest planning risk is the Docker hostname issue for browser-accessible presigned URLs — the generated URL contains the internal Docker hostname (`minio:9000`) which the browser cannot resolve; a second MinIO client instance configured with the public hostname is required.
|
||||
|
||||
**Primary recommendation:** Implement the phase in four waves: (1) Alembic migration + quota foundation, (2) presigned upload flow backend + auth guards, (3) Celery beat + classifier refactor, (4) frontend upload flow + quota bar.
|
||||
|
||||
---
|
||||
|
||||
## Architectural Responsibility Map
|
||||
|
||||
| Capability | Primary Tier | Secondary Tier | Rationale |
|
||||
|------------|-------------|----------------|-----------|
|
||||
| Quota enforcement | API / Backend | Database | Atomic UPDATE must be DB-side; API layer calls it and interprets result |
|
||||
| Presigned URL generation | API / Backend | MinIO | FastAPI generates URL using SDK; browser uses URL to PUT directly to MinIO |
|
||||
| File bytes transfer | MinIO (direct) | — | Bytes never pass through API layer (CLAUDE.md architectural rule) |
|
||||
| Ownership assertion | API / Backend | Database | Every handler verifies `doc.user_id == current_user.id` before responding |
|
||||
| Topic namespace | Database | API / Backend | DB filter `user_id IS NULL OR user_id = :uid`; API enforces writes into correct namespace |
|
||||
| AI config resolution | Celery task | Database | Task does second DB lookup; no task signature change |
|
||||
| Abandoned upload cleanup | Celery beat | MinIO + Database | Periodic task; both DB row and MinIO object must be cleaned |
|
||||
| Quota bar display | Frontend (client) | API / Backend | `GET /api/me/quota` endpoint; Vue component polls/renders |
|
||||
| Upload progress | Frontend (client) | — | XMLHttpRequest upload events; fetch API does not support upload progress |
|
||||
|
||||
---
|
||||
|
||||
## Key Findings
|
||||
|
||||
### Finding 1: Alembic Migration with Side Effects — Safe Pattern
|
||||
|
||||
[VERIFIED: codebase inspection + SQLAlchemy docs]
|
||||
|
||||
The Phase 3 migration (`0003_multi_user_isolation.py`) must perform: (1) delete null-user documents + their MinIO objects, (2) delete all topics, (3) add NOT NULL constraint to `documents.user_id`, (4) reconcile `quotas.used_bytes`.
|
||||
|
||||
**Critical constraint:** Alembic `upgrade()` runs synchronously. MinIO SDK calls are also synchronous (the async wrappers in `MinIOBackend` use `asyncio.to_thread()` which requires a running event loop). Inside Alembic, there is no event loop by default. The safe pattern is to call the synchronous MinIO SDK directly — not through `MinIOBackend`, which is the async wrapper. `asyncio.run()` inside `upgrade()` also works but adds unnecessary complexity.
|
||||
|
||||
**Established pattern in this codebase:** Migrations use `op.execute(text(...))` for data manipulation. For the MinIO side, inline synchronous SDK calls are appropriate:
|
||||
|
||||
```python
|
||||
# In upgrade() — synchronous MinIO SDK, not the async wrapper
|
||||
from minio import Minio
|
||||
client = Minio(endpoint, access_key, secret_key, secure=False)
|
||||
for obj_key in keys_to_delete:
|
||||
client.remove_object(bucket, obj_key)
|
||||
```
|
||||
|
||||
**Rollback safety:** The `downgrade()` function cannot restore deleted MinIO objects or recreate deleted rows — document this explicitly. The migration is intentionally one-way for the cleanup step (D-01). The NOT NULL constraint reversal is safe via `op.alter_column('documents', 'user_id', nullable=True)`.
|
||||
|
||||
**Transaction boundary:** The DB operations (DELETE rows, ALTER COLUMN, UPDATE quotas) run inside the Alembic transaction. The MinIO deletions happen outside any DB transaction — if MinIO deletion partially fails, the DB transaction can still commit. This is intentional: leftover MinIO objects are orphaned but harmless; they will not appear in the UI because the DB rows are gone.
|
||||
|
||||
**Privilege grants:** The existing pattern (from migration 0001/0002) requires `GRANT` statements for any new tables. Migration 0003 creates no new tables — no additional grants needed.
|
||||
|
||||
### Finding 2: MinIO `presigned_put_object()` API
|
||||
|
||||
[VERIFIED: `python3 -c "from minio import Minio; help(Minio.presigned_put_object)"`]
|
||||
|
||||
```python
|
||||
client.presigned_put_object(
|
||||
bucket_name: str,
|
||||
object_name: str, # the pre-computed object key
|
||||
expires: timedelta = timedelta(days=7), # use timedelta(minutes=15) per D-05
|
||||
) -> str # returns the presigned URL string
|
||||
```
|
||||
|
||||
The object key must be pre-computed BEFORE calling this method, since the presigned URL is tied to that exact key. The Phase 3 flow creates the key in FastAPI at upload-url time: `f"{user_id}/{document_id}/{uuid4()}{ext}"`. This key is stored in the `Document` row (`object_key` column) at `status='pending'` time, so `stat_object()` at confirm time uses the same key.
|
||||
|
||||
**Async wrapping:** `asyncio.to_thread(self._client.presigned_put_object, bucket, key, timedelta(minutes=15))` — same pattern as existing `put_object` calls in `MinIOBackend`.
|
||||
|
||||
### Finding 3: Docker Hostname Pitfall for Presigned URLs — CRITICAL
|
||||
|
||||
[VERIFIED: community documentation + MinIO GitHub discussions]
|
||||
|
||||
**The problem:** `MinIOBackend` is initialized with `endpoint="minio:9000"` (the Docker internal hostname). `presigned_put_object()` generates URLs using this hostname: `http://minio:9000/docuvault/...?X-Amz-Signature=...`. Browsers running on the host machine cannot resolve `minio` as a DNS name — they see a network error before the PUT even starts.
|
||||
|
||||
**Why you cannot just replace the hostname:** The hostname is part of the HMAC-SHA256 signature. Changing `minio:9000` to `localhost:9000` in the URL string invalidates the signature; MinIO returns `SignatureDoesNotMatch`.
|
||||
|
||||
**The solution: dual MinIO client.** `MinIOBackend` needs a second Minio client instance initialized with the public/browser-accessible endpoint:
|
||||
|
||||
```python
|
||||
class MinIOBackend(StorageBackend):
|
||||
def __init__(self, endpoint, access_key, secret_key, bucket, secure=False,
|
||||
public_endpoint=None):
|
||||
self._bucket = bucket
|
||||
self._client = Minio(endpoint, access_key, secret_key, secure=secure)
|
||||
# Second client for presigned URL generation — uses browser-accessible hostname.
|
||||
# Falls back to internal client if not configured.
|
||||
self._public_client = Minio(
|
||||
public_endpoint or endpoint, access_key, secret_key, secure=secure
|
||||
)
|
||||
```
|
||||
|
||||
`presigned_put_object()` uses `self._public_client`; all other operations (`put_object`, `get_object`, `stat_object`, `delete_object`) use `self._client` (internal).
|
||||
|
||||
**Configuration:** Add `MINIO_PUBLIC_ENDPOINT` env var to `config.py` (optional, defaults to `MINIO_ENDPOINT`). In `.env.example`: `MINIO_PUBLIC_ENDPOINT=localhost:9000`. For production where MinIO is behind a load balancer or nginx, set this to the public URL.
|
||||
|
||||
**MinIO CORS for browser PUT:** MinIO open-source supports a global CORS setting via `MINIO_API_CORS_ALLOW_ORIGIN` environment variable (or `mc admin config set <alias> api cors_allow_origin=...`). For development: `MINIO_API_CORS_ALLOW_ORIGIN: "http://localhost:5173"`. Add to `docker-compose.yml` MinIO service environment block.
|
||||
|
||||
Per-bucket CORS configuration (S3 `PutBucketCors` API) is an enterprise-only feature in MinIO. The `minio` Python SDK 7.2.20 has no `set_bucket_cors()` method — confirmed by `dir(Minio)` inspection. The global env var approach is the correct solution for the open-source edition. [ASSUMED: the global `MINIO_API_CORS_ALLOW_ORIGIN` env var is reliable in current MinIO docker image — early reports (2021 issue #11258) showed it unreliable, but more recent reports (2024 issue #16479) indicate `mc admin config set` works; env var should be equivalent on fresh container start]
|
||||
|
||||
### Finding 4: Atomic Quota Enforcement Pattern
|
||||
|
||||
[VERIFIED: SQLAlchemy docs + psycopg v3 docs + codebase pattern inspection]
|
||||
|
||||
**Upload enforcement (STORE-03, STORE-05):**
|
||||
|
||||
```python
|
||||
from sqlalchemy import text
|
||||
|
||||
async def enforce_quota(session: AsyncSession, user_id: uuid.UUID, delta: int) -> int:
|
||||
"""Atomically increment quota. Returns new used_bytes, or raises HTTPException(413)."""
|
||||
result = await session.execute(
|
||||
text("""
|
||||
UPDATE quotas
|
||||
SET used_bytes = used_bytes + :delta
|
||||
WHERE user_id = :uid
|
||||
AND (used_bytes + :delta) <= limit_bytes
|
||||
RETURNING used_bytes, limit_bytes
|
||||
"""),
|
||||
{"delta": delta, "uid": str(user_id)},
|
||||
)
|
||||
row = result.fetchone()
|
||||
if row is None:
|
||||
# Quota exceeded — fetch current state for the 413 body
|
||||
quota_row = await session.execute(
|
||||
text("SELECT used_bytes, limit_bytes FROM quotas WHERE user_id = :uid"),
|
||||
{"uid": str(user_id)},
|
||||
)
|
||||
q = quota_row.fetchone()
|
||||
raise HTTPException(
|
||||
status_code=413,
|
||||
detail={
|
||||
"used_bytes": q.used_bytes if q else 0,
|
||||
"limit_bytes": q.limit_bytes if q else 0,
|
||||
"rejected_bytes": delta,
|
||||
},
|
||||
)
|
||||
return row.used_bytes
|
||||
```
|
||||
|
||||
**Why `fetchone() is None` and not `result.rowcount == 0`:** With psycopg v3 and `RETURNING`, `rowcount` reflects the number of rows in the result set. However, the SQLAlchemy async layer behavior can vary across backends (psycopg, asyncpg). Using `fetchone()` returning `None` is the most portable and explicit check — it directly tests "did the UPDATE match any rows?" [VERIFIED: psycopg v3 docs confirm `rowcount` reflects RETURNING result set count; fetchone() is unambiguous]
|
||||
|
||||
**Delete decrement (STORE-06):**
|
||||
|
||||
```python
|
||||
await session.execute(
|
||||
text("""
|
||||
UPDATE quotas
|
||||
SET used_bytes = GREATEST(0, used_bytes - :delta)
|
||||
WHERE user_id = :uid
|
||||
"""),
|
||||
{"delta": doc.size_bytes, "uid": str(doc.user_id)},
|
||||
)
|
||||
```
|
||||
|
||||
`GREATEST(0, ...)` prevents underflow if `size_bytes` was somehow recorded incorrectly. No RETURNING needed — the update always succeeds.
|
||||
|
||||
**Important:** The atomic quota check uses `doc.size_bytes` from `stat_object()` at confirm time, NOT client-supplied size. This matches D-07.
|
||||
|
||||
### Finding 5: MinIO `stat_object()` API
|
||||
|
||||
[VERIFIED: `python3 -c "from minio import Minio; help(Minio.stat_object)"`]
|
||||
|
||||
```python
|
||||
result = client.stat_object(bucket_name, object_name)
|
||||
# result.size — int, file size in bytes (authoritative)
|
||||
# result.last_modified — datetime
|
||||
# result.etag — str
|
||||
# result.content_type — str
|
||||
```
|
||||
|
||||
**Timing:** `stat_object()` is called at the `/confirm` step, after the browser has completed its PUT to MinIO. There is no meaningful eventual-consistency concern here since both the presigned PUT and the stat call go to the same MinIO instance. The presigned PUT completes before the browser calls `/confirm`, so the object exists by the time `stat_object()` is called.
|
||||
|
||||
**Error handling:** `stat_object()` raises `minio.error.S3Error` with `code="NoSuchKey"` if the object does not exist (upload never happened or TTL exceeded). Catch this and return 422: "Upload not found — presigned URL may have expired."
|
||||
|
||||
**Async wrapping:** `await asyncio.to_thread(self._client.stat_object, bucket, key)` — standard pattern.
|
||||
|
||||
### Finding 6: Topics Isolation — Index Strategy and Query Pattern
|
||||
|
||||
[VERIFIED: codebase inspection of Topic model + SQLAlchemy query patterns]
|
||||
|
||||
The `Topic` model already has `user_id` as a nullable UUID FK. The existing `UniqueConstraint("user_id", "name", name="uq_topics_user_name")` correctly handles the layered namespace: a system topic named "Finance" (`user_id=NULL`) and a user topic named "Finance" (`user_id=some_uuid`) can coexist.
|
||||
|
||||
**Index required:** The existing codebase has no dedicated index on `topics.user_id`. The phase needs to add one in the migration for the `user_id IS NULL OR user_id = :uid` query pattern used at classification time and topic listing:
|
||||
|
||||
```python
|
||||
op.create_index("ix_topics_user_id", "topics", ["user_id"])
|
||||
```
|
||||
|
||||
**Query pattern (D-17):**
|
||||
|
||||
```python
|
||||
from sqlalchemy import or_, is_
|
||||
|
||||
stmt = select(Topic).where(
|
||||
or_(Topic.user_id == current_user.id, Topic.user_id.is_(None))
|
||||
).order_by(Topic.name)
|
||||
```
|
||||
|
||||
**Topic creation for new AI suggestions (D-11):** When the classifier suggests a new topic, `create_topic()` in `services/storage.py` must accept an optional `user_id` parameter (defaulting to `None` for backward compatibility). Phase 3 calls it with `user_id=doc.user_id` to put suggested topics into the user's namespace.
|
||||
|
||||
**The deduplication query in `create_topic()` must also be scoped:**
|
||||
|
||||
```python
|
||||
# Current (wrong after Phase 3): matches any topic with same name regardless of user
|
||||
select(Topic).where(func.lower(Topic.name) == name.lower())
|
||||
|
||||
# Correct (Phase 3): deduplication within the same namespace only
|
||||
select(Topic).where(
|
||||
func.lower(Topic.name) == name.lower(),
|
||||
Topic.user_id == user_id # None for system topics, user UUID for per-user
|
||||
)
|
||||
```
|
||||
|
||||
### Finding 7: Auth Guard Pattern — 404 vs 403, Admin 403
|
||||
|
||||
[VERIFIED: codebase inspection of `deps/auth.py` + CONTEXT.md D-16]
|
||||
|
||||
`get_current_user` and `get_current_admin` dependencies are fully implemented in `backend/deps/auth.py`. Adding them to document/topic endpoints is a mechanical dependency injection.
|
||||
|
||||
**Ownership assertion (SEC-04):**
|
||||
|
||||
```python
|
||||
@router.get("/{doc_id}")
|
||||
async def get_document(
|
||||
doc_id: str,
|
||||
current_user: User = Depends(get_current_user),
|
||||
session: AsyncSession = Depends(get_db),
|
||||
):
|
||||
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")
|
||||
return _doc_to_dict(doc, ...)
|
||||
```
|
||||
|
||||
Returning 404 for both "not found" and "wrong owner" avoids information leakage (D-16). An attacker cannot distinguish between "document doesn't exist" and "document exists but belongs to someone else."
|
||||
|
||||
**Admin 403 on `/api/documents/*`:** Admins should not access document content (per CLAUDE.md and Phase 2 D-07 deferred rule). The simplest implementation is a separate dependency or guard at the router level:
|
||||
|
||||
```python
|
||||
# Option A: inline check in each handler
|
||||
if current_user.role == "admin":
|
||||
raise HTTPException(403, "Admin accounts cannot access document content")
|
||||
|
||||
# Option B: a dedicated dependency
|
||||
async def get_regular_user(user: User = Depends(get_current_user)) -> User:
|
||||
if user.role == "admin":
|
||||
raise HTTPException(403, "Admin accounts cannot access document content")
|
||||
return user
|
||||
```
|
||||
|
||||
Option B is cleaner and prevents forgetting the check in any one handler.
|
||||
|
||||
**Admin topics endpoint:** `POST /api/admin/topics` uses `get_current_admin` (already available). Regular `/api/topics/*` uses `get_regular_user` (or `get_current_user` with write operations scoped to own namespace).
|
||||
|
||||
### Finding 8: Per-User AI Classification — Celery Task Refactor
|
||||
|
||||
[VERIFIED: codebase inspection of `tasks/document_tasks.py` + `services/classifier.py`]
|
||||
|
||||
**Current flow (broken for Phase 3):**
|
||||
|
||||
`classifier.classify_document()` calls `storage.load_settings()` which reads the flat-file `settings.json`. This gives global AI config. Phase 3 eliminates this.
|
||||
|
||||
**Target flow (D-14, DOC-03, DOC-05):**
|
||||
|
||||
```python
|
||||
# In _run() inside document_tasks.py (after looking up doc):
|
||||
user = await session.get(User, doc.user_id)
|
||||
ai_provider = user.ai_provider or settings.default_ai_provider
|
||||
ai_model = user.ai_model or settings.default_ai_model
|
||||
|
||||
topics = await classifier.classify_document(
|
||||
session, document_id,
|
||||
ai_provider=ai_provider,
|
||||
ai_model=ai_model,
|
||||
)
|
||||
```
|
||||
|
||||
**`classifier.py` signature change:**
|
||||
|
||||
```python
|
||||
async def classify_document(
|
||||
session: AsyncSession,
|
||||
doc_id: str,
|
||||
topic_names: list[str] | None = None,
|
||||
ai_provider: str | None = None, # NEW
|
||||
ai_model: str | None = None, # NEW
|
||||
) -> list[str]:
|
||||
# ... uses ai_provider/ai_model to call get_provider() instead of load_settings()
|
||||
```
|
||||
|
||||
The `get_provider()` factory in `backend/ai/__init__.py` currently takes a `settings` dict. It needs to accept `(provider_name, model)` or be called with a minimal settings dict constructed from the parameters. Inspect the factory signature before planning the refactor.
|
||||
|
||||
**`config.py` additions (D-13, D-15):**
|
||||
|
||||
```python
|
||||
system_prompt: str = "" # SYSTEM_PROMPT env var
|
||||
default_ai_provider: str = "ollama" # DEFAULT_AI_PROVIDER env var
|
||||
default_ai_model: str = "llama3.2" # DEFAULT_AI_MODEL env var
|
||||
```
|
||||
|
||||
### Finding 9: `GET /api/me/quota` Endpoint + Quota Bar Frontend
|
||||
|
||||
[VERIFIED: codebase inspection + CONTEXT.md specifics]
|
||||
|
||||
**New endpoint needed:** The CONTEXT.md specifies `GET /api/me/quota` returning `{used_bytes, limit_bytes}`. This endpoint belongs in `api/auth.py` (where `/api/auth/me` already lives) or a new `api/me.py` router. Adding it to `api/auth.py` is simpler:
|
||||
|
||||
```python
|
||||
@router.get("/me/quota")
|
||||
async def get_my_quota(
|
||||
current_user: User = Depends(get_current_user),
|
||||
session: AsyncSession = Depends(get_db),
|
||||
):
|
||||
q = await session.get(Quota, current_user.id)
|
||||
return {"used_bytes": q.used_bytes, "limit_bytes": q.limit_bytes}
|
||||
```
|
||||
|
||||
**Frontend quota bar (STORE-04):** `AppSidebar.vue` already uses `useAuthStore`. A new `useQuotaStore` (or inline reactive data in `AppSidebar`) fetches `GET /api/me/quota` on mount and after each upload completion. The bar logic:
|
||||
|
||||
```
|
||||
pct = used_bytes / limit_bytes * 100
|
||||
color = pct >= 95 ? 'red' : pct >= 80 ? 'amber' : 'gray'
|
||||
label = `${(used_bytes / 1048576).toFixed(1)} MB of ${(limit_bytes / 1048576).toFixed(0)} MB`
|
||||
```
|
||||
|
||||
**Upload progress (STORE-04 context):** The `UploadProgress.vue` component currently shows a spinner + "Uploading…" text. To show real progress during the MinIO PUT step, XMLHttpRequest is required — `fetch` does not expose upload progress events. The existing `api/client.js` uses `fetch`; the new upload flow needs a separate `uploadToMinIO(url, file, onProgress)` helper using XHR:
|
||||
|
||||
```javascript
|
||||
function uploadToMinIO(url, file, onProgress) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const xhr = new XMLHttpRequest()
|
||||
xhr.upload.addEventListener('progress', e => {
|
||||
if (e.lengthComputable) onProgress(Math.round(e.loaded / e.total * 100))
|
||||
})
|
||||
xhr.addEventListener('load', () => xhr.status < 400 ? resolve() : reject(new Error(`PUT failed: ${xhr.status}`)))
|
||||
xhr.addEventListener('error', () => reject(new Error('Network error during upload')))
|
||||
xhr.open('PUT', url)
|
||||
xhr.setRequestHeader('Content-Type', file.type || 'application/octet-stream')
|
||||
xhr.send(file)
|
||||
})
|
||||
}
|
||||
```
|
||||
|
||||
### Finding 10: Celery Beat Abandoned Upload Cleanup (D-06)
|
||||
|
||||
[VERIFIED: `python3 -c "import celery; from datetime import timedelta; ..."` + codebase inspection]
|
||||
|
||||
**Celery beat schedule syntax (Celery 5.6.3):**
|
||||
|
||||
```python
|
||||
from datetime import timedelta
|
||||
|
||||
celery_app.conf.beat_schedule = {
|
||||
"cleanup-abandoned-uploads": {
|
||||
"task": "tasks.document_tasks.cleanup_abandoned_uploads",
|
||||
"schedule": timedelta(minutes=30),
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Add to `celery_app.py`. The existing `celery-worker` service in `docker-compose.yml` only runs `celery -A celery_app worker`. Beat requires a separate process: `celery -A celery_app beat --loglevel=info`. Add a `celery-beat` service to `docker-compose.yml`.
|
||||
|
||||
**Task implementation:**
|
||||
|
||||
```python
|
||||
@celery_app.task(name="tasks.document_tasks.cleanup_abandoned_uploads")
|
||||
def cleanup_abandoned_uploads() -> dict:
|
||||
return asyncio.run(_cleanup_abandoned())
|
||||
|
||||
async def _cleanup_abandoned() -> dict:
|
||||
from datetime import datetime, timezone, timedelta
|
||||
from sqlalchemy import select, delete
|
||||
from db.session import AsyncSessionLocal
|
||||
from db.models import Document
|
||||
from storage import get_storage_backend
|
||||
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(hours=1)
|
||||
async with AsyncSessionLocal() as session:
|
||||
result = await session.execute(
|
||||
select(Document).where(
|
||||
Document.status == "pending",
|
||||
Document.created_at < cutoff,
|
||||
)
|
||||
)
|
||||
docs = result.scalars().all()
|
||||
backend = get_storage_backend()
|
||||
cleaned = 0
|
||||
for doc in docs:
|
||||
try:
|
||||
if doc.object_key:
|
||||
await backend.delete_object(doc.object_key)
|
||||
except Exception:
|
||||
pass # MinIO object may not exist yet — safe to ignore
|
||||
await session.delete(doc)
|
||||
cleaned += 1
|
||||
await session.commit()
|
||||
return {"cleaned": cleaned}
|
||||
```
|
||||
|
||||
**Idempotency:** Re-running the task while some cleanup is in progress is safe — the `WHERE status='pending' AND created_at < cutoff` filter is idempotent. Partially cleaned rows (MinIO gone, DB row not yet deleted) are handled by ignoring MinIO deletion errors.
|
||||
|
||||
**Beat routing:** Add beat task to the `documents` queue or a separate `beat` queue. The existing `task_routes` in `celery_app.py` covers `tasks.document_tasks.*` → `documents` queue, so the cleanup task will route there automatically.
|
||||
|
||||
### Finding 11: Settings Flat-File Retirement Scope
|
||||
|
||||
[VERIFIED: codebase inspection of all affected files]
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/api/settings.py` — delete entire file (router removed from `main.py`)
|
||||
- `backend/services/storage.py` — remove `load_settings()`, `save_settings()`, `mask_api_key()`, `settings_masked()`, `DEFAULT_SETTINGS` import, `SETTINGS_FILE` import
|
||||
- `backend/config.py` — remove `SETTINGS_FILE`, `DEFAULT_SETTINGS`, `DEFAULT_SYSTEM_PROMPT`; add `SYSTEM_PROMPT`, `DEFAULT_AI_PROVIDER`, `DEFAULT_AI_MODEL`
|
||||
- `backend/services/classifier.py` — remove `storage.load_settings()` and `get_provider(settings)` calls; accept `ai_provider`/`ai_model` parameters
|
||||
- `backend/main.py` — remove `api/settings.py` router include
|
||||
- `frontend/src/stores/settings.js` — remove (or gut to minimal placeholder)
|
||||
- `frontend/src/views/SettingsView.vue` — replace with placeholder or remove route
|
||||
- `frontend/src/api/client.js` — remove `getSettings()`, `patchSettings()`, `testProvider()`, `getDefaultPrompt()`
|
||||
- `frontend/src/router/index.js` — remove `/settings` route (or keep as redirect to account)
|
||||
- `frontend/src/components/layout/AppSidebar.vue` — remove Settings nav link
|
||||
|
||||
**SettingsView.vue fate:** The existing view lets users configure AI provider. Phase 3 removes user control of AI config (admin-only via admin panel). The `/settings` route should display a message: "AI configuration is managed by your admin." The SettingsView replacement should be minimal — a single message card, no form.
|
||||
|
||||
### Finding 12: `ai/__init__.py` — `get_provider()` Factory
|
||||
|
||||
[VERIFIED: codebase inspection of `backend/ai/__init__.py`]
|
||||
|
||||
The classifier refactor requires understanding the `get_provider()` factory. Let me check its current signature:
|
||||
|
||||
The factory currently takes a `settings` dict (legacy format). For Phase 3, the classifier will have `ai_provider` (string like `"ollama"`, `"anthropic"`) and `ai_model` (string). The factory call should become:
|
||||
|
||||
```python
|
||||
# Construct a minimal settings-like dict from parameters
|
||||
_settings = {
|
||||
"active_provider": ai_provider,
|
||||
"providers": {
|
||||
ai_provider: {"model": ai_model}
|
||||
# For API-key providers, keys come from env vars
|
||||
}
|
||||
}
|
||||
provider = get_provider(_settings)
|
||||
```
|
||||
|
||||
Or refactor `get_provider()` to accept `(provider_name: str, model: str)` directly — cleaner but requires updating the factory signature. The planner should choose: minimal change (dict construction) vs proper refactor (new signature). Minimal change is safer for Phase 3 scope.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Risks
|
||||
|
||||
### Risk 1: MinIO CORS — Browser PUT May Silently Fail
|
||||
|
||||
**Severity:** HIGH — blocks the entire presigned upload flow from working in the browser
|
||||
|
||||
**What goes wrong:** If `MINIO_API_CORS_ALLOW_ORIGIN` is not set in the MinIO container environment, the browser's OPTIONS preflight for the PUT request will receive no `Access-Control-Allow-Origin` header and abort. The JavaScript error is `ERR_FAILED` or a CORS error with no useful detail.
|
||||
|
||||
**How to prevent:**
|
||||
1. Add `MINIO_API_CORS_ALLOW_ORIGIN: "${CORS_ORIGINS}"` (or `http://localhost:5173`) to the MinIO service in `docker-compose.yml`.
|
||||
2. Add `MINIO_PUBLIC_ENDPOINT` env var and dual MinIO client (Finding 3) so presigned URLs use `localhost:9000` not `minio:9000`.
|
||||
3. Integration test must exercise the full browser path (not just the FastAPI endpoints).
|
||||
|
||||
**Early detection:** Create a minimal curl-based test: `curl -X OPTIONS http://localhost:9000/docuvault/test-key -H "Origin: http://localhost:5173" -H "Access-Control-Request-Method: PUT"` — should return `Access-Control-Allow-Origin`.
|
||||
|
||||
### Risk 2: Concurrent Upload Race Condition (SC2)
|
||||
|
||||
**Severity:** HIGH — Phase 3 SC2 requires exactly one success and one 413 when two uploads exceed quota
|
||||
|
||||
**What goes wrong:** Two concurrent `/confirm` calls both see `used_bytes=90MB`, both calculate `90+15=105 > 100` as still under limit, both succeed — quota goes to 120MB.
|
||||
|
||||
**Why this cannot happen with the atomic pattern:** The `UPDATE quotas SET used_bytes = used_bytes + :delta WHERE (used_bytes + :delta) <= limit_bytes` is atomic at the PostgreSQL row level. PostgreSQL's row-level locking ensures the second `UPDATE` sees the result of the first. The WHERE clause re-evaluates against the committed state.
|
||||
|
||||
**Verification:** The SC2 test must fire two concurrent requests using `asyncio.gather()` and assert exactly one 200 and one 413.
|
||||
|
||||
### Risk 3: Alembic Migration with MinIO Connection
|
||||
|
||||
**Severity:** MEDIUM — migration may fail if MinIO is unreachable at migration time
|
||||
|
||||
**What goes wrong:** If `alembic upgrade head` is run while MinIO is not yet healthy (e.g., early in `docker compose up`), the migration's MinIO object deletion step will fail, aborting the migration.
|
||||
|
||||
**How to prevent:** The migration reads MinIO connection details from environment variables (`MINIO_ENDPOINT`, etc.). These are available in the `backend` container. `docker-compose.yml` already has `minio: condition: service_healthy` in `backend` `depends_on` — migrations should be run after services are healthy. Document that migrations must run after `docker compose up` completes health checks.
|
||||
|
||||
**Rollback safety:** If the migration fails mid-way through MinIO deletions but before the `ALTER COLUMN`, the DB is in a consistent state (nullable user_id still allowed). Re-running `alembic upgrade head` is idempotent at the schema level (Alembic tracks applied revisions), but the MinIO cleanup step will re-attempt on partial data. This is safe — `remove_object` on an already-deleted key returns silently.
|
||||
|
||||
### Risk 4: `stat_object()` Size vs. Actual Quota Needs
|
||||
|
||||
**Severity:** LOW — edge case with content-type negotiation
|
||||
|
||||
**What goes wrong:** The file size from `stat_object().size` is the raw byte count of the uploaded object. This matches what was PUT. No edge cases expected — the presigned URL is for a single-part PUT, not multipart.
|
||||
|
||||
**Resolved:** Simple, no special handling needed.
|
||||
|
||||
### Risk 5: Topics Deduplication Cross-Namespace
|
||||
|
||||
**Severity:** MEDIUM — subtle bug if not addressed
|
||||
|
||||
**What goes wrong (without fix):** If `create_topic()` is called with name "Finance" and `user_id=None` (system topic), and later called with name "Finance" and `user_id=some_uuid`, the current deduplication query `WHERE lower(name) = 'finance'` would return the system topic and skip creating a per-user topic. The user's document would then be tagged to the system topic (`user_id=NULL`), not their personal topic.
|
||||
|
||||
**How to prevent:** Scope the deduplication query by `user_id` as described in Finding 6.
|
||||
|
||||
### Risk 6: SettingsView Navigation After Retirement
|
||||
|
||||
**Severity:** LOW — UX regression if not handled
|
||||
|
||||
**What goes wrong:** Users who navigate to `/settings` (or have it bookmarked) will see a broken page or 404 after `SettingsView.vue` is removed.
|
||||
|
||||
**How to prevent:** Keep the `/settings` route but replace `SettingsView.vue` content with a simple "AI configuration is managed by your admin" message. Remove the form, API calls, and store interactions.
|
||||
|
||||
---
|
||||
|
||||
## Validation Architecture
|
||||
|
||||
### Test Framework
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| Framework | pytest + pytest-asyncio (existing in codebase) |
|
||||
| Config | `backend/pytest.ini` or `backend/pyproject.toml` (check before Wave 0) |
|
||||
| Quick run command | `cd backend && pytest tests/test_documents.py tests/test_topics.py -x -q` |
|
||||
| Full suite command | `cd backend && pytest -v` |
|
||||
|
||||
### Phase Requirements → Test Map
|
||||
|
||||
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||
|--------|----------|-----------|-------------------|-------------|
|
||||
| STORE-03 | Atomic quota enforced at confirm; no double-spend | unit + integration | `pytest tests/test_quota.py -x` | No — Wave 0 |
|
||||
| STORE-03 (SC2) | Two concurrent uploads at limit → exactly one 413 | integration | `pytest tests/test_quota.py::test_concurrent_quota_race -x` | No — Wave 0 |
|
||||
| STORE-04 | `GET /api/me/quota` returns `{used_bytes, limit_bytes}` | unit | `pytest tests/test_documents.py::test_get_quota -x` | No — Wave 0 |
|
||||
| STORE-05 | Confirm endpoint returns 413 with `{used_bytes, limit_bytes, rejected_bytes}` | unit | `pytest tests/test_quota.py::test_quota_exceeded_response -x` | No — Wave 0 |
|
||||
| STORE-06 | Delete decrements quota atomically | unit | `pytest tests/test_quota.py::test_delete_decrements_quota -x` | No — Wave 0 |
|
||||
| SEC-04 | Cross-user document access returns 404 | unit | `pytest tests/test_documents.py::test_cross_user_access_404 -x` | No — Wave 0 |
|
||||
| SEC-04 | Admin JWT on document endpoint returns 403 | unit | `pytest tests/test_documents.py::test_admin_cannot_access_documents -x` | No — Wave 0 |
|
||||
| DOC-03 | AI classification uses user's assigned provider not global config | unit | `pytest tests/test_classifier.py::test_per_user_provider -x` | No — Wave 0 |
|
||||
| DOC-04 | Topic list returns system topics + user topics (not other users') | unit | `pytest tests/test_topics.py::test_topic_namespace -x` | No — Wave 0 |
|
||||
| DOC-05 | Celery task resolves provider from document owner's DB record | unit | `pytest tests/test_classifier.py::test_celery_task_uses_user_provider -x` | No — Wave 0 |
|
||||
| D-02 | Alembic migration 0003 applies cleanly, null-user docs deleted | integration | `pytest tests/test_alembic.py::test_migration_0003 -x` | No — Wave 0 |
|
||||
| D-05 | Upload-url endpoint creates pending Document row + returns presigned URL | unit | `pytest tests/test_documents.py::test_upload_url_endpoint -x` | No — Wave 0 |
|
||||
| D-05 | Confirm endpoint calls stat_object, updates size, sets status=uploaded | unit | `pytest tests/test_documents.py::test_confirm_endpoint -x` | No — Wave 0 |
|
||||
| D-12 | `/api/settings` returns 404 (removed) | unit | `pytest tests/test_settings.py::test_settings_endpoint_removed -x` | No — Wave 0 |
|
||||
|
||||
### Sampling Rate
|
||||
- **Per task commit:** `cd backend && pytest tests/test_documents.py tests/test_quota.py tests/test_topics.py -x -q`
|
||||
- **Per wave merge:** `cd backend && pytest -v`
|
||||
- **Phase gate:** Full suite green before `/gsd:verify-work`
|
||||
|
||||
### Wave 0 Gaps
|
||||
- `tests/test_quota.py` — covers STORE-03, STORE-05, STORE-06, SC2 race condition
|
||||
- Test fixture `auth_user` — authenticated user with quota row (needed across quota and document tests)
|
||||
- Test fixture `admin_user` — admin-role user (needed for admin 403 tests)
|
||||
- Mock for MinIO `presigned_put_object` and `stat_object` in unit tests (document tests must not require live MinIO)
|
||||
|
||||
The existing `conftest.py` already provides `db_session` (in-memory SQLite) and `async_client`. The new fixtures extend this.
|
||||
|
||||
---
|
||||
|
||||
## Recommended Approach
|
||||
|
||||
### Wave Structure
|
||||
|
||||
**Wave 1: Migration + Quota Foundation (no external deps required)**
|
||||
1. Alembic migration `0003_multi_user_isolation.py`:
|
||||
- Delete null-user Document rows (collect object_keys first, then DB delete, then MinIO delete)
|
||||
- Delete all Topic rows
|
||||
- Add `NOT NULL` to `documents.user_id`
|
||||
- Reconcile quotas
|
||||
- Add index `ix_topics_user_id`
|
||||
2. `MinIOBackend`: add `presigned_put_url()` method (dual client for public endpoint)
|
||||
3. `StorageBackend` ABC: add `presigned_put_url()` abstract method
|
||||
4. `config.py`: add `MINIO_PUBLIC_ENDPOINT`, `SYSTEM_PROMPT`, `DEFAULT_AI_PROVIDER`, `DEFAULT_AI_MODEL`
|
||||
5. `docker-compose.yml`: add `MINIO_API_CORS_ALLOW_ORIGIN`, `MINIO_PUBLIC_ENDPOINT` to MinIO service; add `celery-beat` service
|
||||
|
||||
**Wave 2: Backend API (blocked on Wave 1)**
|
||||
1. `api/documents.py`: replace `/upload` with `/upload-url` + `/{id}/confirm`; add `get_current_user` (or `get_regular_user`) to all handlers; add ownership assertions (D-16)
|
||||
2. `api/topics.py`: add `get_current_user`; scope queries by `user_id IN (current_user.id, NULL)`; add `POST /api/admin/topics` endpoint
|
||||
3. `api/auth.py`: add `GET /api/me/quota` endpoint
|
||||
4. `api/settings.py`: delete file; remove from `main.py`
|
||||
5. `services/storage.py`: remove settings functions; update `create_topic()` to accept `user_id`; update `delete_document()` to decrement quota
|
||||
6. `services/classifier.py`: accept `ai_provider`/`ai_model` parameters; remove `load_settings()` call
|
||||
|
||||
**Wave 3: Celery (blocked on Wave 2)**
|
||||
1. `tasks/document_tasks.py`: update `_run()` to look up `user.ai_provider`/`user.ai_model`; pass to classifier
|
||||
2. `tasks/document_tasks.py`: add `cleanup_abandoned_uploads` task
|
||||
3. `celery_app.py`: add `beat_schedule` configuration
|
||||
|
||||
**Wave 4: Frontend (blocked on Wave 2)**
|
||||
1. `api/client.js`: add `getUploadUrl()`, `confirmUpload()`, `getMyQuota()`; remove settings functions
|
||||
2. `stores/documents.js`: update `upload()` to three-step flow with XHR progress
|
||||
3. `components/upload/UploadProgress.vue`: add progress percentage display
|
||||
4. `components/layout/AppSidebar.vue`: add quota bar
|
||||
5. `views/SettingsView.vue`: replace with "managed by admin" placeholder
|
||||
6. `stores/settings.js`: remove or gut
|
||||
|
||||
### Architecture Diagram
|
||||
|
||||
```
|
||||
Browser
|
||||
│
|
||||
├─ POST /api/documents/upload-url ──► FastAPI ──► DB: INSERT Document(status=pending)
|
||||
│ └──► MinIO: presigned_put_object()
|
||||
│ └──► returns {upload_url, document_id}
|
||||
│
|
||||
├─ PUT {upload_url} ──────────────────────────────────────────────────────► MinIO (direct)
|
||||
│ (XHR with progress events)
|
||||
│
|
||||
└─ POST /api/documents/{id}/confirm ─► FastAPI ──► MinIO: stat_object() → size
|
||||
└──► DB: atomic quota UPDATE
|
||||
└──► DB: Document.status = 'uploaded'
|
||||
└──► Celery: extract_and_classify.delay()
|
||||
└──► returns {document_id, used_bytes}
|
||||
|
||||
Celery Worker
|
||||
└─ extract_and_classify(document_id)
|
||||
├─ DB: SELECT user.ai_provider, user.ai_model WHERE user.id = doc.user_id
|
||||
├─ MinIO: get_object(object_key)
|
||||
├─ extractor.extract_text()
|
||||
├─ classifier.classify_document(ai_provider, ai_model)
|
||||
│ └─ DB: SELECT topics WHERE user_id IS NULL OR user_id = doc.user_id
|
||||
└─ DB: UPDATE document SET status='classified', extracted_text=...
|
||||
|
||||
Celery Beat (every 30 min)
|
||||
└─ cleanup_abandoned_uploads()
|
||||
├─ DB: SELECT documents WHERE status='pending' AND created_at < now()-1h
|
||||
├─ MinIO: remove_object() for each
|
||||
└─ DB: DELETE documents
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Project Constraints (from CLAUDE.md)
|
||||
|
||||
| Directive | Impact on Phase 3 |
|
||||
|-----------|-------------------|
|
||||
| JWT access token in Pinia memory only | `uploadDocument()` flow in Vue must use `useAuthStore().accessToken` for the `/upload-url` and `/confirm` API calls (via existing `request()` helper); XHR for MinIO PUT does NOT send auth header (MinIO presigned URL is self-authenticating) |
|
||||
| MinIO object keys UUID-based `{user_id}/{document_id}/{uuid4()}{ext}` | Key generated in `upload-url` handler, stored in `Document.object_key`, used unchanged at confirm and cleanup |
|
||||
| Atomic quota UPDATE pattern | Exact SQL from CLAUDE.md: `UPDATE quotas SET used_bytes = used_bytes + $delta WHERE (used_bytes + $delta) <= limit_bytes RETURNING used_bytes` |
|
||||
| Admin endpoints never return document content or `credentials_enc` | Admin topics endpoint at `POST /api/admin/topics` returns topic metadata only; admin cannot call document content endpoints (SEC-04 + D-16) |
|
||||
| Every document/folder endpoint asserts `resource.user_id == current_user.id` | Ownership assertion in every handler — return 404 for cross-user access |
|
||||
| All DB queries via ORM / parameterized statements | `text()` with bound parameters is used for the atomic quota SQL — always pass `:delta` and `:uid` as parameters, never interpolate |
|
||||
| SEC-04 | Object keys are stored in DB at upload-url time; confirm endpoint fetches key from DB (`Document.object_key`) — never reconstructs from request params |
|
||||
|
||||
---
|
||||
|
||||
## Environment Availability
|
||||
|
||||
| Dependency | Required By | Available | Version | Fallback |
|
||||
|------------|------------|-----------|---------|----------|
|
||||
| MinIO SDK (`minio`) | Presigned PUT, stat_object | Yes | 7.2.20 | — |
|
||||
| Celery + Redis | Beat schedule, cleanup task | Yes | 5.6.3 | — |
|
||||
| SQLAlchemy `text()` | Atomic quota SQL | Yes | 2.0+ | — |
|
||||
| psycopg v3 | Async DB driver | Yes (3.2.13) | 3.2.13 | — |
|
||||
| `aiosqlite` | Test in-memory DB | Yes (0.20.0) | 0.20.0 | — |
|
||||
| `asyncio.to_thread()` | Sync MinIO SDK wrapping | Yes (Python 3.10+) | Python 3.12 | — |
|
||||
|
||||
No blocking missing dependencies.
|
||||
|
||||
---
|
||||
|
||||
## Assumptions Log
|
||||
|
||||
| # | Claim | Section | Risk if Wrong |
|
||||
|---|-------|---------|---------------|
|
||||
| A1 | `MINIO_API_CORS_ALLOW_ORIGIN` env var works reliably on current MinIO docker image for browser PUT CORS | Finding 3 | Browser PUT blocked by CORS; fallback is `mc admin config set` in `docker-compose.yml` entrypoint or nginx CORS proxy |
|
||||
| A2 | PostgreSQL row-level locking on the quota `UPDATE WHERE` is sufficient to prevent concurrent over-quota uploads | Finding 4 / Risk 2 | Two simultaneous confirms could both succeed — would require `SELECT FOR UPDATE` on quota row or advisory lock |
|
||||
| A3 | `MinIOBackend.stat_object()` sees the completed PUT immediately with no eventual-consistency delay | Finding 5 | `NoSuchKey` error on stat_object even though PUT succeeded — add retry with exponential backoff as fallback |
|
||||
| A4 | `ai/__init__.py` `get_provider()` factory accepts a dict with `active_provider` and `providers` keys | Finding 12 | Factory may have a different internal signature — inspect before refactoring classifier |
|
||||
|
||||
**Note on A2:** PostgreSQL's `UPDATE ... WHERE` with a conditional is a standard serializable approach for quota enforcement. PostgreSQL's statement-level locking ensures the second concurrent UPDATE sees the committed result of the first. This is the same pattern used by many SaaS systems for rate limiting and quota enforcement. Risk is LOW given the choice of PostgreSQL (which has strong MVCC guarantees), but the SC2 integration test must verify this explicitly.
|
||||
|
||||
---
|
||||
|
||||
## Sources
|
||||
|
||||
### Primary (HIGH confidence)
|
||||
- MinIO Python SDK v7.2.20 — verified via `help(Minio.presigned_put_object)`, `help(Minio.stat_object)`, `dir(Minio)` — local installation
|
||||
- SQLAlchemy `text()` and `CursorResult` — verified via Python inspection of installed package
|
||||
- Celery 5.6.3 `beat_schedule` syntax — verified via `python3 -c "import celery; ..."`
|
||||
- psycopg v3.2.13 — verified `rowcount` + `fetchone()` behavior
|
||||
- Codebase inspection: `backend/storage/minio_backend.py`, `backend/db/models.py`, `backend/services/storage.py`, `backend/tasks/document_tasks.py`, `backend/services/classifier.py`, `backend/deps/auth.py`, `backend/api/documents.py`, `backend/api/topics.py`, `frontend/src/api/client.js`, `frontend/src/stores/documents.js`
|
||||
- CONTEXT.md — all locked decisions (D-01 through D-17)
|
||||
- CLAUDE.md — architectural constraints
|
||||
|
||||
### Secondary (MEDIUM confidence)
|
||||
- [MinIO GitHub discussion #20555](https://github.com/minio/minio/discussions/20555) — per-bucket CORS is enterprise-only in MinIO open source
|
||||
- [MinIO GitHub issue #16479](https://github.com/minio/minio/issues/16479) — `mc admin config set` as authoritative CORS configuration method
|
||||
- MinIO CORS `MINIO_API_CORS_ALLOW_ORIGIN` env var — referenced in multiple GitHub issues and community reports; enterprise docs confirm it exists at the global level
|
||||
- [Medium article on Docker hostname resolution](https://medium.com/@codyalexanderraymond/solving-presigned-url-issues-in-dockerized-development-with-minio-internal-dns-61a8b7c7c0ce) — dual-client / nginx proxy approaches for presigned URL Docker hostname issue
|
||||
|
||||
### Tertiary (LOW confidence)
|
||||
- XMLHttpRequest upload progress events vs. fetch — referenced in multiple blog posts; treated as LOW confidence due to unverified sources, but cross-confirmed by MDN documentation (fetch does not expose upload progress)
|
||||
|
||||
---
|
||||
|
||||
## Metadata
|
||||
|
||||
**Confidence breakdown:**
|
||||
- Alembic migration pattern: HIGH — established in migrations 0001 and 0002 in this codebase
|
||||
- MinIO SDK API (`presigned_put_object`, `stat_object`): HIGH — verified against installed SDK
|
||||
- Atomic quota SQL: HIGH — verified with SQLAlchemy text() and psycopg v3
|
||||
- MinIO CORS open-source solution: MEDIUM — `MINIO_API_CORS_ALLOW_ORIGIN` is documented but has had reliability issues; `mc admin config set` is more reliable but harder to automate in compose
|
||||
- Docker presigned URL hostname issue: HIGH — well-documented, dual-client solution is clean
|
||||
- Auth guard patterns: HIGH — `get_current_user`/`get_current_admin` already in production
|
||||
|
||||
**Research date:** 2026-05-23
|
||||
**Valid until:** 2026-06-22 (30 days — stable libraries)
|
||||
@@ -0,0 +1,66 @@
|
||||
---
|
||||
phase: 3
|
||||
slug: document-migration-multi-user-isolation
|
||||
status: draft
|
||||
nyquist_compliant: false
|
||||
wave_0_complete: false
|
||||
created: 2026-05-23
|
||||
---
|
||||
|
||||
# Phase 3 — Validation Strategy
|
||||
|
||||
> Per-phase validation contract for feedback sampling during execution.
|
||||
|
||||
---
|
||||
|
||||
## Test Infrastructure
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| **Framework** | pytest + pytest-asyncio (existing in codebase) |
|
||||
| **Config file** | `backend/pytest.ini` or `backend/pyproject.toml` |
|
||||
| **Quick run command** | `cd backend && pytest tests/test_documents.py tests/test_quota.py tests/test_topics.py -x -q` |
|
||||
| **Full suite command** | `cd backend && pytest -v` |
|
||||
| **Estimated runtime** | ~30–60 seconds |
|
||||
|
||||
---
|
||||
|
||||
## Sampling Rate
|
||||
|
||||
- **After every task commit:** Run `cd backend && pytest tests/test_documents.py tests/test_quota.py tests/test_topics.py -x -q`
|
||||
- **After every plan wave:** Run `cd backend && pytest -v`
|
||||
- **Before `/gsd:verify-work`:** Full suite must be green
|
||||
- **Max feedback latency:** 60 seconds
|
||||
|
||||
---
|
||||
|
||||
## Per-Task Verification Map
|
||||
|
||||
| Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status |
|
||||
|---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------|
|
||||
| Migration null-user cleanup | 01 | 1 | D-02 | SEC-04 | Null-user docs deleted before NOT NULL constraint | integration | `pytest tests/test_alembic.py::test_migration_0003 -x` | ❌ W0 | ⬜ pending |
|
||||
| Quota reconciliation | 01 | 1 | D-03 | STORE-03 | used_bytes matches SUM(size_bytes) post-migration | integration | `pytest tests/test_alembic.py::test_migration_0003 -x` | ❌ W0 | ⬜ pending |
|
||||
| Atomic quota enforce | 02 | 2 | STORE-03 | STORE-03 | No double-spend on concurrent uploads | unit+integration | `pytest tests/test_quota.py -x` | ❌ W0 | ⬜ pending |
|
||||
| Concurrent quota race | 02 | 2 | STORE-03 (SC2) | STORE-03 | Two concurrent uploads at limit → exactly one 413 | integration | `pytest tests/test_quota.py::test_concurrent_quota_race -x` | ❌ W0 | ⬜ pending |
|
||||
| Quota exceeded response | 02 | 2 | STORE-05 | STORE-05 | 413 with {used_bytes, limit_bytes, rejected_bytes} | unit | `pytest tests/test_quota.py::test_quota_exceeded_response -x` | ❌ W0 | ⬜ pending |
|
||||
| Atomic quota decrement | 02 | 2 | STORE-06 | STORE-06 | Delete decrements quota atomically | unit | `pytest tests/test_quota.py::test_delete_decrements_quota -x` | ❌ W0 | ⬜ pending |
|
||||
| Upload-url endpoint | 02 | 2 | D-05 | SEC-04 | Creates pending Document row + returns presigned URL | unit | `pytest tests/test_documents.py::test_upload_url_endpoint -x` | ❌ W0 | ⬜ pending |
|
||||
| Confirm endpoint | 02 | 2 | D-05 | STORE-03 | stat_object size used, status=uploaded set | unit | `pytest tests/test_documents.py::test_confirm_endpoint -x` | ❌ W0 | ⬜ pending |
|
||||
| Quota bar endpoint | 02 | 2 | STORE-04 | — | GET /api/me/quota returns {used_bytes, limit_bytes} | unit | `pytest tests/test_documents.py::test_get_quota -x` | ❌ W0 | ⬜ pending |
|
||||
| Cross-user access | 03 | 3 | SEC-04 | SEC-04 | Cross-user document access returns 404 | unit | `pytest tests/test_documents.py::test_cross_user_access_404 -x` | ❌ W0 | ⬜ pending |
|
||||
| Admin 403 on documents | 03 | 3 | SEC-04 (SC4) | SEC-04 | Admin JWT on /api/documents/* returns 403 | unit | `pytest tests/test_documents.py::test_admin_cannot_access_documents -x` | ❌ W0 | ⬜ pending |
|
||||
| Topic namespace isolation | 03 | 3 | DOC-04 | — | Topic list = system topics + own topics only | unit | `pytest tests/test_topics.py::test_topic_namespace -x` | ❌ W0 | ⬜ pending |
|
||||
| Per-user AI provider | 04 | 4 | DOC-03/DOC-05 | — | Classifier uses user's assigned provider not global | unit | `pytest tests/test_classifier.py::test_per_user_provider -x` | ❌ W0 | ⬜ pending |
|
||||
| Celery task provider lookup | 04 | 4 | DOC-05 | — | Celery task resolves provider from document owner's DB | unit | `pytest tests/test_classifier.py::test_celery_task_uses_user_provider -x` | ❌ W0 | ⬜ pending |
|
||||
| Settings endpoint removed | 04 | 4 | D-12 | — | /api/settings returns 404 | unit | `pytest tests/test_settings.py::test_settings_endpoint_removed -x` | ❌ W0 | ⬜ pending |
|
||||
|
||||
*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky*
|
||||
|
||||
---
|
||||
|
||||
## Wave 0 Requirements
|
||||
|
||||
- [ ] `backend/tests/test_quota.py` — stubs for STORE-03, STORE-05, STORE-06, concurrent race
|
||||
- [ ] `backend/tests/test_alembic.py` — stub for migration 0003 test
|
||||
- [ ] `backend/tests/test_classifier.py` — stubs for DOC-03, DOC-05 per-user provider
|
||||
- [ ] `backend/tests/conftest.py` — `auth_user` fixture (authenticated user with quota row), `admin_user` fixture, MinIO mock fixtures for `presigned_put_object` and `stat_object`
|
||||
Reference in New Issue
Block a user