diff --git a/.planning/phases/06.1-close-v1-audit-gaps/06.1-01-SUMMARY.md b/.planning/phases/06.1-close-v1-audit-gaps/06.1-01-SUMMARY.md new file mode 100644 index 0000000..d31e6b2 --- /dev/null +++ b/.planning/phases/06.1-close-v1-audit-gaps/06.1-01-SUMMARY.md @@ -0,0 +1,94 @@ +--- +phase: "6.1" +plan: "06.1-01" +subsystem: testing +tags: [shares, test-promotion, xfail-removal, SHARE-01, SHARE-02, SHARE-03, SHARE-04, SHARE-05] +dependency_graph: + requires: [04-04] + provides: [SHARE-01-tests, SHARE-02-tests, SHARE-03-tests, SHARE-04-tests, SHARE-05-tests] + affects: [backend/tests/test_shares.py, backend/tests/conftest.py] +tech_stack: + added: [] + patterns: [pytest_asyncio fixture, ORM direct-insert helper, pytestmark module-level] +key_files: + created: [] + modified: + - backend/tests/test_shares.py + - backend/tests/conftest.py +decisions: + - "second_auth_user fixture uses user2_ handle prefix to prevent collisions with auth_user's testuser_ prefix" + - "_make_doc() helper inserts Document row directly via ORM (no upload endpoint) — same pattern as test_documents.py" + - "pytestmark = pytest.mark.asyncio at module level replaces per-test decorators — consistent with other test files" +metrics: + duration: "~15 minutes" + completed_date: "2026-05-30" + tasks_completed: 2 + tasks_total: 2 + files_modified: 2 +--- + +# Phase 6.1 Plan 01: Promote test_shares.py stubs to real tests (SHARE-01..05) Summary + +**One-liner:** Seven xfail stubs replaced with real integration tests that exercise POST/GET/DELETE /api/shares via in-memory SQLite, plus a second_auth_user fixture enabling sharer/recipient scenarios. + +## Tasks Completed + +| Task | Description | Commit | +|------|-------------|--------| +| 1 | Add second_auth_user fixture to conftest.py | b7df971 | +| 2 | Rewrite test_shares.py — 7 real tests replacing xfail stubs | 9973f42 | + +## What Was Built + +### Task 1 — second_auth_user fixture (conftest.py) + +Added `@pytest_asyncio.fixture async def second_auth_user(db_session)` immediately after `auth_user`. The fixture creates a second User with handle prefix `user2_` and email `user2_{hex8}@example.com`, plus a Quota row with limit_bytes=100MB and used_bytes=0. Returns the same `{user, token, headers}` dict shape as `auth_user`. This fixture enables sharing tests that need a distinct sharer and recipient within the same test case. + +### Task 2 — Real tests in test_shares.py + +Completely rewrote `backend/tests/test_shares.py`: +- Removed: all 7 `@pytest.mark.xfail(strict=False)` decorators, all 7 `pytest.xfail("not implemented yet")` calls, `import os` (was unused) +- Added: `pytestmark = pytest.mark.asyncio`, `import uuid as _uuid`, `import pytest_asyncio` +- Added: `async def _make_doc(db_session, owner_user)` helper that inserts an uploaded Document row via ORM and returns `str(doc_id)` +- Implemented 7 real tests: + +| Test | Requirement | Assertion | +|------|-------------|-----------| +| test_share_success | SHARE-01 | POST 201, body has id/document_id/recipient_id; recipient sees doc in /received | +| test_share_handle_not_found | SHARE-01 | POST with nonexistent handle → 404 | +| test_shared_with_me | SHARE-02 | /received has required fields; extracted_text absent (T-04-04-03); owner_handle correct | +| test_share_no_quota_impact | SHARE-03 | Recipient /quota used_bytes == 0 after share (T-04-04-04) | +| test_revoke_share | SHARE-04 | DELETE 204; doc no longer in recipient /received | +| test_share_revoke_wrong_owner_404 | SHARE-04 | Recipient DELETE → 404 not 403 (IDOR protection T-04-04-02) | +| test_share_duplicate | SHARE-05 | Second POST with same doc+recipient → 409 | + +## Verification + +``` +docker compose exec backend python -m pytest tests/test_shares.py -v +``` + +Result: **7 passed, 0 failed, 0 xfailed, 0 xpassed** (verified in Docker with pytest 9.0.3, asyncio mode=AUTO). + +## Deviations from Plan + +None — plan executed exactly as written. The second_auth_user fixture was added to the worktree conftest.py at the exact position specified (after auth_user, before admin_user). All 7 tests match the plan spec. + +## Known Stubs + +None. All 7 tests have real assertions against the live API. + +## Threat Flags + +None. No new network endpoints, auth paths, file access patterns, or schema changes introduced. This plan is test-only. + +## Self-Check: PASSED + +- [x] `backend/tests/conftest.py` contains `second_auth_user` fixture at line 229 +- [x] `backend/tests/test_shares.py` has zero `pytest.xfail` calls +- [x] `backend/tests/test_shares.py` has zero `@pytest.mark.xfail` decorators +- [x] `import os` is absent from test_shares.py +- [x] `pytestmark = pytest.mark.asyncio` is present at module level +- [x] Commit b7df971 exists (Task 1) +- [x] Commit 9973f42 exists (Task 2) +- [x] Docker test run: 7 passed, 0 failed diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 71990cf..e75e6a2 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -226,6 +226,45 @@ async def auth_user(db_session: AsyncSession): } +@pytest_asyncio.fixture +async def second_auth_user(db_session: AsyncSession): + """Create a second regular user with a Quota row and return auth context. + + Returns the same dict shape as auth_user but with a distinct handle prefix + ("user2_") so sharing tests can have a sharer and a recipient in the same + test without handle collisions. + """ + import uuid as _uuid + from db.models import User, Quota + from services.auth import hash_password, create_access_token + + user_id = _uuid.uuid4() + user = User( + id=user_id, + handle=f"user2_{user_id.hex[:8]}", + email=f"user2_{user_id.hex[:8]}@example.com", + password_hash=hash_password("Testpassword123!"), + role="user", + is_active=True, + password_must_change=False, + ) + quota = Quota( + user_id=user_id, + limit_bytes=104857600, # 100 MB + used_bytes=0, + ) + db_session.add(user) + db_session.add(quota) + await db_session.commit() + + token = create_access_token(str(user_id), "user") + return { + "user": user, + "token": token, + "headers": {"Authorization": f"Bearer {token}"}, + } + + @pytest_asyncio.fixture async def admin_user(db_session: AsyncSession): """Create an admin user with a Quota row and return auth context. diff --git a/backend/tests/test_shares.py b/backend/tests/test_shares.py index 6dc41c7..91370c5 100644 --- a/backend/tests/test_shares.py +++ b/backend/tests/test_shares.py @@ -1,15 +1,41 @@ """ -Share API tests — Wave 0 xfail stubs for Phase 4. +Share API tests — Phase 6.1, Plan 06.1-01. -All tests in this file are xfail stubs. They will be implemented in Plan 04-05. -The stubs ensure pytest collects them and keeps CI green before implementation -code exists. +Promotes all 7 xfail stubs to real tests covering SHARE-01 through SHARE-05. """ from __future__ import annotations -import os +import uuid as _uuid import pytest +import pytest_asyncio + +pytestmark = pytest.mark.asyncio + + +# --------------------------------------------------------------------------- +# Helper: create an uploaded Document row directly via ORM +# --------------------------------------------------------------------------- + + +async def _make_doc(db_session, owner_user) -> str: + """Create an uploaded Document row owned by owner_user and return its str UUID.""" + from db.models import Document + + doc_id = _uuid.uuid4() + doc = Document( + id=doc_id, + user_id=owner_user["user"].id, + filename="test.txt", + content_type="text/plain", + size_bytes=1000, + storage_backend="minio", + status="uploaded", + object_key=f"{owner_user['user'].id}/{doc_id}/{_uuid.uuid4()}.txt", + ) + db_session.add(doc) + await db_session.commit() + return str(doc_id) # --------------------------------------------------------------------------- @@ -17,16 +43,48 @@ import pytest # --------------------------------------------------------------------------- -@pytest.mark.xfail(strict=False) -async def test_share_success(async_client, auth_user): +async def test_share_success(async_client, auth_user, second_auth_user, db_session): """POST /api/shares grants share; recipient can see doc via GET /api/shares/received.""" - pytest.xfail("not implemented yet") + doc_id = await _make_doc(db_session, auth_user) + + resp = await async_client.post( + "/api/shares", + json={ + "document_id": doc_id, + "recipient_handle": second_auth_user["user"].handle, + }, + headers=auth_user["headers"], + ) + assert resp.status_code == 201, resp.text + body = resp.json() + assert "id" in body + assert body["document_id"] == doc_id + assert body["recipient_id"] == str(second_auth_user["user"].id) + + # Recipient can see the document in their received list + received = await async_client.get( + "/api/shares/received", + headers=second_auth_user["headers"], + ) + assert received.status_code == 200 + items = received.json()["items"] + doc_ids = [item["id"] for item in items] + assert doc_id in doc_ids -@pytest.mark.xfail(strict=False) -async def test_share_handle_not_found(async_client, auth_user): +async def test_share_handle_not_found(async_client, auth_user, db_session): """POST /api/shares with unknown handle returns 404.""" - pytest.xfail("not implemented yet") + doc_id = await _make_doc(db_session, auth_user) + + resp = await async_client.post( + "/api/shares", + json={ + "document_id": doc_id, + "recipient_handle": "nonexistent_handle_xyz", + }, + headers=auth_user["headers"], + ) + assert resp.status_code == 404 # --------------------------------------------------------------------------- @@ -34,10 +92,47 @@ async def test_share_handle_not_found(async_client, auth_user): # --------------------------------------------------------------------------- -@pytest.mark.xfail(strict=False) -async def test_shared_with_me(async_client, auth_user): - """GET /api/shares/received lists docs shared with current user.""" - pytest.xfail("not implemented yet") +async def test_shared_with_me(async_client, auth_user, second_auth_user, db_session): + """GET /api/shares/received lists docs shared with current user (T-04-04-03).""" + doc_id = await _make_doc(db_session, auth_user) + + # Grant share + share_resp = await async_client.post( + "/api/shares", + json={ + "document_id": doc_id, + "recipient_handle": second_auth_user["user"].handle, + }, + headers=auth_user["headers"], + ) + assert share_resp.status_code == 201 + + # Recipient fetches shared-with-me + received = await async_client.get( + "/api/shares/received", + headers=second_auth_user["headers"], + ) + assert received.status_code == 200 + items = received.json()["items"] + assert len(items) >= 1 + + # Find the specific item + matching = [item for item in items if item["id"] == doc_id] + assert len(matching) == 1, f"doc {doc_id} not in received items: {items}" + item = matching[0] + + # Required metadata fields present + for field in ("id", "filename", "content_type", "size_bytes", "created_at", "owner_handle"): + assert field in item, f"Expected field '{field}' in item" + + # T-04-04-03: extracted_text must NEVER be in any item + for received_item in items: + assert "extracted_text" not in received_item, ( + "extracted_text must not be returned in shared-with-me responses (T-04-04-03)" + ) + + # owner_handle should match the sharer + assert item["owner_handle"] == auth_user["user"].handle # --------------------------------------------------------------------------- @@ -45,10 +140,31 @@ async def test_shared_with_me(async_client, auth_user): # --------------------------------------------------------------------------- -@pytest.mark.xfail(strict=False) -async def test_share_no_quota_impact(async_client, auth_user): - """Share does not increment recipient's quota used_bytes.""" - pytest.xfail("not implemented yet") +async def test_share_no_quota_impact(async_client, auth_user, second_auth_user, db_session): + """Share does not increment recipient's quota used_bytes (T-04-04-04).""" + doc_id = await _make_doc(db_session, auth_user) + + # Grant share + share_resp = await async_client.post( + "/api/shares", + json={ + "document_id": doc_id, + "recipient_handle": second_auth_user["user"].handle, + }, + headers=auth_user["headers"], + ) + assert share_resp.status_code == 201 + + # Check recipient quota — must still be 0 used_bytes + quota_resp = await async_client.get( + "/api/auth/me/quota", + headers=second_auth_user["headers"], + ) + assert quota_resp.status_code == 200 + quota = quota_resp.json() + assert quota["used_bytes"] == 0, ( + f"Recipient quota used_bytes should be 0 after share, got {quota['used_bytes']}" + ) # --------------------------------------------------------------------------- @@ -56,16 +172,65 @@ async def test_share_no_quota_impact(async_client, auth_user): # --------------------------------------------------------------------------- -@pytest.mark.xfail(strict=False) -async def test_revoke_share(async_client, auth_user): +async def test_revoke_share(async_client, auth_user, second_auth_user, db_session): """DELETE /api/shares/{id} removes share; GET /api/shares/received no longer lists the doc.""" - pytest.xfail("not implemented yet") + doc_id = await _make_doc(db_session, auth_user) + + # Grant share + share_resp = await async_client.post( + "/api/shares", + json={ + "document_id": doc_id, + "recipient_handle": second_auth_user["user"].handle, + }, + headers=auth_user["headers"], + ) + assert share_resp.status_code == 201 + share_id = share_resp.json()["id"] + + # Revoke share (owner revokes) + revoke_resp = await async_client.delete( + f"/api/shares/{share_id}", + headers=auth_user["headers"], + ) + assert revoke_resp.status_code == 204 + + # Recipient no longer sees the document in received + received = await async_client.get( + "/api/shares/received", + headers=second_auth_user["headers"], + ) + assert received.status_code == 200 + items = received.json()["items"] + doc_ids = [item["id"] for item in items] + assert doc_id not in doc_ids, f"doc {doc_id} should have been revoked but still appears" -@pytest.mark.xfail(strict=False) -async def test_share_revoke_wrong_owner_404(async_client, auth_user): - """DELETE /api/shares/{id} by non-owner returns 404.""" - pytest.xfail("not implemented yet") +async def test_share_revoke_wrong_owner_404(async_client, auth_user, second_auth_user, db_session): + """DELETE /api/shares/{id} by non-owner returns 404 (IDOR protection — T-04-04-02).""" + doc_id = await _make_doc(db_session, auth_user) + + # Grant share + share_resp = await async_client.post( + "/api/shares", + json={ + "document_id": doc_id, + "recipient_handle": second_auth_user["user"].handle, + }, + headers=auth_user["headers"], + ) + assert share_resp.status_code == 201 + share_id = share_resp.json()["id"] + + # Recipient attempts to revoke — must be rejected with 404 (not 403) + revoke_resp = await async_client.delete( + f"/api/shares/{share_id}", + headers=second_auth_user["headers"], + ) + assert revoke_resp.status_code == 404, ( + f"Recipient should get 404 when attempting to revoke a share they don't own, " + f"got {revoke_resp.status_code}" + ) # --------------------------------------------------------------------------- @@ -73,7 +238,29 @@ async def test_share_revoke_wrong_owner_404(async_client, auth_user): # --------------------------------------------------------------------------- -@pytest.mark.xfail(strict=False) -async def test_share_duplicate(async_client, auth_user): +async def test_share_duplicate(async_client, auth_user, second_auth_user, db_session): """POST /api/shares same doc+recipient twice returns 409.""" - pytest.xfail("not implemented yet") + doc_id = await _make_doc(db_session, auth_user) + + payload = { + "document_id": doc_id, + "recipient_handle": second_auth_user["user"].handle, + } + + # First share — should succeed + first_resp = await async_client.post( + "/api/shares", + json=payload, + headers=auth_user["headers"], + ) + assert first_resp.status_code == 201 + + # Second share with same doc+recipient — should return 409 + second_resp = await async_client.post( + "/api/shares", + json=payload, + headers=auth_user["headers"], + ) + assert second_resp.status_code == 409, ( + f"Duplicate share should return 409, got {second_resp.status_code}" + )