chore: merge executor worktree (worktree-agent-a11c9f6d35e8d8721)
This commit is contained in:
Lucas Berger
@@ -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.
|
||||
@@ -0,0 +1,98 @@
|
||||
/**
|
||||
* localAuthMiddleware.ts — local-session cookie → c.set('user') middleware (AUTH-LOCAL-04).
|
||||
*
|
||||
* Reads the `local-session` cookie, validates the JWT, fetches the users row,
|
||||
* and populates `c.get('user')` with the same shape as devAuthBypass so that all
|
||||
* downstream routes work unchanged.
|
||||
*
|
||||
* Mount in index.ts AFTER devAuthBypass() and BEFORE the OIDC guard:
|
||||
* app.use('/api/*', devAuthBypass());
|
||||
* app.use('/api/*', localAuthMiddleware()); ← HERE
|
||||
* if (!devBypassActive) {
|
||||
* app.use('/api/*', oidcAuthMiddleware()); ← skip if c.get('user') set
|
||||
* }
|
||||
*
|
||||
* Security contract:
|
||||
* - If c.get('user') is already set (devAuthBypass ran first): no-op passthrough.
|
||||
* This is Test 4 — dev user is not overwritten.
|
||||
* - If no 'local-session' cookie is present: pure passthrough WITHOUT calling
|
||||
* c.set('user', undefined). The OIDC guard fires on falsy c.get('user') only when
|
||||
* the value was never set — calling c.set('user', undefined) would suppress it.
|
||||
* This is Pitfall-1 / Test 2.
|
||||
* - If cookie is valid but the users row is gone: passthrough (no crash).
|
||||
* This is Test 3.
|
||||
*
|
||||
* ContextVariableMap: the `user` shape is declared in devBypass.ts. Import it as a
|
||||
* side-effect so c.set('user', ...) is typed correctly throughout this file.
|
||||
*/
|
||||
|
||||
// Side-effect import: extends ContextVariableMap with the `user` key (Pitfall — shared shape).
|
||||
import './devBypass.js';
|
||||
import type { MiddlewareHandler } from 'hono';
|
||||
import { eq } from 'drizzle-orm';
|
||||
import { db } from '../db/client.js';
|
||||
import { users } from '../db/schema.js';
|
||||
import { verifyLocalSessionCookie } from './localSession.js';
|
||||
import type { DEV_USER } from './devBypass.js';
|
||||
|
||||
/**
|
||||
* Returns a Hono MiddlewareHandler that:
|
||||
* 1. Checks if c.get('user') is already set — no-op if so (devAuthBypass-first).
|
||||
* 2. Calls verifyLocalSessionCookie(c) — returns null if no cookie / invalid / expired.
|
||||
* 3. On null: calls next() WITHOUT setting user (pure passthrough — OIDC guard can fire).
|
||||
* 4. On valid userId: SELECTs the users row; if found, c.set('user', {...}); always next().
|
||||
*/
|
||||
export function localAuthMiddleware(): MiddlewareHandler {
|
||||
return async (c, next) => {
|
||||
// If a prior middleware (devAuthBypass) already set the user, do not overwrite.
|
||||
if (c.get('user')) {
|
||||
await next();
|
||||
return;
|
||||
}
|
||||
|
||||
// Verify the local-session JWT cookie — returns userId or null.
|
||||
// verifyLocalSessionCookie returns null (never throws) on any error (Pitfall 9 guard).
|
||||
const userId = await verifyLocalSessionCookie(c);
|
||||
|
||||
if (userId === null) {
|
||||
// No valid local session — pass through WITHOUT setting c.get('user').
|
||||
// CRITICAL: Do NOT call c.set('user', undefined) — that sets the key to undefined
|
||||
// which is falsy but "set", breaking the OIDC guard's c.get('user') check (Pitfall 1).
|
||||
await next();
|
||||
return;
|
||||
}
|
||||
|
||||
// Load the users row to populate the same shape as DEV_USER.
|
||||
const [row] = await db
|
||||
.select({
|
||||
id: users.id,
|
||||
oidcIss: users.oidcIss,
|
||||
oidcSub: users.oidcSub,
|
||||
displayName: users.displayName,
|
||||
color: users.color,
|
||||
})
|
||||
.from(users)
|
||||
.where(eq(users.id, userId))
|
||||
.limit(1);
|
||||
|
||||
if (!row) {
|
||||
// userId from JWT but no users row (deleted user) — passthrough without setting user.
|
||||
await next();
|
||||
return;
|
||||
}
|
||||
|
||||
// Populate c.get('user') with the same shape as DEV_USER (devBypass.ts ContextVariableMap).
|
||||
// oidcIss/oidcSub: local users have nullable oidcIss/oidcSub — use fallback strings so the
|
||||
// shape is compatible with typeof DEV_USER at runtime. Cast required because ContextVariableMap
|
||||
// is narrowed to the const DEV_USER literal type.
|
||||
c.set('user', {
|
||||
id: row.id,
|
||||
oidcIss: row.oidcIss ?? 'local',
|
||||
oidcSub: row.oidcSub ?? String(row.id),
|
||||
displayName: row.displayName ?? null,
|
||||
color: row.color ?? '#4A90D9',
|
||||
} as typeof DEV_USER);
|
||||
|
||||
await next();
|
||||
};
|
||||
}
|
||||
@@ -1,12 +1,12 @@
|
||||
/**
|
||||
* OIDC authentication middleware wiring.
|
||||
*
|
||||
* Configures @hono/oidc-auth for Authelia as the identity provider.
|
||||
* Configures @hono/oidc-auth for the generic OIDC identity provider (D-06).
|
||||
*
|
||||
* Required env vars:
|
||||
* OIDC_AUTH_SECRET — 32+ char random string for JWT cookie signing (T-02-03)
|
||||
* OIDC_ISSUER — Authelia base URL (middleware fetches /.well-known/openid-configuration)
|
||||
* OIDC_CLIENT_ID — registered client ID in Authelia
|
||||
* OIDC_ISSUER — OIDC issuer URL (middleware fetches /.well-known/openid-configuration)
|
||||
* OIDC_CLIENT_ID — registered client ID at the OIDC provider
|
||||
* OIDC_CLIENT_SECRET — PLAIN text secret (NOT the pbkdf2 hash — see Pitfall 7)
|
||||
* OIDC_REDIRECT_URI — https://familysync.<domain>/callback
|
||||
* OIDC_AUTH_EXTERNAL_URL — https://familysync.<domain> — MANDATORY behind Pangolin (Pitfall 1)
|
||||
@@ -30,7 +30,7 @@
|
||||
* Every OIDC_AUTH_REFRESH_INTERVAL (default 15 min) the middleware calls
|
||||
* the token endpoint with the stored refresh token — no iframe required (D-12).
|
||||
* Session lifespan is governed by OIDC_AUTH_EXPIRES (default 1 day) and
|
||||
* Authelia's refresh_token_lifespan.
|
||||
* the OIDC provider's refresh_token_lifespan.
|
||||
*
|
||||
* Scopes: openid, profile, email only — no 'groups' scope (D-11).
|
||||
*
|
||||
|
||||
+92
-6
@@ -11,17 +11,23 @@ import { listsRouter, listItemsRouter } from './routes/lists.js';
|
||||
import { pushRouter } from './routes/push.js';
|
||||
import { adminRouter } from './routes/admin.js';
|
||||
import { setupRouter } from './routes/setup.js';
|
||||
import { authModeRouter } from './routes/authMode.js';
|
||||
import { localAuthRouter } from './routes/localAuth.js';
|
||||
import {
|
||||
oidcAuthMiddleware,
|
||||
processOAuthCallback,
|
||||
oidcConfigFallbackMiddleware,
|
||||
} from './auth/middleware.js';
|
||||
import { devAuthBypass } from './auth/devBypass.js';
|
||||
import { localAuthMiddleware } from './auth/localAuthMiddleware.js';
|
||||
import { persistSessionCookie } from './auth/persistSessionCookie.js';
|
||||
import { startBrokerPoller } from './broker/poller.js';
|
||||
import { startOutboxWorker, initOutboxTrigger } from './broker/outboxWorker.js';
|
||||
import { startReminderScheduler } from './broker/reminderScheduler.js';
|
||||
import { assertNotDevBypassInProduction, assertLocalSessionSecretSet } from './lib/bootGuards.js';
|
||||
import { getAuth } from './auth/middleware.js';
|
||||
import { Jwt } from 'hono/utils/jwt';
|
||||
import { linkOidcToUser, OidcLinkConflictError } from './auth/linkOidc.js';
|
||||
import webpush from 'web-push';
|
||||
|
||||
export const app = new Hono();
|
||||
@@ -36,8 +42,63 @@ if (devBypassActive) {
|
||||
}
|
||||
|
||||
// OIDC callback — must be registered BEFORE oidcAuthMiddleware so the
|
||||
// authorization-code exchange is not itself intercepted by the auth check (T-02-02)
|
||||
app.get('/callback', (c) => processOAuthCallback(c));
|
||||
// authorization-code exchange is not itself intercepted by the auth check (T-02-02).
|
||||
//
|
||||
// Phase 19 — link mode (AUTH-LOCAL-10, T-19-15):
|
||||
// When POST /api/me/link-oidc initiates a link flow, a signed JWT state is included
|
||||
// in the authorization URL as the `state` parameter. On callback, we read that raw URL
|
||||
// state param, try to decode it as our signed JWT, and if it carries `linkUserId`, we
|
||||
// call linkOidcToUser after processOAuthCallback establishes the OIDC session.
|
||||
//
|
||||
// Security: the signed state JWT prevents CSRF (T-19-09); linkOidcToUser preflight
|
||||
// prevents account takeover via conflict (T-19-15 / T-19-08).
|
||||
//
|
||||
// Normal (non-link) callbacks are unaffected — processOAuthCallback is called in all paths.
|
||||
app.get('/callback', async (c) => {
|
||||
// Attempt to extract linkUserId from the signed state param BEFORE processOAuthCallback
|
||||
// consumes it. The state param may be our signed JWT (link mode) or a random string (normal).
|
||||
let linkUserId: number | null = null;
|
||||
const rawState = c.req.query('state');
|
||||
if (rawState) {
|
||||
const secret = process.env.LOCAL_SESSION_SECRET;
|
||||
if (secret) {
|
||||
try {
|
||||
const payload = await Jwt.verify(rawState, secret, 'HS256');
|
||||
if (typeof payload.linkUserId === 'number') {
|
||||
linkUserId = payload.linkUserId;
|
||||
}
|
||||
} catch {
|
||||
// Not our signed link state — normal OIDC callback, proceed normally.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Process the OIDC authorization-code exchange (sets the OIDC session cookie + redirects).
|
||||
const callbackResponse = await processOAuthCallback(c);
|
||||
|
||||
// Link mode: after session is established, bind the OIDC identity to the local user.
|
||||
if (linkUserId !== null) {
|
||||
try {
|
||||
const auth = await getAuth(c);
|
||||
if (auth) {
|
||||
const iss = (auth.iss as string | undefined) ?? '';
|
||||
const sub = auth.sub ?? '';
|
||||
await linkOidcToUser(linkUserId, iss, sub);
|
||||
// On success: user is now OIDC-only; normal redirect via callbackResponse proceeds.
|
||||
}
|
||||
} catch (err) {
|
||||
if (err instanceof OidcLinkConflictError) {
|
||||
// 409: iss+sub already linked to a different user — redirect to conflict error page.
|
||||
// UI-SPEC Surface 13 error copy: "This OIDC identity is already linked to another account."
|
||||
return c.redirect('/?error=oidc-link-conflict');
|
||||
}
|
||||
// Unexpected error during link binding — log and continue with normal redirect.
|
||||
console.error('[callback] linkOidcToUser unexpected error:', err instanceof Error ? err.message : String(err));
|
||||
}
|
||||
}
|
||||
|
||||
return callbackResponse;
|
||||
});
|
||||
|
||||
// GET /health — unauthenticated, mounted BEFORE the OIDC guard (T-01-03, T-02-05)
|
||||
app.route('/health', healthRouter);
|
||||
@@ -48,15 +109,26 @@ app.route('/health', healthRouter);
|
||||
// 423 lock after setup is complete (SETUP-04 / D-10).
|
||||
app.route('/api/setup', setupRouter);
|
||||
|
||||
// Phase 19 — pre-auth auth routes: GET /api/auth/mode and POST /api/auth/local/login, /logout.
|
||||
// Mounted BEFORE devAuthBypass so they are reachable without a session (D-01 / AUTH-LOCAL-05).
|
||||
app.route('/api/auth', authModeRouter);
|
||||
app.route('/api/auth', localAuthRouter);
|
||||
|
||||
// Dev-auth bypass — no-op passthrough unless DEV_AUTH_BYPASS=true AND NODE_ENV!='production'.
|
||||
// When active, injects DEV_USER into the Hono context and the OIDC guard is NOT mounted.
|
||||
// Must be mounted BEFORE oidcAuthMiddleware (T-02-01 mitigation; see auth/devBypass.ts).
|
||||
app.use('/api/*', devAuthBypass());
|
||||
|
||||
// Phase 19 — local-session middleware: sets c.get('user') from 'local-session' JWT cookie.
|
||||
// No-op passthrough when no cookie is present — the OIDC guard fires for unauthenticated.
|
||||
// Runs AFTER devAuthBypass (which may set c.get('user') first) and BEFORE the OIDC guard.
|
||||
// The OIDC guard below is wrapped to skip when c.get('user') is already set (Pitfall 1 guard).
|
||||
app.use('/api/*', localAuthMiddleware());
|
||||
|
||||
// Protect all /api/* routes with OIDC session middleware (AUTH-01, T-02-05).
|
||||
// Skipped entirely when devBypassActive so that local dev works without Authelia.
|
||||
// Skipped entirely when devBypassActive so that local dev works without the OIDC provider.
|
||||
// In production devBypassActive is always false — OIDC is unconditionally mounted.
|
||||
// Unauthenticated requests receive a 302 redirect to Authelia's authorize endpoint.
|
||||
// Unauthenticated requests receive a 302 redirect to the OIDC authorize endpoint.
|
||||
// OIDC_AUTH_EXTERNAL_URL is MANDATORY behind Pangolin to construct the correct
|
||||
// redirect_uri (Pitfall 1). Set it to https://familysync.<domain>.
|
||||
if (!devBypassActive) {
|
||||
@@ -67,7 +139,21 @@ if (!devBypassActive) {
|
||||
// A2 CONFIRMED: oidcAuthMiddleware() reads process.env per-request (call time), so
|
||||
// injecting into process.env here is safe and effective (see auth/middleware.ts A2 note).
|
||||
app.use('/api/*', oidcConfigFallbackMiddleware);
|
||||
app.use('/api/*', oidcAuthMiddleware());
|
||||
// Phase 19 / D-03: OIDC guard wrapped to skip when c.get('user') is already set.
|
||||
// A valid local-session (or dev-bypass) user must NOT be 302-redirected to the OIDC
|
||||
// provider — the skip-when-set wrapper is the coexistence seam (D-03 / RESEARCH Pitfall 1).
|
||||
//
|
||||
// IMPORTANT: oidcAuthMiddleware() factory is called ONCE at app construction time (not per
|
||||
// request) to match the prior behavior and keep test assertions about "called once" valid.
|
||||
// The returned handler is stored and invoked per-request inside the wrapper.
|
||||
const oidcHandler = oidcAuthMiddleware();
|
||||
app.use('/api/*', async (c, next) => {
|
||||
if (c.get('user')) {
|
||||
await next();
|
||||
return;
|
||||
}
|
||||
await oidcHandler(c, next);
|
||||
});
|
||||
// Re-issues the session-scoped oidc-auth cookie as persistent so PWA sessions survive close (AUTH-02).
|
||||
app.use('/api/*', persistSessionCookie());
|
||||
}
|
||||
@@ -76,7 +162,7 @@ if (!devBypassActive) {
|
||||
|
||||
// GET /api/login — login entry point for the PWA.
|
||||
// Flow (production): unauthenticated top-level nav hits the OIDC guard above,
|
||||
// which 302-redirects to Authelia. After login, Authelia POSTs to /callback,
|
||||
// which 302-redirects to the OIDC provider. After login, the provider POSTs to /callback,
|
||||
// the middleware sets a `continue` cookie pointing back to /api/login, and the
|
||||
// browser follows it here — now authenticated. The handler then redirects to /
|
||||
// so the SPA boots with a valid session. Under DEV_AUTH_BYPASS the guard is not
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
/**
|
||||
* authMode.ts — GET /api/auth/mode pre-auth endpoint (AUTH-LOCAL-05).
|
||||
*
|
||||
* Returns { localEnabled: boolean, oidcEnabled: boolean } reflecting the current
|
||||
* authentication configuration. This endpoint is intentionally reachable before
|
||||
* authentication (same pre-auth pattern as GET /api/setup/status).
|
||||
*
|
||||
* Mount in index.ts BEFORE devAuthBypass and OIDC guard:
|
||||
* app.route('/api/auth', authModeRouter); ← pre-auth
|
||||
*
|
||||
* Response contract:
|
||||
* - localEnabled: always true — local auth is the default, always available (D-01).
|
||||
* - oidcEnabled: true when OIDC_ISSUER env var is set, OR when app_config has
|
||||
* an oidc_issuer row (supports wizard-configured OIDC before container restart).
|
||||
*
|
||||
* No auth gate, no isSetupLocked() check — the PWA fetches this on app load before
|
||||
* knowing if the user is authenticated.
|
||||
*/
|
||||
|
||||
import { Hono } from 'hono';
|
||||
import { eq } from 'drizzle-orm';
|
||||
import { db } from '../db/client.js';
|
||||
import { appConfig } from '../db/schema.js';
|
||||
|
||||
export const authModeRouter = new Hono();
|
||||
|
||||
/**
|
||||
* GET / (mounted as /api/auth, so effective path is GET /api/auth/mode)
|
||||
*
|
||||
* Checks OIDC_ISSUER env var first (process.env is cheapest); falls back to
|
||||
* app_config DB row if env var is absent (wizard-configured OIDC).
|
||||
*/
|
||||
authModeRouter.get('/mode', async (c) => {
|
||||
// localEnabled: always true (D-01)
|
||||
// oidcEnabled: env var first, then app_config fallback
|
||||
const issuerFromEnv = process.env.OIDC_ISSUER;
|
||||
let oidcEnabled = Boolean(issuerFromEnv);
|
||||
|
||||
if (!oidcEnabled) {
|
||||
// Check app_config for oidc_issuer (wizard-written, pre-restart-fallback pattern from middleware.ts)
|
||||
const [row] = await db
|
||||
.select({ value: appConfig.value })
|
||||
.from(appConfig)
|
||||
.where(eq(appConfig.key, 'oidc_issuer'))
|
||||
.limit(1);
|
||||
oidcEnabled = Boolean(row?.value);
|
||||
}
|
||||
|
||||
return c.json({ localEnabled: true, oidcEnabled });
|
||||
});
|
||||
@@ -0,0 +1,164 @@
|
||||
/**
|
||||
* localAuth.ts — local authentication routes (AUTH-LOCAL-03, AUTH-LOCAL-06).
|
||||
*
|
||||
* Routes:
|
||||
* POST /api/auth/local/login — rate-limited credential verify + session cookie issue
|
||||
* POST /api/auth/local/logout — clear local-session cookie
|
||||
* GET /api/auth/local/logout — alias for POST /logout (some browsers prefer GET)
|
||||
*
|
||||
* Mounted in index.ts as app.route('/api/auth', localAuthRouter) BEFORE any auth middleware.
|
||||
* This means POST /api/auth/local/login is reachable without a session (pre-auth, per D-01).
|
||||
*
|
||||
* Security (T-19-11, T-19-12, T-19-14):
|
||||
* - noEchoHook on login: Zod errors NEVER return received values (T-19-14 / Pitfall 7).
|
||||
* - Dummy-hash timing defense: verifyPassword is always called, even for unknown usernames,
|
||||
* to prevent timing-oracle username enumeration attacks (T-19-12 / RESEARCH Pitfall 2).
|
||||
* - Same 401 body for wrong password AND unknown username — no field discrimination (T-19-12).
|
||||
* - Per-IP in-memory rate-limiting: 5 failures → 429; 10 failures → 423 (T-19-11).
|
||||
* - Lockout (423) cleared only by admin password reset — no self-service unlock.
|
||||
*/
|
||||
|
||||
import { Hono } from 'hono';
|
||||
import type { Context } from 'hono';
|
||||
import { zValidator } from '@hono/zod-validator';
|
||||
import { z } from 'zod';
|
||||
import { eq } from 'drizzle-orm';
|
||||
import { db } from '../db/client.js';
|
||||
import { localCredentials } from '../db/schema.js';
|
||||
import { verifyPassword, hashPassword } from '../auth/localCredentials.js';
|
||||
import { issueLocalSessionCookie, clearLocalSessionCookie } from '../auth/localSession.js';
|
||||
|
||||
export const localAuthRouter = new Hono();
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// noEchoHook — NEVER return Zod validation error details for the login route.
|
||||
// Zod's error object contains issues[].received which may echo the submitted password.
|
||||
// Always return { error: 'Invalid request' } 400, no other fields (T-19-14).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const noEchoHook = (result: { success: boolean }, c: Context) => {
|
||||
if (!result.success) {
|
||||
return c.json({ error: 'Invalid request' }, 400);
|
||||
}
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Zod schema for login body
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const loginSchema = z.object({
|
||||
username: z.string().min(1).max(128).trim(),
|
||||
password: z.string().min(1).max(1000),
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Per-IP rate-limiting state (in-memory Map — household scale, no Redis needed).
|
||||
//
|
||||
// State shape per IP: { count, lockedUntil (epoch ms), lockedOut (bool) }
|
||||
// count >= RATE_WINDOW_FAILURES AND Date.now() < lockedUntil → 429
|
||||
// count >= LOCKOUT_FAILURES → 423 (permanent until admin reset)
|
||||
// Success → delete entry (clears counter)
|
||||
//
|
||||
// RATE_WINDOW_FAILURES: 5 failures → 60s cooldown (429)
|
||||
// LOCKOUT_FAILURES: 10 failures → account locked (423)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export const loginAttempts = new Map<string, { count: number; lockedUntil: number; lockedOut: boolean }>();
|
||||
|
||||
const RATE_WINDOW_FAILURES = 5;
|
||||
const RATE_WINDOW_SECS = 60;
|
||||
const LOCKOUT_FAILURES = 10;
|
||||
|
||||
// Pre-computed dummy hash used to run verifyPassword on unknown-username paths
|
||||
// (timing defense — prevents timing-oracle username enumeration, RESEARCH Pitfall 2).
|
||||
// Computed once at module load time; the actual value is never used for auth.
|
||||
const DUMMY_HASH = hashPassword('dummy-constant-time-filler-xyzzy');
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// POST /local/login → POST /api/auth/local/login
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook), async (c) => {
|
||||
// Derive client IP from Pangolin-set X-Forwarded-For header; fall back to host header.
|
||||
const ip = c.req.header('x-forwarded-for')?.split(',')[0]?.trim()
|
||||
?? c.req.raw.headers.get('host')
|
||||
?? 'unknown';
|
||||
|
||||
const attempt = loginAttempts.get(ip);
|
||||
|
||||
// 423: account locked (>= LOCKOUT_FAILURES total failures, admin must reset)
|
||||
// Check lockedOut FIRST — lockout takes precedence over rate window.
|
||||
if (attempt?.lockedOut) {
|
||||
return c.json({ error: 'Account locked' }, 423);
|
||||
}
|
||||
|
||||
// 429: rate window — >= RATE_WINDOW_FAILURES failures within the cooldown window.
|
||||
// Increment the counter even on 429 so continued brute-force accumulates toward lockout.
|
||||
if (attempt && attempt.count >= RATE_WINDOW_FAILURES && Date.now() < attempt.lockedUntil) {
|
||||
attempt.count += 1;
|
||||
attempt.lockedUntil = Date.now() + RATE_WINDOW_SECS * 1000;
|
||||
attempt.lockedOut = attempt.count >= LOCKOUT_FAILURES;
|
||||
loginAttempts.set(ip, attempt);
|
||||
if (attempt.lockedOut) {
|
||||
return c.json({ error: 'Account locked' }, 423);
|
||||
}
|
||||
return c.json({ error: 'Too many attempts' }, 429);
|
||||
}
|
||||
|
||||
const { username, password } = c.req.valid('json');
|
||||
|
||||
// Look up local_credentials by username
|
||||
let cred: { userId: number; passwordHash: string } | undefined;
|
||||
try {
|
||||
const [found] = await db
|
||||
.select({ userId: localCredentials.userId, passwordHash: localCredentials.passwordHash })
|
||||
.from(localCredentials)
|
||||
.where(eq(localCredentials.username, username))
|
||||
.limit(1);
|
||||
cred = found;
|
||||
} catch (err) {
|
||||
console.error('[localAuth/POST /local/login] DB error:', err instanceof Error ? err.message : String(err));
|
||||
return c.json({ error: 'Service unavailable' }, 503);
|
||||
}
|
||||
|
||||
// ALWAYS run verifyPassword — even for unknown usernames — to prevent timing-oracle
|
||||
// username enumeration (T-19-12 / RESEARCH Pitfall 2). Use a pre-computed dummy hash
|
||||
// so the scrypt work is always performed regardless of whether username was found.
|
||||
const valid = cred
|
||||
? verifyPassword(cred.passwordHash, password)
|
||||
: verifyPassword(DUMMY_HASH, password);
|
||||
|
||||
if (!valid || !cred) {
|
||||
// Increment failure counter
|
||||
const cur = loginAttempts.get(ip) ?? { count: 0, lockedUntil: 0, lockedOut: false };
|
||||
cur.count += 1;
|
||||
cur.lockedUntil = Date.now() + RATE_WINDOW_SECS * 1000;
|
||||
cur.lockedOut = cur.count >= LOCKOUT_FAILURES;
|
||||
loginAttempts.set(ip, cur);
|
||||
// Same 401 body for wrong password AND unknown username — no field discrimination (T-19-12)
|
||||
return c.json({ error: 'Invalid credentials' }, 401);
|
||||
}
|
||||
|
||||
// Success: clear failure counter, issue the local-session JWT cookie, return ok
|
||||
loginAttempts.delete(ip);
|
||||
try {
|
||||
await issueLocalSessionCookie(c, cred.userId);
|
||||
} catch (err) {
|
||||
console.error('[localAuth/POST /local/login] Cookie issue error:', err instanceof Error ? err.message : String(err));
|
||||
return c.json({ error: 'Service unavailable' }, 503);
|
||||
}
|
||||
return c.json({ ok: true }, 200);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// POST /local/logout → POST /api/auth/local/logout
|
||||
// GET /local/logout → GET /api/auth/local/logout (browser-redirect alias)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
async function handleLogout(c: Context) {
|
||||
clearLocalSessionCookie(c);
|
||||
return c.json({ ok: true }, 200);
|
||||
}
|
||||
|
||||
localAuthRouter.post('/local/logout', handleLogout);
|
||||
localAuthRouter.get('/local/logout', handleLogout);
|
||||
@@ -0,0 +1,246 @@
|
||||
/**
|
||||
* localAuthMiddleware() — unit tests (Plan 19-03, TDD RED → GREEN).
|
||||
*
|
||||
* Covers:
|
||||
* Test 1: valid local-session cookie for an existing user → c.get('user') set; next() called
|
||||
* Test 2: no cookie → pure passthrough; c.get('user') NOT set (OIDC guard fall-through intact)
|
||||
* Test 3: cookie with valid JWT but userId has no users row → passthrough (no crash)
|
||||
* Test 4: c.get('user') already set (devAuthBypass ran first) → not overwritten; next() called
|
||||
*
|
||||
* Security:
|
||||
* - Test 2 is the Pitfall-1 guard: middleware must NEVER call c.set('user', undefined).
|
||||
* The OIDC guard only fires when c.get('user') is falsy; setting it to undefined
|
||||
* would suppress the OIDC guard for unauthenticated requests.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { Hono } from 'hono';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Mock DB — avoids real DB connections in this middleware unit-test
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const mockDbSelectResult: Array<{
|
||||
id: number;
|
||||
oidcIss: string | null;
|
||||
oidcSub: string | null;
|
||||
displayName: string | null;
|
||||
color: string;
|
||||
}> = [];
|
||||
|
||||
vi.mock('../../src/db/client.js', () => ({
|
||||
db: {
|
||||
select: vi.fn().mockReturnValue({
|
||||
from: vi.fn().mockReturnValue({
|
||||
where: vi.fn().mockReturnValue({
|
||||
limit: vi.fn().mockImplementation(() => Promise.resolve(mockDbSelectResult)),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Mock verifyLocalSessionCookie — controls what userId the cookie yields
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
let mockVerifyResult: number | null = null;
|
||||
|
||||
vi.mock('../../src/auth/localSession.js', () => ({
|
||||
verifyLocalSessionCookie: vi.fn().mockImplementation(() => Promise.resolve(mockVerifyResult)),
|
||||
issueLocalSessionCookie: vi.fn(),
|
||||
clearLocalSessionCookie: vi.fn(),
|
||||
}));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
async function getMiddleware() {
|
||||
const { localAuthMiddleware } = await import('../../src/auth/localAuthMiddleware.js');
|
||||
return localAuthMiddleware;
|
||||
}
|
||||
|
||||
function makeApp(middleware: ReturnType<typeof vi.fn>, presetUser?: unknown) {
|
||||
const app = new Hono();
|
||||
|
||||
if (presetUser !== undefined) {
|
||||
// Simulate devAuthBypass having already set c.get('user')
|
||||
app.use('/api/*', async (c, next) => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
c.set('user', presetUser as any);
|
||||
await next();
|
||||
});
|
||||
}
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
|
||||
app.use('/api/*', middleware());
|
||||
|
||||
let capturedUser: unknown = 'NOT_SET_SENTINEL';
|
||||
let nextCalled = false;
|
||||
|
||||
app.get('/api/test', (c) => {
|
||||
capturedUser = c.get('user');
|
||||
nextCalled = true;
|
||||
return c.json({ ok: true });
|
||||
});
|
||||
|
||||
return { app, getCapturedUser: () => capturedUser, wasNextCalled: () => nextCalled };
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('localAuthMiddleware', () => {
|
||||
beforeEach(() => {
|
||||
mockVerifyResult = null;
|
||||
mockDbSelectResult.splice(0);
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
it('Test 1: valid local-session cookie for existing user → sets c.get("user") and calls next', async () => {
|
||||
mockVerifyResult = 42;
|
||||
mockDbSelectResult.push({
|
||||
id: 42,
|
||||
oidcIss: 'local',
|
||||
oidcSub: '42',
|
||||
displayName: 'Test User',
|
||||
color: '#4A90D9',
|
||||
});
|
||||
|
||||
const localAuthMiddleware = await getMiddleware();
|
||||
const { app } = makeApp(localAuthMiddleware);
|
||||
|
||||
const res = await app.request('/api/test');
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
// Re-import to inspect captured value via the route handler's closure
|
||||
// We verify by checking response — route returns 200 only if next() was called
|
||||
// The actual user value is verified via the app route handler
|
||||
const body = (await res.json()) as { ok: boolean };
|
||||
expect(body.ok).toBe(true);
|
||||
});
|
||||
|
||||
it('Test 1b: user shape from DB row is correct (id, oidcIss, oidcSub, displayName, color)', async () => {
|
||||
mockVerifyResult = 7;
|
||||
mockDbSelectResult.push({
|
||||
id: 7,
|
||||
oidcIss: 'https://auth.example.com',
|
||||
oidcSub: 'sub-abc',
|
||||
displayName: 'Alice',
|
||||
color: '#FF5733',
|
||||
});
|
||||
|
||||
const localAuthMiddleware = await getMiddleware();
|
||||
const app = new Hono();
|
||||
let capturedUser: unknown;
|
||||
|
||||
app.use('/api/*', localAuthMiddleware());
|
||||
app.get('/api/test', (c) => {
|
||||
capturedUser = c.get('user');
|
||||
return c.json({ ok: true });
|
||||
});
|
||||
|
||||
const res = await app.request('/api/test');
|
||||
expect(res.status).toBe(200);
|
||||
expect(capturedUser).toBeDefined();
|
||||
const u = capturedUser as { id: number; oidcIss: string; oidcSub: string; displayName: string | null; color: string };
|
||||
expect(u.id).toBe(7);
|
||||
expect(u.oidcIss).toBe('https://auth.example.com');
|
||||
expect(u.oidcSub).toBe('sub-abc');
|
||||
expect(u.displayName).toBe('Alice');
|
||||
expect(u.color).toBe('#FF5733');
|
||||
});
|
||||
|
||||
it('Test 2: no cookie → pure passthrough; c.get("user") remains unset (Pitfall-1 guard)', async () => {
|
||||
mockVerifyResult = null; // No cookie / invalid
|
||||
|
||||
const localAuthMiddleware = await getMiddleware();
|
||||
const app = new Hono();
|
||||
let capturedUser: unknown = 'NOT_SET_SENTINEL';
|
||||
let nextCalled = false;
|
||||
|
||||
app.use('/api/*', localAuthMiddleware());
|
||||
app.get('/api/test', (c) => {
|
||||
capturedUser = c.get('user');
|
||||
nextCalled = true;
|
||||
return c.json({ ok: true });
|
||||
});
|
||||
|
||||
const res = await app.request('/api/test');
|
||||
expect(res.status).toBe(200);
|
||||
expect(nextCalled).toBe(true);
|
||||
// CRITICAL: user must remain UNSET (undefined), NOT set to undefined explicitly.
|
||||
// The OIDC guard checks c.get('user') — if it's undefined (not set), the guard fires.
|
||||
// The middleware must call next() without c.set('user') on the no-cookie path.
|
||||
expect(capturedUser).toBeUndefined();
|
||||
});
|
||||
|
||||
it('Test 3: valid cookie but userId has no users row → passthrough (no crash)', async () => {
|
||||
mockVerifyResult = 999; // Valid JWT payload, userId=999
|
||||
mockDbSelectResult.splice(0); // No DB row for userId 999
|
||||
|
||||
const localAuthMiddleware = await getMiddleware();
|
||||
const app = new Hono();
|
||||
let capturedUser: unknown = 'NOT_SET_SENTINEL';
|
||||
let nextCalled = false;
|
||||
|
||||
app.use('/api/*', localAuthMiddleware());
|
||||
app.get('/api/test', (c) => {
|
||||
capturedUser = c.get('user');
|
||||
nextCalled = true;
|
||||
return c.json({ ok: true });
|
||||
});
|
||||
|
||||
const res = await app.request('/api/test');
|
||||
expect(res.status).toBe(200);
|
||||
expect(nextCalled).toBe(true);
|
||||
// No row found — should passthrough without setting user
|
||||
expect(capturedUser).toBeUndefined();
|
||||
});
|
||||
|
||||
it('Test 4: c.get("user") already set (devAuthBypass ran first) → not overwritten; next() called', async () => {
|
||||
const preExistingUser = {
|
||||
id: 1,
|
||||
oidcIss: 'dev',
|
||||
oidcSub: 'dev-user',
|
||||
displayName: 'Dev User',
|
||||
color: '#4A90D9',
|
||||
};
|
||||
|
||||
// Even if verifyLocalSessionCookie would succeed, the existing user must not be overwritten
|
||||
mockVerifyResult = 99; // A DIFFERENT userId
|
||||
mockDbSelectResult.push({
|
||||
id: 99,
|
||||
oidcIss: 'local',
|
||||
oidcSub: '99',
|
||||
displayName: 'Another User',
|
||||
color: '#FF0000',
|
||||
});
|
||||
|
||||
const localAuthMiddleware = await getMiddleware();
|
||||
const app = new Hono();
|
||||
let capturedUser: unknown;
|
||||
|
||||
// Simulate devAuthBypass having set the user first
|
||||
app.use('/api/*', async (c, next) => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
c.set('user', preExistingUser as any);
|
||||
await next();
|
||||
});
|
||||
app.use('/api/*', localAuthMiddleware());
|
||||
app.get('/api/test', (c) => {
|
||||
capturedUser = c.get('user');
|
||||
return c.json({ ok: true });
|
||||
});
|
||||
|
||||
const res = await app.request('/api/test');
|
||||
expect(res.status).toBe(200);
|
||||
// User must remain the dev-bypass user, not overwritten
|
||||
expect(capturedUser).toEqual(preExistingUser);
|
||||
expect((capturedUser as typeof preExistingUser).id).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,123 @@
|
||||
/**
|
||||
* GET /api/auth/mode — unit tests (Plan 19-03, TDD RED → GREEN).
|
||||
*
|
||||
* Covers:
|
||||
* Test 5: returns { localEnabled:true, oidcEnabled:false } when no oidc_issuer in env or app_config
|
||||
* Test 6: returns { oidcEnabled:true } when OIDC_ISSUER env var is set
|
||||
* Test 6b: returns { oidcEnabled:true } when oidc_issuer is in app_config (no env var)
|
||||
*
|
||||
* Pre-auth surface: reachable without OIDC session (same as /api/setup/status).
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// DB mock — controls app_config rows returned for oidc_issuer
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
let mockAppConfigOidcIssuer: string | null = null;
|
||||
|
||||
vi.mock('../../src/db/client.js', () => ({
|
||||
db: {
|
||||
execute: vi.fn().mockResolvedValue([[{ '1': 1 }]]),
|
||||
select: vi.fn().mockImplementation(() => ({
|
||||
from: vi.fn().mockReturnValue({
|
||||
where: vi.fn().mockReturnValue({
|
||||
limit: vi.fn().mockImplementation(() => {
|
||||
if (mockAppConfigOidcIssuer) {
|
||||
return Promise.resolve([{ value: mockAppConfigOidcIssuer }]);
|
||||
}
|
||||
return Promise.resolve([]);
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
})),
|
||||
insert: vi.fn().mockReturnValue({
|
||||
values: vi.fn().mockReturnValue({
|
||||
onDuplicateKeyUpdate: vi.fn().mockResolvedValue(undefined),
|
||||
}),
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('@hono/oidc-auth', () => ({
|
||||
oidcAuthMiddleware: () => async (_c: unknown, next: () => Promise<void>) => next(),
|
||||
processOAuthCallback: () => async (c: { json: (v: unknown) => unknown }) => c.json({ ok: true }),
|
||||
getAuth: vi.fn().mockResolvedValue(null),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/auth/devBypass.js', () => ({
|
||||
devAuthBypass:
|
||||
() => async (_c: unknown, next: () => Promise<void>) => next(),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/auth/localAuthMiddleware.js', () => ({
|
||||
localAuthMiddleware:
|
||||
() => async (_c: unknown, next: () => Promise<void>) => next(),
|
||||
}));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Env snapshot
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const originalOidcIssuer = process.env.OIDC_ISSUER;
|
||||
|
||||
beforeEach(() => {
|
||||
mockAppConfigOidcIssuer = null;
|
||||
delete process.env.OIDC_ISSUER;
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalOidcIssuer === undefined) {
|
||||
delete process.env.OIDC_ISSUER;
|
||||
} else {
|
||||
process.env.OIDC_ISSUER = originalOidcIssuer;
|
||||
}
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('GET /api/auth/mode', () => {
|
||||
it('Test 5: returns { localEnabled:true, oidcEnabled:false } when no oidc_issuer in env or app_config', async () => {
|
||||
delete process.env.OIDC_ISSUER;
|
||||
mockAppConfigOidcIssuer = null;
|
||||
|
||||
const { app } = await import('../../src/index.js');
|
||||
const res = await app.request('/api/auth/mode');
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as { localEnabled: boolean; oidcEnabled: boolean };
|
||||
expect(body.localEnabled).toBe(true);
|
||||
expect(body.oidcEnabled).toBe(false);
|
||||
});
|
||||
|
||||
it('Test 6: returns { oidcEnabled:true } when OIDC_ISSUER env var is set', async () => {
|
||||
process.env.OIDC_ISSUER = 'https://auth.example.com';
|
||||
mockAppConfigOidcIssuer = null;
|
||||
|
||||
const { app } = await import('../../src/index.js');
|
||||
const res = await app.request('/api/auth/mode');
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as { localEnabled: boolean; oidcEnabled: boolean };
|
||||
expect(body.localEnabled).toBe(true);
|
||||
expect(body.oidcEnabled).toBe(true);
|
||||
});
|
||||
|
||||
it('Test 6b: returns { oidcEnabled:true } when oidc_issuer is in app_config (no env var)', async () => {
|
||||
delete process.env.OIDC_ISSUER;
|
||||
mockAppConfigOidcIssuer = 'https://auth-from-config.example.com';
|
||||
|
||||
const { app } = await import('../../src/index.js');
|
||||
const res = await app.request('/api/auth/mode');
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as { localEnabled: boolean; oidcEnabled: boolean };
|
||||
expect(body.localEnabled).toBe(true);
|
||||
expect(body.oidcEnabled).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,313 @@
|
||||
/**
|
||||
* POST /api/auth/local/login + POST/GET /api/auth/local/logout — tests (Plan 19-03, TDD RED → GREEN).
|
||||
*
|
||||
* Covers:
|
||||
* Test 1: valid username+password → 200 { ok:true } + Set-Cookie for local-session
|
||||
* Test 2: wrong password → 401 { error: 'Invalid credentials' }
|
||||
* Test 3: unknown username → 401 with SAME body as Test 2 (no enumeration / no field discrimination)
|
||||
* Test 4: 5 consecutive failures from one IP → 6th returns 429
|
||||
* Test 5: 10 failures → 423 (lockedOut); a cleared map resets the counter
|
||||
* Test 6: POST /api/auth/local/logout clears the local-session cookie (expired Set-Cookie)
|
||||
* Test 6b: GET /api/auth/local/logout (alias) also clears the local-session cookie
|
||||
* Test 7 (no-echo): malformed body (missing password) → 400 { error: 'Invalid request' };
|
||||
* body must NOT contain submitted value or Zod 'received' field
|
||||
*
|
||||
* Architecture:
|
||||
* Tests mock DB client and issueLocalSessionCookie/clearLocalSessionCookie.
|
||||
* loginAttempts Map is imported directly and cleared between tests.
|
||||
* IP is derived from x-forwarded-for header.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Mock DB client
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
type LocalCredRow = { userId: number; passwordHash: string } | undefined;
|
||||
let mockCredRow: LocalCredRow;
|
||||
|
||||
vi.mock('../../src/db/client.js', () => ({
|
||||
db: {
|
||||
execute: vi.fn().mockResolvedValue([[{ '1': 1 }]]),
|
||||
select: vi.fn().mockImplementation(() => ({
|
||||
from: vi.fn().mockReturnValue({
|
||||
where: vi.fn().mockReturnValue({
|
||||
limit: vi.fn().mockImplementation(() =>
|
||||
Promise.resolve(mockCredRow ? [mockCredRow] : [])
|
||||
),
|
||||
}),
|
||||
}),
|
||||
})),
|
||||
insert: vi.fn().mockReturnValue({
|
||||
values: vi.fn().mockReturnValue({
|
||||
onDuplicateKeyUpdate: vi.fn().mockResolvedValue(undefined),
|
||||
}),
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Mock localSession helpers — track calls and control cookie behavior
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
let issueSessionCalled = false;
|
||||
let issuedUserId: number | null = null;
|
||||
let clearSessionCalled = false;
|
||||
|
||||
vi.mock('../../src/auth/localSession.js', () => ({
|
||||
issueLocalSessionCookie: vi.fn().mockImplementation(
|
||||
(_c: unknown, userId: number) => {
|
||||
issueSessionCalled = true;
|
||||
issuedUserId = userId;
|
||||
// Simulate setting a cookie on the context
|
||||
return Promise.resolve();
|
||||
}
|
||||
),
|
||||
clearLocalSessionCookie: vi.fn().mockImplementation((_c: unknown) => {
|
||||
clearSessionCalled = true;
|
||||
}),
|
||||
verifyLocalSessionCookie: vi.fn().mockResolvedValue(null),
|
||||
}));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Mock devAuthBypass and OIDC — standard passthrough for route tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
vi.mock('../../src/auth/devBypass.js', () => ({
|
||||
devAuthBypass:
|
||||
() => async (_c: unknown, next: () => Promise<void>) => next(),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/auth/localAuthMiddleware.js', () => ({
|
||||
localAuthMiddleware:
|
||||
() => async (_c: unknown, next: () => Promise<void>) => next(),
|
||||
}));
|
||||
|
||||
vi.mock('@hono/oidc-auth', () => ({
|
||||
oidcAuthMiddleware: () => async (_c: unknown, next: () => Promise<void>) => next(),
|
||||
processOAuthCallback: () => async (c: { json: (v: unknown) => unknown }) => c.json({ ok: true }),
|
||||
getAuth: vi.fn().mockResolvedValue(null),
|
||||
}));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
function makeLoginRequest(body: Record<string, string>, ip = '1.2.3.4'): Request {
|
||||
return new Request('http://localhost/api/auth/local/login', {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
'x-forwarded-for': ip,
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
}
|
||||
|
||||
async function getApp() {
|
||||
const { app } = await import('../../src/index.js');
|
||||
return app;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Test state
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const originalNodeEnv = process.env.NODE_ENV;
|
||||
const originalBypass = process.env.DEV_AUTH_BYPASS;
|
||||
const originalSecret = process.env.LOCAL_SESSION_SECRET;
|
||||
|
||||
beforeEach(async () => {
|
||||
// Use dev-bypass mode so no OIDC redirect occurs
|
||||
process.env.NODE_ENV = 'test';
|
||||
process.env.DEV_AUTH_BYPASS = 'true';
|
||||
// Provide a valid LOCAL_SESSION_SECRET for the session helpers
|
||||
process.env.LOCAL_SESSION_SECRET = 'test-secret-that-is-at-least-32-chars!!';
|
||||
|
||||
issueSessionCalled = false;
|
||||
issuedUserId = null;
|
||||
clearSessionCalled = false;
|
||||
mockCredRow = undefined;
|
||||
|
||||
vi.resetModules();
|
||||
|
||||
// Clear the rate-limit map between tests
|
||||
const { loginAttempts } = await import('../../src/routes/localAuth.js');
|
||||
loginAttempts.clear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env.NODE_ENV = originalNodeEnv;
|
||||
if (originalBypass === undefined) delete process.env.DEV_AUTH_BYPASS;
|
||||
else process.env.DEV_AUTH_BYPASS = originalBypass;
|
||||
if (originalSecret === undefined) delete process.env.LOCAL_SESSION_SECRET;
|
||||
else process.env.LOCAL_SESSION_SECRET = originalSecret;
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Import verifyPassword/hashPassword for test credential setup
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
async function getLocalCredentials() {
|
||||
return import('../../src/auth/localCredentials.js');
|
||||
}
|
||||
|
||||
// ===========================================================================
|
||||
// POST /api/auth/local/login
|
||||
// ===========================================================================
|
||||
|
||||
describe('POST /api/auth/local/login', () => {
|
||||
it('Test 1: valid username+password → 200 { ok:true } and issueLocalSessionCookie called', async () => {
|
||||
const { hashPassword } = await getLocalCredentials();
|
||||
const hash = hashPassword('correcthorse');
|
||||
mockCredRow = { userId: 5, passwordHash: hash };
|
||||
|
||||
const app = await getApp();
|
||||
const res = await app.fetch(makeLoginRequest({ username: 'alice', password: 'correcthorse' }));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as { ok: boolean };
|
||||
expect(body.ok).toBe(true);
|
||||
expect(issueSessionCalled).toBe(true);
|
||||
expect(issuedUserId).toBe(5);
|
||||
});
|
||||
|
||||
it('Test 2: wrong password → 401 { error: "Invalid credentials" }', async () => {
|
||||
const { hashPassword } = await getLocalCredentials();
|
||||
const hash = hashPassword('correcthorse');
|
||||
mockCredRow = { userId: 5, passwordHash: hash };
|
||||
|
||||
const app = await getApp();
|
||||
const res = await app.fetch(makeLoginRequest({ username: 'alice', password: 'wrongpassword' }));
|
||||
|
||||
expect(res.status).toBe(401);
|
||||
const body = (await res.json()) as { error: string };
|
||||
expect(body.error).toBe('Invalid credentials');
|
||||
expect(issueSessionCalled).toBe(false);
|
||||
});
|
||||
|
||||
it('Test 3: unknown username → 401 with SAME body as wrong password (no enumeration)', async () => {
|
||||
mockCredRow = undefined; // No credential row found
|
||||
|
||||
const app = await getApp();
|
||||
const res = await app.fetch(makeLoginRequest({ username: 'unknown-user', password: 'anypassword' }));
|
||||
|
||||
expect(res.status).toBe(401);
|
||||
const body = (await res.json()) as { error: string };
|
||||
// CRITICAL: same body as wrong-password case (Test 2) — no field discrimination
|
||||
expect(body.error).toBe('Invalid credentials');
|
||||
expect(issueSessionCalled).toBe(false);
|
||||
});
|
||||
|
||||
it('Test 4: 5 consecutive failures → 6th attempt returns 429', async () => {
|
||||
mockCredRow = undefined; // Always unknown — every attempt fails
|
||||
|
||||
const app = await getApp();
|
||||
|
||||
// 5 failures to trigger the rate window
|
||||
for (let i = 0; i < 5; i++) {
|
||||
const res = await app.fetch(
|
||||
makeLoginRequest({ username: 'alice', password: 'wrong' }, '10.0.0.1')
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
}
|
||||
|
||||
// 6th attempt from same IP → 429
|
||||
const res6 = await app.fetch(
|
||||
makeLoginRequest({ username: 'alice', password: 'wrong' }, '10.0.0.1')
|
||||
);
|
||||
expect(res6.status).toBe(429);
|
||||
const body = (await res6.json()) as { error: string };
|
||||
expect(body.error).toBe('Too many attempts');
|
||||
});
|
||||
|
||||
it('Test 5: 10 failures → 423 (account locked); cleared map resets counter', async () => {
|
||||
mockCredRow = undefined;
|
||||
|
||||
const app = await getApp();
|
||||
|
||||
// 10 failures from same IP → lockout
|
||||
for (let i = 0; i < 10; i++) {
|
||||
await app.fetch(
|
||||
makeLoginRequest({ username: 'alice', password: 'wrong' }, '10.0.0.2')
|
||||
);
|
||||
}
|
||||
|
||||
// 11th attempt → 423 (locked)
|
||||
const res11 = await app.fetch(
|
||||
makeLoginRequest({ username: 'alice', password: 'wrong' }, '10.0.0.2')
|
||||
);
|
||||
expect(res11.status).toBe(423);
|
||||
const body = (await res11.json()) as { error: string };
|
||||
expect(body.error).toBe('Account locked');
|
||||
|
||||
// Clear the map (simulates admin reset) → counter gone → next attempt is 401 again (not locked)
|
||||
const { loginAttempts } = await import('../../src/routes/localAuth.js');
|
||||
loginAttempts.delete('10.0.0.2');
|
||||
|
||||
const resAfterReset = await app.fetch(
|
||||
makeLoginRequest({ username: 'alice', password: 'wrong' }, '10.0.0.2')
|
||||
);
|
||||
expect(resAfterReset.status).toBe(401);
|
||||
});
|
||||
|
||||
it('Test 7 (no-echo): malformed body (missing password) → 400 { error: "Invalid request" }; body has no echoed value or Zod received field', async () => {
|
||||
const app = await getApp();
|
||||
// Body with username but missing password (Zod will reject)
|
||||
const res = await app.fetch(
|
||||
new Request('http://localhost/api/auth/local/login', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', 'x-forwarded-for': '1.2.3.4' },
|
||||
body: JSON.stringify({ username: 'mysecretusername' }),
|
||||
})
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
const bodyText = await res.text();
|
||||
const parsed = JSON.parse(bodyText) as { error: string };
|
||||
expect(parsed.error).toBe('Invalid request');
|
||||
// CRITICAL (no-echo): the response must NOT contain the submitted value or Zod 'received' field
|
||||
expect(bodyText).not.toContain('mysecretusername');
|
||||
expect(bodyText).not.toContain('received');
|
||||
});
|
||||
});
|
||||
|
||||
// ===========================================================================
|
||||
// POST /api/auth/local/logout + GET /api/auth/local/logout
|
||||
// ===========================================================================
|
||||
|
||||
describe('POST /api/auth/local/logout', () => {
|
||||
it('Test 6: POST /logout → 200 { ok:true } and clearLocalSessionCookie called', async () => {
|
||||
const app = await getApp();
|
||||
const res = await app.fetch(
|
||||
new Request('http://localhost/api/auth/local/logout', {
|
||||
method: 'POST',
|
||||
headers: { 'x-forwarded-for': '1.2.3.4' },
|
||||
})
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as { ok: boolean };
|
||||
expect(body.ok).toBe(true);
|
||||
expect(clearSessionCalled).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('GET /api/auth/local/logout', () => {
|
||||
it('Test 6b: GET /logout (alias) → 200 { ok:true } and clearLocalSessionCookie called', async () => {
|
||||
const app = await getApp();
|
||||
const res = await app.fetch(
|
||||
new Request('http://localhost/api/auth/local/logout', {
|
||||
method: 'GET',
|
||||
headers: { 'x-forwarded-for': '1.2.3.4' },
|
||||
})
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as { ok: boolean };
|
||||
expect(body.ok).toBe(true);
|
||||
expect(clearSessionCalled).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user