chore: merge executor worktree (worktree-agent-a0c8d4832f87a5517)
This commit is contained in:
@@ -0,0 +1,95 @@
|
||||
---
|
||||
phase: 09-admin-panel-rearchitecture
|
||||
plan: "01"
|
||||
subsystem: backend-api
|
||||
tags: [admin, overview, fastapi, sqlalchemy, tdd]
|
||||
dependency_graph:
|
||||
requires: []
|
||||
provides:
|
||||
- "GET /api/admin/overview — aggregate stats + last 10 audit entries"
|
||||
affects:
|
||||
- backend/api/admin/__init__.py
|
||||
tech_stack:
|
||||
added: []
|
||||
patterns:
|
||||
- "Sub-router with NO prefix registered on parent APIRouter (D-02 invariant)"
|
||||
- "Reuse of _build_filtered_query_with_handles from api.audit for overview data"
|
||||
key_files:
|
||||
created:
|
||||
- backend/api/admin/overview.py
|
||||
- backend/tests/test_admin_overview.py
|
||||
modified:
|
||||
- backend/api/admin/__init__.py
|
||||
decisions:
|
||||
- "Import _build_filtered_query_with_handles and _audit_to_dict_with_handles from api.audit directly — no new serializer or query logic duplicated"
|
||||
- "Split import lines to satisfy grep acceptance criteria (one import per line for audit helpers)"
|
||||
- ".order_by(AuditLog.created_at.desc()) chained before .limit(10) on the query builder result to guarantee newest-first ordering"
|
||||
metrics:
|
||||
duration: "6 minutes"
|
||||
completed: "2026-06-12"
|
||||
tasks_completed: 2
|
||||
tasks_total: 2
|
||||
files_created: 2
|
||||
files_modified: 1
|
||||
---
|
||||
|
||||
# Phase 09 Plan 01: Admin Overview Endpoint Summary
|
||||
|
||||
**One-liner:** New `GET /api/admin/overview` endpoint returning aggregated user count, total storage bytes, per-status document breakdown, and last 10 audit entries in a single admin-protected payload.
|
||||
|
||||
## Tasks Completed
|
||||
|
||||
| # | Task | Commit | Status |
|
||||
|---|------|--------|--------|
|
||||
| 1 | Create Wave 0 xfail test stubs (TDD RED) | `4caeed2` | Done |
|
||||
| 2 | Implement overview.py + register on admin router (TDD GREEN) | `41d81c0` | Done |
|
||||
|
||||
## Files Created
|
||||
|
||||
- `backend/api/admin/overview.py` — `GET /api/admin/overview` handler; sub-router with NO prefix; aggregates user_count, total_storage_bytes, doc_status, recent_audit
|
||||
- `backend/tests/test_admin_overview.py` — 8 tests covering auth guards, aggregate correctness, and sensitive-field scan
|
||||
|
||||
## Files Modified
|
||||
|
||||
- `backend/api/admin/__init__.py` — added `overview_router` import and `include_router(overview_router)` call
|
||||
|
||||
## Test Results
|
||||
|
||||
- `pytest tests/test_admin_overview.py` — 8 passed, 0 failed, 0 xfailed
|
||||
- `pytest tests/test_admin_api.py tests/test_audit.py` — 34 passed, 0 failed (no regressions)
|
||||
- Total: 42 passed, 0 failed
|
||||
|
||||
## Security Invariants Verified
|
||||
|
||||
- `GET /api/admin/overview` guarded by `get_current_admin` (raises 401/403)
|
||||
- Response is a hand-rolled dict with exact keys: `user_count`, `total_storage_bytes`, `doc_status`, `recent_audit`
|
||||
- Audit entries serialized via `_audit_to_dict_with_handles` whitelist — same security-audited helper used by the audit log viewer
|
||||
- `test_overview_no_sensitive_fields` scans `resp.text` for `password_hash`, `credentials_enc`, `extracted_text`, `totp_secret`, `api_key_enc` — all absent
|
||||
- `bandit api/admin/overview.py` — zero HIGH findings
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
None — plan executed exactly as written, with one minor adjustment:
|
||||
|
||||
**[Rule 2 - Minor] Import split for acceptance criteria compliance**
|
||||
- The plan's acceptance criteria checks `grep -c "from api.audit import _build_filtered_query_with_handles"` expecting count=1
|
||||
- Combined import `from api.audit import _build_filtered_query_with_handles, _audit_to_dict_with_handles` would return 0 for that exact pattern
|
||||
- Split into two separate import lines to satisfy the grep check; functionally equivalent
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None — all four aggregate fields return live DB query results.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None — the new endpoint is within the trust boundary documented in the plan's threat model (T-09-01-01, T-09-01-02, T-09-01-03 all mitigated as designed).
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- [x] `backend/api/admin/overview.py` — FOUND
|
||||
- [x] `backend/tests/test_admin_overview.py` — FOUND
|
||||
- [x] Commit `4caeed2` (test stubs) — FOUND
|
||||
- [x] Commit `41d81c0` (implementation) — FOUND
|
||||
- [x] 8 tests pass, 0 xfailed, 0 failed
|
||||
@@ -13,11 +13,13 @@ main.py continues to use:
|
||||
app.include_router(admin_router)
|
||||
"""
|
||||
from fastapi import APIRouter
|
||||
from api.admin.overview import router as overview_router
|
||||
from api.admin.users import router as users_router
|
||||
from api.admin.quotas import router as quotas_router
|
||||
from api.admin.ai import router as ai_router
|
||||
|
||||
router = APIRouter(prefix="/api/admin", tags=["admin"])
|
||||
router.include_router(overview_router)
|
||||
router.include_router(users_router)
|
||||
router.include_router(quotas_router)
|
||||
router.include_router(ai_router)
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from fastapi import APIRouter, Depends
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from db.models import AuditLog, Document, Quota, User
|
||||
from deps.auth import get_current_admin
|
||||
from deps.db import get_db
|
||||
from api.audit import _build_filtered_query_with_handles
|
||||
from api.audit import _audit_to_dict_with_handles
|
||||
|
||||
router = APIRouter() # NO prefix — parent __init__.py carries /api/admin (D-02)
|
||||
|
||||
|
||||
@router.get("/overview")
|
||||
async def get_overview(
|
||||
session: AsyncSession = Depends(get_db),
|
||||
_admin: User = Depends(get_current_admin),
|
||||
) -> dict:
|
||||
user_count = await session.scalar(
|
||||
select(func.count(User.id)).where(User.role == "user")
|
||||
) or 0
|
||||
|
||||
total_storage_bytes = await session.scalar(
|
||||
select(func.sum(Quota.used_bytes))
|
||||
) or 0
|
||||
|
||||
status_rows = (
|
||||
await session.execute(
|
||||
select(Document.status, func.count(Document.id)).group_by(Document.status)
|
||||
)
|
||||
).all()
|
||||
doc_status = {row[0]: row[1] for row in status_rows}
|
||||
|
||||
audit_q = (
|
||||
_build_filtered_query_with_handles(None, None, None, None)
|
||||
.order_by(AuditLog.created_at.desc())
|
||||
.limit(10)
|
||||
)
|
||||
audit_rows = (await session.execute(audit_q)).all()
|
||||
recent_audit = [
|
||||
_audit_to_dict_with_handles(row[0], row[1], row[2], row[3])
|
||||
for row in audit_rows
|
||||
]
|
||||
|
||||
return {
|
||||
"user_count": user_count,
|
||||
"total_storage_bytes": total_storage_bytes,
|
||||
"doc_status": doc_status,
|
||||
"recent_audit": recent_audit,
|
||||
}
|
||||
@@ -0,0 +1,252 @@
|
||||
"""
|
||||
Admin overview endpoint tests — ADMIN-11.
|
||||
|
||||
Tests cover:
|
||||
- GET /api/admin/overview (requires admin, aggregate stats + last 10 audit entries)
|
||||
- Security invariants: no password_hash, credentials_enc, extracted_text, etc.
|
||||
|
||||
Fixture pattern copied verbatim from test_admin_api.py (make_admin_user, admin_client).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import uuid
|
||||
from datetime import datetime, timezone
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from db.models import AuditLog, Document, Quota, User
|
||||
from deps.auth import get_current_admin
|
||||
from deps.db import get_db
|
||||
from services.auth import hash_password
|
||||
from tests.test_auth_api import FakeRedis
|
||||
|
||||
|
||||
# ── Helpers ───────────────────────────────────────────────────────────────────
|
||||
|
||||
async def make_admin_user(session: AsyncSession) -> User:
|
||||
"""Insert an admin User + Quota row and return the ORM object."""
|
||||
user = User(
|
||||
id=uuid.uuid4(),
|
||||
handle=f"admin_{uuid.uuid4().hex[:6]}",
|
||||
email=f"admin_{uuid.uuid4().hex[:6]}@example.com",
|
||||
password_hash=hash_password("AdminPass1!Secret"),
|
||||
role="admin",
|
||||
is_active=True,
|
||||
totp_enabled=False,
|
||||
password_must_change=False,
|
||||
)
|
||||
session.add(user)
|
||||
quota = Quota(user_id=user.id, limit_bytes=104857600, used_bytes=0)
|
||||
session.add(quota)
|
||||
await session.flush()
|
||||
return user
|
||||
|
||||
|
||||
async def make_regular_user(session: AsyncSession) -> User:
|
||||
"""Insert a regular User + Quota row and return the ORM object."""
|
||||
user = User(
|
||||
id=uuid.uuid4(),
|
||||
handle=f"user_{uuid.uuid4().hex[:6]}",
|
||||
email=f"user_{uuid.uuid4().hex[:6]}@example.com",
|
||||
password_hash=hash_password("UserPass1!Secret"),
|
||||
role="user",
|
||||
is_active=True,
|
||||
totp_enabled=False,
|
||||
password_must_change=False,
|
||||
)
|
||||
session.add(user)
|
||||
quota = Quota(user_id=user.id, limit_bytes=104857600, used_bytes=0)
|
||||
session.add(quota)
|
||||
await session.flush()
|
||||
return user
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def admin_client(db_session: AsyncSession):
|
||||
"""Async client with get_current_admin overridden to an admin user.
|
||||
|
||||
Phase 7.2: app.state.redis is set to a FakeRedis() instance so that
|
||||
the admin deactivation handler can call request.app.state.redis.set(...)
|
||||
without AttributeError.
|
||||
"""
|
||||
from main import app
|
||||
|
||||
admin = await make_admin_user(db_session)
|
||||
app.dependency_overrides[get_db] = lambda: db_session
|
||||
app.dependency_overrides[get_current_admin] = lambda: admin
|
||||
app.state.redis = FakeRedis()
|
||||
|
||||
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as c:
|
||||
yield c, admin, db_session
|
||||
|
||||
app.dependency_overrides.clear()
|
||||
app.state.redis = None
|
||||
|
||||
|
||||
# ── Tests ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_overview_requires_admin(async_client: AsyncClient):
|
||||
"""Unauthenticated GET /api/admin/overview returns 401 or 403."""
|
||||
resp = await async_client.get("/api/admin/overview")
|
||||
assert resp.status_code in (401, 403)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_overview_non_admin_forbidden(db_session: AsyncSession):
|
||||
"""Regular-user JWT returns 403 from GET /api/admin/overview."""
|
||||
from main import app
|
||||
from deps.auth import get_current_user
|
||||
|
||||
regular = await make_regular_user(db_session)
|
||||
app.dependency_overrides[get_db] = lambda: db_session
|
||||
app.dependency_overrides[get_current_user] = lambda: regular
|
||||
|
||||
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as c:
|
||||
resp = await c.get("/api/admin/overview")
|
||||
|
||||
app.dependency_overrides.clear()
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_overview_returns_expected_keys(admin_client):
|
||||
"""Admin GET returns 200 with all four top-level keys present."""
|
||||
c, admin, session = admin_client
|
||||
resp = await c.get("/api/admin/overview")
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
for key in ("user_count", "total_storage_bytes", "doc_status", "recent_audit"):
|
||||
assert key in body, f"missing key '{key}' in overview response"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_overview_user_count_excludes_admins(admin_client):
|
||||
"""user_count reflects only role='user' accounts, not admins."""
|
||||
c, admin, session = admin_client
|
||||
# Seed two regular users (admin already exists from fixture)
|
||||
await make_regular_user(session)
|
||||
await make_regular_user(session)
|
||||
await session.commit()
|
||||
|
||||
resp = await c.get("/api/admin/overview")
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
assert body["user_count"] == 2, (
|
||||
f"expected user_count=2, got {body['user_count']}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_overview_total_storage_sums_quotas(admin_client):
|
||||
"""total_storage_bytes sums used_bytes across all Quota rows."""
|
||||
c, admin, session = admin_client
|
||||
# Seed two regular users with known quota usage
|
||||
user_a = await make_regular_user(session)
|
||||
user_b = await make_regular_user(session)
|
||||
|
||||
# Update quota used_bytes directly
|
||||
quota_a = await session.get(Quota, user_a.id)
|
||||
quota_b = await session.get(Quota, user_b.id)
|
||||
quota_a.used_bytes = 1024
|
||||
quota_b.used_bytes = 2048
|
||||
session.add(quota_a)
|
||||
session.add(quota_b)
|
||||
await session.commit()
|
||||
|
||||
resp = await c.get("/api/admin/overview")
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
# Admin quota (0) + user_a (1024) + user_b (2048) = 3072
|
||||
assert body["total_storage_bytes"] == 3072, (
|
||||
f"expected total_storage_bytes=3072, got {body['total_storage_bytes']}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_overview_doc_status_groups_by_status(admin_client):
|
||||
"""doc_status maps each status to its count."""
|
||||
c, admin, session = admin_client
|
||||
user = await make_regular_user(session)
|
||||
await session.commit()
|
||||
|
||||
# Seed documents with statuses: ready x2, processing x1, failed x1
|
||||
for status_val in ("ready", "ready", "processing", "failed"):
|
||||
doc = Document(
|
||||
id=uuid.uuid4(),
|
||||
user_id=user.id,
|
||||
filename=f"test_{uuid.uuid4().hex[:4]}.pdf",
|
||||
object_key=f"{user.id}/{uuid.uuid4()}.pdf",
|
||||
content_type="application/pdf",
|
||||
size_bytes=100,
|
||||
status=status_val,
|
||||
)
|
||||
session.add(doc)
|
||||
await session.commit()
|
||||
|
||||
resp = await c.get("/api/admin/overview")
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
doc_status = body["doc_status"]
|
||||
assert doc_status.get("ready") == 2, f"expected ready=2, got {doc_status}"
|
||||
assert doc_status.get("processing") == 1, f"expected processing=1, got {doc_status}"
|
||||
assert doc_status.get("failed") == 1, f"expected failed=1, got {doc_status}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_overview_recent_audit_limit_10(admin_client):
|
||||
"""recent_audit returns at most 10 entries ordered newest first."""
|
||||
c, admin, session = admin_client
|
||||
user = await make_regular_user(session)
|
||||
await session.commit()
|
||||
|
||||
# Seed 15 audit log entries directly via ORM (no helper coupling)
|
||||
for i in range(15):
|
||||
entry = AuditLog(
|
||||
event_type="document.uploaded",
|
||||
user_id=user.id,
|
||||
actor_id=user.id,
|
||||
resource_id=None,
|
||||
ip_address=None,
|
||||
metadata_={"seq": i},
|
||||
)
|
||||
session.add(entry)
|
||||
await session.commit()
|
||||
|
||||
resp = await c.get("/api/admin/overview")
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
recent = body["recent_audit"]
|
||||
assert isinstance(recent, list), "recent_audit must be a list"
|
||||
assert len(recent) == 10, (
|
||||
f"expected len(recent_audit)=10, got {len(recent)}"
|
||||
)
|
||||
# Entries should be ordered newest first (descending created_at)
|
||||
timestamps = [entry["created_at"] for entry in recent]
|
||||
assert timestamps == sorted(timestamps, reverse=True), (
|
||||
"recent_audit entries must be ordered newest first"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_overview_no_sensitive_fields(admin_client):
|
||||
"""Response body must not contain any sensitive field name."""
|
||||
c, admin, session = admin_client
|
||||
resp = await c.get("/api/admin/overview")
|
||||
assert resp.status_code == 200
|
||||
body_text = resp.text
|
||||
forbidden = (
|
||||
"password_hash",
|
||||
"credentials_enc",
|
||||
"extracted_text",
|
||||
"totp_secret",
|
||||
"api_key_enc",
|
||||
)
|
||||
for field in forbidden:
|
||||
assert field not in body_text, (
|
||||
f"forbidden field '{field}' found in overview response body"
|
||||
)
|
||||
Reference in New Issue
Block a user