docs(19-03): complete local-auth middleware wiring plan

2026-06-17 17:00:42 -04:00
parent 9b569efeab
commit e0d471a5d5
1 changed files with 182 additions and 0 deletions
@@ -0,0 +1,182 @@
 ---
 phase: 19-local-auth-no-oidc-mode
 plan: "03"
 subsystem: auth
 tags: [local-auth, middleware, rate-limit, lockout, session-cookie, oidc-link, de-authelia, tdd]
 status: complete
 dependency_graph:
  requires:
    - verifyLocalSessionCookie / issueLocalSessionCookie / clearLocalSessionCookie (from 19-01)
    - localCredentials Drizzle table (from 19-01)
    - hashPassword / verifyPassword (from 19-01)
    - linkOidcToUser / OidcLinkConflictError (from 19-02)
  provides:
    - localAuthMiddleware: cookie → c.set('user') middleware (AUTH-LOCAL-04)
    - GET /api/auth/mode: pre-auth OIDC config endpoint (AUTH-LOCAL-05)
    - POST /api/auth/local/login: rate-limited + timing-safe login (AUTH-LOCAL-03, AUTH-LOCAL-19)
    - POST/GET /api/auth/local/logout: session cookie clear (AUTH-LOCAL-06)
    - index.ts: pre-auth mounts + localAuthMiddleware slot + OIDC guard skip-when-user-set + /callback link branch (AUTH-LOCAL-04, AUTH-LOCAL-10)
    - middleware.ts: de-Authelia-ized comments (AUTH-LOCAL-18)
  affects:
    - apps/api/src/index.ts (route mounts, middleware chain, /callback extension)
    - apps/api/src/auth/middleware.ts (comment-only D-06 changes)
 tech_stack:
  added: []
  patterns:
    - TDD RED/GREEN per task (failing test committed before implementation)
    - In-memory loginAttempts Map: per-IP rate-limit (5→429) + lockout (10→423)
    - DUMMY_HASH timing defense: verifyPassword always runs, even for unknown usernames (T-19-12)
    - noEchoHook on login: Zod errors never echo submitted values (T-19-14)
    - oidcAuthMiddleware() factory called once at construction, returned handler in skip-when-user-set wrapper (D-03)
    - Jwt.verify on URL state param to extract linkUserId in /callback link branch (T-19-15)
 key_files:
  created:
    - apps/api/src/auth/localAuthMiddleware.ts
    - apps/api/src/routes/authMode.ts
    - apps/api/src/routes/localAuth.ts
    - apps/api/tests/auth/localAuthMiddleware.test.ts
    - apps/api/tests/routes/authMode.test.ts
    - apps/api/tests/routes/localAuth.test.ts
  modified:
    - apps/api/src/index.ts
    - apps/api/src/auth/middleware.ts
 decisions:
  - "loginAttempts counter increments even on 429 responses — brute-force accumulates toward lockout (10→423) even during rate-window; original implementation only incremented on final auth check"
  - "oidcAuthMiddleware() factory called once at app construction (not per-request) to preserve test assertion that it is called exactly once during app init"
  - "localAuthMiddleware casts user value to typeof DEV_USER for ContextVariableMap compatibility — the narrow const type from devBypass.ts as const requires an explicit cast"
  - "Authelia removed from 2 comments in index.ts and 2 comments in middleware.ts (D-06 / AUTH-LOCAL-18)"
 metrics:
  duration: "~16 minutes"
  completed: "2026-06-17"
  tasks_completed: 3
  tasks_total: 3
  files_created: 6
  files_modified: 2
 ---
 # Phase 19 Plan 03: Auth Routes + Middleware Wiring Summary
 **One-liner:** localAuthMiddleware (cookie→c.set('user')), GET /api/auth/mode pre-auth endpoint, rate-limited POST /api/auth/local/login with timing-safe dummy-hash, logout, index.ts middleware chain wired with OIDC-guard skip-when-user-set and /callback link branch for linkOidcToUser, de-Authelia-ized middleware comments.
 ## Tasks Completed
 | Task | RED Commit | GREEN Commit | Key Files |
 |------|-----------|-------------|-----------|
 | 1: localAuthMiddleware + GET /api/auth/mode | ac32bd4 | be7a0ae | localAuthMiddleware.ts, authMode.ts, index.ts |
 | 2: POST /api/auth/local/login (rate-limit + lockout) + logout | db66295 | c437f40 | localAuth.ts |
 | 3: index.ts wiring + /callback link branch + de-Authelia comments | — | 9b569ef | index.ts, middleware.ts |
 ## What Was Built
 ### Task 1: localAuthMiddleware + GET /api/auth/mode (TDD)
 **`apps/api/src/auth/localAuthMiddleware.ts`** — exports `localAuthMiddleware(): MiddlewareHandler`:
 - If `c.get('user')` already set (devAuthBypass ran first): no-op, calls next()
 - Calls `verifyLocalSessionCookie(c)` — returns null if no cookie / invalid / expired
 - On null: calls next() WITHOUT c.set('user') — Pitfall-1 guard; OIDC guard fires on absent key
 - On valid userId: SELECTs users row; if found, `c.set('user', {...} as typeof DEV_USER)` with matching shape
 **`apps/api/src/routes/authMode.ts`** — exports `authModeRouter` with GET /mode:
 - `localEnabled: true` unconditionally (D-01)
 - `oidcEnabled: Boolean(OIDC_ISSUER env)` || falls back to `app_config` oidc_issuer row
 - No auth gate — pre-auth endpoint
 Tests: 8 tests pass (4 middleware + 4 mode tests)
 ### Task 2: POST /api/auth/local/login + logout (TDD)
 **`apps/api/src/routes/localAuth.ts`** — exports `localAuthRouter` and `loginAttempts`:
 - `POST /local/login` — zValidator + noEchoHook + per-IP loginAttempts Map
  - lockedOut check (>= LOCKOUT_FAILURES=10) → 423 `{ error: 'Account locked' }`
  - Rate window check (>= RATE_WINDOW_FAILURES=5, within RATE_WINDOW_SECS=60) → increments counter + 429
  - Selects local_credentials by username; ALWAYS runs verifyPassword (DUMMY_HASH on unknown username — T-19-12)
  - Same 401 body for wrong-password AND unknown-username (no enumeration)
  - On success: loginAttempts.delete(ip), issueLocalSessionCookie, 200 `{ ok: true }`
 - `POST /local/logout` + `GET /local/logout` → clearLocalSessionCookie → 200 `{ ok: true }`
 Tests: 8 tests pass (login success/failure/enumeration/rate-limit/lockout/logout/no-echo)
 ### Task 3: index.ts wiring + /callback link branch + de-Authelia comments
 **`apps/api/src/index.ts`** changes:
 - Pre-auth mounts: `app.route('/api/auth', authModeRouter)` + `app.route('/api/auth', localAuthRouter)` before devAuthBypass
 - `app.use('/api/*', localAuthMiddleware())` after devAuthBypass, before OIDC guard
 - OIDC guard wrapper: `oidcHandler = oidcAuthMiddleware()` stored once at construction, invoked per-request only when `c.get('user')` is falsy (D-03 coexistence seam)
 - `/callback` extended: reads URL `state` param, tries Jwt.verify with LOCAL_SESSION_SECRET; if `linkUserId` in payload → call linkOidcToUser after processOAuthCallback; OidcLinkConflictError → redirect `/?error=oidc-link-conflict`
 - Authelia references removed from 2 comments (D-06)
 **`apps/api/src/auth/middleware.ts`** changes:
 - Header: "Authelia as the identity provider" → "generic OIDC identity provider" (D-06)
 - "Authelia base URL" → "OIDC issuer URL" (D-06)
 - "Authelia's refresh_token_lifespan" → "the OIDC provider's refresh_token_lifespan" (D-06)
 Full suite: 446/446 tests pass; `pnpm --filter @familysync/api typecheck` exits 0.
 ## Deviations from Plan
 ### Auto-fixed Issues
 **1. [Rule 1 - Bug] Rate-limit counter not incrementing during 429 window**
 - **Found during:** Task 2 GREEN phase (Test 5 returning 429 instead of 423 for the 11th attempt)
 - **Issue:** After 5 failures, subsequent attempts returned 429 (early return) without incrementing the counter. The counter never reached LOCKOUT_FAILURES=10 because the early-return prevented accumulation.
 - **Fix:** Inside the rate-window 429 branch: increment counter, update lockedUntil, check if lockedOut (returns 423 if so), else return 429. This way brute-force attacks accumulate toward lockout even during the rate window.
 - **Files modified:** apps/api/src/routes/localAuth.ts
 - **Commit:** c437f40
 **2. [Rule 1 - Bug] oidcAuthMiddleware() factory called per-request in OIDC guard wrapper**
 - **Found during:** Task 3 execution — me.test.ts assertion that oidcAuthMiddleware is called exactly once during app init
 - **Issue:** Original OIDC guard wrapper called `oidcAuthMiddleware()(c, next)` per-request; existing test `wires oidcAuthMiddleware on /api/* when bypass is not active` asserts `oidcMiddlewareSpy.toHaveBeenCalledTimes(1)` (factory called once at construction).
 - **Fix:** Store `const oidcHandler = oidcAuthMiddleware()` at construction time; invoke `oidcHandler(c, next)` per-request inside the wrapper.
 - **Files modified:** apps/api/src/index.ts
 - **Commit:** 9b569ef
 **3. [Rule 1 - Bug] Type incompatibility: localAuthMiddleware c.set('user') type error**
 - **Found during:** Task 3 typecheck
 - **Issue:** `ContextVariableMap` maps 'user' to `typeof DEV_USER` (narrow `as const` literal). The middleware constructs `{ id: number; oidcIss: string; ... }` which TypeScript rejects as incompatible.
 - **Fix:** Add `import type { DEV_USER }` and cast with `as typeof DEV_USER` on the c.set call.
 - **Files modified:** apps/api/src/auth/localAuthMiddleware.ts
 - **Commit:** 9b569ef
 ## Threat Surface Scan
 All new/modified routes in this plan:
 - `GET /api/auth/mode` — pre-auth, no credentials, no sensitive data; reads only env/app_config
 - `POST /api/auth/local/login` — new attack surface; mitigated by T-19-11 (rate-limit), T-19-12 (timing-safe dummy hash, no-enumeration 401), T-19-14 (noEchoHook)
 - `POST/GET /api/auth/local/logout` — clears cookie only; no sensitive data exposed
 No new trust boundaries beyond those in the plan's threat model.
 ## TDD Gate Compliance
 **Task 1 (TDD):**
 - RED commit: ac32bd4 — test(19-03): add failing tests for localAuthMiddleware and GET /api/auth/mode
 - GREEN commit: be7a0ae — feat(19-03): implement localAuthMiddleware, GET /api/auth/mode...
 **Task 2 (TDD):**
 - RED commit: db66295 — test(19-03): add failing tests for POST /api/auth/local/login + logout
 - GREEN commit: c437f40 — feat(19-03): implement POST /api/auth/local/login (rate-limit + lockout) + logout
 **Task 3 (auto):** No TDD cycle required.
 ## Known Stubs
 None. All new endpoints return real data and perform real operations.
 ## Self-Check: PASSED
 All created files confirmed present on disk:
 - FOUND: apps/api/src/auth/localAuthMiddleware.ts
 - FOUND: apps/api/src/routes/authMode.ts
 - FOUND: apps/api/src/routes/localAuth.ts
 - FOUND: apps/api/tests/auth/localAuthMiddleware.test.ts
 - FOUND: apps/api/tests/routes/authMode.test.ts
 - FOUND: apps/api/tests/routes/localAuth.test.ts
 All commits confirmed in git log:
 - ac32bd4: test(19-03): add failing tests for localAuthMiddleware and GET /api/auth/mode
 - be7a0ae: feat(19-03): implement localAuthMiddleware, GET /api/auth/mode, and pre-auth route mounts
 - db66295: test(19-03): add failing tests for POST /api/auth/local/login + logout
 - c437f40: feat(19-03): implement POST /api/auth/local/login (rate-limit + lockout) + logout
 - 9b569ef: feat(19-03): wire /callback link branch, OIDC-guard skip, de-Authelia comments
 Test results: 446/446 pass (34 test files); `pnpm --filter @familysync/api typecheck` exits 0.