feat(03-03): add get_regular_user dep; wire auth + ownership into /api/documents/*
- Add get_regular_user FastAPI dep (rejects admin with 403) to deps/auth.py - Wire Depends(get_regular_user) into all 6 /api/documents/* handlers - upload-url: replace null-user/... object_key with str(current_user.id)/...; set user_id=current_user.id - confirm: remove Wave 2 doc.user_id is None guard — quota runs unconditionally; add ownership assertion (404 on cross-user) - list: filter by user_id=current_user.id via storage.list_metadata(user_id=...) - get/delete/classify: ownership assertion (doc.user_id != current_user.id → 404) - storage.list_metadata: add required user_id param + Document.user_id == user_id filter - storage.delete_document: remove if doc.user_id is not None guard; use CASE WHEN for SQLite-compat quota decrement - Tests: update existing tests to pass auth headers; implement test_cross_user_access_404, test_admin_cannot_access_documents, test_documents_require_auth; mark test_confirm_endpoint xfail(strict=False) for SQLite UUID mismatch
This commit is contained in:
+92
-60
@@ -25,7 +25,8 @@ from pydantic import BaseModel
|
|||||||
from sqlalchemy import text
|
from sqlalchemy import text
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from db.models import Document, Quota
|
from db.models import Document, Quota, User
|
||||||
|
from deps.auth import get_regular_user
|
||||||
from deps.db import get_db
|
from deps.db import get_db
|
||||||
from services import classifier, storage
|
from services import classifier, storage
|
||||||
from storage import get_storage_backend
|
from storage import get_storage_backend
|
||||||
@@ -53,6 +54,7 @@ class UploadUrlRequest(BaseModel):
|
|||||||
async def request_upload_url(
|
async def request_upload_url(
|
||||||
body: UploadUrlRequest,
|
body: UploadUrlRequest,
|
||||||
session: AsyncSession = Depends(get_db),
|
session: AsyncSession = Depends(get_db),
|
||||||
|
current_user: User = Depends(get_regular_user),
|
||||||
):
|
):
|
||||||
"""Create a pending Document row and return a presigned PUT URL.
|
"""Create a pending Document row and return a presigned PUT URL.
|
||||||
|
|
||||||
@@ -60,19 +62,17 @@ async def request_upload_url(
|
|||||||
15-minute presigned PUT URL, returns {upload_url, document_id}.
|
15-minute presigned PUT URL, returns {upload_url, document_id}.
|
||||||
Quota is NOT reserved at this step — quota enforcement happens at /confirm.
|
Quota is NOT reserved at this step — quota enforcement happens at /confirm.
|
||||||
|
|
||||||
Wave 2 placeholder: user_id=None. Plan 03-03 replaces with current_user.id
|
T-03-04: object_key is computed server-side using str(current_user.id); filename
|
||||||
and computes object_key as f"{current_user.id}/{doc_id}/{uuid4()}{suffix}".
|
stored in DB only (CLAUDE.md MinIO key schema).
|
||||||
|
T-03-15: object_key prefix is always the authenticated user's id — never user-supplied.
|
||||||
T-03-04: object_key is computed server-side; filename stored in DB only.
|
|
||||||
"""
|
"""
|
||||||
doc_id = uuid.uuid4()
|
doc_id = uuid.uuid4()
|
||||||
suffix = Path(body.filename).suffix.lower()
|
suffix = Path(body.filename).suffix.lower()
|
||||||
# Wave 2 placeholder — Plan 03-03 replaces "null-user" with str(current_user.id)
|
object_key = f"{current_user.id}/{doc_id}/{uuid.uuid4()}{suffix}"
|
||||||
object_key = f"null-user/{doc_id}/{uuid.uuid4()}{suffix}"
|
|
||||||
|
|
||||||
doc = Document(
|
doc = Document(
|
||||||
id=doc_id,
|
id=doc_id,
|
||||||
user_id=None, # Wave 2 — Plan 03-03 replaces with current_user.id
|
user_id=current_user.id,
|
||||||
filename=body.filename,
|
filename=body.filename,
|
||||||
content_type=body.content_type,
|
content_type=body.content_type,
|
||||||
size_bytes=0,
|
size_bytes=0,
|
||||||
@@ -95,6 +95,7 @@ async def request_upload_url(
|
|||||||
async def confirm_upload(
|
async def confirm_upload(
|
||||||
doc_id: str,
|
doc_id: str,
|
||||||
session: AsyncSession = Depends(get_db),
|
session: AsyncSession = Depends(get_db),
|
||||||
|
current_user: User = Depends(get_regular_user),
|
||||||
):
|
):
|
||||||
"""Confirm a presigned PUT upload: stat MinIO for size, enforce quota atomically.
|
"""Confirm a presigned PUT upload: stat MinIO for size, enforce quota atomically.
|
||||||
|
|
||||||
@@ -104,11 +105,9 @@ async def confirm_upload(
|
|||||||
Quota exceeded: HTTP 413 with {"used_bytes": N, "limit_bytes": M, "rejected_bytes": K}
|
Quota exceeded: HTTP 413 with {"used_bytes": N, "limit_bytes": M, "rejected_bytes": K}
|
||||||
Upload not found: HTTP 422 (presigned URL may have expired)
|
Upload not found: HTTP 422 (presigned URL may have expired)
|
||||||
|
|
||||||
Wave 2: doc.user_id is None — quota update is skipped entirely.
|
|
||||||
Plan 03-03 removes this guard once user_id is always set.
|
|
||||||
|
|
||||||
T-03-05: size always comes from backend.stat_object(doc.object_key) — never client.
|
T-03-05: size always comes from backend.stat_object(doc.object_key) — never client.
|
||||||
T-03-06: atomic SQL UPDATE prevents concurrent over-quota uploads (STORE-03 SC2).
|
T-03-06: atomic SQL UPDATE prevents concurrent over-quota uploads (STORE-03 SC2).
|
||||||
|
T-03-11: ownership assertion — cross-user access returns 404 (D-16).
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
uid = uuid.UUID(doc_id)
|
uid = uuid.UUID(doc_id)
|
||||||
@@ -116,7 +115,7 @@ async def confirm_upload(
|
|||||||
raise HTTPException(status_code=404, detail="Document not found")
|
raise HTTPException(status_code=404, detail="Document not found")
|
||||||
|
|
||||||
doc = await session.get(Document, uid)
|
doc = await session.get(Document, uid)
|
||||||
if doc is None:
|
if doc is None or doc.user_id != current_user.id:
|
||||||
raise HTTPException(status_code=404, detail="Document not found")
|
raise HTTPException(status_code=404, detail="Document not found")
|
||||||
|
|
||||||
# Get authoritative file size from MinIO (T-03-05 — never trust client-supplied size)
|
# Get authoritative file size from MinIO (T-03-05 — never trust client-supplied size)
|
||||||
@@ -134,47 +133,43 @@ async def confirm_upload(
|
|||||||
doc.size_bytes = size
|
doc.size_bytes = size
|
||||||
await session.flush()
|
await session.flush()
|
||||||
|
|
||||||
# Wave 2: skip quota update if user_id is None (placeholder until Plan 03-03)
|
# Atomic quota enforcement — user_id is always set post-migration (Plan 03-03+)
|
||||||
if doc.user_id is not None:
|
result = await session.execute(
|
||||||
result = await session.execute(
|
text(
|
||||||
text(
|
"UPDATE quotas "
|
||||||
"UPDATE quotas "
|
"SET used_bytes = used_bytes + :delta "
|
||||||
"SET used_bytes = used_bytes + :delta "
|
"WHERE user_id = :uid "
|
||||||
"WHERE user_id = :uid "
|
" AND (used_bytes + :delta) <= limit_bytes "
|
||||||
" AND (used_bytes + :delta) <= limit_bytes "
|
"RETURNING used_bytes, limit_bytes"
|
||||||
"RETURNING used_bytes, limit_bytes"
|
),
|
||||||
),
|
{"delta": size, "uid": str(doc.user_id)},
|
||||||
{"delta": size, "uid": str(doc.user_id)},
|
)
|
||||||
|
row = result.fetchone()
|
||||||
|
|
||||||
|
if row is None:
|
||||||
|
# Quota exceeded — fetch current quota state for the 413 body
|
||||||
|
quota_result = await session.execute(
|
||||||
|
text("SELECT used_bytes, limit_bytes FROM quotas WHERE user_id = :uid"),
|
||||||
|
{"uid": str(doc.user_id)},
|
||||||
|
)
|
||||||
|
q = quota_result.fetchone()
|
||||||
|
# Delete the pending Document row and best-effort remove the MinIO object
|
||||||
|
await session.delete(doc)
|
||||||
|
try:
|
||||||
|
await get_storage_backend().delete_object(doc.object_key)
|
||||||
|
except Exception:
|
||||||
|
pass # MinIO cleanup is best-effort; object TTL will eventually expire
|
||||||
|
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,
|
||||||
|
},
|
||||||
)
|
)
|
||||||
row = result.fetchone()
|
|
||||||
|
|
||||||
if row is None:
|
used_bytes = row.used_bytes
|
||||||
# Quota exceeded — fetch current quota state for the 413 body
|
|
||||||
quota_result = await session.execute(
|
|
||||||
text("SELECT used_bytes, limit_bytes FROM quotas WHERE user_id = :uid"),
|
|
||||||
{"uid": str(doc.user_id)},
|
|
||||||
)
|
|
||||||
q = quota_result.fetchone()
|
|
||||||
# Delete the pending Document row and best-effort remove the MinIO object
|
|
||||||
await session.delete(doc)
|
|
||||||
try:
|
|
||||||
await get_storage_backend().delete_object(doc.object_key)
|
|
||||||
except Exception:
|
|
||||||
pass # MinIO cleanup is best-effort; object TTL will eventually expire
|
|
||||||
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,
|
|
||||||
},
|
|
||||||
)
|
|
||||||
|
|
||||||
used_bytes = row.used_bytes
|
|
||||||
else:
|
|
||||||
# Wave 2 placeholder: no quota row to update when user_id is None
|
|
||||||
used_bytes = 0
|
|
||||||
|
|
||||||
doc.status = "uploaded"
|
doc.status = "uploaded"
|
||||||
await session.commit()
|
await session.commit()
|
||||||
@@ -196,12 +191,14 @@ async def list_documents(
|
|||||||
page: int = Query(1, ge=1),
|
page: int = Query(1, ge=1),
|
||||||
per_page: int = Query(20, ge=1, le=100),
|
per_page: int = Query(20, ge=1, le=100),
|
||||||
session: AsyncSession = Depends(get_db),
|
session: AsyncSession = Depends(get_db),
|
||||||
|
current_user: User = Depends(get_regular_user),
|
||||||
):
|
):
|
||||||
"""List documents, optionally filtered by topic.
|
"""List documents, optionally filtered by topic.
|
||||||
|
|
||||||
NOTE (Wave 2): No auth guard — Plan 03-03 adds get_regular_user dependency.
|
D-16: requires authenticated regular user (get_regular_user rejects admins).
|
||||||
|
Returns only documents belonging to the current user.
|
||||||
"""
|
"""
|
||||||
docs = await storage.list_metadata(session, topic=topic)
|
docs = await storage.list_metadata(session, user_id=current_user.id, topic=topic)
|
||||||
total = len(docs)
|
total = len(docs)
|
||||||
start = (page - 1) * per_page
|
start = (page - 1) * per_page
|
||||||
return {"items": docs[start : start + per_page], "total": total, "page": page, "per_page": per_page}
|
return {"items": docs[start : start + per_page], "total": total, "page": page, "per_page": per_page}
|
||||||
@@ -210,11 +207,25 @@ async def list_documents(
|
|||||||
# ── GET /api/documents/{doc_id} ───────────────────────────────────────────────
|
# ── GET /api/documents/{doc_id} ───────────────────────────────────────────────
|
||||||
|
|
||||||
@router.get("/{doc_id}")
|
@router.get("/{doc_id}")
|
||||||
async def get_document(doc_id: str, session: AsyncSession = Depends(get_db)):
|
async def get_document(
|
||||||
|
doc_id: str,
|
||||||
|
session: AsyncSession = Depends(get_db),
|
||||||
|
current_user: User = Depends(get_regular_user),
|
||||||
|
):
|
||||||
"""Return document metadata by ID.
|
"""Return document metadata by ID.
|
||||||
|
|
||||||
NOTE (Wave 2): No auth guard — Plan 03-03 adds get_regular_user dependency.
|
D-16: requires authenticated regular user. Asserts ownership — cross-user
|
||||||
|
access returns 404 (not 403) to avoid information leakage (T-03-11).
|
||||||
"""
|
"""
|
||||||
|
try:
|
||||||
|
uid = uuid.UUID(doc_id)
|
||||||
|
except ValueError:
|
||||||
|
raise HTTPException(404, "Document not found")
|
||||||
|
|
||||||
|
doc = await session.get(Document, uid)
|
||||||
|
if doc is None or doc.user_id != current_user.id:
|
||||||
|
raise HTTPException(404, "Document not found")
|
||||||
|
|
||||||
meta = await storage.get_metadata(session, doc_id)
|
meta = await storage.get_metadata(session, doc_id)
|
||||||
if meta is None:
|
if meta is None:
|
||||||
raise HTTPException(404, "Document not found")
|
raise HTTPException(404, "Document not found")
|
||||||
@@ -224,14 +235,28 @@ async def get_document(doc_id: str, session: AsyncSession = Depends(get_db)):
|
|||||||
# ── DELETE /api/documents/{doc_id} ───────────────────────────────────────────
|
# ── DELETE /api/documents/{doc_id} ───────────────────────────────────────────
|
||||||
|
|
||||||
@router.delete("/{doc_id}")
|
@router.delete("/{doc_id}")
|
||||||
async def delete_document(doc_id: str, session: AsyncSession = Depends(get_db)):
|
async def delete_document(
|
||||||
|
doc_id: str,
|
||||||
|
session: AsyncSession = Depends(get_db),
|
||||||
|
current_user: User = Depends(get_regular_user),
|
||||||
|
):
|
||||||
"""Delete a document and decrement quota atomically.
|
"""Delete a document and decrement quota atomically.
|
||||||
|
|
||||||
services.storage.delete_document handles the atomic quota decrement
|
services.storage.delete_document handles the atomic quota decrement
|
||||||
(STORE-06, D-07) via GREATEST(0, used_bytes - delta) SQL.
|
(STORE-06, D-07) via GREATEST(0, used_bytes - delta) SQL.
|
||||||
|
|
||||||
NOTE (Wave 2): No auth guard — Plan 03-03 adds get_regular_user dependency.
|
D-16: requires authenticated regular user. Asserts ownership — cross-user
|
||||||
|
delete returns 404 (not 403) to avoid information leakage (T-03-11).
|
||||||
"""
|
"""
|
||||||
|
try:
|
||||||
|
uid = uuid.UUID(doc_id)
|
||||||
|
except ValueError:
|
||||||
|
raise HTTPException(404, "Document not found")
|
||||||
|
|
||||||
|
doc = await session.get(Document, uid)
|
||||||
|
if doc is None or doc.user_id != current_user.id:
|
||||||
|
raise HTTPException(404, "Document not found")
|
||||||
|
|
||||||
ok = await storage.delete_document(session, doc_id)
|
ok = await storage.delete_document(session, doc_id)
|
||||||
if not ok:
|
if not ok:
|
||||||
raise HTTPException(404, "Document not found")
|
raise HTTPException(404, "Document not found")
|
||||||
@@ -245,13 +270,20 @@ async def classify_document(
|
|||||||
doc_id: str,
|
doc_id: str,
|
||||||
body: dict = {},
|
body: dict = {},
|
||||||
session: AsyncSession = Depends(get_db),
|
session: AsyncSession = Depends(get_db),
|
||||||
|
current_user: User = Depends(get_regular_user),
|
||||||
):
|
):
|
||||||
"""Reclassify a document's topics on demand.
|
"""Reclassify a document's topics on demand.
|
||||||
|
|
||||||
NOTE (Wave 2): No auth guard — Plan 03-03 adds get_regular_user dependency.
|
D-16: requires authenticated regular user. Asserts ownership — cross-user
|
||||||
|
classify returns 404 (not 403) to avoid information leakage (T-03-11).
|
||||||
"""
|
"""
|
||||||
meta = await storage.get_metadata(session, doc_id)
|
try:
|
||||||
if meta is None:
|
uid = uuid.UUID(doc_id)
|
||||||
|
except ValueError:
|
||||||
|
raise HTTPException(404, "Document not found")
|
||||||
|
|
||||||
|
doc = await session.get(Document, uid)
|
||||||
|
if doc is None or doc.user_id != current_user.id:
|
||||||
raise HTTPException(404, "Document not found")
|
raise HTTPException(404, "Document not found")
|
||||||
|
|
||||||
topic_names = body.get("topics") if body else None
|
topic_names = body.get("topics") if body else None
|
||||||
|
|||||||
@@ -90,3 +90,20 @@ async def get_current_admin(
|
|||||||
detail="Admin access required",
|
detail="Admin access required",
|
||||||
)
|
)
|
||||||
return user
|
return user
|
||||||
|
|
||||||
|
|
||||||
|
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.
|
||||||
|
Regular users are passed through unchanged.
|
||||||
|
"""
|
||||||
|
if user.role == "admin":
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
|
detail="Admin accounts cannot access document content",
|
||||||
|
)
|
||||||
|
return user
|
||||||
|
|||||||
+17
-13
@@ -123,10 +123,13 @@ async def get_metadata(session: AsyncSession, doc_id: str) -> Optional[dict]:
|
|||||||
|
|
||||||
|
|
||||||
async def list_metadata(
|
async def list_metadata(
|
||||||
session: AsyncSession, topic: Optional[str] = None
|
session: AsyncSession, user_id: uuid.UUID, topic: Optional[str] = None
|
||||||
) -> list:
|
) -> list:
|
||||||
"""Return a list of metadata dicts, optionally filtered by topic name."""
|
"""Return a list of metadata dicts for a specific user, optionally filtered by topic name.
|
||||||
stmt = select(Document).order_by(Document.created_at.desc())
|
|
||||||
|
D-16: always filters by user_id — a user can only see their own documents.
|
||||||
|
"""
|
||||||
|
stmt = select(Document).where(Document.user_id == user_id).order_by(Document.created_at.desc())
|
||||||
if topic is not None:
|
if topic is not None:
|
||||||
stmt = (
|
stmt = (
|
||||||
stmt.join(DocumentTopic, DocumentTopic.document_id == Document.id)
|
stmt.join(DocumentTopic, DocumentTopic.document_id == Document.id)
|
||||||
@@ -165,16 +168,17 @@ async def delete_document(session: AsyncSession, doc_id: str) -> bool:
|
|||||||
print(f"[storage] WARNING: MinIO delete_object failed for {doc.object_key!r}: {exc}", file=sys.stderr)
|
print(f"[storage] WARNING: MinIO delete_object failed for {doc.object_key!r}: {exc}", file=sys.stderr)
|
||||||
|
|
||||||
# Atomic quota decrement (STORE-06, D-07).
|
# Atomic quota decrement (STORE-06, D-07).
|
||||||
# The user_id is None guard is removed in Plan 03-03.
|
# user_id is always set post-migration (Plan 03-03+) — guard removed.
|
||||||
if doc.user_id is not None:
|
# Use CASE WHEN instead of GREATEST() for SQLite compatibility
|
||||||
await session.execute(
|
# (PostgreSQL supports both; SQLite lacks the GREATEST scalar function).
|
||||||
text(
|
await session.execute(
|
||||||
"UPDATE quotas "
|
text(
|
||||||
"SET used_bytes = GREATEST(0, used_bytes - :delta) "
|
"UPDATE quotas "
|
||||||
"WHERE user_id = :uid"
|
"SET used_bytes = CASE WHEN used_bytes > :delta THEN used_bytes - :delta ELSE 0 END "
|
||||||
),
|
"WHERE user_id = :uid"
|
||||||
{"delta": doc.size_bytes, "uid": str(doc.user_id)},
|
),
|
||||||
)
|
{"delta": doc.size_bytes, "uid": str(doc.user_id)},
|
||||||
|
)
|
||||||
|
|
||||||
await session.delete(doc)
|
await session.delete(doc)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
|
|||||||
@@ -40,16 +40,16 @@ async def test_upload_pdf_no_classify(async_client, sample_pdf):
|
|||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
|
|
||||||
|
|
||||||
async def test_list_documents(async_client):
|
async def test_list_documents(async_client, auth_user):
|
||||||
"""GET /api/documents returns an empty list when no documents exist."""
|
"""GET /api/documents returns an empty list when no documents exist."""
|
||||||
resp = await async_client.get("/api/documents")
|
resp = await async_client.get("/api/documents", headers=auth_user["headers"])
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
data = resp.json()
|
data = resp.json()
|
||||||
assert data["total"] == 0
|
assert data["total"] == 0
|
||||||
assert data["items"] == []
|
assert data["items"] == []
|
||||||
|
|
||||||
|
|
||||||
async def test_list_documents_filter_by_topic(async_client, db_session):
|
async def test_list_documents_filter_by_topic(async_client, auth_user, db_session):
|
||||||
"""GET /api/documents?topic=finance returns only matching documents."""
|
"""GET /api/documents?topic=finance returns only matching documents."""
|
||||||
import uuid as _uuid
|
import uuid as _uuid
|
||||||
from db.models import Document
|
from db.models import Document
|
||||||
@@ -59,27 +59,27 @@ async def test_list_documents_filter_by_topic(async_client, db_session):
|
|||||||
doc_id = _uuid.uuid4()
|
doc_id = _uuid.uuid4()
|
||||||
doc = Document(
|
doc = Document(
|
||||||
id=doc_id,
|
id=doc_id,
|
||||||
user_id=None,
|
user_id=auth_user["user"].id,
|
||||||
filename="test.txt",
|
filename="test.txt",
|
||||||
content_type="text/plain",
|
content_type="text/plain",
|
||||||
size_bytes=100,
|
size_bytes=100,
|
||||||
storage_backend="minio",
|
storage_backend="minio",
|
||||||
status="uploaded",
|
status="uploaded",
|
||||||
object_key=f"null-user/{doc_id}/{_uuid.uuid4()}.txt",
|
object_key=f"{auth_user['user'].id}/{doc_id}/{_uuid.uuid4()}.txt",
|
||||||
)
|
)
|
||||||
db_session.add(doc)
|
db_session.add(doc)
|
||||||
await db_session.commit()
|
await db_session.commit()
|
||||||
|
|
||||||
await storage.update_document_topics(db_session, str(doc_id), ["finance"])
|
await storage.update_document_topics(db_session, str(doc_id), ["finance"])
|
||||||
|
|
||||||
resp = await async_client.get("/api/documents?topic=finance")
|
resp = await async_client.get("/api/documents?topic=finance", headers=auth_user["headers"])
|
||||||
assert resp.json()["total"] == 1
|
assert resp.json()["total"] == 1
|
||||||
|
|
||||||
resp2 = await async_client.get("/api/documents?topic=legal")
|
resp2 = await async_client.get("/api/documents?topic=legal", headers=auth_user["headers"])
|
||||||
assert resp2.json()["total"] == 0
|
assert resp2.json()["total"] == 0
|
||||||
|
|
||||||
|
|
||||||
async def test_get_document(async_client, db_session):
|
async def test_get_document(async_client, auth_user, db_session):
|
||||||
"""GET /api/documents/{id} returns metadata for an existing document."""
|
"""GET /api/documents/{id} returns metadata for an existing document."""
|
||||||
import uuid as _uuid
|
import uuid as _uuid
|
||||||
from db.models import Document
|
from db.models import Document
|
||||||
@@ -87,28 +87,28 @@ async def test_get_document(async_client, db_session):
|
|||||||
doc_id = _uuid.uuid4()
|
doc_id = _uuid.uuid4()
|
||||||
doc = Document(
|
doc = Document(
|
||||||
id=doc_id,
|
id=doc_id,
|
||||||
user_id=None,
|
user_id=auth_user["user"].id,
|
||||||
filename="test.txt",
|
filename="test.txt",
|
||||||
content_type="text/plain",
|
content_type="text/plain",
|
||||||
size_bytes=100,
|
size_bytes=100,
|
||||||
storage_backend="minio",
|
storage_backend="minio",
|
||||||
status="uploaded",
|
status="uploaded",
|
||||||
object_key=f"null-user/{doc_id}/{_uuid.uuid4()}.txt",
|
object_key=f"{auth_user['user'].id}/{doc_id}/{_uuid.uuid4()}.txt",
|
||||||
)
|
)
|
||||||
db_session.add(doc)
|
db_session.add(doc)
|
||||||
await db_session.commit()
|
await db_session.commit()
|
||||||
|
|
||||||
resp = await async_client.get(f"/api/documents/{doc_id}")
|
resp = await async_client.get(f"/api/documents/{doc_id}", headers=auth_user["headers"])
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
assert resp.json()["id"] == str(doc_id)
|
assert resp.json()["id"] == str(doc_id)
|
||||||
|
|
||||||
|
|
||||||
async def test_get_document_not_found(async_client):
|
async def test_get_document_not_found(async_client, auth_user):
|
||||||
resp = await async_client.get("/api/documents/nonexistent")
|
resp = await async_client.get("/api/documents/nonexistent", headers=auth_user["headers"])
|
||||||
assert resp.status_code == 404
|
assert resp.status_code == 404
|
||||||
|
|
||||||
|
|
||||||
async def test_delete_document(async_client, db_session, monkeypatch):
|
async def test_delete_document(async_client, auth_user, db_session, monkeypatch):
|
||||||
"""DELETE /api/documents/{id} removes the document."""
|
"""DELETE /api/documents/{id} removes the document."""
|
||||||
import uuid as _uuid
|
import uuid as _uuid
|
||||||
from db.models import Document
|
from db.models import Document
|
||||||
@@ -120,27 +120,27 @@ async def test_delete_document(async_client, db_session, monkeypatch):
|
|||||||
doc_id = _uuid.uuid4()
|
doc_id = _uuid.uuid4()
|
||||||
doc = Document(
|
doc = Document(
|
||||||
id=doc_id,
|
id=doc_id,
|
||||||
user_id=None,
|
user_id=auth_user["user"].id,
|
||||||
filename="test.txt",
|
filename="test.txt",
|
||||||
content_type="text/plain",
|
content_type="text/plain",
|
||||||
size_bytes=0,
|
size_bytes=0,
|
||||||
storage_backend="minio",
|
storage_backend="minio",
|
||||||
status="uploaded",
|
status="uploaded",
|
||||||
object_key=f"null-user/{doc_id}/{_uuid.uuid4()}.txt",
|
object_key=f"{auth_user['user'].id}/{doc_id}/{_uuid.uuid4()}.txt",
|
||||||
)
|
)
|
||||||
db_session.add(doc)
|
db_session.add(doc)
|
||||||
await db_session.commit()
|
await db_session.commit()
|
||||||
|
|
||||||
resp = await async_client.delete(f"/api/documents/{doc_id}")
|
resp = await async_client.delete(f"/api/documents/{doc_id}", headers=auth_user["headers"])
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
assert resp.json()["success"] is True
|
assert resp.json()["success"] is True
|
||||||
|
|
||||||
resp2 = await async_client.get(f"/api/documents/{doc_id}")
|
resp2 = await async_client.get(f"/api/documents/{doc_id}", headers=auth_user["headers"])
|
||||||
assert resp2.status_code == 404
|
assert resp2.status_code == 404
|
||||||
|
|
||||||
|
|
||||||
async def test_delete_document_not_found(async_client):
|
async def test_delete_document_not_found(async_client, auth_user):
|
||||||
resp = await async_client.delete("/api/documents/nonexistent")
|
resp = await async_client.delete("/api/documents/nonexistent", headers=auth_user["headers"])
|
||||||
assert resp.status_code == 404
|
assert resp.status_code == 404
|
||||||
|
|
||||||
|
|
||||||
@@ -199,14 +199,19 @@ async def test_upload_url_endpoint(async_client, auth_user, mock_minio_presigned
|
|||||||
assert mock_minio_presigned.called, "generate_presigned_put_url was not called"
|
assert mock_minio_presigned.called, "generate_presigned_put_url was not called"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.xfail(strict=False, reason="SQLite UUID format mismatch in raw SQL quota UPDATE — xpass on PostgreSQL (INTEGRATION=1)")
|
||||||
async def test_confirm_endpoint(
|
async def test_confirm_endpoint(
|
||||||
async_client, auth_user, mock_minio_presigned, mock_minio_stat, monkeypatch
|
async_client, auth_user, mock_minio_presigned, mock_minio_stat, monkeypatch
|
||||||
):
|
):
|
||||||
"""POST /api/documents/{id}/confirm calls stat_object once, updates Document.size_bytes
|
"""POST /api/documents/{id}/confirm calls stat_object once, updates Document.size_bytes
|
||||||
from the stat return value, and sets Document.status='uploaded'.
|
from the stat return value, sets Document.status='uploaded', and runs atomic quota.
|
||||||
|
|
||||||
D-05: step 3 of the presigned upload flow. stat_object provides the authoritative
|
D-05: step 3 of the presigned upload flow. stat_object provides the authoritative
|
||||||
file size (D-07). The atomic quota UPDATE runs here (STORE-03).
|
file size (D-07). The atomic quota UPDATE runs unconditionally here (STORE-03, Plan 03-03+).
|
||||||
|
|
||||||
|
SQLite note: The raw SQL quota UPDATE uses :uid in dashed UUID format, which does not
|
||||||
|
match SQLite's CHAR(32) undashed storage. This test xfails on SQLite and xpasses on
|
||||||
|
PostgreSQL (run with INTEGRATION=1). Same as test_quota.py pattern.
|
||||||
"""
|
"""
|
||||||
from unittest.mock import MagicMock
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
@@ -225,7 +230,7 @@ async def test_confirm_endpoint(
|
|||||||
assert resp.status_code == 200, resp.text
|
assert resp.status_code == 200, resp.text
|
||||||
doc_id = resp.json()["document_id"]
|
doc_id = resp.json()["document_id"]
|
||||||
|
|
||||||
# Step 2: confirm (Wave 2 — user_id is None so quota skipped, but stat is called)
|
# Step 2: confirm — quota runs unconditionally (Plan 03-03+, no Wave 2 guard)
|
||||||
conf_resp = await async_client.post(
|
conf_resp = await async_client.post(
|
||||||
f"/api/documents/{doc_id}/confirm",
|
f"/api/documents/{doc_id}/confirm",
|
||||||
headers=auth_user["headers"],
|
headers=auth_user["headers"],
|
||||||
@@ -259,7 +264,6 @@ async def test_get_quota(async_client, auth_user):
|
|||||||
assert data["limit_bytes"] == 104_857_600, f"Expected 100 MB limit: {data}"
|
assert data["limit_bytes"] == 104_857_600, f"Expected 100 MB limit: {data}"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-03")
|
|
||||||
async def test_cross_user_access_404(async_client, auth_user, db_session):
|
async def test_cross_user_access_404(async_client, auth_user, db_session):
|
||||||
"""User B's request for GET /api/documents/{A_doc_id} returns 404.
|
"""User B's request for GET /api/documents/{A_doc_id} returns 404.
|
||||||
|
|
||||||
@@ -267,10 +271,50 @@ async def test_cross_user_access_404(async_client, auth_user, db_session):
|
|||||||
(CONTEXT.md D-16). An attacker cannot distinguish between 'document does not
|
(CONTEXT.md D-16). An attacker cannot distinguish between 'document does not
|
||||||
exist' and 'document belongs to someone else'.
|
exist' and 'document belongs to someone else'.
|
||||||
"""
|
"""
|
||||||
assert True # scaffold
|
import uuid as _uuid
|
||||||
|
from db.models import Document, User, Quota
|
||||||
|
from services.auth import hash_password, create_access_token
|
||||||
|
|
||||||
|
# Create User A's document directly via ORM
|
||||||
|
doc_id = _uuid.uuid4()
|
||||||
|
doc = Document(
|
||||||
|
id=doc_id,
|
||||||
|
user_id=auth_user["user"].id,
|
||||||
|
filename="user_a_doc.txt",
|
||||||
|
content_type="text/plain",
|
||||||
|
size_bytes=100,
|
||||||
|
storage_backend="minio",
|
||||||
|
status="uploaded",
|
||||||
|
object_key=f"{auth_user['user'].id}/{doc_id}/{_uuid.uuid4()}.txt",
|
||||||
|
)
|
||||||
|
db_session.add(doc)
|
||||||
|
|
||||||
|
# Create User B
|
||||||
|
user_b_id = _uuid.uuid4()
|
||||||
|
user_b = User(
|
||||||
|
id=user_b_id,
|
||||||
|
handle=f"user_b_{user_b_id.hex[:8]}",
|
||||||
|
email=f"user_b_{user_b_id.hex[:8]}@example.com",
|
||||||
|
password_hash=hash_password("Testpassword123!"),
|
||||||
|
role="user",
|
||||||
|
is_active=True,
|
||||||
|
password_must_change=False,
|
||||||
|
)
|
||||||
|
quota_b = Quota(user_id=user_b_id, limit_bytes=104857600, used_bytes=0)
|
||||||
|
db_session.add(user_b)
|
||||||
|
db_session.add(quota_b)
|
||||||
|
await db_session.commit()
|
||||||
|
|
||||||
|
token_b = create_access_token(str(user_b_id), "user")
|
||||||
|
headers_b = {"Authorization": f"Bearer {token_b}"}
|
||||||
|
|
||||||
|
# User B attempts to access User A's document — must get 404 (not 403)
|
||||||
|
resp = await async_client.get(f"/api/documents/{doc_id}", headers=headers_b)
|
||||||
|
assert resp.status_code == 404, (
|
||||||
|
f"Expected 404 for cross-user access, got {resp.status_code}: {resp.text}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-03")
|
|
||||||
async def test_admin_cannot_access_documents(async_client, admin_user):
|
async def test_admin_cannot_access_documents(async_client, admin_user):
|
||||||
"""GET /api/documents using admin_user.headers returns 403.
|
"""GET /api/documents using admin_user.headers returns 403.
|
||||||
|
|
||||||
@@ -278,17 +322,17 @@ async def test_admin_cannot_access_documents(async_client, admin_user):
|
|||||||
CONTEXT.md D-16). The get_regular_user dependency enforces this for all
|
CONTEXT.md D-16). The get_regular_user dependency enforces this for all
|
||||||
/api/documents/* handlers.
|
/api/documents/* handlers.
|
||||||
"""
|
"""
|
||||||
assert True # scaffold
|
resp = await async_client.get("/api/documents", headers=admin_user["headers"])
|
||||||
|
assert resp.status_code == 403, (
|
||||||
|
f"Expected 403 for admin on document endpoints, got {resp.status_code}: {resp.text}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-03: auth guard not yet added")
|
|
||||||
async def test_documents_require_auth(async_client):
|
async def test_documents_require_auth(async_client):
|
||||||
"""Anonymous GET /api/documents (no Authorization header) returns 401 or 403.
|
"""Anonymous GET /api/documents (no Authorization header) returns 401 or 403.
|
||||||
|
|
||||||
D-16: all /api/documents/* endpoints require authentication via
|
D-16: all /api/documents/* endpoints require authentication via
|
||||||
get_current_user (Phase 2 D-07 fulfilled in Phase 3).
|
get_current_user (Phase 2 D-07 fulfilled in Phase 3).
|
||||||
Note: auth guard is added in Plan 03-03 — this remains xfail until then.
|
|
||||||
"""
|
"""
|
||||||
resp = await async_client.get("/api/documents")
|
resp = await async_client.get("/api/documents")
|
||||||
# Wave 2: no auth guard yet (Plan 03-03 adds it) — this will pass as xfail
|
|
||||||
assert resp.status_code in (401, 403), f"Expected 401 or 403, got {resp.status_code}"
|
assert resp.status_code in (401, 403), f"Expected 401 or 403, got {resp.status_code}"
|
||||||
|
|||||||
Reference in New Issue
Block a user