From e0d471a5d56490fabcf462e70b385c35e2bf2eb7 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Wed, 17 Jun 2026 17:00:42 -0400 Subject: [PATCH] docs(19-03): complete local-auth middleware wiring plan --- .../19-03-SUMMARY.md | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 .planning/phases/19-local-auth-no-oidc-mode/19-03-SUMMARY.md diff --git a/.planning/phases/19-local-auth-no-oidc-mode/19-03-SUMMARY.md b/.planning/phases/19-local-auth-no-oidc-mode/19-03-SUMMARY.md new file mode 100644 index 0000000..2725938 --- /dev/null +++ b/.planning/phases/19-local-auth-no-oidc-mode/19-03-SUMMARY.md @@ -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.