docs(12-02): complete setup-routes plan — guard + router + OIDC fallback

2026-06-15 14:06:29 -04:00
parent 67a9d29dc1
commit 9d0aa6296a
4 changed files with 165 additions and 13 deletions
@@ -0,0 +1,151 @@
+---
+phase: 12-initial-setup-wizard
+plan: 02
+subsystem: api, auth, testing
+tags: [hono, drizzle, vitest, tdd, setup-wizard, oidc, vapid, pre-auth, guard]
+
+# Dependency graph
+requires:
+  - phase: 12-01
+    provides: setupGuard.ts stub, setup.ts stub router, Wave-0 RED test scaffolds, schema claimed column
+provides:
+  - apps/api/src/lib/setupGuard.ts — real isSetupLocked() per-call DB evaluation (SETUP-04/D-10)
+  - apps/api/src/routes/setup.ts — setupRouter with all 7 pre-auth handlers
+  - apps/api/src/index.ts — setupRouter mounted pre-auth before devAuthBypass
+  - apps/api/src/auth/middleware.ts — oidcConfigFallbackMiddleware (env-OR-app_config, D-02/D-03)
+  - apps/api/tests/routes/setup.test.ts — 17 integration tests all GREEN
+affects:
+  - 12-03-pwa-setup-page (consumes /api/setup/* routes, esp. GET /status)
+  - 12-04-integration (full setup flow)
+
+# Tech tracking
+tech-stack:
+  added: []  # Zero new packages (RESEARCH §No New Packages)
+  patterns:
+    - isSetupLocked() per-call freshness pattern (D-10) — imported in every handler, no module-cache
+    - guard-first handler pattern — isSetupLocked() is the FIRST await in every setup handler
+    - noEchoHook anti-echo pattern (from admin.ts) — Zod error details never returned on credential routes
+    - validateEncryptAndStoreCredential reuse (D-09) — no new crypto; shared helper for PROPFIND+encrypt+store
+    - env-OR-app_config fallback middleware — reads DB per-request when env absent; injects into process.env
+    - mysql2 $returningId() + re-select for local user insert (Pattern 4 from user.ts)
+    - onDuplicateKeyUpdate upsert for app_config writes (Shared Pattern 1 from admin.ts)
+
+key-files:
+  created: []
+  modified:
+    - apps/api/src/lib/setupGuard.ts
+    - apps/api/src/routes/setup.ts
+    - apps/api/src/index.ts
+    - apps/api/src/auth/middleware.ts
+    - apps/api/tests/routes/setup.test.ts
+    - apps/api/tests/routes/push.test.ts
+
+key-decisions:
+  - "A2-CONFIRMED: @hono/oidc-auth reads OIDC_ISSUER/OIDC_CLIENT_ID/OIDC_AUTH_EXTERNAL_URL at per-request call time via env(c)→process.env — NOT at import time; fresh boot without OIDC env is safe (HTTP 500 only on protected /api/* requests)"
+  - "D-02-FALLBACK: env-OR-app_config Recommendation (a) implemented: oidcConfigFallbackMiddleware reads from app_config when process.env absent, injects into process.env before oidcAuthMiddleware() per-request read"
+  - "GUARD-ON-STATUS: GET /api/setup/status uses isSetupLocked() directly (covers effective-config branch too) — returns {setupComplete:true} when locked, {setupComplete:false} when not; aligns with must_haves.truths"
+  - "LOCAL-USER-ROLLBACK: POST /api/setup/credential rolls back the local user insert if validateEncryptAndStoreCredential throws, preventing orphaned unclaimed user rows"
+
+# Metrics
+duration: 15min
+completed: 2026-06-15
+---
+
+# Phase 12 Plan 02: Setup Routes Summary
+
+**Real isSetupLocked() 423 guard + all 7 pre-auth /api/setup/* routes + OIDC env-OR-app_config fallback; 394 tests green including Pitfall 8 regression**
+
+## Performance
+
+- **Duration:** 15 min
+- **Started:** 2026-06-15T17:48:42Z
+- **Completed:** 2026-06-15T18:03:21Z
+- **Tasks:** 3
+- **Files modified:** 6
+
+## Accomplishments
+
+- Implemented real `isSetupLocked()` in `setupGuard.ts`: reads `app_config.setup_complete` (check 1) and then checks `member_credentials` row + `VAPID_PRIVATE_KEY`/`VAPID_PUBLIC_KEY` env for effective-config branch (D-10 check 2). Re-queries DB fresh every call — no module-level cache.
+- Converted all 20 Wave-0 `it.todo()` scaffolds in `setup.test.ts` into real integration tests (17 tests) — all GREEN after Task 2.
+- Implemented full `setupRouter` in `setup.ts` with all 7 routes:
+  - `GET /status` — uses `isSetupLocked()` directly; returns `{setupComplete: boolean}`
+  - `POST /config` — zod-validates https-URL issuer; upserts `oidc_issuer`, `oidc_client_id`, `vapid_public_key`, `app_external_url`
+  - `POST /validate/db` — `SELECT 1` connectivity check; 200/503
+  - `POST /validate/oidc` — fetches discovery doc with 5s timeout; 200/400
+  - `POST /validate/vapid` — `webpush.setVapidDetails()` structural check; env-only key read; 200/400
+  - `POST /credential` — inserts local user first (Pitfall 5 FK), calls shared helper; noEchoHook; rollback on failure
+  - `POST /complete` — upserts `setup_complete='true'`; 200 first call, 423 second (Pitfall 8/SETUP-04)
+- Mounted `setupRouter` in `index.ts` BEFORE `devAuthBypass()` (line 49 < line 54, T-12-09/Pitfall 1 acceptance-checked).
+- Implemented `oidcConfigFallbackMiddleware` in `auth/middleware.ts`: reads OIDC config from `app_config` when env absent, injects into `process.env` for downstream `oidcAuthMiddleware()` pickup. Mounted before OIDC guard when `!devBypassActive`.
+- Confirmed A2: `@hono/oidc-auth` reads env at per-request call time — boot is safe without OIDC env.
+- Fixed `push.test.ts` `vi.doMock` to include `oidcConfigFallbackMiddleware` stub (Rule 3 auto-fix).
+
+## Task Commits
+
+1. **Task 1: isSetupLocked() real impl + RED-first setup tests** — `4748d57` (test)
+2. **Task 2: Setup router — all 7 routes + pre-auth mount** — `20f91e4` (feat)
+3. **Task 3: OIDC boot env-OR-app_config fallback + mount verification** — `67a9d29` (feat)
+
+## Files Created/Modified
+
+- `apps/api/src/lib/setupGuard.ts` — real `isSetupLocked()`: `setup_complete` check + effective-config branch (D-10); no module-level cache
+- `apps/api/src/routes/setup.ts` — `setupRouter` with 7 handlers; guard-first; noEchoHook; shared helper reuse; VAPID env-only
+- `apps/api/src/index.ts` — `setupRouter` import + pre-auth mount; `oidcConfigFallbackMiddleware` import + mount before OIDC guard
+- `apps/api/src/auth/middleware.ts` — `oidcConfigFallbackMiddleware` added (env-OR-app_config fallback); re-exports unchanged
+- `apps/api/tests/routes/setup.test.ts` — 17 real integration tests (all GREEN); full mock scaffolding
+- `apps/api/tests/routes/push.test.ts` — `vi.doMock` updated to include `oidcConfigFallbackMiddleware` stub
+
+## Decisions Made
+
+- **A2-CONFIRMED:** `@hono/oidc-auth` reads OIDC env vars at per-request call time via `env(c) → process.env` (source: `@hono/oidc-auth` dist/index.js line 30). NOT at import time. A fresh unconfigured instance boots without crashing; HTTP 500 only occurs on OIDC-protected `/api/*` requests when env is absent — acceptable since `/api/setup/*` is pre-auth and is the only pre-setup surface. Recommendation (a) implemented.
+
+- **D-02-FALLBACK:** `oidcConfigFallbackMiddleware` injects `oidc_issuer` / `oidc_client_id` / `app_external_url` from `app_config` into `process.env` when the env var is absent, before `oidcAuthMiddleware()` reads it per-request. Non-secret values only (D-01 env floor: `OIDC_CLIENT_SECRET`, `OIDC_AUTH_SECRET` stay in env always). Options (b) and (c) (defer mount, lazy-per-request) not needed — option (a) is simpler and correct per A2 confirmation.
+
+- **GUARD-ON-STATUS:** `GET /api/setup/status` calls `isSetupLocked()` to populate `setupComplete`. This makes the status response consistent with the guard state (covers the effective-config branch too) and satisfies the must_haves truth that `/status` returns `{setupComplete:true}` after setup is complete. The route never returns 423 — it always returns 200 with the boolean.
+
+- **LOCAL-USER-ROLLBACK:** `POST /api/setup/credential` deletes the inserted local user row if `validateEncryptAndStoreCredential()` throws, preventing orphaned `claimed=false` rows in the `users` table that would permanently increment color slot usage and confuse the first-login-claims query.
+
+## Deviations from Plan
+
+### Auto-fixed Issues
+
+**1. [Rule 3 - Blocking] push.test.ts vi.doMock missing oidcConfigFallbackMiddleware**
+- **Found during:** Task 3 test run
+- **Issue:** `push.test.ts` uses `vi.doMock('../../src/auth/middleware.js', ...)` but the mock omitted the new `oidcConfigFallbackMiddleware` export. Vitest raises `No "oidcConfigFallbackMiddleware" export is defined on the mock` at runtime.
+- **Fix:** Added `oidcConfigFallbackMiddleware: async (_c, next) => next()` to the doMock factory.
+- **Files modified:** `apps/api/tests/routes/push.test.ts`
+- **Commit:** `67a9d29` (Task 3)
+
+---
+
+**Total deviations:** 1 auto-fixed (Rule 3 blocking — test mock missing new export)
+**Impact on plan:** Zero scope creep. Fix was mechanical and localized to a test file.
+
+## Threat Surface Scan
+
+No new threat surface beyond what is explicitly modeled in the plan's `<threat_model>`. All mitigations verified:
+
+| Threat | Mitigation | Verified |
+|--------|-----------|---------|
+| T-12-04: Setup endpoint replay after completion | `isSetupLocked()` first in every handler; 423; re-queried per call | All 7 handlers call `isSetupLocked()` — source-grep ≥7 passed |
+| T-12-05: App password echoed in 400 | `noEchoHook`; no `console.log` of password or `valid('json')` | grep returns 0 echo/log hits |
+| T-12-06: VAPID_PRIVATE_KEY in DB or response | `/validate/vapid` reads ONLY from `process.env`; never from app_config; never returned | grep confirms env-only read |
+| T-12-08: OIDC issuer SSRF via /config | Zod `.refine(v => v.startsWith('https://'))` rejects non-https URLs | Test `returns 400 when oidcIssuer is not an https URL` passes |
+| T-12-09: /api/setup/* caught by OIDC guard | Mounted at line 49, `devAuthBypass()` at line 54 — ordering verified | awk mount-order acceptance gate passes |
+
+## Self-Check: PASSED
+
+Files exist:
+- `apps/api/src/lib/setupGuard.ts` — FOUND
+- `apps/api/src/routes/setup.ts` — FOUND
+- `apps/api/src/auth/middleware.ts` — FOUND
+- `apps/api/src/index.ts` — FOUND
+- `apps/api/tests/routes/setup.test.ts` — FOUND
+
+Commits exist:
+- `4748d57` — FOUND
+- `20f91e4` — FOUND
+- `67a9d29` — FOUND
+
+Test suite: 394 passed | 5 todo | 0 failed
+TypeCheck: clean (0 errors)