--- phase: 02-users-authentication plan: 04 subsystem: auth tags: [fastapi, admin, user-management, quota, ai-config, tdd, security, role-based-access] # Dependency graph requires: - phase: 02-users-authentication plan: 02 provides: "backend/api/auth.py (auth router, limiter), deps/auth.py (get_current_admin), services/auth.py (hash_password, revoke_all_refresh_tokens, create_password_reset_token)" - phase: 02-users-authentication plan: 03 provides: "backend/config.py: frontend_url setting for reset link construction, tasks/email_tasks.py: send_reset_email.delay pattern" provides: - "backend/api/admin.py: GET /api/admin/users, POST /api/admin/users, PATCH /api/admin/users/{id}/status, POST /api/admin/users/{id}/password-reset, GET /api/admin/users/{id}/quota, PATCH /api/admin/users/{id}/quota, PATCH /api/admin/users/{id}/ai-config" - "backend/tests/test_admin_api.py: 18 tests covering all admin endpoints (role enforcement, password_must_change, no impersonation, quota warning, AI config)" affects: [02-05, 03-user-document-isolation, 04-folders-sharing-quotas] # Tech tracking tech-stack: added: [] # No new packages — all dependencies established in Plans 01/02 patterns: - "get_current_admin Depends() on every handler — no handler uses get_current_user alone (SEC-07)" - "_user_to_dict() whitelist helper — explicitly enumerates safe fields; password_hash/credentials_enc excluded by construction" - "password_must_change=True on admin-created users — forced password change on first login (ADMIN-01)" - "Sole-admin deactivation guard — counts active admins before applying deactivation; raises 400 if count would reach 0 (T-02-29)" - "Celery email dispatch with deferred import — send_reset_email.delay inside handler body to avoid circular imports" - "Quota warning — 200 OK with warning=True when new limit_bytes < used_bytes; change still applies" - "field_validator on UserCreate.password and QuotaUpdate.limit_bytes — Pydantic v2 style; raises 422 on invalid input" key-files: created: - "backend/api/admin.py — 7 admin endpoints with get_current_admin on every handler" - "backend/tests/test_admin_api.py — 18 tests: list/create/deactivate/reset/quota/AI-config/no-impersonation/no-password_hash" modified: - "backend/main.py — added admin_router include after auth_router" key-decisions: - "ADMIN-07 enforced by omission: no impersonation route exists; verified by AST check and test_admin_impersonation_not_found" - "password_hash only appears in admin.py at line 186 (User constructor for DB write, not response); _user_to_dict() whitelist prevents any leakage" - "Celery mock in test_password_reset_initiates_email to avoid Redis connection in unit tests — patch targets tasks.email_tasks.send_reset_email.delay" - "Sole-admin guard uses COUNT query on (role='admin', is_active=True) — prevents lockout scenario (T-02-29)" patterns-established: - "Admin router pattern: APIRouter(prefix='/api/admin') + get_current_admin Depends() on every handler" - "Safe response helper pattern: _user_to_dict() whitelist — use for any multi-field User response" - "Quota warning pattern: return 200 with warning=True field rather than error — change is applied, caller informed" requirements-completed: [ADMIN-01, ADMIN-02, ADMIN-03, ADMIN-04, ADMIN-05, ADMIN-07, SEC-07] # Metrics duration: ~8min completed: 2026-05-22 --- # Phase 2 Plan 04: Admin Backend API Summary **7-endpoint admin API (user CRUD, quota management, AI provider assignment) with get_current_admin on every handler, _user_to_dict whitelist, password_must_change=True on create, and no impersonation endpoint** ## Performance - **Duration:** ~8 min - **Started:** 2026-05-22T17:58:00Z - **Completed:** 2026-05-22T18:06:00Z - **Tasks:** 2 (both TDD — Task 1: admin.py, Task 2: test file) - **Files created:** 2, Files modified: 1 ## Accomplishments - All 7 admin endpoints implemented in `backend/api/admin.py` with `get_current_admin` on every handler (SEC-07) - Admin-created users have `password_must_change=True` — forced password change on first login (ADMIN-01, T-02-32) - `_user_to_dict()` whitelist helper explicitly enumerates safe response fields; `password_hash` and `credentials_enc` are excluded by construction (T-02-27) - Sole-admin deactivation guard: raises 400 if deactivating the only active admin (T-02-29) - Password reset via Celery email dispatch — does not return a token, does not impersonate (T-02-30, ADMIN-07) - Quota warning: 200 OK + `warning=True` when new `limit_bytes` < `used_bytes`; change is applied (ADMIN-04) - 18 tests passing: role enforcement, create/deactivate/reset/quota/AI-config, no impersonation route, no password_hash in responses - No impersonation endpoint — ADMIN-07 verified by AST check and dedicated test ## Task Commits 1. **Task 1 RED — test file:** `cbad9ac` (test: RED phase admin tests) 2. **Task 1 GREEN — admin.py + main.py:** `f94e8d8` (feat: admin API endpoints) ## Files Created/Modified - `backend/api/admin.py` — 7 admin endpoints; `_user_to_dict()` helper; `_validate_password_strength()`; no impersonation code - `backend/tests/test_admin_api.py` — 18 tests: list/create/deactivate/reset/quota/AI-config, sole-admin guard, impersonation 404, password_hash absent from responses - `backend/main.py` — added `from api.admin import router as admin_router` + `app.include_router(admin_router)` ## Decisions Made - **ADMIN-07 by omission**: No `/impersonate` or `/login-as` route. Verified by AST walk checking for `impersonate`/`login_as` string in dump — returns zero matches. `test_admin_impersonation_not_found` asserts 404/422 for `/api/admin/users/impersonate` - **`password_hash` appears 3 times in admin.py** — lines 55 and 141 are doc comments; line 186 is `password_hash=hash_password(body.password)` in the User constructor (DB write, not API response). `_user_to_dict()` never includes `password_hash` in its whitelist - **Celery mock in tests**: `patch("tasks.email_tasks.send_reset_email.delay")` used in `test_password_reset_initiates_email` to avoid Redis connection attempt in unit tests — same mock-target pattern as Plan 03 deviation fix - **Quota warning is 200 not 4xx**: The plan spec says "still apply the update" and return `warning=True`. This is correct — it's an advisory, not a rejection ## Deviations from Plan ### Auto-fixed Issues **1. [Rule 1 - Test Fix] Celery task causes Redis connection attempt in unit test** - **Found during:** Task 1 (GREEN phase — first test run) - **Issue:** `test_password_reset_initiates_email` called the real `send_reset_email.delay()` which triggered Celery trying to connect to Redis (not available in unit tests), causing a RuntimeError after 20 retry attempts - **Fix:** Added `unittest.mock.patch("tasks.email_tasks.send_reset_email.delay")` context manager in the test; also added assertions that `mock_delay.call_args[0][0] == target.email` (correct address) and `"token=" in call_args[1]` (reset link contains token) - **Files modified:** `backend/tests/test_admin_api.py` - **Verification:** All 18 tests pass in 1.5s without Redis - **Committed in:** `f94e8d8` (GREEN phase commit) --- **Total deviations:** 1 auto-fixed (Rule 1 — test Celery mock) **Impact on plan:** Single test-level fix — identical to the Celery mock fix pattern established in Plan 03. No implementation changes. No scope creep. ## Issues Encountered None beyond the Celery mock deviation above. All implementation proceeded cleanly. ## Known Stubs None — all 7 endpoints are fully implemented and returning correct data. ## Threat Flags All STRIDE mitigations from the plan's threat model are implemented: - T-02-26: `get_current_admin` Depends() on every handler — grep count = 10 (7 handlers + 3 in helper pattern references) - T-02-27: `_user_to_dict()` whitelist — never includes `password_hash`, `credentials_enc`, or document content - T-02-28: No impersonation endpoint — AST check + `test_admin_impersonation_not_found` asserts 404/422 - T-02-29: Sole-admin guard — COUNT query before deactivation; `test_cannot_deactivate_only_admin` asserts 400 - T-02-30: Password reset endpoint returns 202 + message, not a token; `send_reset_email.delay` goes to user's inbox - T-02-31: Quota endpoint accepted as admin-visible operational data — no PII, no document content - T-02-32: `password_must_change=True` set in `POST /api/admin/users`; `test_create_user_sets_password_must_change` verifies DB state No new threat surface beyond what was planned. ## Next Phase Readiness - All 7 admin endpoints operational with correct auth enforcement - Plan 02-05 (admin panel frontend) can wire to real endpoints - `password_must_change` flag respected by login flow (Plan 02) — admin-created users are forced to set new password on first login - All 53 auth + admin tests passing --- ## Self-Check: PASSED **Files verified:** - `backend/api/admin.py` — FOUND, contains `/api/admin/users` route - `backend/tests/test_admin_api.py` — FOUND (18 tests passing) - `backend/main.py` — FOUND, contains `admin_router` **Commits verified:** - `cbad9ac` (test: RED phase) — verified in git log - `f94e8d8` (feat: admin API) — verified in git log **Test results:** 18 passed (test_admin_api.py), 53 passed (all auth + admin tests combined) **grep -c get_current_admin backend/api/admin.py:** 10 (at least 7 required) **grep -c password_must_change backend/api/admin.py:** 4 (at least 1 required) **grep -c impersonate backend/api/admin.py:** 0 (required) --- *Phase: 02-users-authentication* *Completed: 2026-05-22*