From 803a7cb64a3a537a96017054ded3e22d2d12b4cf Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 13 Sep 2026 22:03:24 +0200 Subject: [PATCH 1/2] fix(oidc): bind the SSO auth code to the browser that earned it The callback hands the finished login back as a code in a redirect URL, and /exchange turned any code into a session. That made the code the whole credential while it travelled somewhere URLs are readable: browser history on a shared machine, a referrer, a reverse proxy log. It cuts both ways. Redeem a code you saw over someone's shoulder and you are them; or complete a login as yourself, hand the victim's browser your code, and they are silently signed into your account and write their next trip into it. The callback now also sets a one-minute httpOnly cookie holding a random secret, and /exchange wants the code plus that cookie. Only the browser that came back from the provider holds both, which is the same shape the state cookie already gives the callback. The code is burnt on every attempt, right or wrong, and an unbound code answers exactly like an unknown one so presenting one confirms nothing. --- server/src/nest/oidc/oidc.controller.ts | 26 +++- server/src/nest/oidc/oidc.service.ts | 45 +++++- server/tests/integration/oidc.test.ts | 134 +++++++++++++++++- .../tests/unit/nest/oidc.controller.test.ts | 47 ++++-- server/tests/unit/nest/oidc.service.test.ts | 55 +++++-- 5 files changed, 271 insertions(+), 36 deletions(-) diff --git a/server/src/nest/oidc/oidc.controller.ts b/server/src/nest/oidc/oidc.controller.ts index 37cc8daec..d9dffba4b 100644 --- a/server/src/nest/oidc/oidc.controller.ts +++ b/server/src/nest/oidc/oidc.controller.ts @@ -2,7 +2,7 @@ import { Body, Controller, Get, HttpException, Put, Query, Req, Res, UseGuards } import type { Request, Response } from 'express'; import { oidcLoginQuerySchema } from '@trek/shared'; import { readEnv } from '../../app-config'; -import { OidcService, OIDC_STATE_TTL_MS } from './oidc.service'; +import { OidcService, OIDC_STATE_TTL_MS, OIDC_AUTH_CODE_TTL_MS } from './oidc.service'; import { cookieOptions } from '../common/cookie'; import { AdminGuard } from '../auth/admin.guard'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; @@ -15,15 +15,21 @@ import { Public } from '../auth/public.decorator'; import { ManagedForbidden } from '../common/managed'; const OIDC_STATE_COOKIE = 'trek_oidc_state'; +const OIDC_EXCHANGE_COOKIE = 'trek_oidc_exchange'; /** * /api/auth/oidc — OIDC SSO login flow (Authorization Code + PKCE). * - * Byte-identical to the legacy Express route (server/src/routes/oidc.ts): + * Carried over from the legacy Express route (server/src/routes/oidc.ts): * unauthenticated, the sso-disabled / not-configured / HTTPS-issuer guards, the * strict id_token + userinfo.sub cross-check, all the frontend redirect error * codes, and the auth-code → cookie hand-off on /exchange. Uses @Res directly * because the flow mixes provider redirects with JSON error bodies. + * + * Two cookies bind the flow to one browser, and both are single use: the state + * cookie set at /login is required at /callback, and the binding cookie set at + * /callback is required at /exchange. Neither the provider state nor the auth + * code is a credential on its own — both travel in URLs. */ @Public('the OIDC handshake happens before a TREK session exists; the provider state is the credential') @Controller('api/auth/oidc') @@ -161,7 +167,14 @@ export class OidcController { this.oidc.touchLastLogin(result.user.id); const jwtToken = this.oidc.generateToken(result.user, pending.remember === true); - const authCode = this.oidc.createAuthCode(jwtToken, pending.remember); + const { code: authCode, binding } = this.oidc.createAuthCode(jwtToken, pending.remember); + // Bind the code to THIS browser, the way the state cookie binds the callback. + // The code rides home in a URL, so it is readable from history, from a + // referrer and from anything that logs URLs; without a second half nobody + // else holds, /exchange would hand a session to whoever presents it — either + // stealing this login, or (a code the attacker minted for their own account, + // fed to a victim's browser) fixing the victim into the attacker's session. + res.cookie(OIDC_EXCHANGE_COOKIE, binding, { ...cookieOptions(false, req), maxAge: OIDC_AUTH_CODE_TTL_MS }); return f('/login?oidc_code=' + authCode); } catch (err: unknown) { console.error('[OIDC] Callback error:', err); @@ -171,11 +184,16 @@ export class OidcController { @Get('exchange') exchange(@Query('code') code: string | undefined, @Req() req: Request, @Res() res: Response): void { + // The binding cookie is single-use like the state cookie: one redemption + // attempt per callback, whatever its outcome. + const binding = (req.cookies as Record | undefined)?.[OIDC_EXCHANGE_COOKIE]; + res.clearCookie(OIDC_EXCHANGE_COOKIE, cookieOptions(true, req)); + if (!code) { res.status(400).json({ error: 'Code required' }); return; } - const result = this.oidc.consumeAuthCode(code); + const result = this.oidc.consumeAuthCode(code, binding); if ('error' in result) { res.status(400).json({ error: result.error }); return; diff --git a/server/src/nest/oidc/oidc.service.ts b/server/src/nest/oidc/oidc.service.ts index 94d934997..36ef5cabe 100644 --- a/server/src/nest/oidc/oidc.service.ts +++ b/server/src/nest/oidc/oidc.service.ts @@ -60,7 +60,9 @@ export interface OidcConfig { // Constants / TTLs // --------------------------------------------------------------------------- -const AUTH_CODE_TTL = 60000; // 1 minute +/** 1 minute — the auth-code lifetime AND the controller's binding-cookie maxAge. */ +export const OIDC_AUTH_CODE_TTL_MS = 60000; +const AUTH_CODE_TTL = OIDC_AUTH_CODE_TTL_MS; const AUTH_CODE_CLEANUP = 30000; // 30 seconds /** 5 minutes — the server-side pending-state TTL AND the controller's state-cookie maxAge. */ export const OIDC_STATE_TTL_MS = 5 * 60 * 1000; @@ -87,6 +89,19 @@ function base64url(buf: Buffer): string { return buf.toString('base64url'); } +function sha256Hex(value: string): string { + return crypto.createHash('sha256').update(value, 'utf8').digest('hex'); +} + +/** Constant-time compare of a presented binding secret against its stored hash. */ +function bindingMatches(expectedHash: string, presented: string): boolean { + const expected = Buffer.from(expectedHash, 'hex'); + const actual = crypto.createHash('sha256').update(presented, 'utf8').digest(); + // Both are sha256 digests, so the lengths always agree; the guard is there + // because timingSafeEqual throws rather than returning false on a mismatch. + return expected.length === actual.length && crypto.timingSafeEqual(expected, actual); +} + const isRecord = (v: unknown): v is Record => typeof v === 'object' && v !== null && !Array.isArray(v); @@ -170,7 +185,11 @@ export class OidcService implements OnModuleDestroy { // Auth code management – short-lived codes exchanged for JWT // ------------------------------------------------------------------------- - private readonly authCodes = new Map(); + // `bindingHash` is the sha256 of a secret that only the browser which finished + // the callback holds, in a cookie. The code itself travels in a URL — through + // history, referrers and any log in between — so on its own it is not a + // credential, and /exchange must not accept it as one. + private readonly authCodes = new Map(); // Discovery document cache (1 h TTL), keyed by discovery URL so two // configured issuers no longer thrash a single slot. @@ -229,17 +248,31 @@ export class OidcService implements OnModuleDestroy { return pending; } - createAuthCode(token: string, remember?: boolean): string { + /** + * Mint a one-time login code plus the secret that redeems it. + * + * The caller puts `code` in the redirect URL and `binding` in an httpOnly + * cookie, so redeeming the code takes both halves and only the browser that + * completed the provider handshake has both. + */ + createAuthCode(token: string, remember?: boolean): { code: string; binding: string } { const authCode: string = uuidv4(); - this.authCodes.set(authCode, { token, created: Date.now(), remember }); - return authCode; + const binding = crypto.randomBytes(32).toString('base64url'); + this.authCodes.set(authCode, { token, created: Date.now(), remember, bindingHash: sha256Hex(binding) }); + return { code: authCode, binding }; } - consumeAuthCode(code: string): { token: string; remember?: boolean } | { error: string } { + consumeAuthCode(code: string, binding?: string): { token: string; remember?: boolean } | { error: string } { const entry = this.authCodes.get(code); if (!entry) return { error: 'Invalid or expired code' }; + // Single use, burnt on every outcome: a code seen by someone else must not + // survive their attempt for a second guess, and the browser that owns it can + // simply log in again. this.authCodes.delete(code); if (Date.now() - entry.created > AUTH_CODE_TTL) return { error: 'Code expired' }; + // Same wording as the unknown-code case on purpose — whoever presents a code + // without its binding learns nothing about whether the code was real. + if (!binding || !bindingMatches(entry.bindingHash, binding)) return { error: 'Invalid or expired code' }; return { token: entry.token, remember: entry.remember }; } diff --git a/server/tests/integration/oidc.test.ts b/server/tests/integration/oidc.test.ts index 70f55ae29..8755d5ea7 100644 --- a/server/tests/integration/oidc.test.ts +++ b/server/tests/integration/oidc.test.ts @@ -56,6 +56,16 @@ import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser } from '../helpers/factories'; import { OidcService } from '../../src/nest/oidc/oidc.service'; +/** Read one cookie's value out of a response's Set-Cookie header, as a browser would. */ +function readCookie(res: request.Response, name: string): string | undefined { + const raw = res.headers['set-cookie']; + const all: string[] = Array.isArray(raw) ? raw : raw ? [raw as unknown as string] : []; + const hit = all.filter((c) => c.startsWith(`${name}=`)).pop(); + const value = hit?.split(';')[0].slice(name.length + 1); + // An expired clear-cookie carries an empty value; treat that as "gone". + return value ? decodeURIComponent(value) : undefined; +} + const MOCK_DISCOVERY_DOC = { authorization_endpoint: 'https://oidc.example.com/auth', token_endpoint: 'https://oidc.example.com/token', @@ -307,9 +317,11 @@ describe('GET /api/auth/oidc/callback', () => { describe('GET /api/auth/oidc/exchange', () => { it('OIDC-011: valid auth code returns JWT and sets cookie', async () => { const fakeToken = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.test.sig'; - const code = oidcSvc.createAuthCode(fakeToken); + const { code, binding } = oidcSvc.createAuthCode(fakeToken); - const res = await request(app).get(`/api/auth/oidc/exchange?code=${code}`); + const res = await request(app) + .get(`/api/auth/oidc/exchange?code=${code}`) + .set('Cookie', `trek_oidc_exchange=${binding}`); expect(res.status).toBe(200); expect(res.body.token).toBe(fakeToken); @@ -332,16 +344,28 @@ describe('GET /api/auth/oidc/exchange', () => { expect(res.body.error).toBeDefined(); }); + it('OIDC-013b: a real code without its binding cookie returns 400 and buys nothing', async () => { + const { code } = oidcSvc.createAuthCode('valid.but.unbound'); + + const res = await request(app).get(`/api/auth/oidc/exchange?code=${code}`); + + expect(res.status).toBe(400); + // Same wording as an unknown code: presenting a code must not confirm it exists. + expect(res.body.error).toBe('Invalid or expired code'); + expect(readCookie(res, 'trek_session')).toBeUndefined(); + }); + it('OIDC-014: auth code is single-use (second use returns 400)', async () => { const fakeToken = 'test.token.here'; - const code = oidcSvc.createAuthCode(fakeToken); + const { code, binding } = oidcSvc.createAuthCode(fakeToken); + const cookie = `trek_oidc_exchange=${binding}`; // First use: success - const res1 = await request(app).get(`/api/auth/oidc/exchange?code=${code}`); + const res1 = await request(app).get(`/api/auth/oidc/exchange?code=${code}`).set('Cookie', cookie); expect(res1.status).toBe(200); - // Second use: rejected - const res2 = await request(app).get(`/api/auth/oidc/exchange?code=${code}`); + // Second use: rejected, even from the same browser + const res2 = await request(app).get(`/api/auth/oidc/exchange?code=${code}`).set('Cookie', cookie); expect(res2.status).toBe(400); }); }); @@ -367,8 +391,13 @@ describe('OIDC remember-me (#1927)', () => { expect(cb.status).toBe(302); const oidcCode = new URL(cb.headers.location!, 'http://localhost').searchParams.get('oidc_code')!; expect(oidcCode).toBeTruthy(); + // The browser carries the binding cookie the callback just set back to /exchange. + const binding = readCookie(cb, 'trek_oidc_exchange')!; + expect(binding).toBeTruthy(); - return request(app).get(`/api/auth/oidc/exchange?code=${oidcCode}`); + return request(app) + .get(`/api/auth/oidc/exchange?code=${oidcCode}`) + .set('Cookie', `trek_oidc_exchange=${binding}`); } function sessionCookie(res: request.Response): string { @@ -402,3 +431,94 @@ describe('OIDC remember-me (#1927)', () => { expect(sessionCookie(res)).toContain('Max-Age=86400'); }); }); + +// ── the auth code is bound to one browser (GHSA / session fixation) ─────────── + +describe('OIDC auth-code binding', () => { + // Walks /login → /callback for one "browser" and hands back everything that + // browser holds afterwards: the code out of the redirect URL, and the binding + // cookie out of the callback response. + async function loginUpToCallback(sub: string, email: string) { + mockDiscover.mockResolvedValue(MOCK_DISCOVERY_DOC); + mockExchangeCode.mockResolvedValue({ access_token: 'tok', id_token: 'fake.id.token', _ok: true, _status: 200 }); + mockVerifyIdToken.mockResolvedValue({ ok: true, claims: { sub } }); + mockGetUserInfo.mockResolvedValue({ sub, email, name: 'Binding User' }); + + const login = await request(app).get('/api/auth/oidc/login'); + const state = new URL(login.headers.location!).searchParams.get('state')!; + const cb = await request(app) + .get(`/api/auth/oidc/callback?code=anycode&state=${state}`) + .set('Cookie', `trek_oidc_state=${state}`); + + return { + code: new URL(cb.headers.location!, 'http://localhost').searchParams.get('oidc_code')!, + binding: readCookie(cb, 'trek_oidc_exchange'), + response: cb, + }; + } + + it('OIDC-018: /callback sets the binding as an httpOnly cookie that dies with the code', async () => { + const { response } = await loginUpToCallback('sub-bind-1', 'bind1@example.com'); + + const raw = (response.headers['set-cookie'] as unknown as string[]).find((c) => c.startsWith('trek_oidc_exchange='))!; + expect(raw).toMatch(/HttpOnly/i); + expect(raw).toMatch(/SameSite=Lax/i); + // Same minute the code lives; a cookie that outlived it could only ever fail. + expect(raw).toContain('Max-Age=60;'); + }); + + it('OIDC-019: a code captured in browser A is refused in browser B', async () => { + // Browser A finishes the handshake. B is anything that only got to see the + // URL: history on a shared machine, a referrer, a proxy log, a screen share. + const a = await loginUpToCallback('sub-bind-2', 'bind2@example.com'); + + const inB = await request(app).get(`/api/auth/oidc/exchange?code=${a.code}`); + + expect(inB.status).toBe(400); + expect(readCookie(inB, 'trek_session')).toBeUndefined(); + }); + + it('OIDC-020: the failed attempt burns the code, so a stolen code is never redeemable', async () => { + const a = await loginUpToCallback('sub-bind-3', 'bind3@example.com'); + + await request(app).get(`/api/auth/oidc/exchange?code=${a.code}`); + // A retries with the right cookie and is refused too. Deliberate: a code that + // someone else has already presented is treated as spent, and A only has to + // log in again. + const retryInA = await request(app) + .get(`/api/auth/oidc/exchange?code=${a.code}`) + .set('Cookie', `trek_oidc_exchange=${a.binding}`); + + expect(retryInA.status).toBe(400); + expect(readCookie(retryInA, 'trek_session')).toBeUndefined(); + }); + + it('OIDC-021: an attacker-minted code cannot be forced onto a victim browser', async () => { + // The session-fixation direction: the attacker completes a real login as + // themselves, then makes the victim's browser call /exchange with that code. + // If it worked, the victim would be silently signed into the attacker's + // account and would write their next trip into it. + const attacker = await loginUpToCallback('sub-attacker', 'attacker@example.com'); + const victimHasHisOwnFlow = await loginUpToCallback('sub-victim', 'victim@example.com'); + + const forced = await request(app) + .get(`/api/auth/oidc/exchange?code=${attacker.code}`) + // The victim's browser carries its own binding, never the attacker's. + .set('Cookie', `trek_oidc_exchange=${victimHasHisOwnFlow.binding}`); + + expect(forced.status).toBe(400); + expect(readCookie(forced, 'trek_session')).toBeUndefined(); + }); + + it('OIDC-022: /exchange clears the binding cookie on the way out', async () => { + const a = await loginUpToCallback('sub-bind-4', 'bind4@example.com'); + + const ok = await request(app) + .get(`/api/auth/oidc/exchange?code=${a.code}`) + .set('Cookie', `trek_oidc_exchange=${a.binding}`); + + expect(ok.status).toBe(200); + const cleared = (ok.headers['set-cookie'] as unknown as string[]).find((c) => c.startsWith('trek_oidc_exchange='))!; + expect(cleared).toContain('Expires=Thu, 01 Jan 1970'); + }); +}); diff --git a/server/tests/unit/nest/oidc.controller.test.ts b/server/tests/unit/nest/oidc.controller.test.ts index c53eeeb7c..3aaf97ca6 100644 --- a/server/tests/unit/nest/oidc.controller.test.ts +++ b/server/tests/unit/nest/oidc.controller.test.ts @@ -19,7 +19,7 @@ function svc(o: Partial = {}): OidcService { findOrCreateUser: vi.fn(), touchLastLogin: vi.fn(), generateToken: vi.fn().mockReturnValue('jwt'), - createAuthCode: vi.fn().mockReturnValue('ac'), + createAuthCode: vi.fn().mockReturnValue({ code: 'ac', binding: 'bnd' }), consumeAuthCode: vi.fn(), frontendUrl: vi.fn((p: string) => 'https://app' + p), setAuthCookie: vi.fn(), @@ -42,6 +42,8 @@ function makeRes() { } const req = { query: {}, headers: {} } as Request; +// Exchange request carrying the binding cookie the callback handed this browser. +const reqEx = (binding = 'bnd') => ({ query: {}, headers: {}, cookies: { trek_oidc_exchange: binding } } as unknown as Request); // Callback request carrying the state-binding cookie a real browser would send // after going through /login. const reqCb = (state = 's') => ({ query: {}, headers: {}, cookies: { trek_oidc_state: state } } as unknown as Request); @@ -192,6 +194,9 @@ describe('OidcController /callback', () => { })); await c.callback('c', 's', undefined, reqCb('s'), ok); expect(ok.redirectedTo).toBe('https://app/login?oidc_code=ac'); + // The code alone is not the credential: its other half goes out as a cookie + // that only this browser holds, and expires with the code. + expect(ok.cookie).toHaveBeenCalledWith('trek_oidc_exchange', 'bnd', expect.objectContaining({ httpOnly: true, maxAge: 60000, sameSite: 'lax' })); }); it('rejects a callback whose state cookie does not match the query state', async () => { @@ -360,7 +365,7 @@ describe('OidcController /callback', () => { [undefined, false], ] as const) { const generateToken = vi.fn().mockReturnValue('jwt'); - const createAuthCode = vi.fn().mockReturnValue('ac'); + const createAuthCode = vi.fn().mockReturnValue({ code: 'ac', binding: 'bnd' }); const res = makeRes(); await new OidcController(svc({ consumeState: vi.fn().mockReturnValue({ redirectUri: 'https://app/api/auth/oidc/callback', codeVerifier: 'cv', inviteToken: undefined, remember }), @@ -380,19 +385,20 @@ describe('OidcController /callback', () => { describe('OidcController /exchange', () => { it('400 without a code, 400 on an invalid code, else sets the cookie + returns the token', () => { const r1 = makeRes(); - new OidcController(svc()).exchange(undefined, req, r1); + new OidcController(svc()).exchange(undefined, reqEx(), r1); expect(r1.statusCode).toBe(400); expect(r1.body).toEqual({ error: 'Code required' }); const r2 = makeRes(); - new OidcController(svc({ consumeAuthCode: vi.fn().mockReturnValue({ error: 'invalid_code' }) })).exchange('x', req, r2); + new OidcController(svc({ consumeAuthCode: vi.fn().mockReturnValue({ error: 'invalid_code' }) })).exchange('x', reqEx(), r2); expect(r2.statusCode).toBe(400); expect(r2.body).toEqual({ error: 'invalid_code' }); const r3 = makeRes(); const setAuthCookie = vi.fn(); - new OidcController(svc({ consumeAuthCode: vi.fn().mockReturnValue({ token: 'jwt' }), setAuthCookie })).exchange('x', req, r3); - expect(setAuthCookie).toHaveBeenCalledWith(r3, 'jwt', req, undefined); + const req3 = reqEx(); + new OidcController(svc({ consumeAuthCode: vi.fn().mockReturnValue({ token: 'jwt' }), setAuthCookie })).exchange('x', req3, r3); + expect(setAuthCookie).toHaveBeenCalledWith(r3, 'jwt', req3, undefined); expect(r3.body).toEqual({ token: 'jwt' }); }); @@ -400,9 +406,34 @@ describe('OidcController /exchange', () => { for (const remember of [true, false] as const) { const res = makeRes(); const setAuthCookie = vi.fn(); - new OidcController(svc({ consumeAuthCode: vi.fn().mockReturnValue({ token: 'jwt', remember }), setAuthCookie })).exchange('x', req, res); - expect(setAuthCookie).toHaveBeenCalledWith(res, 'jwt', req, remember); + const r = reqEx(); + new OidcController(svc({ consumeAuthCode: vi.fn().mockReturnValue({ token: 'jwt', remember }), setAuthCookie })).exchange('x', r, res); + expect(setAuthCookie).toHaveBeenCalledWith(res, 'jwt', r, remember); expect(res.body).toEqual({ token: 'jwt' }); } }); + + it('hands the binding cookie to consumeAuthCode and clears it on the way out', () => { + const consumeAuthCode = vi.fn().mockReturnValue({ token: 'jwt' }); + const res = makeRes(); + new OidcController(svc({ consumeAuthCode })).exchange('x', reqEx('the-secret'), res); + expect(consumeAuthCode).toHaveBeenCalledWith('x', 'the-secret'); + expect(res.clearCookie).toHaveBeenCalledWith('trek_oidc_exchange', expect.objectContaining({ httpOnly: true, path: '/' })); + }); + + it('passes undefined when the browser has no binding cookie, and still clears it', () => { + const consumeAuthCode = vi.fn().mockReturnValue({ error: 'Invalid or expired code' }); + const res = makeRes(); + // A request from any other context: no cookie jar for this origin at all. + new OidcController(svc({ consumeAuthCode })).exchange('stolen', { query: {}, headers: {} } as Request, res); + expect(consumeAuthCode).toHaveBeenCalledWith('stolen', undefined); + expect(res.statusCode).toBe(400); + expect(res.clearCookie).toHaveBeenCalledWith('trek_oidc_exchange', expect.anything()); + }); + + it('clears the binding cookie even when no code was presented', () => { + const res = makeRes(); + new OidcController(svc()).exchange(undefined, reqEx(), res); + expect(res.clearCookie).toHaveBeenCalledWith('trek_oidc_exchange', expect.anything()); + }); }); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index 189189391..ee72408b5 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -200,38 +200,71 @@ describe('createState / consumeState', () => { // ── createAuthCode / consumeAuthCode ───────────────────────────────────────── describe('createAuthCode / consumeAuthCode', () => { - it('OIDC-SVC-005: createAuthCode returns a UUID-like string', () => { - const code = svc.createAuthCode('my.jwt.token'); + it('OIDC-SVC-005: createAuthCode returns a code and the secret that redeems it', () => { + const { code, binding } = svc.createAuthCode('my.jwt.token'); expect(typeof code).toBe('string'); expect(code.length).toBeGreaterThan(0); + // 32 random bytes as base64url; the code alone must never be enough. + expect(binding).toMatch(/^[A-Za-z0-9_-]{43}$/); + expect(binding).not.toBe(code); + }); + + it('OIDC-SVC-005b: every code gets its own binding secret', () => { + const a = svc.createAuthCode('t'); + const b = svc.createAuthCode('t'); + expect(a.binding).not.toBe(b.binding); + expect(a.code).not.toBe(b.code); }); it('OIDC-SVC-006: consumeAuthCode returns the stored token', () => { - const code = svc.createAuthCode('real.jwt.here'); - const result = svc.consumeAuthCode(code); + const { code, binding } = svc.createAuthCode('real.jwt.here'); + const result = svc.consumeAuthCode(code, binding); expect('token' in result).toBe(true); expect((result as { token: string }).token).toBe('real.jwt.here'); }); it('OIDC-SVC-007: auth code is single-use (second consume returns error)', () => { - const code = svc.createAuthCode('single.use.token'); - svc.consumeAuthCode(code); // first use - const second = svc.consumeAuthCode(code); + const { code, binding } = svc.createAuthCode('single.use.token'); + svc.consumeAuthCode(code, binding); // first use + const second = svc.consumeAuthCode(code, binding); expect('error' in second).toBe(true); }); it('OIDC-SVC-008: consumeAuthCode returns error for unknown code', () => { - const result = svc.consumeAuthCode('not-a-real-code'); + const result = svc.consumeAuthCode('not-a-real-code', 'whatever'); expect('error' in result).toBe(true); }); + it('OIDC-SVC-008b: a valid code without its binding is refused, and indistinguishable from an unknown one', () => { + const { code } = svc.createAuthCode('bound.token'); + expect(svc.consumeAuthCode(code, undefined)).toEqual({ error: 'Invalid or expired code' }); + expect(svc.consumeAuthCode('not-a-real-code', undefined)).toEqual({ error: 'Invalid or expired code' }); + }); + + it('OIDC-SVC-008c: another browser binding is refused', () => { + const mine = svc.createAuthCode('mine'); + const theirs = svc.createAuthCode('theirs'); + expect('error' in svc.consumeAuthCode(mine.code, theirs.binding)).toBe(true); + }); + + it('OIDC-SVC-008d: a wrong binding burns the code, so it cannot be retried', () => { + const { code, binding } = svc.createAuthCode('burn.me'); + svc.consumeAuthCode(code, 'not-the-binding'); + expect('error' in svc.consumeAuthCode(code, binding)).toBe(true); + }); + + it('OIDC-SVC-008e: a binding that is not even base64 is refused instead of throwing', () => { + const { code } = svc.createAuthCode('t'); + expect('error' in svc.consumeAuthCode(code, '!!!!')).toBe(true); + }); + it('OIDC-SVC-056: auth code round-trips the remember flag', () => { const cTrue = svc.createAuthCode('t1', true); const cFalse = svc.createAuthCode('t2', false); const cAbsent = svc.createAuthCode('t3'); - expect((svc.consumeAuthCode(cTrue) as { remember?: boolean }).remember).toBe(true); - expect((svc.consumeAuthCode(cFalse) as { remember?: boolean }).remember).toBe(false); - expect((svc.consumeAuthCode(cAbsent) as { remember?: boolean }).remember).toBeUndefined(); + expect((svc.consumeAuthCode(cTrue.code, cTrue.binding) as { remember?: boolean }).remember).toBe(true); + expect((svc.consumeAuthCode(cFalse.code, cFalse.binding) as { remember?: boolean }).remember).toBe(false); + expect((svc.consumeAuthCode(cAbsent.code, cAbsent.binding) as { remember?: boolean }).remember).toBeUndefined(); }); }); From a2585beb13747c121a307936937a0d4dfd6647b5 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 13 Sep 2026 22:05:54 +0200 Subject: [PATCH 2/2] docs(wiki): the SSO exchange takes a cookie as well as the code --- wiki/OIDC-SSO.md | 1 + 1 file changed, 1 insertion(+) diff --git a/wiki/OIDC-SSO.md b/wiki/OIDC-SSO.md index 135515dc8..c1aa4ec54 100644 --- a/wiki/OIDC-SSO.md +++ b/wiki/OIDC-SSO.md @@ -13,6 +13,7 @@ OpenID Connect (OIDC) lets users log in with an existing identity provider — G 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. + 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. **Remember me.** The Remember-me switch on the login page is carried into SSO as a query flag on the login URL: `GET /api/auth/oidc/login?remember=1` (or `remember=0`), appended with `&` when an invite token is already present. Only `0` and `1` are accepted; any other value — and starting SSO from the register tab, where the switch is not shown — is treated as if the parameter were absent. The choice is held in the server-side login state, survives the provider round-trip and the one-time-code exchange, and decides both the session lifetime and the cookie lifetime: