Files
familysync/.planning/phases/10-admin-role-settings/10-02-SUMMARY.md
T

7.1 KiB

phase, plan, subsystem, tags, dependency_graph, tech_stack, key_files, decisions, metrics
phase plan subsystem tags dependency_graph tech_stack key_files decisions metrics
10-admin-role-settings 02 api-auth
requireAdmin
admin-role
middleware
upsertUser
first-login-wins
me-api
tdd
requires provides affects
users.is_admin column (10-01)
member_credentials table with UNIQUE(user_id) (10-01)
app_config table (10-01)
requireAdmin MiddlewareHandler (DB-backed role enforcement, T-10-04/T-10-05)
first-login-wins is_admin bootstrap in upsertUser (D-01)
isAdmin + needsProviderSetup on /api/me response (D-03)
Phase 10 Plan 03 (adminRouter mounts requireAdmin)
Phase 10 Plan 04 (PWA nav gating reads isAdmin from /api/me)
Phase 12 (first-login-wins hook point documented for setup_complete tightening)
added patterns
MiddlewareHandler inline export (requireAdmin pattern, not factory function)
sql<number> COUNT(*) with .limit(1) for scalar aggregate in Drizzle
resolveAdminAndSetupStatus helper — two sequential DB selects in a route
TDD RED→GREEN: 6 RED commits → 3 GREEN commits
created modified
apps/api/src/lib/requireAdmin.ts
apps/api/tests/lib/requireAdmin.test.ts
apps/api/src/auth/user.ts
apps/api/tests/auth/user.test.ts
apps/api/src/routes/me.ts
apps/api/tests/routes/me.test.ts
sql<number> COUNT(*) with .limit(1) — not .limit() on Drizzle aggregate; scalar aggregate needs explicit limit for mock-chain compatibility and Drizzle's select-where pattern
resolveAdminAndSetupStatus extracted as a shared helper in me.ts — used by both bypass and OIDC paths to avoid duplication
requireAdmin is an inline MiddlewareHandler constant, not a factory function — applied as adminRouter.use('*', requireAdmin)
duration_seconds completed_date tasks_completed files_modified
700 2026-06-13 3 6

Phase 10 Plan 02: Admin Role Primitives Summary

One-liner: DB-backed requireAdmin MiddlewareHandler, first-login-wins is_admin bootstrap in upsertUser, and /api/me extended with isAdmin + needsProviderSetup — all TDD-verified with 22 tests.

Tasks Completed

Task Name Commits Files
1 requireAdmin middleware (RED→GREEN) 9217930 (RED), f9c70ab (GREEN) requireAdmin.ts, requireAdmin.test.ts
2 First-login-wins is_admin bootstrap in upsertUser (RED→GREEN) 9e1507f (RED), 72e0140 (GREEN) user.ts, user.test.ts
3 Extend /api/me with isAdmin + needsProviderSetup (RED→GREEN) e5889df (RED), 1adff61 (GREEN) me.ts, me.test.ts

What Was Built

Task 1: requireAdmin middleware

apps/api/src/lib/requireAdmin.ts exports requireAdmin: MiddlewareHandler:

  • Reads c.get('user')?.id; if no id → 403 { error: 'Forbidden' } immediately (no DB query)
  • Queries db.select({ isAdmin: users.isAdmin }).from(users).where(eq(users.id, userId)).limit(1)
  • If !row?.isAdmin → 403; else await next()
  • Side-effect import of ../auth/devBypass.js carries the ContextVariableMap augmentation
  • Never reads isAdmin from the context user object — DB is the sole authority (T-10-04)
  • The dev-auth bypass skips OIDC; requireAdmin still hits the DB for every request (T-10-05)
  • No console.log of user object or credentials (T-10-07)

4 test cases covering: non-admin DB row → 403, admin DB row → next(), no user → 403 (no DB call), spoofed isAdmin: true on context but non-admin DB row → 403.

Task 2: First-login-wins is_admin bootstrap in upsertUser

apps/api/src/auth/user.ts extended before the INSERT block:

  • Added import { sql } from 'drizzle-orm'
  • Zero-admin COUNT check: db.select({ count: sql<number>\COUNT(*)` }).from(users).where(eq(users.isAdmin, true)).limit(1)`
  • shouldBeAdmin = Number(count) === 0
  • INSERT .values({ ..., isAdmin: shouldBeAdmin }) — first user when zero admins → is_admin=true; subsequent users → is_admin=false
  • Existing-user early-return path unchanged (no is_admin modification on re-upsert)
  • Phase-12 hook comment: "Phase 12 tightens to: first user after app_config.setup_complete"

3 new test cases + existing tests updated for the new 4-select call sequence (identity lookup → used-colors → admin COUNT → re-fetch).

Task 3: /api/me extended with isAdmin + needsProviderSetup

apps/api/src/routes/me.ts extended with:

  • resolveAdminAndSetupStatus(userId) helper — two DB selects:
    1. users.isAdmin via db.select({ isAdmin: users.isAdmin }).from(users).where(eq(users.id, userId)).limit(1)
    2. memberCredentials.id via db.select({ id: memberCredentials.id }).from(memberCredentials).where(eq(memberCredentials.userId, userId)).limit(1)
    • Returns { isAdmin: row?.isAdmin ?? false, needsProviderSetup: !cred }
  • Dev-bypass path: now calls resolveAdminAndSetupStatus(devUser.id) — DB-backed, not hardcoded (T-10-05)
  • OIDC path: calls resolveAdminAndSetupStatus(user.id) after upsertUser
  • Response: { user: { id, displayName, color, isAdmin, needsProviderSetup } } on both paths
  • No /api/me/credential POST added (Plan 03)

3 new test cases: isAdmin from DB (not hardcoded), needsProviderSetup=true (no cred), needsProviderSetup=false (cred exists).

Deviations from Plan

Auto-fixed Issues

1. [Rule 1 - Bug] Drizzle aggregate mock chaining — added .limit(1) to COUNT query

  • Found during: Task 2 (GREEN phase)
  • Issue: The COUNT query const [{ count }] = await db.select({...}).from(users).where(...) was awaiting the .where() return directly. In mocked tests, makeSelectChain.where() returns the chain object (not a Promise), so destructuring [{ count }] failed with "is not iterable".
  • Fix: Added .limit(1) to the COUNT query, making it terminate at .limit() which returns a Promise in the mock (consistent with all other select patterns in this codebase).
  • Files modified: apps/api/src/auth/user.ts (.limit(1) on COUNT query)
  • Commit: 72e0140

Known Stubs

None. This plan is API-only (no UI components). All DB queries are real and fully implemented.

Threat Flags

None new beyond the plan's threat model. All T-10-04/T-10-05/T-10-06/T-10-07 mitigations implemented:

  • T-10-04: requireAdmin reads users.is_admin from DB, never trusts context user's isAdmin
  • T-10-05: Both requireAdmin and /api/me do DB lookups even on the dev-bypass path
  • T-10-06: isAdmin on /api/me is documented UX-only; Plan 03's requireAdmin is the server boundary
  • T-10-07: No console.log of user object or credentials in any modified file

Self-Check: PASSED

  • apps/api/src/lib/requireAdmin.ts exists and exports requireAdmin: PASS
  • grep -q "users.isAdmin" apps/api/src/lib/requireAdmin.ts: PASS
  • grep -q "isAdmin: shouldBeAdmin" apps/api/src/auth/user.ts: PASS
  • grep -q "needsProviderSetup" apps/api/src/routes/me.ts: PASS
  • grep -q "memberCredentials" apps/api/src/routes/me.ts: PASS
  • All 22 tests pass (requireAdmin: 4, user: 10, me: 8): PASS
  • pnpm --filter @familysync/api exec tsc --noEmit exits 0: PASS
  • Commits 9217930, f9c70ab, 9e1507f, 72e0140, e5889df, 1adff61 in git log: PASS