docs(10-02): complete admin-role-primitives plan summary and state update
This commit is contained in:
Lucas Berger
@@ -177,7 +177,7 @@ Plans:
|
||||
|
||||
**Wave 2** *(blocked on Wave 1 completion)*
|
||||
|
||||
- [ ] 10-02-PLAN.md — requireAdmin guard + first-login-wins bootstrap + /api/me isAdmin/needsProviderSetup (TDD)
|
||||
- [x] 10-02-PLAN.md — requireAdmin guard + first-login-wins bootstrap + /api/me isAdmin/needsProviderSetup (TDD)
|
||||
|
||||
**Wave 3** *(blocked on Wave 2 completion)*
|
||||
|
||||
@@ -369,7 +369,7 @@ Plans:
|
||||
| 7. Mobile Test Harness | v1.1 | 4/4 | Complete | 2026-06-11 |
|
||||
| 8. Gitea CI | v1.1 | 4/4 | Complete | 2026-06-11 |
|
||||
| 9. Faster Write-Back | v1.1 | 2/2 | Complete | 2026-06-12 |
|
||||
| 10. Admin Role & Settings | v1.1 | 1/4 | In Progress| |
|
||||
| 10. Admin Role & Settings | v1.1 | 2/4 | In Progress| |
|
||||
| 11. Per-Event Reminders | v1.1 | 0/? | Not started | - |
|
||||
| 12. Initial Setup Wizard | v1.1 | 0/? | Not started | - |
|
||||
| 13. Real Lint Gate (ESLint) | v1.1 | 3/3 | Complete | 2026-06-12 |
|
||||
@@ -383,7 +383,7 @@ Plans:
|
||||
|
||||
**Goal:** [Captured for future planning] Abstract the calendar backend behind a provider interface so Fastmail/CalDAV is one implementation among potentially many. Shipping with a single provider is fine, but the broker, sync, and event-expansion layers should be structured so additional providers (e.g. other CalDAV hosts, Google Calendar, generic ICS feeds) can be added without rework. Captures the "provider" seam as an explicit architectural concern.
|
||||
**Requirements:** TBD
|
||||
**Plans:** 1/4 plans executed
|
||||
**Plans:** 2/4 plans executed
|
||||
|
||||
Plans:
|
||||
|
||||
|
||||
+8
-6
@@ -3,14 +3,14 @@ gsd_state_version: 1.0
|
||||
milestone: v1.1
|
||||
milestone_name: Operability & Polish
|
||||
status: executing
|
||||
stopped_at: Completed Phase 10 Plan 01 (v1.1 DB foundation)
|
||||
last_updated: "2026-06-13T18:25:15.171Z"
|
||||
stopped_at: "Completed Phase 10 Plan 02 (admin role primitives: requireAdmin, upsertUser is_admin, /api/me isAdmin+needsProviderSetup)"
|
||||
last_updated: "2026-06-13T18:39:38.129Z"
|
||||
last_activity: 2026-06-13 -- Phase 10 execution started
|
||||
progress:
|
||||
total_phases: 20
|
||||
completed_phases: 7
|
||||
total_plans: 27
|
||||
completed_plans: 24
|
||||
completed_plans: 25
|
||||
percent: 35
|
||||
---
|
||||
|
||||
@@ -26,7 +26,7 @@ See: .planning/PROJECT.md (updated 2026-06-10)
|
||||
## Current Position
|
||||
|
||||
Phase: 10 (admin-role-settings) — EXECUTING
|
||||
Plan: 2 of 4
|
||||
Plan: 3 of 4
|
||||
Status: Ready to execute
|
||||
Last activity: 2026-06-13 -- Phase 10 execution started
|
||||
|
||||
@@ -101,6 +101,7 @@ _Updated after each plan completion_
|
||||
| Phase 16 P04 | 45 | 4 tasks | 3 files |
|
||||
| Phase 16 P05 | 7 | 2 tasks | 1 files |
|
||||
| Phase 10-admin-role-settings P01 | 265 | - tasks | - files |
|
||||
| Phase 10-admin-role-settings P02 | 700 | 3 tasks | 6 files |
|
||||
|
||||
## Accumulated Context
|
||||
|
||||
@@ -171,6 +172,7 @@ Recent decisions affecting current work:
|
||||
- [Phase ?]: D-12-security-job: gitleaks runs unconditionally, pnpm audit/outdated code-gated at step level
|
||||
- [Phase ?]: D-14-gate-security: security wired into gate with individual needs.security.result==success check (not success-or-skipped, Gitea #31007)
|
||||
- [Phase ?]: D-MIGRATION-10-01: v1.1 DB migration applied via direct mysql2 DDL (drizzle-kit migrate silently failed due to journal hash mismatch with legacy tracking; hash recorded in __drizzle_migrations for forward compatibility)
|
||||
- [Phase ?]: D-10-02-aggregate-limit1: Drizzle COUNT aggregate uses .limit(1) for mock-chain compatibility
|
||||
|
||||
### Roadmap Evolution
|
||||
|
||||
@@ -237,8 +239,8 @@ Recent decisions affecting current work:
|
||||
|
||||
## Session Continuity
|
||||
|
||||
Last session: 2026-06-13T18:25:15.155Z
|
||||
Stopped at: Completed Phase 10 Plan 01 (v1.1 DB foundation)
|
||||
Last session: 2026-06-13T18:39:38.119Z
|
||||
Stopped at: Completed Phase 10 Plan 02 (admin role primitives: requireAdmin, upsertUser is_admin, /api/me isAdmin+needsProviderSetup)
|
||||
Resume file: None
|
||||
|
||||
## Operator Next Steps
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
---
|
||||
phase: "10-admin-role-settings"
|
||||
plan: "02"
|
||||
subsystem: "api-auth"
|
||||
tags: ["requireAdmin", "admin-role", "middleware", "upsertUser", "first-login-wins", "me-api", "tdd"]
|
||||
dependency_graph:
|
||||
requires:
|
||||
- "users.is_admin column (10-01)"
|
||||
- "member_credentials table with UNIQUE(user_id) (10-01)"
|
||||
- "app_config table (10-01)"
|
||||
provides:
|
||||
- "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)"
|
||||
affects:
|
||||
- "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)"
|
||||
tech_stack:
|
||||
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"
|
||||
key_files:
|
||||
created:
|
||||
- "apps/api/src/lib/requireAdmin.ts"
|
||||
- "apps/api/tests/lib/requireAdmin.test.ts"
|
||||
modified:
|
||||
- "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"
|
||||
decisions:
|
||||
- "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)"
|
||||
metrics:
|
||||
duration_seconds: 700
|
||||
completed_date: "2026-06-13"
|
||||
tasks_completed: 3
|
||||
files_modified: 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
|
||||
Reference in New Issue
Block a user