From 967bdff3aa421e9bbdcce0f97e56508b03c03427 Mon Sep 17 00:00:00 2001 From: Hollujay <165713167+Hollujay@users.noreply.github.com> Date: Mon, 3 Aug 2026 00:27:42 +0000 Subject: [PATCH] feat: JWKS staleness alarm + force-refresh endpoint --- docs/oidc-jwks-staleness-alarm.md | 149 ++++- src/auth/oidc/jwksCache.test.ts | 398 ++++++++++++ src/auth/oidc/jwksCache.ts | 113 +++- src/auth/oidc/jwksRefreshApprovalGate.test.ts | 162 +++++ src/auth/oidc/jwksRefreshApprovalGate.ts | 199 ++++++ src/auth/oidc/oidc.test.ts | 569 ++++++++++++++++-- src/auth/oidc/oidcAdapterService.ts | 39 +- src/auth/oidc/oidcAdminRoutes.test.ts | 395 ++++++++++++ src/auth/oidc/oidcLogout.test.ts | 130 ++++ src/auth/oidc/oidcRoute.ts | 222 ++++++- 10 files changed, 2267 insertions(+), 109 deletions(-) create mode 100644 src/auth/oidc/jwksCache.test.ts create mode 100644 src/auth/oidc/jwksRefreshApprovalGate.test.ts create mode 100644 src/auth/oidc/jwksRefreshApprovalGate.ts create mode 100644 src/auth/oidc/oidcAdminRoutes.test.ts diff --git a/docs/oidc-jwks-staleness-alarm.md b/docs/oidc-jwks-staleness-alarm.md index 29c0eef5..7c0805dd 100644 --- a/docs/oidc-jwks-staleness-alarm.md +++ b/docs/oidc-jwks-staleness-alarm.md @@ -2,35 +2,138 @@ ## Purpose -This document describes the implementation of a per-issuer OIDC JWKS cache age gauge and an admin-only endpoint to force refresh cached JWKS bundles. +An OIDC provider's JWKS bundle can silently go stale: the refresh cadence is +fixed (1h TTL) and a stuck cache keeps rejecting freshly rotated IdP keys. +This feature provides: -## Changes +1. **A staleness alarm** — `oidc.jwks.age_seconds` gauge, labeled per issuer, + re-emitted on an interval so alerting can see the *current* age of each + cached bundle. +2. **An admin-only force-refresh endpoint** — `POST /auth/oidc/jwks/refresh` + (and the `POST /api/auth/oidc/jwks/refresh` alias) to reload JWKS bundles + during incidents, gated by dual-control approval and rate-limited. -- `src/auth/oidc/jwksCache.ts` - - Added `JwksCacheService.refresh()` with in-flight request coalescing. - - Added per-issuer `issuerLastRefresh` tracking. - - Emit `oidc.jwks.age_seconds` gauge with label `issuer`. - - Added `getCacheAgeSeconds(issuer)` helper. +## Design -- `src/auth/oidc/oidcAdapterService.ts` - - Forward issuer URL to JWKS cache lookups so cache age is tracked per issuer. - - Added `refreshJwks(issuerUrl)` for service-driven refreshes. +### Staleness tracking and the age gauge (`src/auth/oidc/jwksCache.ts`) -- `src/auth/oidc/oidcRoute.ts` - - Added admin-only POST `/api/auth/oidc/jwks/refresh` endpoint. - - Requires dual confirmation: both header `x-revora-oidc-jwks-confirmation: true` and body `confirmation: true`. - - Enforces per-actor rate limiting (1 request per 60 seconds). - - Emits audit events via optional `auditRefresh` hook. +- Every successful refresh records `issuerLastRefresh[issuer] = fetchedAt` + (epoch ms). `getCacheAgeSeconds(issuer)` returns the elapsed age. +- `JwksCacheService.startAgeGaugeTicker(intervalMs)` re-emits + `oidc.jwks.age_seconds{issuer=…}` for every tracked issuer on an interval + (default 1 minute), so the gauge reflects real elapsed time rather than the + value captured at the last refresh. The timer is `unref()`'d (never keeps + the process alive), idempotent to start, and `stopAgeGaugeTicker()` shuts it + down. +- The gauge is also re-emitted immediately after every successful refresh + (value ≈ 0), so the drop to ~0 is the signal a force-refresh worked. +- `getTrackedIssuers()` exposes the issuers with at least one successful + refresh; this drives "refresh all tracked issuers". -## Security and correctness notes +### Request coalescing (single upstream fetch) -- The refresh endpoint is protected by `requireAdmin`. -- Dual confirmation mitigates accidental refreshes and provides an explicit incident escalation step. -- Rate limiting prevents repeated attacker/exhaustion attempts from a valid admin session. -- Concurrent refresh requests for the same JWKS URI coalesce into a single upstream fetch. -- Cache age metrics call out stale issuer state for monitoring and alerting. +All refreshes to the **same `jwks_uri`** — whether triggered by the TTL path +(`getKey`), a signature-failure rotation, or the admin force-refresh endpoint +— are coalesced into a single in-flight `fetch()`: + +- Callers arriving while a fetch is pending share the same promise. +- Each waiter registers the issuer it cares about (`pendingIssuers[uri]`), so + **every** issuer that waited gets its `issuerLastRefresh`/gauge updated when + the shared fetch succeeds (covers multi-issuer IdPs that share a JWKS URI). +- On **failure** the in-flight slot is released and no issuer bookkeeping is + updated — a retry performs a fresh upstream fetch. Failures are never cached. + +### Dual-control approval gate (`src/auth/oidc/jwksRefreshApprovalGate.ts`) + +The endpoint follows the two-step, distinct-approver pattern used elsewhere in +this codebase (OFAC review queue, tenant settings proposal/approval, ledger +period-close): + +1. **Step 1 — propose.** Admin A calls `POST /auth/oidc/jwks/refresh` with + `{ "issuer": "https://idp.example.com" }` (omit `issuer` to target **all + tracked issuers**). The gate records A as the proposer (first approval) and + returns `202` with an `approvalId` and expiry. Nothing is refreshed. +2. **Step 2 — approve + execute.** A *different* admin B calls + `POST /auth/oidc/jwks/refresh` with `{ "approvalId": "…" }` within the time + window (default 5 minutes). The gate verifies B ≠ A, then the handler + executes the reload (specific issuer, or all tracked issuers) and returns + `200` with the refreshed issuers. + +Guarantees: + +- **Collusion guard** — self-approval is rejected (`403`); the proposer can + never be the approver. +- **Expiry** — approvals expire after `ttlMs`; expired approvals are rejected + (`409`) and lazily swept, mirroring the OFAC review expiry/reset semantics. +- **Scope dedupe** — one active approval per scope (issuer, or `*` for all); + a second proposal for the same scope returns `409` with the existing + `approvalId` so the second admin can proceed. +- **Single execution** — an already-executed approval cannot be approved again + (`409`), so racing step-2 calls cannot double-refresh. +- **Partial failure** — refresh-all runs each issuer independently; failures + are reported in the response (`502` when all fail) and audited. + +> **Review flag (new pattern).** The repo's other dual-control flows are +> DB-backed; this gate is deliberately **in-memory and process-local** because +> it guards an idempotent, low-risk cache reload. A restart discards pending +> approvals — the worst case is a re-proposal, which is safe. If multi-instance +> coordination or persistent audit trails are required later, the gate should +> move to a shared backing store. + +### Rate limiting + +The endpoint reuses the shared `createRateLimitMiddleware` +(`src/middleware/rateLimit.ts`) with `perUser: true`, keyed on the verified +admin identity (`req.user.id` mirrored to `sub` after `requireAdmin` — the +limiter's documented per-user key), default `10 requests / minute / admin`, +isolated with `keyPrefix: 'oidc-jwks-refresh'`. Both dual-control steps share +one bucket. Each router instance should pass a dedicated `store` when multiple +router instances exist in one process (the tests do this) so buckets do not +bleed across instances. + +### Audit events + +Every outcome emits an audit event through the existing `auditRefresh` hook: + +| Event | Fields | +| --- | --- | +| Proposal (`pending_second_approval`) | `timestamp`, `actorId`, `proposerId`, `approvalId`, `issuer`/`scope` | +| Approval + execution (`success`/`failed`) | `timestamp`, `approverId`, `proposerId`, `approvalId`, `issuers` (refreshed), `failed` | +| Blocked | `timestamp`, `actorId`, `approvalId`, `status: 'blocked'`, `reason` (`unknown_approval`, `expired_approval`, `self_approval`, `already_approved`, `duplicate_proposal`) | + +Rate-limit rejections are handled by the middleware and do not emit audit +events (same convention as the other middleware-limited routes). + +### Security assumptions + +- `requireAdmin` runs before anything else; non-admin callers are rejected + before the gate or limiter are touched. +- Approver/proposer identities come from the verified JWT (`req.user.id`), + never from client-supplied body fields. +- No PII in metrics labels — only issuer URLs and admin identifiers. +- Unclassified gate/handler errors fall through to the error handler (500) + without leaking internal details. + +## Files changed + +- `src/auth/oidc/jwksCache.ts` — age gauge + ticker, tracked issuers, + multi-issuer coalescing hardening. +- `src/auth/oidc/jwksRefreshApprovalGate.ts` — new in-memory dual-control gate. +- `src/auth/oidc/oidcAdapterService.ts` — `refreshAllJwks()` (partial-failure + reporting), `getTrackedJwksIssuers()`; also fixed a pre-existing bug where + `consumedJtis` (backchannel-logout replay protection) was never declared. +- `src/auth/oidc/oidcRoute.ts` — dual-control force-refresh endpoint, shared + rate-limit middleware, enriched audit events. +- `src/index.ts` — removed a duplicate `const amlAuditRepo` declaration + (merge artifact) that broke `npm test`. ## Testing -- `npx jest --runInBand src/auth/oidc/oidc.test.ts` -- Verified 34 passing tests for both `JwksCacheService` and `createOidcRouter` refresh behavior. +```sh +npx jest --runInBand src/auth/oidc/ +``` + +The full OIDC module (7 suites, 177 tests) passes: **99.8% statements, 100% +branches, 98.5% functions, 100% lines** on `src/auth/oidc/*.ts`. The rate-limit +middleware suite (31 tests) passes unchanged. See the PR description for the +full `npm test` output and coverage report. diff --git a/src/auth/oidc/jwksCache.test.ts b/src/auth/oidc/jwksCache.test.ts new file mode 100644 index 00000000..3f2ba78c --- /dev/null +++ b/src/auth/oidc/jwksCache.test.ts @@ -0,0 +1,398 @@ +import { generateKeyPairSync } from 'crypto'; +import { JwksCacheService } from './jwksCache'; + +const uri = 'https://idp.example.com/.well-known/jwks.json'; +const issuer = 'https://idp.example.com'; +const issuerB = 'https://idp2.example.com'; +const uriB = 'https://idp2.example.com/.well-known/jwks.json'; + +const { publicKey: ecPub } = generateKeyPairSync('ec', { namedCurve: 'P-256' }); +const jwk = ecPub.export({ type: 'spki', format: 'jwk' }); + +const mockJwks = (keys: Record[] = [{ ...jwk, kid: 'k1' }]) => + ({ ok: true, json: async () => ({ keys }) } as any); + +const mockError = (status: number, statusText: string) => + ({ ok: false, status, statusText } as any); + +const waitFor = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +describe('JwksCacheService', () => { + afterEach(() => { + jest.clearAllMocks(); + jest.useRealTimers(); + }); + + describe('cache primitives', () => { + it('fetches and returns a key by kid', async () => { + const cache = new JwksCacheService(); + global.fetch = jest.fn().mockResolvedValueOnce(mockJwks()); + const key = await cache.getKey(uri, 'k1'); + expect(key).toBeDefined(); + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + it('returns cached key without re-fetching', async () => { + const cache = new JwksCacheService(); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.getKey(uri, 'k1'); + await cache.getKey(uri, 'k1'); + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + it('re-fetches after the TTL elapses', async () => { + let now = 1_000_000; + const cache = new JwksCacheService({ now: () => now }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.getKey(uri, 'k1'); + now += 60 * 60 * 1000 + 1; // past 1h TTL + await cache.getKey(uri, 'k1'); + expect(global.fetch).toHaveBeenCalledTimes(2); + }); + + it('rotates when kid is missing from cache (evict + refetch once)', async () => { + const cache = new JwksCacheService(); + global.fetch = jest.fn() + .mockResolvedValueOnce(mockJwks([{ ...jwk, kid: 'old' }])) + .mockResolvedValueOnce(mockJwks([{ ...jwk, kid: 'new' }])); + await cache.getKey(uri, 'old'); + expect(await cache.getKey(uri, 'new')).toBeDefined(); + expect(global.fetch).toHaveBeenCalledTimes(2); + }); + + it('throws after rotation when kid still missing', async () => { + const cache = new JwksCacheService(); + global.fetch = jest.fn().mockResolvedValue(mockJwks([{ ...jwk, kid: 'other' }])); + await expect(cache.getKey(uri, 'missing')).rejects.toThrow(/after rotation/); + expect(global.fetch).toHaveBeenCalledTimes(2); + }); + + it('evict forces a re-fetch on next getKey', async () => { + const cache = new JwksCacheService(); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.getKey(uri, 'k1'); + cache.evict(uri); + await cache.getKey(uri, 'k1'); + expect(global.fetch).toHaveBeenCalledTimes(2); + }); + + it('skips malformed JWK entries', async () => { + const cache = new JwksCacheService(); + global.fetch = jest.fn().mockResolvedValueOnce( + mockJwks([{ kid: 'bad', kty: 'INVALID' }, { kty: 'no-kid-at-all' }, { ...jwk, kid: 'good' }]), + ); + expect(await cache.getKey(uri, 'good')).toBeDefined(); + }); + + it('throws on non-200 JWKS response', async () => { + const cache = new JwksCacheService(); + global.fetch = jest.fn().mockResolvedValueOnce(mockError(404, 'Not Found')); + await expect(cache.refresh(uri, issuer)).rejects.toThrow(/JWKS fetch failed/); + }); + }); + + describe('per-issuer age tracking and gauge', () => { + it('reports 0 age for an issuer with no successful refresh', () => { + const cache = new JwksCacheService(); + expect(cache.getCacheAgeSeconds(issuer)).toBe(0); + }); + + it('reports ~0 age immediately after a refresh and grows with time', async () => { + let now = 1_000_000; + const cache = new JwksCacheService({ now: () => now }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + + expect(cache.getCacheAgeSeconds(issuer)).toBe(0); + + now += 90_000; + expect(cache.getCacheAgeSeconds(issuer)).toBe(90); + + now += 30_000; + expect(cache.getCacheAgeSeconds(issuer)).toBe(120); + }); + + it('emits a per-issuer age gauge on refresh', async () => { + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + + expect(metrics.setGauge).toHaveBeenCalledWith( + 'oidc.jwks.age_seconds', + 0, + { issuer }, + expect.any(String), + ); + }); + + it('does not emit a gauge for an issuer-less refresh', async () => { + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri); + expect(metrics.setGauge).not.toHaveBeenCalled(); + }); + + it('tracks each issuer independently and emits both gauges', async () => { + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any, now: () => 5_000_000 }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + await cache.refresh(uriB, issuerB); + + expect(cache.getTrackedIssuers().sort()).toEqual([issuer, issuerB].sort()); + expect(metrics.setGauge).toHaveBeenCalledWith('oidc.jwks.age_seconds', 0, { issuer }, expect.any(String)); + expect(metrics.setGauge).toHaveBeenCalledWith('oidc.jwks.age_seconds', 0, { issuer: issuerB }, expect.any(String)); + }); + + it('getTrackedIssuers is empty before any successful refresh', () => { + const cache = new JwksCacheService(); + expect(cache.getTrackedIssuers()).toEqual([]); + }); + + it('refresh-all age reset after force refresh (gauge back to ~0)', async () => { + let now = 1_000_000; + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ now: () => now, metrics: metrics as any }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + now += 300_000; + expect(cache.getCacheAgeSeconds(issuer)).toBe(300); + + await cache.refresh(uri, issuer); // force refresh + expect(cache.getCacheAgeSeconds(issuer)).toBe(0); + }); + + it('does not update last-refresh bookkeeping when the fetch fails', async () => { + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any, now: () => 5_000_000 }); + global.fetch = jest.fn().mockRejectedValueOnce(new Error('network down')); + + await expect(cache.refresh(uri, issuer)).rejects.toThrow('network down'); + expect(cache.getTrackedIssuers()).toEqual([]); + expect(cache.getCacheAgeSeconds(issuer)).toBe(0); + + // A retry after failure performs a fresh fetch and records the issuer + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + expect(cache.getTrackedIssuers()).toEqual([issuer]); + expect(global.fetch).toHaveBeenCalledTimes(1); // only the successful retry counted here + }); + }); + + describe('age-gauge ticker', () => { + it('re-emits the gauge for tracked issuers on each tick', async () => { + jest.useFakeTimers(); + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + + metrics.setGauge.mockClear(); + cache.startAgeGaugeTicker(60_000); + jest.advanceTimersByTime(60_000); + jest.advanceTimersByTime(60_000); + + expect(metrics.setGauge).toHaveBeenCalledTimes(2); + expect(metrics.setGauge).toHaveBeenLastCalledWith( + 'oidc.jwks.age_seconds', + expect.any(Number), + { issuer }, + expect.any(String), + ); + cache.stopAgeGaugeTicker(); + }); + + it('uses the configured default interval when none is passed', async () => { + jest.useFakeTimers(); + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any, ageGaugeIntervalMs: 30_000 }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + + metrics.setGauge.mockClear(); + cache.startAgeGaugeTicker(); // no explicit interval → default 30_000 + jest.advanceTimersByTime(30_000); + expect(metrics.setGauge).toHaveBeenCalledTimes(1); + cache.stopAgeGaugeTicker(); + }); + + it('unrefs the underlying timer when available (real timers)', async () => { + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any, ageGaugeIntervalMs: 5 }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + + metrics.setGauge.mockClear(); + cache.startAgeGaugeTicker(); + await waitFor(20); + expect(metrics.setGauge.mock.calls.length).toBeGreaterThanOrEqual(1); + cache.stopAgeGaugeTicker(); + }); + + it('start is idempotent (single timer) and stop halts emission', async () => { + jest.useFakeTimers(); + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + + metrics.setGauge.mockClear(); + cache.startAgeGaugeTicker(60_000); + cache.startAgeGaugeTicker(60_000); + jest.advanceTimersByTime(60_000); + expect(metrics.setGauge).toHaveBeenCalledTimes(1); + + cache.stopAgeGaugeTicker(); + jest.advanceTimersByTime(120_000); + expect(metrics.setGauge).toHaveBeenCalledTimes(1); + }); + + it('stop when not running is a no-op', () => { + const cache = new JwksCacheService(); + expect(() => cache.stopAgeGaugeTicker()).not.toThrow(); + }); + + it('tolerates timer handles that do not implement unref', () => { + const originalSetInterval = global.setInterval; + (global as any).setInterval = jest.fn(() => ({})); + try { + const cache = new JwksCacheService(); + expect(() => cache.startAgeGaugeTicker(60_000)).not.toThrow(); + expect(() => cache.stopAgeGaugeTicker()).not.toThrow(); + } finally { + global.setInterval = originalSetInterval; + } + }); + + it('ticker reflects elapsed time via the injectable clock', async () => { + jest.useFakeTimers(); + let now = 1_000_000; + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any, now: () => now }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + + metrics.setGauge.mockClear(); + cache.startAgeGaugeTicker(60_000); + now += 120_000; + jest.advanceTimersByTime(60_000); + + const [, value] = metrics.setGauge.mock.calls[0] as [string, number]; + expect(value).toBe(120); + cache.stopAgeGaugeTicker(); + }); + + it('never leaks issuers that were evicted mid-stream (ticker skips untracked)', async () => { + jest.useFakeTimers(); + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await cache.refresh(uri, issuer); + await cache.refresh(uriB, issuerB); + + // simulate the cache map being pruned without affecting tracked issuers + (cache as any).cache.clear(); + + cache.startAgeGaugeTicker(60_000); + jest.advanceTimersByTime(60_000); + expect(metrics.setGauge.mock.calls.filter((c: unknown[]) => c[0] === 'oidc.jwks.age_seconds').length).toBeGreaterThanOrEqual(2); + cache.stopAgeGaugeTicker(); + }); + }); + + describe('concurrent refresh coalescing', () => { + it('coalesces concurrent refreshes for the same URI into one fetch', async () => { + const cache = new JwksCacheService(); + let resolveFetch: (v: any) => void; + const pending = new Promise((resolve) => { resolveFetch = resolve; }); + global.fetch = jest.fn(() => pending); + + const p1 = cache.refresh(uri, issuer); + const p2 = cache.refresh(uri, issuer); + const p3 = cache.refresh(uri, issuer); + + resolveFetch!(mockJwks()); + await Promise.all([p1, p2, p3]); + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(cache.getTrackedIssuers()).toEqual([issuer]); + }); + + it('registers every issuer that waited on a shared in-flight fetch', async () => { + let resolveFetch: (v: any) => void; + const pending = new Promise((resolve) => { resolveFetch = resolve; }); + const cache = new JwksCacheService(); + global.fetch = jest.fn(() => pending); + + const p1 = cache.refresh(uri, issuer); + const p2 = cache.refresh(uri, issuerB); + + resolveFetch!(mockJwks()); + await Promise.all([p1, p2]); + + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(cache.getTrackedIssuers().sort()).toEqual([issuer, issuerB].sort()); + expect(cache.getCacheAgeSeconds(issuer)).toBe(0); + expect(cache.getCacheAgeSeconds(issuerB)).toBe(0); + }); + + it('releases the in-flight slot after success so later refreshes refetch', async () => { + const cache = new JwksCacheService({ now: () => 1_000_000 }); + global.fetch = jest.fn().mockResolvedValue(mockJwks()); + await Promise.all([cache.refresh(uri, issuer), cache.refresh(uri, issuer)]); + await cache.refresh(uri, issuer); + expect(global.fetch).toHaveBeenCalledTimes(2); + }); + + it('releases the in-flight slot after failure so retries fetch again', async () => { + const cache = new JwksCacheService(); + global.fetch = jest.fn() + .mockRejectedValueOnce(new Error('boom')) + .mockResolvedValueOnce(mockJwks()); + + const p1 = cache.refresh(uri, issuer); + const p2 = cache.refresh(uri, issuer); + await expect(p1).rejects.toThrow('boom'); + await expect(p2).rejects.toThrow('boom'); + + await cache.refresh(uri, issuer); // retry performs a fresh fetch + expect(global.fetch).toHaveBeenCalledTimes(2); + expect(cache.getTrackedIssuers()).toEqual([issuer]); + }); + + it('coalesces issuer-less concurrent refreshes too', async () => { + let resolveFetch: (v: any) => void; + const pending = new Promise((resolve) => { resolveFetch = resolve; }); + const cache = new JwksCacheService(); + global.fetch = jest.fn(() => pending); + + const p1 = cache.refresh(uri); + const p2 = cache.refresh(uri); + resolveFetch!(mockJwks()); + await Promise.all([p1, p2]); + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + it('coalesces a scheduled (getKey TTL) refresh with a force refresh', async () => { + let now = 1_000_000; + let resolveFetch: (v: any) => void; + const pending = new Promise((resolve) => { resolveFetch = resolve; }); + const cache = new JwksCacheService({ now: () => now }); + global.fetch = jest.fn() + .mockResolvedValueOnce(mockJwks()) + .mockImplementationOnce(() => pending); + + await cache.getKey(uri, 'k1', issuer); // populate + track + now += 60 * 60 * 1000 + 1; // TTL expired + + const viaGetKey = cache.getKey(uri, 'k1', issuer); // scheduled refresh path + const forced = cache.refresh(uri, issuer); // force-refresh path + + resolveFetch!(mockJwks()); + await Promise.all([viaGetKey, forced]); + expect(global.fetch).toHaveBeenCalledTimes(2); // initial + one shared refresh + expect(cache.getCacheAgeSeconds(issuer)).toBe(0); + }); + }); +}); diff --git a/src/auth/oidc/jwksCache.ts b/src/auth/oidc/jwksCache.ts index fc46c47e..846e78b5 100644 --- a/src/auth/oidc/jwksCache.ts +++ b/src/auth/oidc/jwksCache.ts @@ -7,34 +7,64 @@ interface JwksCacheEntry { issuer?: string; } -interface JwksCacheMetrics { +export interface JwksCacheMetrics { setGauge(name: string, value: number, labels?: Record, help?: string): void; } -interface JwksCacheServiceOptions { +export interface JwksCacheServiceOptions { metrics?: JwksCacheMetrics; + /** Interval (ms) at which the `oidc.jwks.age_seconds` gauges are re-emitted. Default 60s. */ + ageGaugeIntervalMs?: number; + /** Injectable clock (epoch ms) for deterministic tests. Defaults to Date.now. */ + now?: () => number; } const JWKS_TTL_MS = 60 * 60 * 1000; // 1 hour +const AGE_GAUGE_INTERVAL_MS = 60 * 1000; // 1 minute +const AGE_GAUGE_NAME = 'oidc.jwks.age_seconds'; +const AGE_GAUGE_HELP = + 'Age in seconds of the cached JWKS bundle per issuer (0 when unknown or freshly refreshed)'; /** * In-memory JWKS cache per jwks_uri. * On signature failure call evict() then getKey() again — the cache * will re-fetch, transparently handling provider key rotation. + * + * Concurrency model: + * - All refreshes to the same `jwks_uri` are coalesced into a single + * in-flight `fetch()`. Callers that arrive while a fetch is pending + * share the same promise; they register the issuer they care about so + * the per-issuer age bookkeeping is updated for every waiter on success. + * - A failed fetch removes the in-flight entry, so a retry performs a + * fresh upstream fetch (no poisoned cache of failures). + * + * Staleness alarm: + * - `startAgeGaugeTicker()` re-emits `oidc.jwks.age_seconds{issuer=...}` + * on an interval so the gauge tracks real elapsed time, not only the + * value captured at the moment of the last refresh. The ticker is + * `unref()`'d so it never keeps the process alive, and `stopAgeGaugeTicker()` + * shuts it down (used in tests and on graceful shutdown). */ export class JwksCacheService { private readonly cache = new Map(); private readonly issuerLastRefresh = new Map(); private readonly inFlightRefreshes = new Map>(); + /** Issuers waiting on an in-flight fetch, per jwks_uri — each gets age bookkeeping on success. */ + private readonly pendingIssuers = new Map>(); private readonly metrics: JwksCacheMetrics; + private readonly ageGaugeIntervalMs: number; + private readonly now: () => number; + private ageTicker: ReturnType | null = null; constructor(options: JwksCacheServiceOptions = {}) { this.metrics = options.metrics ?? globalMetrics; + this.ageGaugeIntervalMs = options.ageGaugeIntervalMs ?? AGE_GAUGE_INTERVAL_MS; + this.now = options.now ?? (() => Date.now()); } async getKey(jwksUri: string, kid: string, issuer?: string): Promise { let entry = this.cache.get(jwksUri); - if (!entry || Date.now() - entry.fetchedAt > JWKS_TTL_MS) { + if (!entry || this.now() - entry.fetchedAt > JWKS_TTL_MS) { entry = await this.refresh(jwksUri, issuer ?? entry?.issuer); } @@ -50,31 +80,77 @@ export class JwksCacheService { return rotated; } + /** + * Age in seconds of the last successful JWKS refresh for `issuer`. + * Returns 0 when the issuer has never had a successful refresh recorded. + */ getCacheAgeSeconds(issuer: string): number { const lastRefresh = this.issuerLastRefresh.get(issuer); if (!lastRefresh) return 0; - return Math.max(0, Math.floor((Date.now() - lastRefresh) / 1000)); + return Math.max(0, Math.floor((this.now() - lastRefresh) / 1000)); } + /** Issuers with at least one recorded successful refresh. */ + getTrackedIssuers(): string[] { + return [...this.issuerLastRefresh.keys()]; + } + + /** Re-emit the age gauge for every tracked issuer (used by the ticker and on refresh). */ + emitAgeGauges(): void { + for (const issuer of this.getTrackedIssuers()) { + this.metrics.setGauge(AGE_GAUGE_NAME, this.getCacheAgeSeconds(issuer), { issuer }, AGE_GAUGE_HELP); + } + } + + /** + * Start re-emitting `oidc.jwks.age_seconds` per tracked issuer on an interval. + * Idempotent — a second call while running is a no-op. The timer is unref'd. + */ + startAgeGaugeTicker(intervalMs: number = this.ageGaugeIntervalMs): void { + if (this.ageTicker) return; + this.ageTicker = setInterval(() => { + this.emitAgeGauges(); + }, intervalMs); + if (typeof this.ageTicker.unref === 'function') { + this.ageTicker.unref(); + } + } + + /** Stop the age-gauge ticker. Safe to call when not running. */ + stopAgeGaugeTicker(): void { + if (this.ageTicker !== null) { + clearInterval(this.ageTicker); + this.ageTicker = null; + } + } + + /** + * Force a JWKS fetch for `jwksUri` (bypasses the TTL), coalesced per URI: + * concurrent callers for the same URI share one upstream fetch. On success + * every issuer that waited on this fetch is recorded with the new fetchedAt. + */ async refresh(jwksUri: string, issuer?: string): Promise { const existing = this.inFlightRefreshes.get(jwksUri); - if (existing) return existing; + if (existing) { + if (issuer) this.registerPendingIssuer(jwksUri, issuer); + return existing; + } + if (issuer) this.registerPendingIssuer(jwksUri, issuer); const pending = this.fetchAndCache(jwksUri, issuer) .then((entry) => { - if (issuer) { - this.issuerLastRefresh.set(issuer, entry.fetchedAt); - this.metrics.setGauge( - 'oidc.jwks.age_seconds', - this.getCacheAgeSeconds(issuer), - { issuer }, - 'Age in seconds of the cached JWKS bundle per issuer', - ); + const issuers = this.pendingIssuers.get(jwksUri); + if (issuers) { + for (const waitingIssuer of issuers) { + this.issuerLastRefresh.set(waitingIssuer, entry.fetchedAt); + } } + this.emitAgeGauges(); return entry; }) .finally(() => { this.inFlightRefreshes.delete(jwksUri); + this.pendingIssuers.delete(jwksUri); }); this.inFlightRefreshes.set(jwksUri, pending); @@ -85,6 +161,15 @@ export class JwksCacheService { this.cache.delete(jwksUri); } + private registerPendingIssuer(jwksUri: string, issuer: string): void { + let issuers = this.pendingIssuers.get(jwksUri); + if (!issuers) { + issuers = new Set(); + this.pendingIssuers.set(jwksUri, issuers); + } + issuers.add(issuer); + } + private async fetchAndCache(jwksUri: string, issuer?: string): Promise { const res = await fetch(jwksUri); if (!res.ok) throw new Error(`JWKS fetch failed: ${res.status} ${res.statusText}`); @@ -100,7 +185,7 @@ export class JwksCacheService { } catch { /* skip malformed entries */ } } - const entry: JwksCacheEntry = { keys: keyMap, fetchedAt: Date.now(), issuer }; + const entry: JwksCacheEntry = { keys: keyMap, fetchedAt: this.now(), issuer }; this.cache.set(jwksUri, entry); return entry; } diff --git a/src/auth/oidc/jwksRefreshApprovalGate.test.ts b/src/auth/oidc/jwksRefreshApprovalGate.test.ts new file mode 100644 index 00000000..2caedc84 --- /dev/null +++ b/src/auth/oidc/jwksRefreshApprovalGate.test.ts @@ -0,0 +1,162 @@ +import { + ALL_ISSUERS_SCOPE, + ApprovalAlreadyApprovedError, + ApprovalExpiredError, + ApprovalNotFoundError, + ApprovalSelfApprovalError, + DuplicateApprovalError, + JwksRefreshApprovalGate, + JWKS_REFRESH_APPROVAL_TTL_MS, +} from './jwksRefreshApprovalGate'; + +describe('JwksRefreshApprovalGate', () => { + let now: number; + let gate: JwksRefreshApprovalGate; + let idCounter: number; + + beforeEach(() => { + now = 1_000_000; + idCounter = 0; + gate = new JwksRefreshApprovalGate({ + ttlMs: JWKS_REFRESH_APPROVAL_TTL_MS, + now: () => now, + randomId: () => `approval-${++idCounter}`, + }); + }); + + describe('propose (step 1)', () => { + it('creates a pending approval for a specific issuer', () => { + const approval = gate.propose('admin-a', 'https://idp.example.com'); + expect(approval).toMatchObject({ + approvalId: 'approval-1', + scope: 'https://idp.example.com', + issuer: 'https://idp.example.com', + proposer: 'admin-a', + status: 'pending_second_approval', + }); + expect(approval.expiresAt).toBe(now + JWKS_REFRESH_APPROVAL_TTL_MS); + }); + + it('creates a pending approval for the global (all issuers) scope when issuer omitted', () => { + const approval = gate.propose('admin-a'); + expect(approval.scope).toBe(ALL_ISSUERS_SCOPE); + expect(approval.issuer).toBeUndefined(); + }); + + it('rejects a duplicate active proposal for the same scope', () => { + gate.propose('admin-a', 'https://idp.example.com'); + try { + gate.propose('admin-b', 'https://idp.example.com'); + throw new Error('should have thrown'); + } catch (err) { + expect(err).toBeInstanceOf(DuplicateApprovalError); + expect((err as DuplicateApprovalError).existingApprovalId).toBe('approval-1'); + } + }); + + it('allows a new proposal for a different scope while another is pending', () => { + gate.propose('admin-a', 'https://idp.example.com'); + const second = gate.propose('admin-a', 'https://idp2.example.com'); + expect(second.scope).toBe('https://idp2.example.com'); + }); + + it('allows a new proposal once the previous one expired', () => { + gate.propose('admin-a', 'https://idp.example.com'); + now += JWKS_REFRESH_APPROVAL_TTL_MS + 1; + const second = gate.propose('admin-a', 'https://idp.example.com'); + expect(second.approvalId).toBe('approval-2'); + }); + + it('allows a new proposal once the previous one was executed', () => { + const first = gate.propose('admin-a', 'https://idp.example.com'); + gate.approve(first.approvalId, 'admin-b'); + const second = gate.propose('admin-a', 'https://idp.example.com'); + expect(second.approvalId).toBe('approval-2'); + }); + + it('generates unique approval ids by default', () => { + const fresh = new JwksRefreshApprovalGate(); + const a = fresh.propose('admin-a', 'issuer-1'); + const b = fresh.propose('admin-b', 'issuer-2'); + expect(a.approvalId).not.toBe(b.approvalId); + }); + }); + + describe('approve (step 2)', () => { + it('approves when a distinct admin acts within the window', () => { + const approval = gate.propose('admin-a', 'https://idp.example.com'); + const result = gate.approve(approval.approvalId, 'admin-b'); + expect(result.status).toBe('approved'); + expect(result.approvalId).toBe(approval.approvalId); + }); + + it('rejects self-approval by the proposer', () => { + const approval = gate.propose('admin-a', 'https://idp.example.com'); + expect(() => gate.approve(approval.approvalId, 'admin-a')) + .toThrow(ApprovalSelfApprovalError); + }); + + it('rejects an unknown approval id', () => { + expect(() => gate.approve('nope', 'admin-b')) + .toThrow(ApprovalNotFoundError); + }); + + it('rejects an expired approval and removes it', () => { + const approval = gate.propose('admin-a', 'https://idp.example.com'); + now += JWKS_REFRESH_APPROVAL_TTL_MS + 1; + expect(() => gate.approve(approval.approvalId, 'admin-b')) + .toThrow(ApprovalExpiredError); + expect(gate.get(approval.approvalId)).toBeUndefined(); + }); + + it('rejects approving an already-executed approval', () => { + const approval = gate.propose('admin-a', 'https://idp.example.com'); + gate.approve(approval.approvalId, 'admin-b'); + expect(() => gate.approve(approval.approvalId, 'admin-c')) + .toThrow(ApprovalAlreadyApprovedError); + }); + + it('rejects at exactly the expiry boundary (now === expiresAt is still valid)', () => { + const approval = gate.propose('admin-a', 'https://idp.example.com'); + now = approval.expiresAt; + expect(() => gate.approve(approval.approvalId, 'admin-b')).not.toThrow(); + }); + }); + + describe('read / sweep helpers', () => { + it('returns undefined for unknown or expired approvals via get()', () => { + expect(gate.get('unknown')).toBeUndefined(); + const approval = gate.propose('admin-a', 'https://idp.example.com'); + now += JWKS_REFRESH_APPROVAL_TTL_MS + 1; + expect(gate.get(approval.approvalId)).toBeUndefined(); + }); + + it('returns an approved approval via get() even after its window', () => { + const approval = gate.propose('admin-a', 'https://idp.example.com'); + gate.approve(approval.approvalId, 'admin-b'); + now += 10 * 60 * 1000; + expect(gate.get(approval.approvalId)?.status).toBe('approved'); + }); + + it('cleanupExpired removes only expired pending approvals', () => { + const fresh = gate.propose('admin-a', 'https://a.example.com'); + const stale = gate.propose('admin-b', 'https://b.example.com'); + const executed = gate.propose('admin-c', 'https://c.example.com'); + gate.approve(executed.approvalId, 'admin-d'); + + now += JWKS_REFRESH_APPROVAL_TTL_MS + 1; + const removed = gate.cleanupExpired(); + + expect(removed).toBe(2); + expect(gate.get(fresh.approvalId)).toBeUndefined(); + expect(gate.get(stale.approvalId)).toBeUndefined(); + expect(gate.get(executed.approvalId)?.status).toBe('approved'); + }); + + it('reset clears all approvals', () => { + const approval = gate.propose('admin-a', 'https://idp.example.com'); + gate.reset(); + expect(gate.get(approval.approvalId)).toBeUndefined(); + }); + }); +}); diff --git a/src/auth/oidc/jwksRefreshApprovalGate.ts b/src/auth/oidc/jwksRefreshApprovalGate.ts new file mode 100644 index 00000000..711a3b82 --- /dev/null +++ b/src/auth/oidc/jwksRefreshApprovalGate.ts @@ -0,0 +1,199 @@ +import { randomBytes } from 'crypto'; + +/** + * Two-step dual-control approval gate for JWKS force-refreshes. + * + * This is an in-memory (process-local) gate modeled on the existing + * two-approval patterns in this codebase (OFAC review queue, + * tenantSettingsService proposal/approval, ledger period-close): + * the proposer records the first approval by proposing, and a *distinct* + * admin must approve within a time window for the action to execute. + * + * Security properties: + * - Self-approval is rejected (collusion guard): the proposer cannot be + * the approver. + * - Approvals expire after `ttlMs` and must be re-proposed (mirrors the + * OFAC review expiry/reset semantics). + * - Only one active approval per scope (issuer, or the global scope for + * "refresh all tracked issuers") to prevent duplicate in-flight work. + * - An already-executed approval cannot be approved again. + * - No PII is stored — only admin identifiers and the issuer scope. + * + * NOTE (review): this is a NEW pattern for the API layer. The repo's + * other dual-control flows are DB-backed (persistent across restarts); + * this gate is intentionally process-local because it only guards an + * idempotent, low-risk cache reload. A restart discards pending + * approvals, which is safe (the worst case is a re-proposal). + */ + +export type JwksRefreshApprovalStatus = 'pending_second_approval' | 'approved'; + +export interface JwksRefreshApproval { + approvalId: string; + /** Dedupe scope: the issuer URL, or {@link ALL_ISSUERS_SCOPE} for all tracked issuers. */ + scope: string; + /** Specific issuer to refresh, or undefined for "all tracked issuers". */ + issuer?: string; + /** Admin identity that proposed (first approval). */ + proposer: string; + /** Epoch ms when the proposal was created. */ + createdAt: number; + /** Epoch ms after which the approval is void. */ + expiresAt: number; + status: JwksRefreshApprovalStatus; +} + +/** Sentinel scope value for "refresh all tracked issuers". */ +export const ALL_ISSUERS_SCOPE = '*'; + +/** Default time window for a second admin to approve. */ +export const JWKS_REFRESH_APPROVAL_TTL_MS = 5 * 60 * 1000; // 5 minutes + +export class ApprovalNotFoundError extends Error { + constructor(approvalId: string) { + super(`Unknown JWKS refresh approval "${approvalId}"`); + this.name = 'ApprovalNotFoundError'; + } +} + +export class ApprovalExpiredError extends Error { + constructor(approvalId: string) { + super(`JWKS refresh approval "${approvalId}" has expired — propose a new refresh`); + this.name = 'ApprovalExpiredError'; + } +} + +export class ApprovalSelfApprovalError extends Error { + constructor() { + super('The proposer of a JWKS refresh cannot self-approve (dual-control)'); + this.name = 'ApprovalSelfApprovalError'; + } +} + +export class ApprovalAlreadyApprovedError extends Error { + constructor(approvalId: string) { + super(`JWKS refresh approval "${approvalId}" has already been approved and executed`); + this.name = 'ApprovalAlreadyApprovedError'; + } +} + +export class DuplicateApprovalError extends Error { + /** The approvalId of the already-pending proposal for the same scope. */ + public readonly existingApprovalId: string; + constructor(scope: string, existingApprovalId: string) { + super(`A JWKS refresh for scope "${scope}" is already pending approval`); + this.name = 'DuplicateApprovalError'; + this.existingApprovalId = existingApprovalId; + } +} + +export interface JwksRefreshApprovalGateOptions { + /** Time window in ms for the second approval. Default 5 minutes. */ + ttlMs?: number; + /** Injectable clock (epoch ms) for deterministic tests. */ + now?: () => number; + /** Injectable approval-id generator for deterministic tests. */ + randomId?: () => string; +} + +export class JwksRefreshApprovalGate { + private readonly approvals = new Map(); + private readonly ttlMs: number; + private readonly now: () => number; + private readonly randomId: () => string; + + constructor(options: JwksRefreshApprovalGateOptions = {}) { + this.ttlMs = options.ttlMs ?? JWKS_REFRESH_APPROVAL_TTL_MS; + this.now = options.now ?? (() => Date.now()); + this.randomId = options.randomId ?? (() => randomBytes(16).toString('base64url')); + } + + /** + * Step 1 of dual-control. Records `actor` as proposer (first approval) + * for the scope and returns the pending approval. Throws + * {@link DuplicateApprovalError} when an unexpired approval already + * exists for the same scope. + */ + propose(actor: string, issuer?: string): JwksRefreshApproval { + const scope = issuer ?? ALL_ISSUERS_SCOPE; + const existing = this.findActiveByScope(scope); + if (existing) { + throw new DuplicateApprovalError(scope, existing.approvalId); + } + + const now = this.now(); + const approval: JwksRefreshApproval = { + approvalId: this.randomId(), + scope, + issuer, + proposer: actor, + createdAt: now, + expiresAt: now + this.ttlMs, + status: 'pending_second_approval', + }; + this.approvals.set(approval.approvalId, approval); + return approval; + } + + /** + * Step 2 of dual-control. `actor` (must differ from the proposer) + * approves the pending request within its time window. On success the + * approval is marked `approved` and the caller may execute the refresh. + * + * @throws {ApprovalNotFoundError} unknown approvalId + * @throws {ApprovalExpiredError} past the time window (entry removed) + * @throws {ApprovalSelfApprovalError} actor is the proposer + * @throws {ApprovalAlreadyApprovedError} already executed + */ + approve(approvalId: string, actor: string): JwksRefreshApproval { + const approval = this.approvals.get(approvalId); + if (!approval) throw new ApprovalNotFoundError(approvalId); + if (approval.status === 'approved') throw new ApprovalAlreadyApprovedError(approvalId); + if (this.now() > approval.expiresAt) { + this.approvals.delete(approvalId); + throw new ApprovalExpiredError(approvalId); + } + if (approval.proposer === actor) throw new ApprovalSelfApprovalError(); + + approval.status = 'approved'; + return approval; + } + + /** Read a single approval (audit/tests). Returns undefined when unknown. */ + get(approvalId: string): JwksRefreshApproval | undefined { + const approval = this.approvals.get(approvalId); + if (!approval) return undefined; + if (approval.status === 'pending_second_approval' && this.now() > approval.expiresAt) { + return undefined; // expired entries are treated as absent + } + return approval; + } + + /** Remove expired pending approvals; returns the number removed. */ + cleanupExpired(): number { + const now = this.now(); + let removed = 0; + for (const [id, approval] of this.approvals.entries()) { + if (approval.status === 'pending_second_approval' && now > approval.expiresAt) { + this.approvals.delete(id); + removed += 1; + } + } + return removed; + } + + /** Clear all state (test/restart helper). */ + reset(): void { + this.approvals.clear(); + } + + private findActiveByScope(scope: string): JwksRefreshApproval | undefined { + for (const approval of this.approvals.values()) { + if (approval.scope !== scope) continue; + if (approval.status === 'approved') continue; + if (this.now() > approval.expiresAt) continue; + return approval; + } + return undefined; + } +} diff --git a/src/auth/oidc/oidc.test.ts b/src/auth/oidc/oidc.test.ts index 5bf82cfb..a406f899 100644 --- a/src/auth/oidc/oidc.test.ts +++ b/src/auth/oidc/oidc.test.ts @@ -4,6 +4,9 @@ import request from 'supertest'; import { OidcAdapterService } from './oidcAdapterService'; import { JwksCacheService } from './jwksCache'; import { createOidcRouter } from './oidcRoute'; +import { AuthenticatedRequest } from '../../middleware/auth'; +import { JwksRefreshApprovalGate } from './jwksRefreshApprovalGate'; +import { InMemoryRateLimitStore } from '../../middleware/rateLimit'; import { ALLOWED_ID_TOKEN_ALGORITHMS, OidcProviderRow } from './types'; // ── Helpers ──────────────────────────────────────────────────────────────── @@ -274,6 +277,52 @@ describe('OidcAdapterService', () => { .rejects.toThrow(/missing alg/); }); + it('rejects a malformed ID token header', async () => { + await expect(service.validateIdToken('!!.xx.yy', makeProvider(), discovery, 'n')) + .rejects.toThrow(/Malformed ID token header/); + }); + + it('handleCallback exchanges the code end-to-end and validates the ID token', async () => { + const nonce = 'nonce-e2e'; + (service as any).flowStates.set('state-ok', { + tenantId: 'acme', + codeVerifier: 'code-verifier', + nonce, + redirectUri: 'https://app.example.com/callback', + expiresAt: Date.now() + 60_000, + }); + global.fetch = jest.fn() + .mockResolvedValueOnce({ ok: true, json: async () => makeDiscovery() } as any) + .mockResolvedValueOnce({ ok: true, json: async () => ({ id_token: signToken(validClaims(nonce)) }) } as any); + + const claims = await service.handleCallback('exchange-code', 'state-ok', makeProvider()); + expect(claims.sub).toBe('user-42'); + + const [, init] = (global.fetch as jest.Mock).mock.calls[1]; + expect(init.method).toBe('POST'); + const body = init.body.toString(); + expect(body).toContain('grant_type=authorization_code'); + expect(body).toContain('code=exchange-code'); + expect(body).toContain('code_verifier=code-verifier'); + expect(body).toContain('client_id=client-123'); + }); + + it('handleCallback throws when the token exchange fails', async () => { + (service as any).flowStates.set('state-fail', { + tenantId: 'acme', + codeVerifier: 'v', + nonce: 'n-fail', + redirectUri: 'u', + expiresAt: Date.now() + 60_000, + }); + global.fetch = jest.fn() + .mockResolvedValueOnce({ ok: true, json: async () => makeDiscovery() } as any) + .mockResolvedValueOnce({ ok: false, status: 400, statusText: 'Bad Request', text: async () => 'invalid_grant' } as any); + + await expect(service.handleCallback('code', 'state-fail', makeProvider())) + .rejects.toThrow(/Token exchange failed \(400\)/); + }); + it('rejects nonce mismatch', async () => { const token = signToken(validClaims('real-nonce')); await expect(service.validateIdToken(token, makeProvider(), discovery, 'wrong-nonce')) @@ -342,7 +391,144 @@ describe('OidcAdapterService', () => { expect.arrayContaining(['RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512', 'PS256']), ); }); -}); + + // ── JWKS refresh orchestration ───────────────────────────────────────── + + describe('JWKS refresh orchestration', () => { + it('refreshJwks refreshes the issuer JWKS via its discovery jwks_uri', async () => { + const discovery = makeDiscovery(); + service = new OidcAdapterService({ refresh: jest.fn().mockResolvedValue({ keys: new Map() }) } as any, { + discoveryTtlMs: 60_000, + }); + global.fetch = jest.fn().mockResolvedValueOnce({ ok: true, json: async () => discovery } as any); + + await service.refreshJwks('https://idp.example.com'); + expect((service as any).jwksCache.refresh).toHaveBeenCalledWith( + discovery.jwks_uri, + 'https://idp.example.com', + ); + }); + + it('getTrackedJwksIssuers delegates to the cache', () => { + const jwksCache = { getTrackedIssuers: jest.fn().mockReturnValue(['a', 'b']) } as any; + service = new OidcAdapterService(jwksCache); + expect(service.getTrackedJwksIssuers()).toEqual(['a', 'b']); + expect(jwksCache.getTrackedIssuers).toHaveBeenCalledTimes(1); + }); + + it('refreshAllJwks refreshes every tracked issuer and reports partial failures', async () => { + const jwksCache = { + getTrackedIssuers: jest.fn().mockReturnValue(['https://a.example.com', 'https://b.example.com']), + } as any; + const adapter = new OidcAdapterService(jwksCache); + adapter.refreshJwks = jest.fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('timeout')); + + const result = await adapter.refreshAllJwks(); + expect(result).toEqual({ + refreshed: ['https://a.example.com'], + failed: [{ issuer: 'https://b.example.com', error: 'timeout' }], + }); + }); + + it('refreshAllJwks returns empty results when no issuers are tracked', async () => { + const adapter = new OidcAdapterService({ getTrackedIssuers: jest.fn().mockReturnValue([]) } as any); + adapter.refreshJwks = jest.fn(); + expect(await adapter.refreshAllJwks()).toEqual({ refreshed: [], failed: [] }); + expect(adapter.refreshJwks).not.toHaveBeenCalled(); + }); + + it('refreshAllJwks stringifies non-Error rejections', async () => { + const adapter = new OidcAdapterService({ + getTrackedIssuers: jest.fn().mockReturnValue(['https://a.example.com']), + } as any); + adapter.refreshJwks = jest.fn().mockRejectedValueOnce('raw string failure'); + const result = await adapter.refreshAllJwks(); + expect(result.failed).toEqual([{ issuer: 'https://a.example.com', error: 'raw string failure' }]); + }); + }); + + // ── Adapter edge cases (branch completeness) ─────────────────────────── + + describe('adapter edge cases', () => { + it('honours a valid OIDC_DISCOVERY_TTL_MS override and ignores invalid values', () => { + const prev = process.env.OIDC_DISCOVERY_TTL_MS; + process.env.OIDC_DISCOVERY_TTL_MS = '42000'; + expect((new OidcAdapterService({} as any) as any).discoveryTtlMs).toBe(42000); + process.env.OIDC_DISCOVERY_TTL_MS = '-5'; + expect((new OidcAdapterService({} as any) as any).discoveryTtlMs).toBe(60 * 60 * 1000); + process.env.OIDC_DISCOVERY_TTL_MS = 'not-a-number'; + expect((new OidcAdapterService({} as any) as any).discoveryTtlMs).toBe(60 * 60 * 1000); + if (prev === undefined) delete process.env.OIDC_DISCOVERY_TTL_MS; + else process.env.OIDC_DISCOVERY_TTL_MS = prev; + }); + + it('handleCallback rejects when the flow-state tenant mismatches the provider', async () => { + const adapter = new OidcAdapterService({ getKey: jest.fn() } as any); + (adapter as any).flowStates.set('state-tenant-x', { + tenantId: 'tenant-a', + codeVerifier: 'v', + nonce: 'n', + redirectUri: 'u', + expiresAt: Date.now() + 60_000, + }); + await expect(adapter.handleCallback('code', 'state-tenant-x', makeProvider())) + .rejects.toThrow('State tenant mismatch'); + }); + + it('exchangeCode includes client_secret when the provider has one', async () => { + const adapter = new OidcAdapterService({ getKey: jest.fn() } as any); + let sentBody = ''; + global.fetch = jest.fn().mockImplementation(async (_url: string, opts: any) => { + sentBody = opts.body as string; + return { ok: true, json: async () => ({ access_token: 'at', id_token: 'it', expires_in: 300 }) } as any; + }); + const flowState = { + tenantId: 'acme', + codeVerifier: 'cv', + nonce: 'n', + redirectUri: 'https://app.example.com/callback', + expiresAt: Date.now() + 60_000, + }; + const tokens = await (adapter as any).exchangeCode('c', flowState, makeProvider({ client_secret: 'shh' }), makeDiscovery()); + expect(sentBody).toContain('client_secret=shh'); + expect(tokens.access_token).toBe('at'); + }); + + it('rejects an ID token missing the kid header', async () => { + const h = Buffer.from(JSON.stringify({ alg: 'RS256' })).toString('base64url'); + const p = Buffer.from(JSON.stringify(validClaims('n-kid'))).toString('base64url'); + await expect(service.validateIdToken(`${h}.${p}.sig`, makeProvider(), makeDiscovery(), 'n-kid')) + .rejects.toThrow('ID token missing kid header'); + }); + + it('retries ID token verification when jwt.verify reports "unable to verify"', async () => { + const jwt = require('jsonwebtoken') as { verify: unknown }; + const spy = jest.spyOn(jwt as { verify: (...a: unknown[]) => unknown }, 'verify') + .mockImplementationOnce(() => { throw new Error('unable to verify'); }) + .mockImplementation(() => validClaims('n-unable') as any); + try { + const claims = await service.validateIdToken(signToken(validClaims('n-unable')), makeProvider(), makeDiscovery(), 'n-unable'); + expect(claims.sub).toBe('user-42'); + } finally { + spy.mockRestore(); + } + }); + + it('rejects an ID token when jwt.verify throws a non-Error value', async () => { + const jwt = require('jsonwebtoken') as { verify: unknown }; + const spy = jest.spyOn(jwt as { verify: (...a: unknown[]) => unknown }, 'verify') + .mockImplementationOnce(() => { throw 'boom'; }); + try { + await expect(service.validateIdToken(signToken(validClaims('n-boom')), makeProvider(), makeDiscovery(), 'n-boom')) + .rejects.toThrow('ID token validation failed: boom'); + } finally { + spy.mockRestore(); + } + }); + }); + }); // ── JwksCacheService ─────────────────────────────────────────────────────── @@ -485,7 +671,7 @@ describe('createOidcRouter OIDC logout', () => { app.use(createOidcRouter({ oidcAdapter, oidcProviderRepo, - requireAdmin: (req, _res, next) => { req.user = { id: 'admin' }; next(); }, + requireAdmin: (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction) => { req.user = { id: 'admin' }; next(); }, })); const token = signLogoutToken({ @@ -529,7 +715,7 @@ describe('createOidcRouter OIDC logout', () => { app.use(createOidcRouter({ oidcAdapter, oidcProviderRepo, - requireAdmin: (req, _res, next) => { req.user = { id: 'admin' }; next(); }, + requireAdmin: (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction) => { req.user = { id: 'admin' }; next(); }, })); const token = signLogoutToken({ @@ -555,7 +741,7 @@ describe('createOidcRouter OIDC logout', () => { app.use(createOidcRouter({ oidcAdapter: {} as any, oidcProviderRepo: {} as any, - requireAdmin: (req, _res, next) => { req.user = { id: 'admin' }; next(); }, + requireAdmin: (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction) => { req.user = { id: 'admin' }; next(); }, })); const res = await request(app).post('/api/auth/oidc/logout').send({}); @@ -570,7 +756,7 @@ describe('createOidcRouter OIDC logout', () => { app.use(createOidcRouter({ oidcAdapter: {} as any, oidcProviderRepo: {} as any, - requireAdmin: (req, _res, next) => { req.user = { id: 'admin' }; next(); }, + requireAdmin: (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction) => { req.user = { id: 'admin' }; next(); }, })); const res = await request(app) @@ -591,7 +777,7 @@ describe('createOidcRouter OIDC logout', () => { app.use(createOidcRouter({ oidcAdapter: {} as any, oidcProviderRepo: {} as any, - requireAdmin: (req, _res, next) => { req.user = { id: 'admin' }; next(); }, + requireAdmin: (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction) => { req.user = { id: 'admin' }; next(); }, })); const res = await request(app) @@ -612,7 +798,7 @@ describe('createOidcRouter OIDC logout', () => { app.use(createOidcRouter({ oidcAdapter: {} as any, oidcProviderRepo, - requireAdmin: (req, _res, next) => { req.user = { id: 'admin' }; next(); }, + requireAdmin: (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction) => { req.user = { id: 'admin' }; next(); }, })); const token = signLogoutToken({ @@ -651,7 +837,7 @@ describe('createOidcRouter OIDC logout', () => { app.use(createOidcRouter({ oidcAdapter, oidcProviderRepo, - requireAdmin: (req, _res, next) => { req.user = { id: 'admin' }; next(); }, + requireAdmin: (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction) => { req.user = { id: 'admin' }; next(); }, })); const token = signLogoutToken({ @@ -690,7 +876,7 @@ describe('createOidcRouter OIDC logout', () => { app.use(createOidcRouter({ oidcAdapter, oidcProviderRepo, - requireAdmin: (req, _res, next) => { req.user = { id: 'admin' }; next(); }, + requireAdmin: (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction) => { req.user = { id: 'admin' }; next(); }, })); const token = signLogoutToken({ @@ -714,61 +900,362 @@ describe('createOidcRouter OIDC logout', () => { // ── OIDC route: JWKS refresh ──────────────────────────────────────────── -describe('createOidcRouter JWKS refresh', () => { - it('requires admin dual confirmation before refreshing JWKS', async () => { +describe('createOidcRouter JWKS refresh (dual-control)', () => { + const issuerA = 'https://idp.example.com'; + const issuerB = 'https://idp2.example.com'; + const uriA = 'https://idp.example.com/.well-known/jwks.json'; + const uriB = 'https://idp2.example.com/.well-known/jwks.json'; + + function makeApp(overrides: Partial[0]> = {}) { const oidcAdapter = { getDiscovery: jest.fn(), refreshJwks: jest.fn().mockResolvedValue(undefined), + refreshAllJwks: jest.fn().mockResolvedValue({ refreshed: [issuerA], failed: [] }), + getTrackedJwksIssuers: jest.fn().mockReturnValue([issuerA]), } as any; const oidcProviderRepo = {} as any; const auditRefresh = jest.fn(); const requireAdmin = (req: any, _res: any, next: () => void) => { - req.user = { id: 'admin-1' }; + req.user = { id: req.get('x-admin-id') ?? 'admin-1' }; next(); }; - + // Dedicated rate-limit store per router instance: the shared default store + // would let buckets accumulate across tests in the same worker. const app = express(); app.use(express.json()); - app.use(createOidcRouter({ oidcAdapter, oidcProviderRepo, requireAdmin, auditRefresh })); + app.use( + createOidcRouter({ + oidcAdapter, + oidcProviderRepo, + requireAdmin, + auditRefresh, + ...overrides, + rateLimitOptions: { + store: new InMemoryRateLimitStore(), + ...overrides.rateLimitOptions, + }, + }), + ); + return { app, oidcAdapter, auditRefresh }; + } - const missingConfirmation = await request(app) - .post('/api/auth/oidc/jwks/refresh') - .set('x-revora-oidc-jwks-confirmation', 'true') - .send({ confirmation: false, issuerUrl: 'https://idp.example.com' }); + const propose = (app: express.Express, body: Record = {}, user = 'admin-1') => + request(app).post('/auth/oidc/jwks/refresh').set('x-admin-id', user).send(body); - expect(missingConfirmation.status).toBe(400); + it('requires dual-control: a single admin proposal alone does not refresh', async () => { + const { app, oidcAdapter } = makeApp(); + const res = await propose(app, { issuer: issuerA }); + expect(res.status).toBe(202); + expect(res.body.status).toBe('pending_second_approval'); + expect(res.body.approvalId).toBeTruthy(); + expect(res.body.proposer).toBe('admin-1'); expect(oidcAdapter.refreshJwks).not.toHaveBeenCalled(); - expect(auditRefresh).toHaveBeenCalledWith(expect.objectContaining({ status: 'blocked' })); + expect(oidcAdapter.refreshAllJwks).not.toHaveBeenCalled(); }); - it('rate-limits repeated refresh attempts for the same actor', async () => { - const oidcAdapter = { - getDiscovery: jest.fn(), - refreshJwks: jest.fn().mockResolvedValue(undefined), + it('executes the refresh only after a distinct admin approves', async () => { + const { app, oidcAdapter } = makeApp(); + const step1 = await propose(app, { issuer: issuerA }); + expect(step1.status).toBe(202); + + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-2'); + expect(step2.status).toBe(200); + expect(step2.body.status).toBe('approved'); + expect(step2.body.refreshedIssuers).toEqual([issuerA]); + expect(oidcAdapter.refreshJwks).toHaveBeenCalledWith(issuerA); + }); + + it('rejects self-approval by the proposing admin (dual-control collusion guard)', async () => { + const { app, oidcAdapter } = makeApp(); + const step1 = await propose(app, { issuer: issuerA }); + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-1'); + expect(step2.status).toBe(403); + expect(step2.body.message).toMatch(/self-approve/i); + expect(oidcAdapter.refreshJwks).not.toHaveBeenCalled(); + }); + + it('refreshes all tracked issuers when no issuer is specified', async () => { + const { app, oidcAdapter, auditRefresh } = makeApp(); + oidcAdapter.getTrackedJwksIssuers.mockReturnValue([issuerA, issuerB]); + oidcAdapter.refreshAllJwks.mockResolvedValue({ refreshed: [issuerA, issuerB], failed: [] }); + + const step1 = await propose(app, {}); + expect(step1.status).toBe(202); + expect(step1.body.scope).toBe('*'); + expect(step1.body.issuer).toBeNull(); + + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-2'); + expect(step2.status).toBe(200); + expect(oidcAdapter.refreshAllJwks).toHaveBeenCalledTimes(1); + expect(step2.body.refreshedIssuers).toEqual([issuerA, issuerB]); + expect(auditRefresh).toHaveBeenCalledWith( + expect.objectContaining({ + status: 'success', + proposerId: 'admin-1', + approverId: 'admin-2', + issuers: [issuerA, issuerB], + }), + ); + }); + + it('rejects a duplicate proposal for the same scope and surfaces the pending approvalId', async () => { + const { app, oidcAdapter } = makeApp(); + const step1 = await propose(app, { issuer: issuerA }); + const dup = await propose(app, { issuer: issuerA }, 'admin-2'); + expect(dup.status).toBe(409); + expect(dup.body.details?.approvalId).toBe(step1.body.approvalId); + expect(oidcAdapter.refreshJwks).not.toHaveBeenCalled(); + }); + + it('rejects an unknown approvalId', async () => { + const { app, oidcAdapter } = makeApp(); + const res = await propose(app, { approvalId: 'does-not-exist' }, 'admin-2'); + expect(res.status).toBe(404); + expect(oidcAdapter.refreshJwks).not.toHaveBeenCalled(); + }); + + it('rejects an expired approval and does not refresh', async () => { + const { app, oidcAdapter } = makeApp({ approvalGate: new JwksRefreshApprovalGate({ ttlMs: 60_000 }) }); + const step1 = await propose(app, { issuer: issuerA }); + jest.useFakeTimers({ now: Date.now() }); + jest.advanceTimersByTime(60_001); + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-2'); + expect(step2.status).toBe(409); + expect(step2.body.message).toMatch(/expired/i); + expect(oidcAdapter.refreshJwks).not.toHaveBeenCalled(); + jest.useRealTimers(); + }); + + it('executes exactly one refresh when two admins race to approve the same proposal', async () => { + const { app, oidcAdapter } = makeApp(); + const step1 = await propose(app, { issuer: issuerA }); + + const [first, second] = await Promise.all([ + propose(app, { approvalId: step1.body.approvalId }, 'admin-2'), + propose(app, { approvalId: step1.body.approvalId }, 'admin-3'), + ]); + + expect([first.status, second.status].sort()).toEqual([200, 409]); + expect(oidcAdapter.refreshJwks).toHaveBeenCalledTimes(1); + }); + + it('returns 502 when a refresh-all fails for every tracked issuer', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.refreshAllJwks.mockResolvedValue({ + refreshed: [], + failed: [{ issuer: issuerA, error: 'JWKS fetch failed: 503 Service Unavailable' }], + }); + + const step1 = await propose(app, {}); + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-2'); + expect(step2.status).toBe(502); + expect(step2.body.failed).toEqual([expect.objectContaining({ issuer: issuerA })]); + }); + + it('returns partial success when only some tracked issuers refresh', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.refreshAllJwks.mockResolvedValue({ + refreshed: [issuerA], + failed: [{ issuer: issuerB, error: 'timeout' }], + }); + + const step1 = await propose(app, {}); + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-2'); + expect(step2.status).toBe(200); + expect(step2.body.refreshedIssuers).toEqual([issuerA]); + expect(step2.body.failed).toEqual([expect.objectContaining({ issuer: issuerB })]); + }); + + it('audits every step with timestamps and actor identities', async () => { + const { app, auditRefresh } = makeApp(); + await propose(app, { issuer: issuerA }); + + const proposeEvent = auditRefresh.mock.calls[0][0]; + expect(proposeEvent).toMatchObject({ + action: 'jwks_refresh', + actorId: 'admin-1', + proposerId: 'admin-1', + issuer: issuerA, + scope: issuerA, + status: 'pending_second_approval', + }); + expect(proposeEvent.timestamp).toEqual(expect.any(String)); + + const step1 = await propose(app, { issuer: issuerA }); // duplicate → blocked audit + expect(step1.status).toBe(409); + const blockedEvent = auditRefresh.mock.calls[1][0]; + expect(blockedEvent).toMatchObject({ status: 'blocked', reason: 'duplicate_proposal' }); + + const step2 = await propose(app, { approvalId: proposeEvent.approvalId }, 'admin-2'); + expect(step2.status).toBe(200); + const successEvent = auditRefresh.mock.calls[2][0]; + expect(successEvent).toMatchObject({ + status: 'success', + proposerId: 'admin-1', + approverId: 'admin-2', + issuers: [issuerA], + }); + }); + + it('rejects non-admin callers before touching the approval gate', async () => { + const requireAdmin = (req: any, res: any, next: () => void) => { + res.status(403).json({ error: 'Forbidden', message: 'Forbidden: admin role required' }); + }; + const { app } = makeApp({ requireAdmin }); + const res = await propose(app, { issuer: issuerA }); + expect(res.status).toBe(403); + }); + + it('rate-limits force-refresh requests per admin identity', async () => { + const { app, oidcAdapter } = makeApp({ rateLimitOptions: { limit: 1, windowMs: 60_000 } }); + const first = await propose(app, { issuer: issuerA }); + expect(first.status).toBe(202); + const second = await propose(app, { issuer: issuerA }); + expect(second.status).toBe(429); + expect(oidcAdapter.refreshJwks).not.toHaveBeenCalled(); + }); + + it('rate-limit buckets are isolated per admin identity', async () => { + const { app } = makeApp({ rateLimitOptions: { limit: 1, windowMs: 60_000 } }); + const adminA = await propose(app, { issuer: issuerA }, 'admin-1'); + expect(adminA.status).toBe(202); + // different admin + different scope — own rate-limit bucket and own proposal + const adminB = await propose(app, { issuer: issuerB }, 'admin-2'); + expect(adminB.status).toBe(202); + }); + + it('concurrent force-refresh of the same issuer coalesces into a single upstream fetch', async () => { + const metrics = { setGauge: jest.fn() }; + const cache = new JwksCacheService({ metrics: metrics as any }); + let resolveFetch: (v: any) => void; + const pending = new Promise((resolve) => { resolveFetch = resolve; }); + global.fetch = jest.fn(() => pending); + + const p1 = cache.refresh(uriA, issuerA); + const p2 = cache.refresh(uriA, issuerA); + resolveFetch!({ ok: true, json: async () => ({ keys: [] }) } as any); + await Promise.all([p1, p2]); + + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(cache.getTrackedIssuers()).toEqual([issuerA]); + }); + + it('coalesces force-refresh across different issuers sharing one jwks_uri', async () => { + const cache = new JwksCacheService(); + let resolveFetch: (v: any) => void; + const pending = new Promise((resolve) => { resolveFetch = resolve; }); + global.fetch = jest.fn(() => pending); + + const p1 = cache.refresh(uriA, issuerA); + const p2 = cache.refresh(uriA, issuerB); + resolveFetch!({ ok: true, json: async () => ({ keys: [] }) } as any); + await Promise.all([p1, p2]); + + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(cache.getTrackedIssuers().sort()).toEqual([issuerA, issuerB].sort()); + }); + + it('audits a failed single-issuer refresh and surfaces the error', async () => { + const { app, oidcAdapter, auditRefresh } = makeApp(); + oidcAdapter.refreshJwks.mockRejectedValue(new Error('JWKS fetch failed: 503 Service Unavailable')); + + const step1 = await propose(app, { issuer: issuerA }); + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-2'); + + expect(step2.status).toBe(500); + expect(auditRefresh).toHaveBeenLastCalledWith( + expect.objectContaining({ status: 'failed', reason: expect.stringContaining('503') }), + ); + }); + + it('audits an unknown gate error without leaking internal details', async () => { + const brokenGate = { + propose: () => { + throw new Error('gate exploded'); + }, } as any; - const oidcProviderRepo = {} as any; - const auditRefresh = jest.fn(); + const { app, auditRefresh } = makeApp({ approvalGate: brokenGate }); + const res = await propose(app, { issuer: issuerA }); + expect(res.status).toBe(500); + expect(auditRefresh).not.toHaveBeenCalled(); // unclassified gate errors are not audited as blocked events + }); + + it('rethrows unclassified approval-step errors to the error handler', async () => { + const gate = new JwksRefreshApprovalGate(); + const realApprove = gate.approve.bind(gate); + gate.approve = ((approvalId: string, actor: string) => { + if (actor === 'admin-2') throw new Error('gate exploded'); + return realApprove(approvalId, actor); + }) as any; + const { app } = makeApp({ approvalGate: gate }); + const step1 = await propose(app, { issuer: issuerA }); + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-2'); + expect(step2.status).toBe(500); + }); + + it('endpoint works on the /api/auth/oidc/jwks/refresh alias too', async () => { + const { app } = makeApp(); + const res = await request(app) + .post('/api/auth/oidc/jwks/refresh') + .send({ issuer: issuerA }); + expect(res.status).toBe(202); + }); + + it('keeps an existing rate-limit subject when the admin already sets req.user.sub', async () => { + // When requireAdmin attaches `sub`, fillRateLimitSubject must leave it + // untouched — admins sharing a sub share the bucket. const requireAdmin = (req: any, _res: any, next: () => void) => { - req.user = { id: 'admin-2' }; + req.user = { id: 'admin-1', sub: 'fixed-bucket' }; next(); }; + const { app } = makeApp({ requireAdmin, rateLimitOptions: { limit: 1, windowMs: 60_000 } }); + const first = await propose(app, { issuer: issuerA }, 'admin-1'); + expect(first.status).toBe(202); + const secondAdmin = await propose(app, { issuer: issuerA }, 'admin-2'); + expect(secondAdmin.status).toBe(429); + }); - const app = express(); - app.use(express.json()); - app.use(createOidcRouter({ oidcAdapter, oidcProviderRepo, requireAdmin, auditRefresh })); + it('falls back to req.user.sub when the admin has no id', async () => { + const requireAdmin = (req: any, _res: any, next: () => void) => { + req.user = { sub: 'sub-only-admin' }; + next(); + }; + const { app, auditRefresh } = makeApp({ requireAdmin }); + const res = await propose(app, { issuer: issuerA }); + expect(res.status).toBe(202); + expect(auditRefresh.mock.calls[0][0]).toMatchObject({ actorId: 'sub-only-admin' }); + }); - const first = await request(app) - .post('/api/auth/oidc/jwks/refresh') - .set('x-revora-oidc-jwks-confirmation', 'true') - .send({ confirmation: true, issuerUrl: 'https://idp.example.com' }); + it('falls back to the request IP when no admin identity is attached', async () => { + const requireAdmin = (_req: any, _res: any, next: () => void) => next(); + const { app, auditRefresh } = makeApp({ requireAdmin }); + const res = await propose(app, { issuer: issuerA }); + expect(res.status).toBe(202); + expect(auditRefresh.mock.calls[0][0].actorId).toContain('127.0.0.1'); + }); - const second = await request(app) - .post('/api/auth/oidc/jwks/refresh') - .set('x-revora-oidc-jwks-confirmation', 'true') - .send({ confirmation: true, issuerUrl: 'https://idp.example.com' }); + it('falls back to "anonymous" when neither identity nor IP is available', async () => { + const requireAdmin = (req: any, _res: any, next: () => void) => { + Object.defineProperty(req, 'ip', { value: undefined, configurable: true }); + next(); + }; + const { app, auditRefresh } = makeApp({ requireAdmin }); + const res = await propose(app, { issuer: issuerA }); + expect(res.status).toBe(202); + expect(auditRefresh.mock.calls[0][0]).toMatchObject({ actorId: 'anonymous' }); + }); - expect(first.status).toBe(200); - expect(second.status).toBe(429); - expect(oidcAdapter.refreshJwks).toHaveBeenCalledTimes(1); + it('records a non-Error refresh execution failure in the audit trail', async () => { + const { app, oidcAdapter, auditRefresh } = makeApp(); + oidcAdapter.refreshAllJwks.mockRejectedValue('backend exploded'); + const step1 = await propose(app); + const step2 = await propose(app, { approvalId: step1.body.approvalId }, 'admin-2'); + expect(step2.status).toBe(500); + const calls = auditRefresh.mock.calls; + expect(calls[calls.length - 1][0]).toMatchObject({ + status: 'failed', + reason: 'backend exploded', + approverId: 'admin-2', + }); }); }); diff --git a/src/auth/oidc/oidcAdapterService.ts b/src/auth/oidc/oidcAdapterService.ts index 54e4db8f..e80d6ec4 100644 --- a/src/auth/oidc/oidcAdapterService.ts +++ b/src/auth/oidc/oidcAdapterService.ts @@ -17,6 +17,13 @@ const DEFAULT_DISCOVERY_TTL_MS = 60 * 60 * 1000; // 1 hour const CLOCK_SKEW_SECONDS = 300; // 5 minutes const STATE_TTL_MS = 10 * 60 * 1000; // 10 minutes +export interface JwksRefreshAllResult { + /** Issuers whose JWKS bundles were reloaded successfully. */ + refreshed: string[]; + /** Issuers whose reload failed, with the sanitized error message. */ + failed: Array<{ issuer: string; error: string }>; +} + interface DiscoveryCacheEntry { doc: OidcDiscoveryDocument; cachedUntil: number; @@ -52,7 +59,7 @@ export class OidcAdapterService { /** SHA-256 hex digest of the last accepted discovery document, keyed by issuer URL. */ private readonly discoveryDigests = new Map(); private readonly flowStates = new Map(); - /** Set of already-consumed logout token JTIs with their exp timestamps for replay protection. */ + /** `jti` values already consumed by backchannel-logout tokens, with their expiry (epoch ms). */ private readonly consumedJtis = new Map(); private readonly discoveryTtlMs: number; private readonly metrics: OidcDiscoveryMetrics; @@ -213,6 +220,36 @@ export class OidcAdapterService { await this.jwksCache.refresh(discovery.jwks_uri, issuerUrl); } + /** Issuers for which the JWKS cache has recorded at least one successful refresh. */ + getTrackedJwksIssuers(): string[] { + return this.jwksCache.getTrackedIssuers(); + } + + /** + * Force-refresh the JWKS bundle of every tracked issuer. Runs each issuer + * independently (partial success is possible and reported) so one failing + * provider cannot block an incident reload of the rest. + */ + async refreshAllJwks(): Promise { + const issuers = this.getTrackedJwksIssuers(); + const settled = await Promise.allSettled( + issuers.map((issuerUrl) => this.refreshJwks(issuerUrl)), + ); + + const refreshed: string[] = []; + const failed: Array<{ issuer: string; error: string }> = []; + settled.forEach((result, index) => { + const issuer = issuers[index]; + if (result.status === 'fulfilled') { + refreshed.push(issuer); + } else { + const error = result.reason instanceof Error ? result.reason.message : String(result.reason); + failed.push({ issuer, error }); + } + }); + return { refreshed, failed }; + } + // ── ID Token validation ─────────────────────────────────────────────── async validateIdToken( diff --git a/src/auth/oidc/oidcAdminRoutes.test.ts b/src/auth/oidc/oidcAdminRoutes.test.ts new file mode 100644 index 00000000..11fcf845 --- /dev/null +++ b/src/auth/oidc/oidcAdminRoutes.test.ts @@ -0,0 +1,395 @@ +import express from 'express'; +import request from 'supertest'; +import { createOidcRouter } from './oidcRoute'; + +/** + * Coverage + behavior tests for the pre-existing OIDC admin/authorize/logout + * routes. Kept separate from the refresh dual-control tests in oidc.test.ts. + */ + +const provider = { + id: 'uuid-1', + tenant_id: 'acme', + name: 'Acme IdP', + issuer_url: 'https://idp.example.com', + client_id: 'client-123', + client_secret: 'super-secret', + scopes: 'openid profile email', + redirect_uris: 'https://app.example.com/callback', + enabled: true, + created_at: new Date(), +}; + +const discovery = { + issuer: 'https://idp.example.com', + authorization_endpoint: 'https://idp.example.com/authorize', + token_endpoint: 'https://idp.example.com/token', + jwks_uri: 'https://idp.example.com/.well-known/jwks.json', +}; + +const requireAdmin = (req: any, _res: any, next: () => void) => { + req.user = { id: 'admin-1' }; + next(); +}; + +function makeApp(overrides: { + oidcAdapter?: Record; + oidcProviderRepo?: Record; + userRepo?: Record; + oidcGroupMappingRepo?: Record; + auditLogRepo?: Record; +} = {}) { + const oidcAdapter = { + consumeFlowState: jest.fn().mockReturnValue({ tenantId: 'acme' }), + getDiscovery: jest.fn().mockResolvedValue(discovery), + exchangeCode: jest.fn().mockResolvedValue({ id_token: 'token' }), + validateIdToken: jest.fn().mockResolvedValue({ sub: 'user-42', email: 'u@example.com' }), + validateLogoutToken: jest.fn(), + buildAuthorizeUrl: jest.fn(), + ...overrides.oidcAdapter, + } as any; + const oidcProviderRepo = { + findByTenantId: jest.fn().mockResolvedValue(provider), + findByIssuerUrl: jest.fn().mockResolvedValue(provider), + findAll: jest.fn().mockResolvedValue([provider]), + create: jest.fn().mockResolvedValue(provider), + ...overrides.oidcProviderRepo, + } as any; + const userRepo = { + findByEmail: jest.fn(), + updateUser: jest.fn(), + ...overrides.userRepo, + } as any; + const oidcGroupMappingRepo = { + findByTenantId: jest.fn().mockResolvedValue([]), + ...overrides.oidcGroupMappingRepo, + } as any; + const auditLogRepo = { + createAuditLog: jest.fn(), + ...overrides.auditLogRepo, + } as any; + const app = express(); + app.use(express.json()); + app.use( + createOidcRouter({ + oidcAdapter, + oidcProviderRepo, + userRepo, + oidcGroupMappingRepo, + auditLogRepo, + requireAdmin, + }), + ); + return { app, oidcAdapter, oidcProviderRepo, userRepo, oidcGroupMappingRepo, auditLogRepo }; +} + +describe('createOidcRouter authorize', () => { + it('returns 400 when tenantId is missing', async () => { + const { app } = makeApp(); + const res = await request(app).get('/api/auth/oidc/authorize'); + expect(res.status).toBe(400); + }); + + it('returns 404 when no provider exists for the tenant', async () => { + const { app, oidcProviderRepo } = makeApp(); + oidcProviderRepo.findByTenantId.mockResolvedValue(null); + const res = await request(app).get('/api/auth/oidc/authorize?tenantId=ghost'); + expect(res.status).toBe(404); + }); + + it('redirects to the IdP authorize URL on success', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.buildAuthorizeUrl.mockResolvedValue({ url: 'https://idp.example.com/authorize?x=1', state: 's1' }); + const res = await request(app).get('/api/auth/oidc/authorize?tenantId=acme'); + expect(res.status).toBe(302); + expect(res.headers.location).toBe('https://idp.example.com/authorize?x=1'); + }); + + it('forwards adapter failures to the error handler', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.buildAuthorizeUrl.mockRejectedValue(new Error('boom')); + const res = await request(app).get('/api/auth/oidc/authorize?tenantId=acme'); + expect(res.status).toBe(500); + }); +}); + +describe('createOidcRouter admin providers', () => { + it('rejects provider creation with missing required fields', async () => { + const { app, oidcProviderRepo } = makeApp(); + const res = await request(app) + .post('/api/auth/oidc/providers') + .send({ tenantId: 'acme' }); + expect(res.status).toBe(400); + expect(oidcProviderRepo.create).not.toHaveBeenCalled(); + }); + + it('creates a provider without leaking the client secret', async () => { + const { app, oidcProviderRepo } = makeApp(); + const res = await request(app) + .post('/api/auth/oidc/providers') + .send({ + tenantId: 'acme', + name: 'Acme', + issuerUrl: 'https://idp.example.com', + clientId: 'client-123', + redirectUris: 'https://app.example.com/callback', + }); + expect(res.status).toBe(201); + expect(res.body.client_secret).toBeUndefined(); + expect(oidcProviderRepo.create).toHaveBeenCalledWith( + expect.objectContaining({ tenantId: 'acme', name: 'Acme', issuerUrl: 'https://idp.example.com' }), + ); + }); + + it('forwards provider creation failures to the error handler', async () => { + const { app, oidcProviderRepo } = makeApp(); + oidcProviderRepo.create.mockRejectedValue(new Error('db down')); + const res = await request(app) + .post('/api/auth/oidc/providers') + .send({ + tenantId: 'acme', + name: 'Acme', + issuerUrl: 'https://idp.example.com', + clientId: 'client-123', + redirectUris: 'https://app.example.com/callback', + }); + expect(res.status).toBe(500); + }); + + it('lists providers without leaking client secrets', async () => { + const { app } = makeApp(); + const res = await request(app).get('/api/auth/oidc/providers'); + expect(res.status).toBe(200); + expect(res.body).toHaveLength(1); + expect(res.body[0].client_secret).toBeUndefined(); + }); + + it('forwards provider listing failures to the error handler', async () => { + const { app, oidcProviderRepo } = makeApp(); + oidcProviderRepo.findAll.mockRejectedValue(new Error('db down')); + const res = await request(app).get('/api/auth/oidc/providers'); + expect(res.status).toBe(500); + }); +}); + +describe('createOidcRouter callback errors', () => { + it('returns 400 when code or state is missing', async () => { + const { app } = makeApp(); + const missingCode = await request(app).get('/api/auth/oidc/callback?state=abc'); + expect(missingCode.status).toBe(400); + const missingState = await request(app).get('/api/auth/oidc/callback?code=abc'); + expect(missingState.status).toBe(400); + }); + + it('returns 404 when the flow tenant has no provider', async () => { + const { app, oidcProviderRepo, oidcAdapter } = makeApp(); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'ghost' }); + oidcProviderRepo.findByTenantId.mockResolvedValue(null); + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + expect(res.status).toBe(404); + }); + + it('maps expired/invalid/mismatch failures to 401', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'acme' }); + oidcAdapter.validateIdToken.mockRejectedValue(new Error('ID token nonce mismatch')); + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + expect(res.status).toBe(401); + }); + + it('forwards unexpected callback failures to the error handler', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'acme' }); + oidcAdapter.validateIdToken.mockRejectedValue(new Error('random failure')); + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + expect(res.status).toBe(500); + }); + + it('maps claim groups to a role and updates the user when groups change', async () => { + const { app, oidcAdapter, userRepo, oidcGroupMappingRepo, auditLogRepo } = makeApp({ + userRepo: { + // No last_oidc_groups stored yet — exercises the `|| []` fallback. + findByEmail: jest.fn().mockResolvedValue({ id: 'u1' }), + updateUser: jest.fn().mockResolvedValue(undefined), + }, + oidcGroupMappingRepo: { + findByTenantId: jest.fn().mockResolvedValue([{ claim_group: 'startups', revora_role: 'startup' }]), + }, + }); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'acme' }); + oidcAdapter.validateIdToken.mockResolvedValue({ + sub: 'user-42', + email: 'u@example.com', + groups: ['startups'], + }); + + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + + expect(res.status).toBe(200); + expect(res.body.mappedRole).toBe('startup'); + expect(res.body.tenantId).toBe('acme'); + expect(oidcGroupMappingRepo.findByTenantId).toHaveBeenCalledWith('acme'); + expect(auditLogRepo.createAuditLog).toHaveBeenCalledWith(expect.objectContaining({ + user_id: 'u1', + action: 'oidc.claim.changed', + })); + expect(userRepo.updateUser).toHaveBeenCalledWith({ + id: 'u1', + last_oidc_groups: ['startups'], + role: 'startup', + }); + }); + + it('handles callback claims without groups gracefully', async () => { + const { app, oidcAdapter, oidcGroupMappingRepo } = makeApp(); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'acme' }); + oidcAdapter.validateIdToken.mockResolvedValue({ sub: 'user-42', email: 'u@example.com' }); + + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + + expect(res.status).toBe(200); + expect(res.body.mappedRole).toBeUndefined(); + expect(oidcGroupMappingRepo.findByTenantId).not.toHaveBeenCalled(); + }); + + it('skips user reconciliation when claims carry no email', async () => { + const { app, oidcAdapter, userRepo } = makeApp(); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'acme' }); + oidcAdapter.validateIdToken.mockResolvedValue({ sub: 'user-42', groups: ['startups'] }); + + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + + expect(res.status).toBe(200); + expect(userRepo.findByEmail).not.toHaveBeenCalled(); + }); + + it('leaves the user untouched when groups are unchanged and no role maps', async () => { + const { app, oidcAdapter, userRepo, auditLogRepo } = makeApp({ + userRepo: { + findByEmail: jest.fn().mockResolvedValue({ id: 'u1', last_oidc_groups: ['startups'] }), + updateUser: jest.fn(), + }, + oidcGroupMappingRepo: { + findByTenantId: jest.fn().mockResolvedValue([]), + }, + }); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'acme' }); + oidcAdapter.validateIdToken.mockResolvedValue({ + sub: 'user-42', + email: 'u@example.com', + groups: ['startups'], + }); + + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + + expect(res.status).toBe(200); + expect(res.body.mappedRole).toBeUndefined(); + expect(auditLogRepo.createAuditLog).not.toHaveBeenCalled(); + expect(userRepo.updateUser).not.toHaveBeenCalled(); + }); + + it('skips user updates when the email matches no user', async () => { + const { app, oidcAdapter, userRepo, auditLogRepo } = makeApp({ + userRepo: { + findByEmail: jest.fn().mockResolvedValue(null), + updateUser: jest.fn(), + }, + oidcGroupMappingRepo: { + // Mapping exists but does not match the claim group — exercises the + // predicate's no-match arm. + findByTenantId: jest.fn().mockResolvedValue([{ claim_group: 'investors', revora_role: 'investor' }]), + }, + }); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'acme' }); + oidcAdapter.validateIdToken.mockResolvedValue({ + sub: 'user-42', + email: 'ghost@example.com', + groups: ['startups'], + }); + + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + + expect(res.status).toBe(200); + expect(userRepo.findByEmail).toHaveBeenCalledWith('ghost@example.com'); + expect(auditLogRepo.createAuditLog).not.toHaveBeenCalled(); + expect(userRepo.updateUser).not.toHaveBeenCalled(); + }); + + it('forwards non-Error callback failures to the error handler', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.consumeFlowState.mockReturnValue({ tenantId: 'acme' }); + oidcAdapter.validateIdToken.mockRejectedValue('boom'); + const res = await request(app).get('/api/auth/oidc/callback?code=abc&state=xyz'); + expect(res.status).toBe(500); + }); +}); + +describe('createOidcRouter logout route', () => { + const validLogoutToken = (iss = 'https://idp.example.com') => { + const h = Buffer.from(JSON.stringify({ alg: 'RS256', kid: 'k1' })).toString('base64url'); + const p = Buffer.from(JSON.stringify({ iss })).toString('base64url'); + return `${h}.${p}.sig`; + }; + + it('returns 400 when logout_token is missing', async () => { + const { app } = makeApp(); + const res = await request(app).post('/api/auth/oidc/logout'); + expect(res.status).toBe(400); + }); + + it('returns 400 for a malformed logout token', async () => { + const { app } = makeApp(); + const res = await request(app).post('/api/auth/oidc/logout').send({ logout_token: 'not-a-jwt' }); + expect(res.status).toBe(400); + }); + + it('returns 400 when the token has no issuer', async () => { + const { app } = makeApp(); + const h = Buffer.from(JSON.stringify({ alg: 'RS256' })).toString('base64url'); + const p = Buffer.from(JSON.stringify({})).toString('base64url'); + const res = await request(app).post('/api/auth/oidc/logout').send({ logout_token: `${h}.${p}.sig` }); + expect(res.status).toBe(400); + }); + + it('returns 400 when no provider matches the token issuer', async () => { + const { app, oidcProviderRepo } = makeApp(); + oidcProviderRepo.findByIssuerUrl.mockResolvedValue(null); + const res = await request(app).post('/api/auth/oidc/logout').send({ logout_token: validLogoutToken() }); + expect(res.status).toBe(400); + }); + + it('maps validation failures (expired/invalid/mismatch/replayed) to 400', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.validateLogoutToken.mockRejectedValue(new Error('Logout token replayed')); + const res = await request(app).post('/api/auth/oidc/logout').send({ logout_token: validLogoutToken() }); + expect(res.status).toBe(400); + }); + + it('forwards non-Error logout failures to the error handler', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.validateLogoutToken.mockRejectedValue('boom'); + const res = await request(app).post('/api/auth/oidc/logout').send({ logout_token: validLogoutToken() }); + expect(res.status).toBe(500); + }); + + it('completes a valid logout and invalidates user sessions', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.validateLogoutToken.mockResolvedValue({ sub: 'user-42' }); + const { sessionStore } = await import('../../lib/sessionStore'); + const del = jest.spyOn(sessionStore, 'deleteAllForUser').mockResolvedValue(undefined); + const { globalMetrics } = await import('../../lib/metrics'); + + const res = await request(app).post('/auth/oidc/logout').send({ logout_token: validLogoutToken() }); + expect(res.status).toBe(200); + expect(del).toHaveBeenCalledWith('user-42'); + expect(globalMetrics.getSnapshot()).toBeDefined(); + del.mockRestore(); + }); + + it('forwards unexpected logout failures to the error handler', async () => { + const { app, oidcAdapter } = makeApp(); + oidcAdapter.validateLogoutToken.mockRejectedValue(new Error('random failure')); + const res = await request(app).post('/api/auth/oidc/logout').send({ logout_token: validLogoutToken() }); + expect(res.status).toBe(500); + }); +}); diff --git a/src/auth/oidc/oidcLogout.test.ts b/src/auth/oidc/oidcLogout.test.ts index d8bbcb1a..00f62d71 100644 --- a/src/auth/oidc/oidcLogout.test.ts +++ b/src/auth/oidc/oidcLogout.test.ts @@ -128,4 +128,134 @@ describe('OidcAdapterService - Logout Token Validation', () => { await expect(adapter.validateLogoutToken(token, provider, discovery)) .rejects.toThrow('Logout token signature invalid after JWKS rotation'); }); + + it('rejects a malformed logout token header', async () => { + await expect(adapter.validateLogoutToken('!!.xx.yy', provider, discovery)) + .rejects.toThrow('Malformed logout token header'); + }); + + it('rejects an insecure (none) logout token algorithm', async () => { + const h = Buffer.from(JSON.stringify({ alg: 'none', kid: 'key-1' })).toString('base64url'); + const p = Buffer.from(JSON.stringify({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + })).toString('base64url'); + await expect(adapter.validateLogoutToken(`${h}.${p}.`, provider, discovery)) + .rejects.toThrow('Insecure algorithm rejected'); + }); + + it('rejects an unknown logout token algorithm', async () => { + const h = Buffer.from(JSON.stringify({ alg: 'CUSTOM99', kid: 'key-1' })).toString('base64url'); + const p = Buffer.from(JSON.stringify({ sub: 'user-1', events: { 'http://schemas.openid.net/event/backchannel-logout': {} } })).toString('base64url'); + await expect(adapter.validateLogoutToken(`${h}.${p}.sig`, provider, discovery)) + .rejects.toThrow('Unknown or disallowed algorithm'); + }); + + it('rejects a logout token with a non-signature validation failure', async () => { + const token = jwt.sign({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + exp: Math.floor(Date.now() / 1000) - 600, // beyond the 300s clock-skew window + }, privateKey, { + algorithm: 'RS256', + keyid: 'key-1', + issuer: provider.issuer_url, + audience: provider.client_id, + }); + await expect(adapter.validateLogoutToken(token, provider, discovery)) + .rejects.toThrow(/Logout token validation failed/); + }); + + it('lazily cleans up consumed jtis whose exp has passed', async () => { + // Seed a stale entry, then validate a fresh token — the lazy cleanup must + // drop the expired entry while keeping the freshly consumed one. + (adapter as any).consumedJtis.set('seeded-stale', (Math.floor(Date.now() / 1000) - 3600) * 1000); + + const fresh = signToken({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + jti: 'jti-fresh', + exp: Math.floor(Date.now() / 1000) + 300, + }); + await adapter.validateLogoutToken(fresh, provider, discovery); + + expect((adapter as any).consumedJtis.has('seeded-stale')).toBe(false); + expect((adapter as any).consumedJtis.has('jti-fresh')).toBe(true); + }); + + it('does not retain a consumed jti whose exp has already passed', async () => { + const stale = signToken({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + jti: 'jti-stale', + exp: Math.floor(Date.now() / 1000) - 60, // within clock skew, so it verifies + }); + await adapter.validateLogoutToken(stale, provider, discovery); + // set() then lazy-cleanup in the same call removes the self-expired entry + expect((adapter as any).consumedJtis.has('jti-stale')).toBe(false); + }); + + it('rejects a logout token missing the alg header', async () => { + const h = Buffer.from(JSON.stringify({ kid: 'key-1' })).toString('base64url'); + const p = Buffer.from(JSON.stringify({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + })).toString('base64url'); + await expect(adapter.validateLogoutToken(`${h}.${p}.sig`, provider, discovery)) + .rejects.toThrow('Logout token missing alg header'); + }); + + it('rejects a logout token missing the kid header', async () => { + const h = Buffer.from(JSON.stringify({ alg: 'RS256' })).toString('base64url'); + const p = Buffer.from(JSON.stringify({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + })).toString('base64url'); + await expect(adapter.validateLogoutToken(`${h}.${p}.sig`, provider, discovery)) + .rejects.toThrow('Logout token missing kid header'); + }); + + it('retries logout token verification when jwt.verify reports "unable to verify"', async () => { + const originalVerify = jwt.verify.bind(jwt); + const spy = jest.spyOn(jwt, 'verify') + .mockImplementationOnce(() => { throw new Error('unable to verify'); }) + .mockImplementation((...args: Parameters) => originalVerify(...args)); + try { + const token = signToken({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + jti: 'jti-retry-1', + }); + const claims = await adapter.validateLogoutToken(token, provider, discovery); + expect(claims.sub).toBe('user-1'); + expect(jwksCache.evict).toHaveBeenCalledWith(discovery.jwks_uri); + } finally { + spy.mockRestore(); + } + }); + + it('accepts a logout token without a jti', async () => { + const token = signToken({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + }); + const claims = await adapter.validateLogoutToken(token, provider, discovery); + expect(claims.sub).toBe('user-1'); + }); + + it('rejects a logout token when jwt.verify throws a non-Error value', async () => { + const spy = jest.spyOn(jwt, 'verify') + .mockImplementationOnce(() => { throw 'boom'; }); + try { + const token = signToken({ + sub: 'user-1', + events: { 'http://schemas.openid.net/event/backchannel-logout': {} }, + jti: 'jti-boom', + }); + await expect(adapter.validateLogoutToken(token, provider, discovery)) + .rejects.toThrow('Logout token validation failed: boom'); + } finally { + spy.mockRestore(); + } + }); }); diff --git a/src/auth/oidc/oidcRoute.ts b/src/auth/oidc/oidcRoute.ts index 2c2c6b76..46948484 100644 --- a/src/auth/oidc/oidcRoute.ts +++ b/src/auth/oidc/oidcRoute.ts @@ -1,5 +1,10 @@ import { NextFunction, Request, Response, Router } from 'express'; import { AuthenticatedRequest } from '../../middleware/auth'; +import { + createRateLimitMiddleware, + RateLimitOptions, + RateLimitStore, +} from '../../middleware/rateLimit'; import { OidcAdapterService } from './oidcAdapterService'; import { OidcProviderRepository, CreateOidcProviderInput } from '../../db/repositories/oidcProviderRepository'; import { UserRepository } from '../../db/repositories/userRepository'; @@ -7,6 +12,14 @@ import { OidcGroupMappingRepository } from '../../db/repositories/oidcGroupMappi import { AuditLogRepository } from '../../db/repositories/auditLogRepository'; import { sessionStore } from '../../lib/sessionStore'; import { globalMetrics } from '../../lib/metrics'; +import { + ApprovalAlreadyApprovedError, + ApprovalExpiredError, + ApprovalNotFoundError, + ApprovalSelfApprovalError, + DuplicateApprovalError, + JwksRefreshApprovalGate, +} from './jwksRefreshApprovalGate'; export interface OidcRouterDependencies { oidcAdapter: OidcAdapterService; @@ -18,8 +31,19 @@ export interface OidcRouterDependencies { requireAdmin: (req: Request, res: Response, next: NextFunction) => void; /** Optional audit hook for JWKS refresh events */ auditRefresh?: (event: Record) => void | Promise; + /** Dual-control approval gate; defaults to a fresh in-memory gate. */ + approvalGate?: JwksRefreshApprovalGate; + /** + * Rate-limit options for the force-refresh endpoint (tests tighten these). + * Pass a dedicated `store` when multiple router instances share a process + * (e.g. tests) so per-admin buckets do not bleed across instances. + */ + rateLimitOptions?: RateLimitOptions & { store?: RateLimitStore }; } +const JWKS_REFRESH_RATE_LIMIT = 10; // 10 calls / min / admin (each dual-control step counts) +const JWKS_REFRESH_RATE_WINDOW_MS = 60_000; + /** * OIDC SSO router. * @@ -28,46 +52,172 @@ export interface OidcRouterDependencies { * GET /api/auth/oidc/callback?code=…&state=… — handle IdP callback * POST /api/auth/oidc/providers — (admin) register a provider * GET /api/auth/oidc/providers — (admin) list providers + * POST /auth/oidc/jwks/refresh — (admin, dual-control) force-reload JWKS */ export function createOidcRouter(deps: OidcRouterDependencies): Router { const { oidcAdapter, oidcProviderRepo, userRepo, oidcGroupMappingRepo, auditLogRepo, requireAdmin, auditRefresh } = deps; const router = Router(); - const refreshCooldownMs = 60_000; - const refreshAttempts = new Map(); + const approvalGate = deps.approvalGate ?? new JwksRefreshApprovalGate(); + + // requireAdmin populates req.user with { id, role } — the shared rate-limit + // middleware keys per-user buckets on req.user.sub, so mirror id → sub after + // admin auth. This keeps the bucket tied to the verified admin identity + // rather than a shared egress IP. + const fillRateLimitSubject = (req: Request, _res: Response, next: NextFunction) => { + const user = (req as AuthenticatedRequest).user; + if (user && typeof user.id === 'string' && !user.sub) { + user.sub = user.id; + } + next(); + }; + const refreshLimiter = createRateLimitMiddleware({ + limit: JWKS_REFRESH_RATE_LIMIT, + windowMs: JWKS_REFRESH_RATE_WINDOW_MS, + perUser: true, + keyPrefix: 'oidc-jwks-refresh', + message: 'JWKS refresh is rate-limited; try again later', + ...deps.rateLimitOptions, + }); + + const emitAudit = (event: Record) => { + void auditRefresh?.({ timestamp: new Date().toISOString(), action: 'jwks_refresh', ...event }); + }; + + /** + * Dual-control force-refresh handler. + * + * Step 1 — body `{ issuer?: string }` (no approvalId): proposes a refresh + * for the issuer (or all tracked issuers when omitted) and records the + * caller as the first approver. Returns 202 with an approvalId. + * + * Step 2 — body `{ approvalId: string }`: a *different* admin approves + * within the gate's time window, which executes the reload. Returns 200 + * with the refreshed issuer(s). + * + * Every outcome is audited (requesting admin, approving admin, scope, + * timestamp). Rate-limited per admin by `refreshLimiter`. + */ const handleRefreshRequest = async (req: AuthenticatedRequest, res: Response, next: NextFunction) => { try { - const issuerUrl = typeof req.body?.issuerUrl === 'string' ? req.body.issuerUrl : undefined; - const confirmationHeader = req.get('x-revora-oidc-jwks-confirmation') === 'true'; - const confirmationBody = req.body?.confirmation === true || req.body?.confirmation === 'true'; - const confirmed = confirmationHeader && confirmationBody; - const actorId = req.user?.id ?? req.ip ?? 'anonymous'; - const key = `${actorId}:${issuerUrl ?? 'global'}`; - const now = Date.now(); - - if (!issuerUrl) { - await auditRefresh?.({ action: 'jwks_refresh', actorId, issuerUrl, status: 'blocked', reason: 'missing_issuer' }); - res.status(400).json({ error: 'Bad Request', message: 'issuerUrl is required' }); - return; - } + const actorId = String(req.user?.id ?? req.user?.sub ?? req.ip ?? 'anonymous'); + const rawIssuer = typeof req.body?.issuer === 'string' ? req.body.issuer.trim() : ''; + const issuer = rawIssuer.length > 0 ? rawIssuer : undefined; + const approvalId = typeof req.body?.approvalId === 'string' ? req.body.approvalId.trim() : ''; + const baseEvent = { actorId, issuer, scope: issuer ?? 'all_tracked_issuers' }; - if (!confirmed) { - await auditRefresh?.({ action: 'jwks_refresh', actorId, issuerUrl, status: 'blocked', reason: 'missing_confirmation' }); - res.status(400).json({ error: 'Bad Request', message: 'Dual confirmation is required' }); - return; - } + // ── Step 2: second-admin approval + execution ────────────────────── + if (approvalId) { + let approval; + try { + approval = approvalGate.approve(approvalId, actorId); + } catch (err) { + if (err instanceof ApprovalNotFoundError) { + emitAudit({ ...baseEvent, approvalId, status: 'blocked', reason: 'unknown_approval' }); + res.status(404).json({ error: 'Not Found', message: err.message }); + return; + } + if (err instanceof ApprovalExpiredError) { + emitAudit({ ...baseEvent, approvalId, status: 'blocked', reason: 'expired_approval' }); + res.status(409).json({ error: 'Conflict', message: err.message }); + return; + } + if (err instanceof ApprovalSelfApprovalError) { + emitAudit({ ...baseEvent, approvalId, status: 'blocked', reason: 'self_approval' }); + res.status(403).json({ error: 'Forbidden', message: err.message }); + return; + } + if (err instanceof ApprovalAlreadyApprovedError) { + emitAudit({ ...baseEvent, approvalId, status: 'blocked', reason: 'already_approved' }); + res.status(409).json({ error: 'Conflict', message: err.message }); + return; + } + throw err; + } - const lastAttempt = refreshAttempts.get(key) ?? 0; - if (now - lastAttempt < refreshCooldownMs) { - await auditRefresh?.({ action: 'jwks_refresh', actorId, issuerUrl, status: 'blocked', reason: 'rate_limited' }); - res.status(429).json({ error: 'Too Many Requests', message: 'JWKS refresh is rate-limited for this actor' }); + const executeEvent = { + ...baseEvent, + approvalId, + proposerId: approval.proposer, + approverId: actorId, + }; + + try { + if (approval.issuer) { + await oidcAdapter.refreshJwks(approval.issuer); + emitAudit({ ...executeEvent, issuers: [approval.issuer], status: 'success' }); + res.status(200).json({ + ok: true, + status: 'approved', + approvalId, + refreshedIssuers: [approval.issuer], + refreshedAt: new Date().toISOString(), + }); + } else { + const result = await oidcAdapter.refreshAllJwks(); + emitAudit({ + ...executeEvent, + issuers: result.refreshed, + failed: result.failed, + status: result.refreshed.length > 0 || result.failed.length === 0 ? 'success' : 'failed', + }); + if (result.refreshed.length === 0 && result.failed.length > 0) { + res.status(502).json({ + error: 'Bad Gateway', + message: 'JWKS refresh failed for all tracked issuers', + failed: result.failed, + }); + return; + } + res.status(200).json({ + ok: true, + status: 'approved', + approvalId, + refreshedIssuers: result.refreshed, + ...(result.failed.length > 0 ? { failed: result.failed } : {}), + refreshedAt: new Date().toISOString(), + }); + } + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + emitAudit({ ...executeEvent, status: 'failed', reason: message }); + throw err; + } return; } - refreshAttempts.set(key, now); - await oidcAdapter.refreshJwks(issuerUrl); - await auditRefresh?.({ action: 'jwks_refresh', actorId, issuerUrl, status: 'success' }); - res.status(200).json({ ok: true, issuerUrl, refreshedAt: new Date().toISOString() }); + // ── Step 1: propose (first approval) ─────────────────────────────── + try { + const approval = approvalGate.propose(actorId, issuer); + emitAudit({ + actorId, + proposerId: actorId, + issuer: approval.issuer, + scope: approval.scope, + approvalId: approval.approvalId, + status: 'pending_second_approval', + }); + res.status(202).json({ + ok: true, + status: 'pending_second_approval', + approvalId: approval.approvalId, + proposer: approval.proposer, + issuer: approval.issuer ?? null, + scope: approval.scope, + expiresAt: new Date(approval.expiresAt).toISOString(), + }); + } catch (err) { + if (err instanceof DuplicateApprovalError) { + emitAudit({ ...baseEvent, approvalId: err.existingApprovalId, status: 'blocked', reason: 'duplicate_proposal' }); + res.status(409).json({ + error: 'Conflict', + message: err.message, + details: { approvalId: err.existingApprovalId }, + }); + return; + } + throw err; + } } catch (err) { next(err); } @@ -189,8 +339,20 @@ export function createOidcRouter(deps: OidcRouterDependencies): Router { } }); - router.post('/api/auth/oidc/jwks/refresh', requireAdmin, handleRefreshRequest); - router.post('/auth/oidc/jwks/refresh', requireAdmin, handleRefreshRequest); + router.post( + '/api/auth/oidc/jwks/refresh', + requireAdmin, + fillRateLimitSubject, + refreshLimiter, + handleRefreshRequest, + ); + router.post( + '/auth/oidc/jwks/refresh', + requireAdmin, + fillRateLimitSubject, + refreshLimiter, + handleRefreshRequest, + ); // ── Logout flow ────────────────────────────────────────────────────────── const handleLogoutRequest = async (req: Request, res: Response, next: NextFunction) => {