diff --git a/client/src/pages/LoginPage.oidc-redirect.test.tsx b/client/src/pages/LoginPage.oidc-redirect.test.tsx index 2d0b8d005..4774f58a6 100644 --- a/client/src/pages/LoginPage.oidc-redirect.test.tsx +++ b/client/src/pages/LoginPage.oidc-redirect.test.tsx @@ -63,7 +63,7 @@ describe('LoginPage — OIDC redirect preservation', () => { beforeEach(() => { server.use( http.get('/api/auth/oidc/exchange', () => - HttpResponse.json({ token: 'mock-oidc-token' }) + HttpResponse.json({ success: true, token: 'mock-oidc-token' }) ), ); }); diff --git a/client/src/pages/login/useLogin.test.tsx b/client/src/pages/login/useLogin.test.tsx index 016901661..28eedf41c 100644 --- a/client/src/pages/login/useLogin.test.tsx +++ b/client/src/pages/login/useLogin.test.tsx @@ -299,7 +299,7 @@ describe('useLogin — OIDC callback', () => { it('FE-LOGIN-HOOK-016: navigates to the stashed redirect after a successful code exchange', async () => { sessionStorage.setItem('oidc_redirect', '/oauth/consent?client_id=foo'); setSearch('?oidc_code=code-1'); - server.use(http.get('/api/auth/oidc/exchange', () => HttpResponse.json({ token: 'tok' }))); + server.use(http.get('/api/auth/oidc/exchange', () => HttpResponse.json({ success: true, token: 'tok' }))); const { result } = renderLogin(); @@ -316,7 +316,7 @@ describe('useLogin — OIDC callback', () => { server.use( http.get('/api/auth/oidc/exchange', () => { exchanges += 1; - return HttpResponse.json({ token: 'tok' }); + return HttpResponse.json({ success: true, token: 'tok' }); }), ); @@ -333,6 +333,28 @@ describe('useLogin — OIDC callback', () => { expect(mockNavigate).toHaveBeenCalledTimes(1); }); + it('FE-LOGIN-HOOK-211: signs in on the success flag alone, never reading the deprecated token', async () => { + setSearch('?oidc_code=code-flag'); + server.use(http.get('/api/auth/oidc/exchange', () => HttpResponse.json({ success: true }))); + + const { result } = renderLogin(); + await waitFor(() => expect(mockNavigate).toHaveBeenCalledWith(START_DESTINATION_ROUTE, { replace: true })); + expect(auth.loadUser).toHaveBeenCalled(); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + }); + + it('FE-LOGIN-HOOK-212: a refused exchange is a failure whatever its body holds', async () => { + setSearch('?oidc_code=code-refused'); + server.use( + http.get('/api/auth/oidc/exchange', () => HttpResponse.json({ success: true, error: 'Invalid or expired code' }, { status: 400 })), + ); + + const { result } = renderLogin(); + await waitFor(() => expect(result.current.error).toBe('Invalid or expired code')); + expect(mockNavigate).not.toHaveBeenCalled(); + expect(auth.loadUser).not.toHaveBeenCalled(); + }); + it('FE-LOGIN-HOOK-017: surfaces the error the exchange endpoint reports', async () => { setSearch('?oidc_code=code-2'); server.use(http.get('/api/auth/oidc/exchange', () => HttpResponse.json({ error: 'State mismatch' }))); @@ -994,7 +1016,7 @@ describe('OIDC-only auto-redirect suppression', () => { server.use( http.get('/api/auth/oidc/exchange', async () => { await new Promise(r => { release = r; }); - return HttpResponse.json({ token: 'tok' }); + return HttpResponse.json({ success: true, token: 'tok' }); }), ); setSearch('?oidc_code=CODE-1'); diff --git a/client/src/pages/login/useLogin.ts b/client/src/pages/login/useLogin.ts index bcfe7bb48..6d77851cc 100644 --- a/client/src/pages/login/useLogin.ts +++ b/client/src/pages/login/useLogin.ts @@ -207,10 +207,12 @@ export function useLogin() { exchangeInitiated.current = true setIsLoading(true) fetch('/api/auth/oidc/exchange?code=' + encodeURIComponent(oidcCode), { credentials: 'include' }) - .then(r => r.json()) - .then(async data => { + .then(async r => ({ ok: r.ok, data: (await r.json()) as { success?: boolean; error?: string } })) + .then(async ({ ok, data }) => { window.history.replaceState({}, '', '/login') - if (data.token) { + // The exchange set the session cookie; the deprecated `token` in the + // body is never read, so it can go without breaking SSO sign-in. + if (ok && data.success === true) { await loadUser() const savedRedirect = sessionStorage.getItem('oidc_redirect') || START_DESTINATION_ROUTE sessionStorage.removeItem('oidc_redirect') diff --git a/server/CLAUDE.md b/server/CLAUDE.md index 78cfc0278..334c84411 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -96,7 +96,7 @@ SQLite is the engine today and Postgres is the planned second one. Code under `s ## Cross-cutting pieces - **`src/nest/common/`**: the global exception filter (error envelope), the Zod validation pipe, the idempotency interceptor (`X-Idempotency-Key` replay), and `validate-route-guards.ts` (boot-time ratchet refusing any public route not on its allow-list). -- **Auth is default-deny**: the global auth guard and the MFA policy guard run as `APP_GUARD`s, followed by the managed-install guard (`managed.guard.ts`), which refuses the routes a centrally administered install withholds from its own admin — it runs last so strangers still get a 401, never a route-revealing 403. Routes opt out with `@Public()`/`@OptionalAuth()`. The global guard stands down for routes declaring their own guard chain, so `@UseGuards(JwtAuthGuard, TripAccessGuard)` + `@RequirePermission` stacks work. Session is a cookie (`trek_session`); `@CurrentUser()` reads the user. Every session token carries a `jti` naming its `user_sessions` row, and `verifyJwtAndLoadUser` refuses one whose row is revoked, expired or gone (logout, `/api/auth/sessions`, a password change or reset, an MFA disable all end rows; `session-purge.job.ts` deletes the dead ones nightly). A token without a `jti` predates the table and passes until it expires, so an upgrade signs nobody out. The `token` field in the login-type JSON bodies is deprecated: kept for API clients, read by nothing of ours. **Never gate a multipart upload with a guard** — guards run before the parser and the client sees ECONNRESET instead of your 403; check in the handler. +- **Auth is default-deny**: the global auth guard and the MFA policy guard run as `APP_GUARD`s, followed by the managed-install guard (`managed.guard.ts`), which refuses the routes a centrally administered install withholds from its own admin — it runs last so strangers still get a 401, never a route-revealing 403. Routes opt out with `@Public()`/`@OptionalAuth()`. The global guard stands down for routes declaring their own guard chain, so `@UseGuards(JwtAuthGuard, TripAccessGuard)` + `@RequirePermission` stacks work. Session is a cookie (`trek_session`); `@CurrentUser()` reads the user. Every session token carries a `jti` naming its `user_sessions` row, and `verifyJwtAndLoadUser` refuses one whose row is revoked, expired or gone (logout, `/api/auth/sessions`, a password change or reset, an MFA disable all end rows; `session-purge.job.ts` deletes the dead ones nightly). A token without a `jti` predates the table and passes until it expires, so an upgrade signs nobody out. The `token` field in the login-type JSON bodies and in the SSO exchange (`GET /api/auth/oidc/exchange`, which also answers `success: true`) is deprecated: kept for API clients, read by nothing of ours. **Never gate a multipart upload with a guard** — guards run before the parser and the client sees ECONNRESET instead of your 403; check in the handler. - **Logging**: `src/nest/audit/audit-log.logger.ts` (`logInfo`/`logWarn`/`logError`/`logDebug`) prints each line to the console and queues it for `data/logs/trek.log`, which `log-file.ts` appends in batches off the event loop and rotates at 10 MB over five files; `index.ts` flushes what is queued on exit. Never write the log file synchronously from a request path. - **Request correlation** (`src/nest/common/request-correlation.ts`): every HTTP request runs under an id (a well-formed incoming `X-Request-Id`, else a fresh UUID) that the response carries in `X-Request-Id`, and every log line written inside it is tagged `[http ]`. The exception filter leaves a 5xx on `res.locals` so the access log writes the request line and the stack as one entry. The other entry points (MCP tool/resource/prompt calls via the registry's `around` seam, WebSocket messages in `TrekWsAdapter`, plugin RPC dispatches in the supervisor, cron ticks in `CronRegistrarService`) each run under their own id through `traceEntry()` (`nest/audit/entry-trace.logger.ts`), which logs one line per call: debug on success, debug as `skipped` when the call declines its work on purpose (a cron tick whose lease another process holds), warn or error with the reason on failure. A new entry point goes through `traceEntry()` too. - **Process safety nets** (`src/index.ts`, `src/shutdown.ts`): `unhandledRejection` and `uncaughtException` are logged to trek.log and end in the same orderly shutdown as SIGTERM, with exit code 1; a second fatal error during it exits at once. A shutdown first marks `ReadinessService` (`nest/health/`) as draining, so `/api/health/ready` answers 503 for the whole drain. The HTTP server keeps idle keep-alive connections for `HTTP_KEEP_ALIVE_TIMEOUT_MS` (95 s, above the 60 s of nginx and the 90 s of Traefik) with the headers timeout one second above it (`applyProxyTimeouts` in `bootstrap.ts`). diff --git a/server/src/nest/auth/auth-public.controller.ts b/server/src/nest/auth/auth-public.controller.ts index 5003749b1..eaf1a3601 100644 --- a/server/src/nest/auth/auth-public.controller.ts +++ b/server/src/nest/auth/auth-public.controller.ts @@ -27,10 +27,11 @@ const delay = (ms: number) => new Promise((r) => setTimeout(r, ms)); * forgot response, the audit writes and the JWT httpOnly cookie set/clear via * the shared cookie service. * - * The `token` in the login, register, demo-login and MFA bodies is deprecated: - * the httpOnly cookie is the session, and the web app has never read the field. - * It stays for API clients that took it as a bearer token, and the API docs - * mark it; nothing here or in the client relies on it. + * The `token` in the login, register, demo-login, MFA and passkey bodies (and + * in the SSO exchange, oidc.controller.ts) is deprecated: the httpOnly cookie + * is the session, and the web app does not read the field. It stays for API + * clients that took it as a bearer token, and the API docs mark it; nothing + * here or in the client relies on it. */ @Controller('api/auth') export class AuthPublicController { diff --git a/server/src/nest/oidc/oidc.controller.ts b/server/src/nest/oidc/oidc.controller.ts index a6c29d428..b588a11a7 100644 --- a/server/src/nest/oidc/oidc.controller.ts +++ b/server/src/nest/oidc/oidc.controller.ts @@ -236,7 +236,9 @@ export class OidcController { return; } this.oidc.setAuthCookie(res, result.token, req, result.remember); - res.json({ token: result.token }); + // `success` is what the web app reads. `token` is deprecated like the one + // in the login bodies: kept for API clients, the cookie is the session. + res.json({ success: true, token: result.token }); } } diff --git a/server/src/nest/platform/api-docs.ts b/server/src/nest/platform/api-docs.ts index 0df5b207e..f83f842c2 100644 --- a/server/src/nest/platform/api-docs.ts +++ b/server/src/nest/platform/api-docs.ts @@ -27,8 +27,9 @@ export function setupApiDocs(app: INestApplication): void { + '`GET /api/auth/sessions` lists the signed-in sessions, `DELETE /api/auth/sessions/{id}` ends one and ' + '`POST /api/auth/sessions/revoke-others` ends all but the current one.\n\n' + '**Deprecated:** the `token` field in the bodies of `POST /api/auth/login`, `/register`, `/demo-login`, ' - + '`/mfa/verify-login` and `/passkey/login/verify`. It is still sent, for clients that read it, but will be ' - + 'removed in a future major version. Use the `trek_session` cookie the same response sets.', + + '`/mfa/verify-login`, `/passkey/login/verify` and `GET /api/auth/oidc/exchange`. It is still sent, for ' + + 'clients that read it, but will be removed in a future major version. Use the `trek_session` cookie the ' + + 'same response sets; the SSO exchange also answers `success: true`.', ) .setVersion(version) .addBearerAuth({ type: 'http', scheme: 'bearer', bearerFormat: 'JWT' }, 'session') diff --git a/server/tests/e2e/oidc.e2e.test.ts b/server/tests/e2e/oidc.e2e.test.ts index 7b244bdc7..346539a86 100644 --- a/server/tests/e2e/oidc.e2e.test.ts +++ b/server/tests/e2e/oidc.e2e.test.ts @@ -103,11 +103,11 @@ describe('OIDC e2e (real cookie service)', () => { expect(res.body).toEqual({ error: 'Code required' }); }); - it('GET /exchange sets the httpOnly trek_session cookie + returns the token', async () => { + it('GET /exchange sets the httpOnly trek_session cookie + answers success (and the deprecated token)', async () => { consumeAuthCode.mockResolvedValue({ token: 'jwt.value' }); const res = await request(server).get('/api/auth/oidc/exchange').query({ code: 'good' }); expect(res.status).toBe(200); - expect(res.body).toEqual({ token: 'jwt.value' }); + expect(res.body).toEqual({ success: true, token: 'jwt.value' }); const setCookie = res.headers['set-cookie'] as unknown as string[]; expect(setCookie.some((c) => c.startsWith('trek_session=') && /HttpOnly/i.test(c))).toBe(true); }); diff --git a/server/tests/integration/oidc.test.ts b/server/tests/integration/oidc.test.ts index b2d9f1252..e1548140d 100644 --- a/server/tests/integration/oidc.test.ts +++ b/server/tests/integration/oidc.test.ts @@ -332,7 +332,7 @@ describe('GET /api/auth/oidc/exchange', () => { .set('Cookie', `trek_oidc_exchange=${binding}`); expect(res.status).toBe(200); - expect(res.body.token).toBe(fakeToken); + expect(res.body).toEqual({ success: true, token: fakeToken }); expect(res.headers['set-cookie']).toBeDefined(); const cookieHeader = Array.isArray(res.headers['set-cookie']) ? res.headers['set-cookie'].join(';') diff --git a/server/tests/unit/nest/oidc.controller.test.ts b/server/tests/unit/nest/oidc.controller.test.ts index d0a2eca5f..320681e79 100644 --- a/server/tests/unit/nest/oidc.controller.test.ts +++ b/server/tests/unit/nest/oidc.controller.test.ts @@ -488,7 +488,7 @@ describe('OidcController /exchange', () => { const req3 = reqEx(); await ctl(svc({ consumeAuthCode: vi.fn().mockResolvedValue({ token: 'jwt' }), setAuthCookie })).exchange('x', req3, r3); expect(setAuthCookie).toHaveBeenCalledWith(r3, 'jwt', req3, undefined); - expect(r3.body).toEqual({ token: 'jwt' }); + expect(r3.body).toEqual({ success: true, token: 'jwt' }); }); it('forwards the stored remember flag to setAuthCookie', async () => { @@ -498,7 +498,7 @@ describe('OidcController /exchange', () => { const r = reqEx(); await ctl(svc({ consumeAuthCode: vi.fn().mockResolvedValue({ token: 'jwt', remember }), setAuthCookie })).exchange('x', r, res); expect(setAuthCookie).toHaveBeenCalledWith(res, 'jwt', r, remember); - expect(res.body).toEqual({ token: 'jwt' }); + expect(res.body).toEqual({ success: true, token: 'jwt' }); } }); diff --git a/wiki/OIDC-SSO.md b/wiki/OIDC-SSO.md index 5a00efa31..0289d8f16 100644 --- a/wiki/OIDC-SSO.md +++ b/wiki/OIDC-SSO.md @@ -12,7 +12,7 @@ OpenID Connect (OIDC) lets users log in with an existing identity provider — G 2. You are redirected to your identity provider's login page. 3. Authenticate and grant consent. 4. The provider redirects back to TREK at `GET /api/auth/oidc/callback`. If this is your first login, an account is created automatically (subject to registration settings). -5. The server issues a short-lived one-time code and redirects your browser to `/login?oidc_code=`. The frontend immediately exchanges that code at `GET /api/auth/oidc/exchange?code=` to obtain the session. +5. The server issues a short-lived one-time code and redirects your browser to `/login?oidc_code=`. The frontend immediately exchanges that code at `GET /api/auth/oidc/exchange?code=` to obtain the session: the exchange sets the `trek_session` cookie and answers `{ "success": true }`. The same body still carries the session JWT as `token` for API clients; that field is deprecated and will be removed in a future major version. The code is only half of what the exchange needs: the callback also sets a one-minute `HttpOnly` cookie (`trek_oidc_exchange`) holding a secret that never appears in a URL, and the exchange requires both. A code copied out of the address bar, out of history, or out of a proxy log is therefore worthless in any other browser, and it is spent by the first attempt to redeem it whether that attempt succeeds or not. If your reverse proxy strips cookies on the way back from the identity provider, SSO login will fail here with `Invalid or expired code`. 6. Your `trek_session` cookie is set and you land on the dashboard. diff --git a/wiki/Security-Hardening.md b/wiki/Security-Hardening.md index d70458001..1cc277fd3 100644 --- a/wiki/Security-Hardening.md +++ b/wiki/Security-Hardening.md @@ -56,7 +56,7 @@ Sessions issued before this was introduced carry no session id. They keep workin A backup restore and the hourly demo reset replace the whole database file. Both carry the sessions that are active at that moment into the new file, for every account it holds under the same id and email, so neither signs anybody out by itself. -> **Deprecated:** the login, registration, demo-login, MFA and passkey sign-in responses still carry the session JWT as `token` in the JSON body, for API clients that read it. The web app does not use it; the session is the httpOnly cookie the same response sets. The field will be removed in a future major version. +> **Deprecated:** the login, registration, demo-login, MFA and passkey sign-in responses and the SSO code exchange (`GET /api/auth/oidc/exchange`) still carry the session JWT as `token` in the JSON body, for API clients that read it. The web app does not use it; the session is the httpOnly cookie the same response sets, and the SSO exchange also answers `success: true`, which is what the web app checks. The field will be removed in a future major version. ## Password Policy