kite/.planning/phases/03-document-migration-multi-user-isolation/03-01-PLAN.md

phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves

phase	plan	type	wave	depends_on	files_modified	autonomous	requirements	must_haves
03-document-migration-multi-user-isolation	01	execute	1		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	true	STORE-03 STORE-04 STORE-05 STORE-06 SEC-04 DOC-03 DOC-04 DOC-05	truths artifacts key_links 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 path provides contains backend/tests/test_quota.py Wave 0 xfail stubs for STORE-03 / STORE-05 / STORE-06 / SC2 test_concurrent_quota_race path provides contains backend/tests/test_classifier.py Wave 0 xfail stubs for DOC-03 / DOC-05 per-user provider resolution test_per_user_provider path provides exports backend/tests/conftest.py auth_user, admin_user, mock_minio_presigned, mock_minio_stat fixtures auth_user admin_user mock_minio_presigned mock_minio_stat path provides contains backend/migrations/versions/0003_multi_user_isolation.py Null-user document cleanup + topic cleanup + NOT NULL constraint + quota reconciliation + ix_topics_user_id revision = "0003" from to via pattern backend/migrations/versions/0003_multi_user_isolation.py MinIO (synchronous SDK) from minio import Minio; client.remove_object(bucket, key) for each null-user object_key remove_object from to via pattern backend/tests/conftest.py backend/services/auth.py create_access_token + User row + Quota row for fixture users create_access_token

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.

<execution_context> @/Users/nik/.claude/get-shit-done/workflows/execute-plan.md @/Users/nik/.claude/get-shit-done/templates/summary.md </execution_context>

@.planning/ROADMAP.md @.planning/STATE.md @.planning/phases/03-document-migration-multi-user-isolation/03-CONTEXT.md @.planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md @.planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md @.planning/phases/03-document-migration-multi-user-isolation/03-VALIDATION.md @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

From backend/db/models.py (Phase 1):

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

def hash_password(password: str) -> str
def create_access_token(user_id: str, role: str) -> str

From backend/tests/conftest.py (existing):

@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()).

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

Task 1: Create Wave 0 test scaffolds and shared fixtures 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/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 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).

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 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. Task 2: Write Alembic migration 0003 (null-user cleanup + NOT NULL + topic cleanup + quota reconcile + index) backend/migrations/versions/0003_multi_user_isolation.py - 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 - 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) 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).

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

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

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.