diff --git a/migrate.md b/migrate.md index 8aef8a1e3..d31bbac55 100644 --- a/migrate.md +++ b/migrate.md @@ -32,7 +32,7 @@ Wave 4 — the coupled cluster (order matters here) Wave 5 — the heavyweights, last -10. adminService (14 Nest + 11 MCP consumers), journeyService/atlasService (done 2026-08 — pulled forward as a zero-dependency frontier member opening the Wave-5 auth chain: the 1612-line core split places-style — the DB half (stats aggregation with its two divergent return shapes, visited countries/regions with the #1490 tombstones + cascade, bucket-list CRUD) folded into the wrapper `AtlasService` over `DatabaseService` alone, while the ~750-line pure-geo half (admin0/admin1 boundary stores with the #1576 streaming builders, poly/box indexes, Nominatim geocoding with its shared throttle, the geocode/region caches incl. the import-time unref'd cleanup interval, the `geocodingInFlight` dedup set) moved verbatim to the plain module `nest/atlas/atlas-geo.ts` so the multi-MB caches stay process-global across the container + bridge instances; the 10-tool registrar `mcp/tools/atlas.ts` **plus all four atlas resources in `mcp/resources.ts`** moved to the decorator-driven `atlas.mcp.ts` (the `when:` atlas-addon gate is parity here — legacy tools/resources gated, REST deliberately doesn't; `resources.test.ts` retired with its last two cases, moved to the atlas suites); the plugin RPC host swapped its 9 atlas imports for the injected service (23rd constructor dep — journeys is the factory's last legacy domain import); a minimal 2-export `atlas.bridge.ts` served legacy `authService.getTravelStats` (the zero-bridge streak ends at five; the bridge died on schedule with the 2026-08 auth fold); the DTO ratchet cleared all three `AtlasController` allow-list entries)/vacayService (done 2026-07 — pulled forward as a zero-dependency frontier member, see migration-graph.md)/collectionsService (done 2026-08 — pulled forward as a frontier-ready member: the 1024-line saved-places core folded into the wrapper `CollectionsService` over DatabaseService + PermissionsService + RealtimeService (the `checkPermission` bridge import and `broadcastToUser` websocket import became injections; the `sendInvite` lazy notificationService `import()` stays, collab precedent; `deleteOldCollectionCover` re-anchored one directory deeper); the 25-tool registrar `mcp/tools/collections.ts` moved to the decorator-driven `collections.mcp.ts` — with NO `when:` addon gate, preserving the legacy registrar's addon-gate asymmetry — plus a new 23-case `tools-collections` characterization suite (the registrar was previously untested); the plugin RPC host swapped its 7 collections imports for the injected service (22nd constructor dep); **no bridge file anywhere**; the DTO ratchet cleared both collections allow-list entries (`reorder`, `delete-many`))/mapsService (done 2026-07 — pulled forward as step 4 of the dependency-honest order: the 1429-line geo core folded into the wrapper `MapsService`, pure helpers to `maps.helpers.ts`, the 3 geo MCP tools to the decorator-driven `maps.mcp.ts` (weather/airport tools stay in the legacy registrar), BookingImportService injects it, and a 3-export `maps.bridge.ts` serves placeEnrichment + the places registrar — transitService → placeService now unblocked)/transitService (done 2026-07 — pulled forward as step 5 of the dependency-honest order: the first fully SQL-free domain fold — the 333-line Transitous/MOTIS proxy became a dep-free `TransitService` (no `DatabaseService`); pure `deriveTransitStats` + mode whitelist + itinerary types moved to `transit.helpers.ts` so the downstream `transitItineraryService` needed no bridge; the whole 3-tool registrar moved to the decorator-driven `transit.mcp.ts`, its days/reservations bridge imports becoming injected `DaysService`/`ReservationsService` — zero bridge files; placeService → transitItineraryService followed, both done 2026-07/08), and the auth stack: authService (done 2026-08 — the 1497-line head of the auth → admin → oauth chain folded into the wrapper `AuthService` over DatabaseService + injected PermissionsService/AtlasService (retiring `atlas.bridge.ts` on schedule); the pure crypto half (backup-code hash/match/generate, `stripUserForClient`, key masking, the import-time `DUMMY_PASSWORD_HASH` timing equaliser + `avatarDir` mkdir, both documented parity exceptions) moved to the plain module `auth.helpers.ts`; `mfaSetupPending` + the per-email reset throttle (with its unref'd cleanup interval) stay module-scoped so the bridge and container instances share them; no MCP registrar existed and the plugin host never imported it, so neither surface changed — but the 15 in-container `*.mcp.ts` `isDemoUser` guards now inject `AuthService` (15 modules import AuthModule), with `atlas.mcp.ts` alone on the new 8-export `auth.bridge.ts` because the AuthModule→AtlasModule edge would otherwise close a cycle (places.mcp precedent); the bridge also serves `mcp/index.ts` token verification, the journey/notifications/transports registrars, and the still-legacy adminService/oidcService/passkeyService; tests moved with IDs preserved (authService.test.ts → auth.helpers.test.ts, authServiceDb.test.ts → auth.service.test.ts + AUTH-BR-001…007 bridge cases), auth.e2e went DI-native (real bcrypt, real audit rows), oidc.e2e switched its dead path-mock for an instance spy), leaving oauthService/oidcService/passkeyService — now unblocked repoint-consumers of the bridge — plus adminService next in the chain. backupService also stays late: it owns closeDb/reinitialize lifecycle, which is deliberately +10. adminService (14 Nest + 11 MCP consumers), journeyService/atlasService (done 2026-08 — pulled forward as a zero-dependency frontier member opening the Wave-5 auth chain: the 1612-line core split places-style — the DB half (stats aggregation with its two divergent return shapes, visited countries/regions with the #1490 tombstones + cascade, bucket-list CRUD) folded into the wrapper `AtlasService` over `DatabaseService` alone, while the ~750-line pure-geo half (admin0/admin1 boundary stores with the #1576 streaming builders, poly/box indexes, Nominatim geocoding with its shared throttle, the geocode/region caches incl. the import-time unref'd cleanup interval, the `geocodingInFlight` dedup set) moved verbatim to the plain module `nest/atlas/atlas-geo.ts` so the multi-MB caches stay process-global across the container + bridge instances; the 10-tool registrar `mcp/tools/atlas.ts` **plus all four atlas resources in `mcp/resources.ts`** moved to the decorator-driven `atlas.mcp.ts` (the `when:` atlas-addon gate is parity here — legacy tools/resources gated, REST deliberately doesn't; `resources.test.ts` retired with its last two cases, moved to the atlas suites); the plugin RPC host swapped its 9 atlas imports for the injected service (23rd constructor dep — journeys is the factory's last legacy domain import); a minimal 2-export `atlas.bridge.ts` served legacy `authService.getTravelStats` (the zero-bridge streak ends at five; the bridge died on schedule with the 2026-08 auth fold); the DTO ratchet cleared all three `AtlasController` allow-list entries)/vacayService (done 2026-07 — pulled forward as a zero-dependency frontier member, see migration-graph.md)/collectionsService (done 2026-08 — pulled forward as a frontier-ready member: the 1024-line saved-places core folded into the wrapper `CollectionsService` over DatabaseService + PermissionsService + RealtimeService (the `checkPermission` bridge import and `broadcastToUser` websocket import became injections; the `sendInvite` lazy notificationService `import()` stays, collab precedent; `deleteOldCollectionCover` re-anchored one directory deeper); the 25-tool registrar `mcp/tools/collections.ts` moved to the decorator-driven `collections.mcp.ts` — with NO `when:` addon gate, preserving the legacy registrar's addon-gate asymmetry — plus a new 23-case `tools-collections` characterization suite (the registrar was previously untested); the plugin RPC host swapped its 7 collections imports for the injected service (22nd constructor dep); **no bridge file anywhere**; the DTO ratchet cleared both collections allow-list entries (`reorder`, `delete-many`))/mapsService (done 2026-07 — pulled forward as step 4 of the dependency-honest order: the 1429-line geo core folded into the wrapper `MapsService`, pure helpers to `maps.helpers.ts`, the 3 geo MCP tools to the decorator-driven `maps.mcp.ts` (weather/airport tools stay in the legacy registrar), BookingImportService injects it, and a 3-export `maps.bridge.ts` serves placeEnrichment + the places registrar — transitService → placeService now unblocked)/transitService (done 2026-07 — pulled forward as step 5 of the dependency-honest order: the first fully SQL-free domain fold — the 333-line Transitous/MOTIS proxy became a dep-free `TransitService` (no `DatabaseService`); pure `deriveTransitStats` + mode whitelist + itinerary types moved to `transit.helpers.ts` so the downstream `transitItineraryService` needed no bridge; the whole 3-tool registrar moved to the decorator-driven `transit.mcp.ts`, its days/reservations bridge imports becoming injected `DaysService`/`ReservationsService` — zero bridge files; placeService → transitItineraryService followed, both done 2026-07/08), and the auth stack: authService (done 2026-08 — the 1497-line head of the auth → admin → oauth chain folded into the wrapper `AuthService` over DatabaseService + injected PermissionsService/AtlasService (retiring `atlas.bridge.ts` on schedule); the pure crypto half (backup-code hash/match/generate, `stripUserForClient`, key masking, the import-time `DUMMY_PASSWORD_HASH` timing equaliser + `avatarDir` mkdir, both documented parity exceptions) moved to the plain module `auth.helpers.ts`; `mfaSetupPending` + the per-email reset throttle (with its unref'd cleanup interval) stay module-scoped so the bridge and container instances share them; no MCP registrar existed and the plugin host never imported it, so neither surface changed — but the 15 in-container `*.mcp.ts` `isDemoUser` guards now inject `AuthService` (15 modules import AuthModule), with `atlas.mcp.ts` alone on the new 8-export `auth.bridge.ts` because the AuthModule→AtlasModule edge would otherwise close a cycle (places.mcp precedent); the bridge also serves `mcp/index.ts` token verification, the journey/notifications/transports registrars, and the still-legacy adminService/passkeyService; tests moved with IDs preserved (authService.test.ts → auth.helpers.test.ts, authServiceDb.test.ts → auth.service.test.ts + AUTH-BR-001…007 bridge cases), auth.e2e went DI-native (real bcrypt, real audit rows), oidc.e2e switched its dead path-mock for an instance spy). oidcService followed (done 2026-08 — the first frontier cash-in of the auth fold: the 508-line SSO module folded into the wrapper `OidcService` over DatabaseService + injected AuthService (the `resolveAuthToggles` `auth.bridge` import became the injection); no MCP surface, no plugin-host import, and no bridge at all — nothing outside the container consumes the domain, so the state/auth-code maps, their sweep intervals, and the discovery/JWKS caches became instance state with `onModuleDestroy` clearing the timers; `uuid`/`bcryptjs` lazy requires, the `invite_exhausted` sentinel transaction and every SQL string relocated verbatim; tests moved with OIDC-SVC IDs preserved, oidc.e2e and the integration suite switched their path mocks for `vi.spyOn(app.get(OidcService), …)` instance spies), leaving oauthService/passkeyService — still unblocked repoint-consumers of the bridge — plus adminService next in the chain. backupService also stays late: it owns closeDb/reinitialize lifecycle, which is deliberately outside the DI wrapper. Two special cases to keep in mind: airportService is lazily required by db/database.ts at boot (the flight-endpoint backfill) — whenever it migrates, that call should move into Nest's bootstrap instead, or you recreate the circular diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 8a2d907b3..1e2459e02 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -29,7 +29,7 @@ mount to Nest and leaves the sibling trip routes (days, places, ...) on Express. - **DI-native services (legacy `src/services/*` deleted):** tags, categories, todo, packing, day-notes, trip-invite, assignments, share, settings, files, collab, vacay, reservations, day, permissions, audit, budget, trip, maps, - transit, place, transit-itinerary, collections, atlas, auth — see the + transit, place, transit-itinerary, collections, atlas, auth, oidc — see the migration recipe below. ## Cross-cutting Foundation pieces @@ -367,12 +367,33 @@ import AuthModule) — except `atlas.mcp.ts`, which keeps a bridge import because AuthService injects AtlasService and the reverse module edge would close a cycle (places.mcp precedent). The 8-export `auth.bridge.ts` serves `mcp/index.ts` token verification, the three legacy tool registrars' -`isDemoUser`, and the un-migrated adminService/oidcService/passkeyService. +`isDemoUser`, and the un-migrated adminService/passkeyService. Tests moved with IDs preserved: authService.test.ts → auth.helpers.test.ts, authServiceDb.test.ts → auth.service.test.ts (+ AUTH-BR-001…007 bridge delegation); auth.e2e converted DI-native — the 30-method whole-module mock died, login now runs real bcrypt and real audit rows; oidc.e2e switched its dead path-mock for a `vi.spyOn(app.get(AuthService), …)` instance stub.) +oidcService followed (the frontier cash-in the auth fold predicted: a pure +fold of the 508-line module into the existing wrapper `OidcService` over +DatabaseService + injected AuthService — the `resolveAuthToggles` +`auth.bridge` import became the injection, the first repoint-consumer of that +bridge to migrate. No MCP registrar, no plugin-host import, and **no bridge +at all**: nothing outside the container consumes the domain (day-notes/ +trip-invite class), so the `pendingStates`/`authCodes` maps, their two sweep +`setInterval`s, the single-slot discovery cache and the JWKS cache became +**instance state** — the module-scope precedent (permissions/atlas-geo/auth +maps) applies only where a bridge instance must share state, and none exists +here; the sweepers start in the constructor and are cleared in +`onModuleDestroy`, the one wire-invisible deviation. The `uuid`/`bcryptjs` +lazy requires, the `invite_exhausted` reference-sentinel transaction (reshaped +one line for `DatabaseService.transaction`) and every SQL string relocated +verbatim; controller and its unit suite needed zero edits. Tests moved with +IDs preserved: oidcService.test.ts → oidc.service.test.ts (OIDC-SVC-001…045, +superseding the 18-case delegation-shim suite; 046–048 carry its wrapper +cases); oidc.e2e swapped its whole-module path mock for +`vi.spyOn(app.get(OidcService), …)` instance spies; the integration suite +spies the four HTTP methods on the container instance while driving the real +state maps on that same instance.) Repeat these steps per service (next up: **the notifications fan-in** — per the dependency-honest order in diff --git a/server/src/nest/auth/auth.bridge.ts b/server/src/nest/auth/auth.bridge.ts index 1f48a4c59..9412d2967 100644 --- a/server/src/nest/auth/auth.bridge.ts +++ b/server/src/nest/auth/auth.bridge.ts @@ -9,7 +9,7 @@ import { User } from '../../types'; * Non-Nest entry point for the auth domain — for code running OUTSIDE the * Nest container: the legacy MCP transport (mcp/index.ts token verification), * the legacy tool registrars (mcp/tools/journey|notifications|transports), the - * not-yet-migrated services/adminService, services/oidcService and + * not-yet-migrated services/adminService and * services/passkeyService, and — as the one in-container exception — * atlas.mcp.ts, which cannot inject AuthService without closing an * AuthModule↔AtlasModule cycle (AuthService injects AtlasService for diff --git a/server/src/nest/oidc/oidc.service.ts b/server/src/nest/oidc/oidc.service.ts index 0fb165415..624f29385 100644 --- a/server/src/nest/oidc/oidc.service.ts +++ b/server/src/nest/oidc/oidc.service.ts @@ -1,33 +1,550 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, OnModuleDestroy } from '@nestjs/common'; +import crypto from 'crypto'; +import jwt from 'jsonwebtoken'; import type { Request, Response } from 'express'; -import * as oidc from '../../services/oidcService'; -import { getAppUrl } from '../../app-config'; -import { AuthService } from '../auth/auth.service'; +import { readEnv, getAppUrl } from '../../app-config'; +import { JWT_SECRET, SESSION_DURATION_SECONDS } from '../../config'; +import { User } from '../../types'; +import { decrypt_api_key } from '../../services/apiKeyCrypto'; +import { joinTripAsMember } from '../../services/tripMembership'; import { setAuthCookie } from '../../services/cookie'; +import { AuthService } from '../auth/auth.service'; +import { DatabaseService } from '../database/database.service'; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export interface OidcDiscoveryDoc { + authorization_endpoint: string; + token_endpoint: string; + userinfo_endpoint: string; + jwks_uri?: string; + issuer?: string; + _issuer?: string; +} + +export interface OidcTokenResponse { + access_token?: string; + id_token?: string; + token_type?: string; +} + +export interface OidcUserInfo { + sub: string; + email?: string; + // Standard OIDC claim. Some IdPs send it as the string "true"/"false". + email_verified?: boolean | string; + name?: string; + preferred_username?: string; + // Standard OIDC profile claim: URL of the user's profile picture. + picture?: string; + groups?: string[]; + roles?: string[]; + [key: string]: unknown; +} + +export interface OidcConfig { + issuer: string; + clientId: string; + clientSecret: string; + displayName: string; + discoveryUrl: string | null; +} + +// --------------------------------------------------------------------------- +// Constants / TTLs +// --------------------------------------------------------------------------- + +const AUTH_CODE_TTL = 60000; // 1 minute +const AUTH_CODE_CLEANUP = 30000; // 30 seconds +const STATE_TTL = 5 * 60 * 1000; // 5 minutes +const STATE_CLEANUP = 60 * 1000; // 1 minute +const DISCOVERY_TTL = 60 * 60 * 1000; // 1 hour + +// 5 minute JWKS cache — short enough to pick up key rotation within a +// reasonable window, long enough that normal login flow doesn't fetch +// JWKS on every callback. +const JWKS_TTL_MS = 5 * 60 * 1000; +type JwksEntry = { keys: Array>; fetchedAt: number }; + +// --------------------------------------------------------------------------- +// Module-private pure helpers +// --------------------------------------------------------------------------- + +function base64url(buf: Buffer): string { + return buf.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); +} + +function base64UrlDecode(input: string): Buffer { + const padded = input.replace(/-/g, '+').replace(/_/g, '/') + '='.repeat((4 - (input.length % 4)) % 4); + return Buffer.from(padded, 'base64'); +} + +// Sanitize the OIDC `picture` claim before we store it as the avatar. Only https +// URLs are usable: the app's CSP allows https image sources but not http, and we +// render the value directly. Non-strings, non-https and oversized values (e.g. a +// large data: URI) are ignored so a user payload never carries junk. #1399 +function safeOidcPicture(picture: unknown): string | null { + if (typeof picture !== 'string') return null; + const url = picture.trim(); + if (!url || url.length > 1024) return null; + return /^https:\/\//i.test(url) ? url : null; +} /** - * Thin Nest wrapper around the existing OIDC service. PKCE state, discovery, - * the strict id_token/JWKS verification, user provisioning and the auth-code - * hand-off all reuse the legacy code unchanged. + * DI-native OIDC service — the legacy services/oidcService.ts folded in whole + * (pure relocation): PKCE state, discovery, the strict id_token/JWKS + * verification, user provisioning and the auth-code hand-off, all over the + * injected DatabaseService, with the resolveAuthToggles bridge import replaced + * by the injected AuthService. + * + * The legacy module-level state (pending-state / auth-code maps and their two + * sweep intervals, the discovery cache, the JWKS cache) lives on the instance: + * nothing outside the container consumes this domain, so no bridge needs to + * share it. The sweepers start in the constructor (legacy started them at + * import) and are cleared in onModuleDestroy. */ @Injectable() -export class OidcService { - constructor(private readonly auth: AuthService) {} +export class OidcService implements OnModuleDestroy { + // ------------------------------------------------------------------------- + // State management – pending OIDC states + // ------------------------------------------------------------------------- + + private readonly pendingStates = new Map(); + + // ------------------------------------------------------------------------- + // Auth code management – short-lived codes exchanged for JWT + // ------------------------------------------------------------------------- + + private readonly authCodes = new Map(); + + // Discovery document cache (1 h TTL) + private discoveryCache: OidcDiscoveryDoc | null = null; + private discoveryCacheTime = 0; + + private readonly jwksCache = new Map(); + + private readonly stateSweeper: NodeJS.Timeout; + private readonly codeSweeper: NodeJS.Timeout; + + constructor( + private readonly db: DatabaseService, + private readonly auth: AuthService, + ) { + this.stateSweeper = setInterval(() => { + const now = Date.now(); + for (const [state, data] of this.pendingStates) { + if (now - data.createdAt > STATE_TTL) this.pendingStates.delete(state); + } + }, STATE_CLEANUP); + this.codeSweeper = setInterval(() => { + const now = Date.now(); + for (const [code, entry] of this.authCodes) { + if (now - entry.created > AUTH_CODE_TTL) this.authCodes.delete(code); + } + }, AUTH_CODE_CLEANUP); + } + + onModuleDestroy(): void { + clearInterval(this.stateSweeper); + clearInterval(this.codeSweeper); + } oidcLoginEnabled(): boolean { return this.auth.resolveAuthToggles().oidc_login; } - getOidcConfig() { return oidc.getOidcConfig(); } + getAppUrl() { return getAppUrl(); } - discover(issuer: string, discoveryUrl?: string | null) { return oidc.discover(issuer, discoveryUrl); } - createState(redirectUri: string, inviteToken?: string) { return oidc.createState(redirectUri, inviteToken); } - consumeState(state: string) { return oidc.consumeState(state); } - exchangeCodeForToken(...args: Parameters) { return oidc.exchangeCodeForToken(...args); } - verifyIdToken(...args: Parameters) { return oidc.verifyIdToken(...args); } - getUserInfo(endpoint: string, accessToken: string) { return oidc.getUserInfo(endpoint, accessToken); } - findOrCreateUser(...args: Parameters) { return oidc.findOrCreateUser(...args); } - touchLastLogin(userId: number) { return oidc.touchLastLogin(userId); } - generateToken(user: { id: number }) { return oidc.generateToken(user); } - createAuthCode(token: string) { return oidc.createAuthCode(token); } - consumeAuthCode(code: string) { return oidc.consumeAuthCode(code); } - frontendUrl(path: string) { return oidc.frontendUrl(path); } + setAuthCookie(res: Response, token: string, req: Request) { setAuthCookie(res, token, req); } + + // Creates the login state and a matching PKCE pair. The verifier stays server + // side (in pendingStates); the S256 challenge goes to the provider so PKCE- + // required setups (e.g. Pocket ID with PKCE = required) work. + createState(redirectUri: string, inviteToken?: string): { state: string; codeChallenge: string } { + const state = crypto.randomBytes(32).toString('hex'); + const codeVerifier = base64url(crypto.randomBytes(32)); + const codeChallenge = base64url(crypto.createHash('sha256').update(codeVerifier).digest()); + this.pendingStates.set(state, { createdAt: Date.now(), redirectUri, inviteToken, codeVerifier }); + return { state, codeChallenge }; + } + + consumeState(state: string) { + const pending = this.pendingStates.get(state); + if (!pending) return null; + this.pendingStates.delete(state); + return pending; + } + + createAuthCode(token: string): string { + const { v4: uuidv4 } = require('uuid'); + const authCode: string = uuidv4(); + this.authCodes.set(authCode, { token, created: Date.now() }); + return authCode; + } + + consumeAuthCode(code: string): { token: string } | { error: string } { + const entry = this.authCodes.get(code); + if (!entry) return { error: 'Invalid or expired code' }; + this.authCodes.delete(code); + if (Date.now() - entry.created > AUTH_CODE_TTL) return { error: 'Code expired' }; + return { token: entry.token }; + } + + // ------------------------------------------------------------------------- + // OIDC configuration (env + DB) + // ------------------------------------------------------------------------- + + getOidcConfig(): OidcConfig | null { + const get = (key: string) => + (this.db.prepare("SELECT value FROM app_settings WHERE key = ?").get(key) as { value: string } | undefined)?.value || null; + + const oidcEnv = readEnv().oidc; + const issuer = oidcEnv.issuer || get('oidc_issuer'); + const clientId = oidcEnv.clientId || get('oidc_client_id'); + const clientSecret = oidcEnv.clientSecret || decrypt_api_key(get('oidc_client_secret')); + const displayName = oidcEnv.displayName || get('oidc_display_name') || 'SSO'; + const discoveryUrl = oidcEnv.discoveryUrl || get('oidc_discovery_url') || null; + + if (!issuer || !clientId || !clientSecret) return null; + return { issuer: issuer.replace(/\/+$/, ''), clientId, clientSecret, displayName, discoveryUrl }; + } + + // ------------------------------------------------------------------------- + // Discovery document (cached, 1 h TTL) + // ------------------------------------------------------------------------- + + async discover(issuer: string, discoveryUrl?: string | null): Promise { + const url = discoveryUrl || `${issuer}/.well-known/openid-configuration`; + if (this.discoveryCache && Date.now() - this.discoveryCacheTime < DISCOVERY_TTL && this.discoveryCache._issuer === url) { + return this.discoveryCache; + } + const res = await fetch(url); + if (!res.ok) throw new Error('Failed to fetch OIDC discovery document'); + const doc = (await res.json()) as OidcDiscoveryDoc; + // Validate that the discovery doc's issuer matches the operator-configured one. + // When no custom discoveryUrl is set, a mismatch signals a MITM or misconfiguration + // and we reject. When the operator explicitly overrides the discovery URL (e.g. + // Authentik realm paths), the discovery doc's issuer is the canonical value — + // trust it and warn rather than blocking login. + const docIssuer = doc.issuer?.replace(/\/+$/, '') ?? ''; + if (docIssuer && docIssuer !== issuer) { + if (discoveryUrl) { + console.warn( + `[OIDC] Discovery doc issuer "${doc.issuer}" differs from configured OIDC_ISSUER "${issuer}". ` + + `Using discovery doc issuer for id_token verification (custom OIDC_DISCOVERY_URL is set).`, + ); + } else { + throw new Error(`OIDC discovery issuer mismatch: expected "${issuer}", got "${doc.issuer}"`); + } + } + doc._issuer = url; + this.discoveryCache = doc; + this.discoveryCacheTime = Date.now(); + return doc; + } + + // ------------------------------------------------------------------------- + // Role resolution via OIDC claims + // ------------------------------------------------------------------------- + + resolveOidcRole(userInfo: OidcUserInfo, isFirstUser: boolean): 'admin' | 'user' { + if (isFirstUser) return 'admin'; + const adminValue = readEnv().oidc.adminValue; + if (!adminValue) return 'user'; + const claimKey = readEnv().oidc.adminClaim; + const claimData = userInfo[claimKey]; + if (Array.isArray(claimData)) { + return claimData.some((v) => String(v) === adminValue) ? 'admin' : 'user'; + } + if (typeof claimData === 'string') { + return claimData === adminValue ? 'admin' : 'user'; + } + return 'user'; + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + frontendUrl(path: string): string { + // Case-sensitive on purpose (legacy parity). + const base = readEnv().app.nodeEnv === 'production' ? '' : 'http://localhost:5173'; + return base + path; + } + + generateToken(user: { id: number }): string { + // Embed the current password_version so an OIDC-issued session is invalidated + // by a password change/reset exactly like a password-login session (the auth + // middleware compares this `pv` against users.password_version). + const pv = (this.db.prepare('SELECT password_version FROM users WHERE id = ?').get(user.id) as { password_version?: number } | undefined)?.password_version ?? 0; + return jwt.sign({ id: user.id, pv }, JWT_SECRET, { expiresIn: SESSION_DURATION_SECONDS, algorithm: 'HS256' }); + } + + // ------------------------------------------------------------------------- + // Token exchange with OIDC provider + // ------------------------------------------------------------------------- + + async exchangeCodeForToken( + doc: OidcDiscoveryDoc, + code: string, + redirectUri: string, + clientId: string, + clientSecret: string, + codeVerifier?: string, + ): Promise { + const body = new URLSearchParams({ + grant_type: 'authorization_code', + code, + redirect_uri: redirectUri, + client_id: clientId, + client_secret: clientSecret, + }); + if (codeVerifier) body.set('code_verifier', codeVerifier); + const tokenRes = await fetch(doc.token_endpoint, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body, + }); + const tokenData = (await tokenRes.json()) as OidcTokenResponse; + return { ...tokenData, _ok: tokenRes.ok, _status: tokenRes.status }; + } + + // ------------------------------------------------------------------------- + // Fetch userinfo from OIDC provider + // ------------------------------------------------------------------------- + + async getUserInfo(userinfoEndpoint: string, accessToken: string): Promise { + const res = await fetch(userinfoEndpoint, { + headers: { Authorization: `Bearer ${accessToken}` }, + }); + return (await res.json()) as OidcUserInfo; + } + + // ------------------------------------------------------------------------- + // id_token verification (signature + iss + aud + exp) + // ------------------------------------------------------------------------- + + private async fetchJwks(jwksUri: string): Promise>> { + const cached = this.jwksCache.get(jwksUri); + if (cached && Date.now() - cached.fetchedAt < JWKS_TTL_MS) return cached.keys; + const res = await fetch(jwksUri); + if (!res.ok) throw new Error(`JWKS fetch failed: HTTP ${res.status}`); + const json = (await res.json()) as { keys?: Array> }; + const keys = json.keys ?? []; + this.jwksCache.set(jwksUri, { keys, fetchedAt: Date.now() }); + return keys; + } + + /** + * Verify an OIDC id_token end-to-end: signature against the provider's + * JWKS, issuer match, audience match, and exp/nbf. Does NOT verify a + * nonce — the server doesn't currently send one in the auth request; + * when that's added, pass the expected nonce here and check `claims.nonce`. + * + * Returning the claims lets callers cross-check `sub` / `email` against + * the userinfo response. A mismatch would mean the provider's userinfo + * endpoint is speaking for a different subject than the id_token — a + * classic IdP-side compromise signal worth refusing login over. + */ + async verifyIdToken( + idToken: string, + doc: OidcDiscoveryDoc, + clientId: string, + expectedIssuer: string, + ): Promise<{ ok: true; claims: Record } | { ok: false; error: string }> { + if (!doc.jwks_uri) return { ok: false, error: 'no_jwks_uri' }; + const parts = idToken.split('.'); + if (parts.length !== 3) return { ok: false, error: 'malformed_token' }; + + let header: { kid?: string; alg?: string }; + try { header = JSON.parse(base64UrlDecode(parts[0]!).toString('utf8')); } + catch { return { ok: false, error: 'bad_header' }; } + + const alg = header.alg; + if (!alg || !/^(RS256|RS384|RS512|ES256|ES384|ES512|PS256|PS384|PS512)$/.test(alg)) { + return { ok: false, error: 'unsupported_alg' }; + } + + let keys: Array>; + try { keys = await this.fetchJwks(doc.jwks_uri); } + catch (e) { return { ok: false, error: 'jwks_fetch_failed' }; } + + // When the token carries a `kid`, refuse to fall back to any other + // key in the JWKS — a mismatch means the token was signed with a key + // the provider no longer publishes, and we should reject rather than + // mask the failure by trying another key. + const jwk = header.kid + ? keys.find((k) => k['kid'] === header.kid) + : keys[0]; + if (!jwk) return { ok: false, error: 'no_matching_key' }; + + let publicKey; + try { + // Node 16+ understands JWK directly; no PEM conversion library needed. + // Node's crypto accepts a JWK object directly as `{ key, format: 'jwk' }`. + // The type signature isn't strict on our TS config so we cast through any. + publicKey = crypto.createPublicKey({ key: jwk as any, format: 'jwk' }); + } catch { + return { ok: false, error: 'key_import_failed' }; + } + + let claims: Record; + try { + const verified = jwt.verify(idToken, publicKey, { + algorithms: [alg as jwt.Algorithm], + audience: clientId, + }); + claims = typeof verified === 'string' ? {} : (verified as Record); + } catch (err) { + const msg = err instanceof Error ? err.message : 'verify_failed'; + return { ok: false, error: `signature_or_claim_mismatch: ${msg}` }; + } + + // Normalize trailing slash before issuer comparison — some IdPs (e.g. Authentik) + // include a trailing slash in the id_token iss claim. + const tokenIssuer = typeof claims['iss'] === 'string' ? claims['iss'].replace(/\/+$/, '') : ''; + if (tokenIssuer !== expectedIssuer) { + return { ok: false, error: `signature_or_claim_mismatch: jwt issuer invalid. expected: ${expectedIssuer}` }; + } + + return { ok: true, claims }; + } + + // ------------------------------------------------------------------------- + // Find or create user by OIDC sub / email + // ------------------------------------------------------------------------- + + findOrCreateUser( + userInfo: OidcUserInfo, + config: OidcConfig, + inviteToken?: string, + ): { user: User } | { error: string } { + const email = userInfo.email!.trim().toLowerCase(); + const name = userInfo.name || userInfo.preferred_username || email.split('@')[0]; + const sub = userInfo.sub; + const picture = safeOidcPicture(userInfo.picture); + + // Try to find existing user by sub, then by email + let user = this.db.prepare('SELECT * FROM users WHERE oidc_sub = ? AND oidc_issuer = ?').get(sub, config.issuer) as User | undefined; + if (!user) { + // Never link/log-in to a guest (#1362) via its synthetic email. + user = this.db.prepare('SELECT * FROM users WHERE LOWER(email) = ? AND COALESCE(is_guest, 0) = 0').get(email) as User | undefined; + } + + if (user) { + // Reaching here without an oidc_sub means we matched an existing local + // account by email. Only auto-link the OIDC identity when the IdP asserts + // the email is verified; an unverified email must not auto-link. + if (!user.oidc_sub) { + const emailVerified = userInfo.email_verified === true || userInfo.email_verified === 'true'; + if (!emailVerified) { + return { error: 'email_not_verified' }; + } + this.db.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?').run(sub, config.issuer, user.id); + } + // Update role based on OIDC claims on every login (if claim mapping is configured) + if (readEnv().oidc.adminValue) { + const newRole = this.resolveOidcRole(userInfo, false); + if (user.role !== newRole) { + // Never let the claim-based downgrade strip the last admin. The bootstrap + // admin (first SSO user) usually doesn't carry the admin claim, so a forced + // re-login — e.g. after a JWT-secret rotation — would otherwise demote it and + // lock an OIDC-only instance out for good. #1274 + const demotingLastAdmin = + user.role === 'admin' && + newRole !== 'admin' && + (this.db.prepare("SELECT COUNT(*) as count FROM users WHERE role = 'admin'").get() as { count: number }).count <= 1; + if (demotingLastAdmin) { + console.warn(`[OIDC] Kept admin role for user ${user.id}: their OIDC claims map to '${newRole}', but they are the only admin — demoting would lock the instance out.`); + } else { + this.db.prepare('UPDATE users SET role = ? WHERE id = ?').run(newRole, user.id); + user = { ...user, role: newRole } as User; + } + } + } + // Keep the avatar in sync with the OIDC picture, but never clobber a custom + // upload: only fill it when empty or when the current value is itself an OIDC + // picture URL, so the picture refreshes on each login without overriding an + // uploaded one. #1399 + if (picture && picture !== user.avatar && (!user.avatar || /^https:\/\//i.test(user.avatar))) { + this.db.prepare('UPDATE users SET avatar = ? WHERE id = ?').run(picture, user.id); + user = { ...user, avatar: picture } as User; + } + return { user }; + } + + // --- New user registration --- + const userCount = (this.db.prepare('SELECT COUNT(*) as count FROM users WHERE COALESCE(is_guest, 0) = 0').get() as { count: number }).count; + const isFirstUser = userCount === 0; + + let validInvite: any = null; + if (inviteToken) { + validInvite = this.db.prepare('SELECT * FROM invite_tokens WHERE token = ?').get(inviteToken); + if (validInvite) { + if (validInvite.max_uses > 0 && validInvite.used_count >= validInvite.max_uses) validInvite = null; + if (validInvite?.expires_at && new Date(validInvite.expires_at) < new Date()) validInvite = null; + } + } + + if (!isFirstUser && !validInvite) { + const { oidc_registration } = this.auth.resolveAuthToggles(); + if (!oidc_registration) { + return { error: 'registration_disabled' }; + } + } + + const role = this.resolveOidcRole(userInfo, isFirstUser); + const randomPass = crypto.randomBytes(32).toString('hex'); + const bcrypt = require('bcryptjs'); + const hash = bcrypt.hashSync(randomPass, 10); + + // Username: sanitize and avoid collisions. Keep dots — they are valid in + // usernames (see the ^[a-zA-Z0-9_.-]+$ validation in authService) and common + // in OIDC name claims like "first.last". + let username = name.replace(/[^a-zA-Z0-9_.-]/g, '').substring(0, 30) || 'user'; + const existing = this.db.prepare('SELECT id FROM users WHERE LOWER(username) = LOWER(?)').get(username); + if (existing) username = `${username}_${Date.now() % 10000}`; + + // Atomic registration: if an invite was presented, the increment IS + // the capacity check — UPDATE matches zero rows the moment another + // concurrent callback wins the last slot, and the transaction aborts + // the user INSERT. Without this, two parallel OIDC callbacks could + // both pass the earlier SELECT-based check and each create a user. + const inviteRaceError = new Error('invite_exhausted'); + try { + const result = this.db.transaction(() => { + if (validInvite) { + const updated = this.db.prepare( + 'UPDATE invite_tokens SET used_count = used_count + 1 WHERE id = ? AND (max_uses = 0 OR used_count < max_uses)', + ).run(validInvite.id); + if (updated.changes === 0) throw inviteRaceError; + } + const ins = this.db.prepare( + 'INSERT INTO users (username, email, password_hash, role, oidc_sub, oidc_issuer, avatar, first_seen_version, login_count) VALUES (?, ?, ?, ?, ?, ?, ?, ?, 0)', + ).run(username, email, hash, role, sub, config.issuer, picture, readEnv().app.appVersion || '0.0.0'); + // Trip-bound invite (#1402): auto-add the new SSO user to the trip inside the + // same atomic step as the invite consume. Idempotent + owner-safe. + if (validInvite?.trip_id) { + joinTripAsMember(Number(validInvite.trip_id), Number(ins.lastInsertRowid), validInvite.created_by ?? null); + } + return ins; + }) as { lastInsertRowid: number | bigint }; + user = { id: Number(result.lastInsertRowid), username, email, role, avatar: picture } as User; + return { user }; + } catch (err) { + if (err === inviteRaceError) { + console.warn(`[OIDC] Invite token ${inviteToken?.slice(0, 8)}... exhausted — concurrent callback won the last slot`); + return { error: 'registration_disabled' }; + } + throw err; + } + } + + // ------------------------------------------------------------------------- + // Update last_login timestamp + // ------------------------------------------------------------------------- + + touchLastLogin(userId: number): void { + this.db.prepare('UPDATE users SET last_login = CURRENT_TIMESTAMP, login_count = login_count + 1 WHERE id = ?').run(userId); + } } diff --git a/server/src/services/oidcService.ts b/server/src/services/oidcService.ts deleted file mode 100644 index 5c2b963cb..000000000 --- a/server/src/services/oidcService.ts +++ /dev/null @@ -1,508 +0,0 @@ -import crypto from 'crypto'; -import jwt from 'jsonwebtoken'; -import { readEnv } from '../app-config'; -import { db } from '../db/database'; -import { JWT_SECRET, SESSION_DURATION_SECONDS } from '../config'; -import { User } from '../types'; -import { decrypt_api_key } from './apiKeyCrypto'; -import { resolveAuthToggles } from '../nest/auth/auth.bridge'; -import { joinTripAsMember } from './tripMembership'; - -// --------------------------------------------------------------------------- -// Types -// --------------------------------------------------------------------------- - -export interface OidcDiscoveryDoc { - authorization_endpoint: string; - token_endpoint: string; - userinfo_endpoint: string; - jwks_uri?: string; - issuer?: string; - _issuer?: string; -} - -export interface OidcTokenResponse { - access_token?: string; - id_token?: string; - token_type?: string; -} - -export interface OidcUserInfo { - sub: string; - email?: string; - // Standard OIDC claim. Some IdPs send it as the string "true"/"false". - email_verified?: boolean | string; - name?: string; - preferred_username?: string; - // Standard OIDC profile claim: URL of the user's profile picture. - picture?: string; - groups?: string[]; - roles?: string[]; - [key: string]: unknown; -} - -export interface OidcConfig { - issuer: string; - clientId: string; - clientSecret: string; - displayName: string; - discoveryUrl: string | null; -} - -// --------------------------------------------------------------------------- -// Constants / TTLs -// --------------------------------------------------------------------------- - -const AUTH_CODE_TTL = 60000; // 1 minute -const AUTH_CODE_CLEANUP = 30000; // 30 seconds -const STATE_TTL = 5 * 60 * 1000; // 5 minutes -const STATE_CLEANUP = 60 * 1000; // 1 minute -const DISCOVERY_TTL = 60 * 60 * 1000; // 1 hour - -// --------------------------------------------------------------------------- -// State management – pending OIDC states -// --------------------------------------------------------------------------- - -const pendingStates = new Map(); - -setInterval(() => { - const now = Date.now(); - for (const [state, data] of pendingStates) { - if (now - data.createdAt > STATE_TTL) pendingStates.delete(state); - } -}, STATE_CLEANUP); - -function base64url(buf: Buffer): string { - return buf.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); -} - -// Creates the login state and a matching PKCE pair. The verifier stays server -// side (in pendingStates); the S256 challenge goes to the provider so PKCE- -// required setups (e.g. Pocket ID with PKCE = required) work. -export function createState(redirectUri: string, inviteToken?: string): { state: string; codeChallenge: string } { - const state = crypto.randomBytes(32).toString('hex'); - const codeVerifier = base64url(crypto.randomBytes(32)); - const codeChallenge = base64url(crypto.createHash('sha256').update(codeVerifier).digest()); - pendingStates.set(state, { createdAt: Date.now(), redirectUri, inviteToken, codeVerifier }); - return { state, codeChallenge }; -} - -export function consumeState(state: string) { - const pending = pendingStates.get(state); - if (!pending) return null; - pendingStates.delete(state); - return pending; -} - -// --------------------------------------------------------------------------- -// Auth code management – short-lived codes exchanged for JWT -// --------------------------------------------------------------------------- - -const authCodes = new Map(); - -setInterval(() => { - const now = Date.now(); - for (const [code, entry] of authCodes) { - if (now - entry.created > AUTH_CODE_TTL) authCodes.delete(code); - } -}, AUTH_CODE_CLEANUP); - -export function createAuthCode(token: string): string { - const { v4: uuidv4 } = require('uuid'); - const authCode: string = uuidv4(); - authCodes.set(authCode, { token, created: Date.now() }); - return authCode; -} - -export function consumeAuthCode(code: string): { token: string } | { error: string } { - const entry = authCodes.get(code); - if (!entry) return { error: 'Invalid or expired code' }; - authCodes.delete(code); - if (Date.now() - entry.created > AUTH_CODE_TTL) return { error: 'Code expired' }; - return { token: entry.token }; -} - -// --------------------------------------------------------------------------- -// OIDC configuration (env + DB) -// --------------------------------------------------------------------------- - -export function getOidcConfig(): OidcConfig | null { - const get = (key: string) => - (db.prepare("SELECT value FROM app_settings WHERE key = ?").get(key) as { value: string } | undefined)?.value || null; - - const oidcEnv = readEnv().oidc; - const issuer = oidcEnv.issuer || get('oidc_issuer'); - const clientId = oidcEnv.clientId || get('oidc_client_id'); - const clientSecret = oidcEnv.clientSecret || decrypt_api_key(get('oidc_client_secret')); - const displayName = oidcEnv.displayName || get('oidc_display_name') || 'SSO'; - const discoveryUrl = oidcEnv.discoveryUrl || get('oidc_discovery_url') || null; - - if (!issuer || !clientId || !clientSecret) return null; - return { issuer: issuer.replace(/\/+$/, ''), clientId, clientSecret, displayName, discoveryUrl }; -} - -// --------------------------------------------------------------------------- -// Discovery document (cached, 1 h TTL) -// --------------------------------------------------------------------------- - -let discoveryCache: OidcDiscoveryDoc | null = null; -let discoveryCacheTime = 0; - -export async function discover(issuer: string, discoveryUrl?: string | null): Promise { - const url = discoveryUrl || `${issuer}/.well-known/openid-configuration`; - if (discoveryCache && Date.now() - discoveryCacheTime < DISCOVERY_TTL && discoveryCache._issuer === url) { - return discoveryCache; - } - const res = await fetch(url); - if (!res.ok) throw new Error('Failed to fetch OIDC discovery document'); - const doc = (await res.json()) as OidcDiscoveryDoc; - // Validate that the discovery doc's issuer matches the operator-configured one. - // When no custom discoveryUrl is set, a mismatch signals a MITM or misconfiguration - // and we reject. When the operator explicitly overrides the discovery URL (e.g. - // Authentik realm paths), the discovery doc's issuer is the canonical value — - // trust it and warn rather than blocking login. - const docIssuer = doc.issuer?.replace(/\/+$/, '') ?? ''; - if (docIssuer && docIssuer !== issuer) { - if (discoveryUrl) { - console.warn( - `[OIDC] Discovery doc issuer "${doc.issuer}" differs from configured OIDC_ISSUER "${issuer}". ` + - `Using discovery doc issuer for id_token verification (custom OIDC_DISCOVERY_URL is set).`, - ); - } else { - throw new Error(`OIDC discovery issuer mismatch: expected "${issuer}", got "${doc.issuer}"`); - } - } - doc._issuer = url; - discoveryCache = doc; - discoveryCacheTime = Date.now(); - return doc; -} - -// --------------------------------------------------------------------------- -// Role resolution via OIDC claims -// --------------------------------------------------------------------------- - -export function resolveOidcRole(userInfo: OidcUserInfo, isFirstUser: boolean): 'admin' | 'user' { - if (isFirstUser) return 'admin'; - const adminValue = readEnv().oidc.adminValue; - if (!adminValue) return 'user'; - const claimKey = readEnv().oidc.adminClaim; - const claimData = userInfo[claimKey]; - if (Array.isArray(claimData)) { - return claimData.some((v) => String(v) === adminValue) ? 'admin' : 'user'; - } - if (typeof claimData === 'string') { - return claimData === adminValue ? 'admin' : 'user'; - } - return 'user'; -} - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -export function frontendUrl(path: string): string { - // Case-sensitive on purpose (legacy parity). - const base = readEnv().app.nodeEnv === 'production' ? '' : 'http://localhost:5173'; - return base + path; -} - -export function generateToken(user: { id: number }): string { - // Embed the current password_version so an OIDC-issued session is invalidated - // by a password change/reset exactly like a password-login session (the auth - // middleware compares this `pv` against users.password_version). - const pv = (db.prepare('SELECT password_version FROM users WHERE id = ?').get(user.id) as { password_version?: number } | undefined)?.password_version ?? 0; - return jwt.sign({ id: user.id, pv }, JWT_SECRET, { expiresIn: SESSION_DURATION_SECONDS, algorithm: 'HS256' }); -} - -// --------------------------------------------------------------------------- -// Token exchange with OIDC provider -// --------------------------------------------------------------------------- - -export async function exchangeCodeForToken( - doc: OidcDiscoveryDoc, - code: string, - redirectUri: string, - clientId: string, - clientSecret: string, - codeVerifier?: string, -): Promise { - const body = new URLSearchParams({ - grant_type: 'authorization_code', - code, - redirect_uri: redirectUri, - client_id: clientId, - client_secret: clientSecret, - }); - if (codeVerifier) body.set('code_verifier', codeVerifier); - const tokenRes = await fetch(doc.token_endpoint, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body, - }); - const tokenData = (await tokenRes.json()) as OidcTokenResponse; - return { ...tokenData, _ok: tokenRes.ok, _status: tokenRes.status }; -} - -// --------------------------------------------------------------------------- -// Fetch userinfo from OIDC provider -// --------------------------------------------------------------------------- - -export async function getUserInfo(userinfoEndpoint: string, accessToken: string): Promise { - const res = await fetch(userinfoEndpoint, { - headers: { Authorization: `Bearer ${accessToken}` }, - }); - return (await res.json()) as OidcUserInfo; -} - -// --------------------------------------------------------------------------- -// id_token verification (signature + iss + aud + exp) -// --------------------------------------------------------------------------- - -// 5 minute JWKS cache — short enough to pick up key rotation within a -// reasonable window, long enough that normal login flow doesn't fetch -// JWKS on every callback. -const JWKS_TTL_MS = 5 * 60 * 1000; -type JwksEntry = { keys: Array>; fetchedAt: number }; -const jwksCache = new Map(); - -async function fetchJwks(jwksUri: string): Promise>> { - const cached = jwksCache.get(jwksUri); - if (cached && Date.now() - cached.fetchedAt < JWKS_TTL_MS) return cached.keys; - const res = await fetch(jwksUri); - if (!res.ok) throw new Error(`JWKS fetch failed: HTTP ${res.status}`); - const json = (await res.json()) as { keys?: Array> }; - const keys = json.keys ?? []; - jwksCache.set(jwksUri, { keys, fetchedAt: Date.now() }); - return keys; -} - -function base64UrlDecode(input: string): Buffer { - const padded = input.replace(/-/g, '+').replace(/_/g, '/') + '='.repeat((4 - (input.length % 4)) % 4); - return Buffer.from(padded, 'base64'); -} - -/** - * Verify an OIDC id_token end-to-end: signature against the provider's - * JWKS, issuer match, audience match, and exp/nbf. Does NOT verify a - * nonce — the server doesn't currently send one in the auth request; - * when that's added, pass the expected nonce here and check `claims.nonce`. - * - * Returning the claims lets callers cross-check `sub` / `email` against - * the userinfo response. A mismatch would mean the provider's userinfo - * endpoint is speaking for a different subject than the id_token — a - * classic IdP-side compromise signal worth refusing login over. - */ -export async function verifyIdToken( - idToken: string, - doc: OidcDiscoveryDoc, - clientId: string, - expectedIssuer: string, -): Promise<{ ok: true; claims: Record } | { ok: false; error: string }> { - if (!doc.jwks_uri) return { ok: false, error: 'no_jwks_uri' }; - const parts = idToken.split('.'); - if (parts.length !== 3) return { ok: false, error: 'malformed_token' }; - - let header: { kid?: string; alg?: string }; - try { header = JSON.parse(base64UrlDecode(parts[0]!).toString('utf8')); } - catch { return { ok: false, error: 'bad_header' }; } - - const alg = header.alg; - if (!alg || !/^(RS256|RS384|RS512|ES256|ES384|ES512|PS256|PS384|PS512)$/.test(alg)) { - return { ok: false, error: 'unsupported_alg' }; - } - - let keys: Array>; - try { keys = await fetchJwks(doc.jwks_uri); } - catch (e) { return { ok: false, error: 'jwks_fetch_failed' }; } - - // When the token carries a `kid`, refuse to fall back to any other - // key in the JWKS — a mismatch means the token was signed with a key - // the provider no longer publishes, and we should reject rather than - // mask the failure by trying another key. - const jwk = header.kid - ? keys.find((k) => k['kid'] === header.kid) - : keys[0]; - if (!jwk) return { ok: false, error: 'no_matching_key' }; - - let publicKey; - try { - // Node 16+ understands JWK directly; no PEM conversion library needed. - // Node's crypto accepts a JWK object directly as `{ key, format: 'jwk' }`. - // The type signature isn't strict on our TS config so we cast through any. - publicKey = crypto.createPublicKey({ key: jwk as any, format: 'jwk' }); - } catch { - return { ok: false, error: 'key_import_failed' }; - } - - let claims: Record; - try { - const verified = jwt.verify(idToken, publicKey, { - algorithms: [alg as jwt.Algorithm], - audience: clientId, - }); - claims = typeof verified === 'string' ? {} : (verified as Record); - } catch (err) { - const msg = err instanceof Error ? err.message : 'verify_failed'; - return { ok: false, error: `signature_or_claim_mismatch: ${msg}` }; - } - - // Normalize trailing slash before issuer comparison — some IdPs (e.g. Authentik) - // include a trailing slash in the id_token iss claim. - const tokenIssuer = typeof claims['iss'] === 'string' ? claims['iss'].replace(/\/+$/, '') : ''; - if (tokenIssuer !== expectedIssuer) { - return { ok: false, error: `signature_or_claim_mismatch: jwt issuer invalid. expected: ${expectedIssuer}` }; - } - - return { ok: true, claims }; -} - -// --------------------------------------------------------------------------- -// Find or create user by OIDC sub / email -// --------------------------------------------------------------------------- - -// Sanitize the OIDC `picture` claim before we store it as the avatar. Only https -// URLs are usable: the app's CSP allows https image sources but not http, and we -// render the value directly. Non-strings, non-https and oversized values (e.g. a -// large data: URI) are ignored so a user payload never carries junk. #1399 -function safeOidcPicture(picture: unknown): string | null { - if (typeof picture !== 'string') return null; - const url = picture.trim(); - if (!url || url.length > 1024) return null; - return /^https:\/\//i.test(url) ? url : null; -} - -export function findOrCreateUser( - userInfo: OidcUserInfo, - config: OidcConfig, - inviteToken?: string, -): { user: User } | { error: string } { - const email = userInfo.email!.trim().toLowerCase(); - const name = userInfo.name || userInfo.preferred_username || email.split('@')[0]; - const sub = userInfo.sub; - const picture = safeOidcPicture(userInfo.picture); - - // Try to find existing user by sub, then by email - let user = db.prepare('SELECT * FROM users WHERE oidc_sub = ? AND oidc_issuer = ?').get(sub, config.issuer) as User | undefined; - if (!user) { - // Never link/log-in to a guest (#1362) via its synthetic email. - user = db.prepare('SELECT * FROM users WHERE LOWER(email) = ? AND COALESCE(is_guest, 0) = 0').get(email) as User | undefined; - } - - if (user) { - // Reaching here without an oidc_sub means we matched an existing local - // account by email. Only auto-link the OIDC identity when the IdP asserts - // the email is verified; an unverified email must not auto-link. - if (!user.oidc_sub) { - const emailVerified = userInfo.email_verified === true || userInfo.email_verified === 'true'; - if (!emailVerified) { - return { error: 'email_not_verified' }; - } - db.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?').run(sub, config.issuer, user.id); - } - // Update role based on OIDC claims on every login (if claim mapping is configured) - if (readEnv().oidc.adminValue) { - const newRole = resolveOidcRole(userInfo, false); - if (user.role !== newRole) { - // Never let the claim-based downgrade strip the last admin. The bootstrap - // admin (first SSO user) usually doesn't carry the admin claim, so a forced - // re-login — e.g. after a JWT-secret rotation — would otherwise demote it and - // lock an OIDC-only instance out for good. #1274 - const demotingLastAdmin = - user.role === 'admin' && - newRole !== 'admin' && - (db.prepare("SELECT COUNT(*) as count FROM users WHERE role = 'admin'").get() as { count: number }).count <= 1; - if (demotingLastAdmin) { - console.warn(`[OIDC] Kept admin role for user ${user.id}: their OIDC claims map to '${newRole}', but they are the only admin — demoting would lock the instance out.`); - } else { - db.prepare('UPDATE users SET role = ? WHERE id = ?').run(newRole, user.id); - user = { ...user, role: newRole } as User; - } - } - } - // Keep the avatar in sync with the OIDC picture, but never clobber a custom - // upload: only fill it when empty or when the current value is itself an OIDC - // picture URL, so the picture refreshes on each login without overriding an - // uploaded one. #1399 - if (picture && picture !== user.avatar && (!user.avatar || /^https:\/\//i.test(user.avatar))) { - db.prepare('UPDATE users SET avatar = ? WHERE id = ?').run(picture, user.id); - user = { ...user, avatar: picture } as User; - } - return { user }; - } - - // --- New user registration --- - const userCount = (db.prepare('SELECT COUNT(*) as count FROM users WHERE COALESCE(is_guest, 0) = 0').get() as { count: number }).count; - const isFirstUser = userCount === 0; - - let validInvite: any = null; - if (inviteToken) { - validInvite = db.prepare('SELECT * FROM invite_tokens WHERE token = ?').get(inviteToken); - if (validInvite) { - if (validInvite.max_uses > 0 && validInvite.used_count >= validInvite.max_uses) validInvite = null; - if (validInvite?.expires_at && new Date(validInvite.expires_at) < new Date()) validInvite = null; - } - } - - if (!isFirstUser && !validInvite) { - const { oidc_registration } = resolveAuthToggles(); - if (!oidc_registration) { - return { error: 'registration_disabled' }; - } - } - - const role = resolveOidcRole(userInfo, isFirstUser); - const randomPass = crypto.randomBytes(32).toString('hex'); - const bcrypt = require('bcryptjs'); - const hash = bcrypt.hashSync(randomPass, 10); - - // Username: sanitize and avoid collisions. Keep dots — they are valid in - // usernames (see the ^[a-zA-Z0-9_.-]+$ validation in authService) and common - // in OIDC name claims like "first.last". - let username = name.replace(/[^a-zA-Z0-9_.-]/g, '').substring(0, 30) || 'user'; - const existing = db.prepare('SELECT id FROM users WHERE LOWER(username) = LOWER(?)').get(username); - if (existing) username = `${username}_${Date.now() % 10000}`; - - // Atomic registration: if an invite was presented, the increment IS - // the capacity check — UPDATE matches zero rows the moment another - // concurrent callback wins the last slot, and the transaction aborts - // the user INSERT. Without this, two parallel OIDC callbacks could - // both pass the earlier SELECT-based check and each create a user. - const inviteRaceError = new Error('invite_exhausted'); - try { - const createUser = db.transaction(() => { - if (validInvite) { - const updated = db.prepare( - 'UPDATE invite_tokens SET used_count = used_count + 1 WHERE id = ? AND (max_uses = 0 OR used_count < max_uses)', - ).run(validInvite.id); - if (updated.changes === 0) throw inviteRaceError; - } - const ins = db.prepare( - 'INSERT INTO users (username, email, password_hash, role, oidc_sub, oidc_issuer, avatar, first_seen_version, login_count) VALUES (?, ?, ?, ?, ?, ?, ?, ?, 0)', - ).run(username, email, hash, role, sub, config.issuer, picture, readEnv().app.appVersion || '0.0.0'); - // Trip-bound invite (#1402): auto-add the new SSO user to the trip inside the - // same atomic step as the invite consume. Idempotent + owner-safe. - if (validInvite?.trip_id) { - joinTripAsMember(Number(validInvite.trip_id), Number(ins.lastInsertRowid), validInvite.created_by ?? null); - } - return ins; - }); - const result = createUser() as { lastInsertRowid: number | bigint }; - user = { id: Number(result.lastInsertRowid), username, email, role, avatar: picture } as User; - return { user }; - } catch (err) { - if (err === inviteRaceError) { - console.warn(`[OIDC] Invite token ${inviteToken?.slice(0, 8)}... exhausted — concurrent callback won the last slot`); - return { error: 'registration_disabled' }; - } - throw err; - } -} - -// --------------------------------------------------------------------------- -// Update last_login timestamp -// --------------------------------------------------------------------------- - -export function touchLastLogin(userId: number): void { - db.prepare('UPDATE users SET last_login = CURRENT_TIMESTAMP, login_count = login_count + 1 WHERE id = ?').run(userId); -} diff --git a/server/tests/e2e/oidc.e2e.test.ts b/server/tests/e2e/oidc.e2e.test.ts index 208d5ed2b..9a946432d 100644 --- a/server/tests/e2e/oidc.e2e.test.ts +++ b/server/tests/e2e/oidc.e2e.test.ts @@ -1,15 +1,17 @@ /** * OIDC e2e — exercises the migrated /api/auth/oidc flow with the real cookie - * service. The OIDC service is mocked and the auth toggles are stubbed on the - * container's real AuthService instance (OidcService injects it since the auth - * DI fold — the old services/authService path mock would silently miss); this - * proves the flow is unauthenticated, the sso-disabled 403, the login redirect, - * and that /exchange sets the httpOnly trek_session cookie from a valid auth code. + * service. The OIDC methods and the auth toggles are stubbed with vi.spyOn on + * the container's real OidcService/AuthService instances (the domain is + * DI-native since the oidc fold — a services/oidcService path mock would + * silently miss); this proves the flow is unauthenticated, the sso-disabled + * 403, the login redirect, and that /exchange sets the httpOnly trek_session + * cookie from a valid auth code. */ import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; +import type { MockInstance } from 'vitest'; import { Test } from '@nestjs/testing'; vi.mock('../../src/app-config', async (importOriginal) => { @@ -20,9 +22,10 @@ vi.mock('../../src/app-config', async (importOriginal) => { const { db } = vi.hoisted(() => { // eslint-disable-next-line @typescript-eslint/no-require-imports const Database = require('better-sqlite3'); - // Bare :memory: db — nothing in these flows queries it (the toggles are - // stubbed on the AuthService instance below), the container just needs a - // connection to construct the auth/permissions/atlas providers. + // Bare :memory: db — nothing in these flows queries it (the db-touching + // OidcService methods and the toggles are stubbed on the container + // instances below), the container just needs a connection to construct the + // oidc/auth/permissions/atlas providers. return { db: new Database(':memory:') }; }); vi.mock('../../src/db/database', () => ({ @@ -38,16 +41,8 @@ vi.mock('../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error', lo const toggles = { oidc_login: true }; -const { oidcSvc } = vi.hoisted(() => ({ - oidcSvc: { - getOidcConfig: vi.fn(), discover: vi.fn(), createState: vi.fn(), consumeState: vi.fn(), createAuthCode: vi.fn(), - consumeAuthCode: vi.fn(), exchangeCodeForToken: vi.fn(), getUserInfo: vi.fn(), verifyIdToken: vi.fn(), - findOrCreateUser: vi.fn(), touchLastLogin: vi.fn(), generateToken: vi.fn(), frontendUrl: (p: string) => 'https://app' + p, - }, -})); -vi.mock('../../src/services/oidcService', () => oidcSvc); - import { OidcModule } from '../../src/nest/oidc/oidc.module'; +import { OidcService } from '../../src/nest/oidc/oidc.service'; import { DatabaseModule } from '../../src/nest/database/database.module'; import { AuthService } from '../../src/nest/auth/auth.service'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; @@ -55,6 +50,7 @@ import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter describe('OIDC e2e (real cookie service)', () => { let server: Server; let app: Awaited>; + let consumeAuthCode: MockInstance; async function build() { const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, OidcModule] }).compile(); @@ -70,10 +66,11 @@ describe('OIDC e2e (real cookie service)', () => { app = await build(); server = app.getHttpServer(); vi.spyOn(app.get(AuthService), 'resolveAuthToggles').mockImplementation(() => toggles as never); - oidcSvc.getOidcConfig.mockReturnValue({ issuer: 'https://idp', clientId: 'c', clientSecret: 's', discoveryUrl: null }); - oidcSvc.discover.mockResolvedValue({ authorization_endpoint: 'https://idp/auth', userinfo_endpoint: 'https://idp/ui', issuer: 'https://idp' }); - oidcSvc.createState.mockReturnValue({ state: 'st', codeChallenge: 'cc' }); - oidcSvc.consumeAuthCode.mockReturnValue({ token: 'jwt.value' }); + const oidc = app.get(OidcService); + vi.spyOn(oidc, 'getOidcConfig').mockReturnValue({ issuer: 'https://idp', clientId: 'c', clientSecret: 's', displayName: 'SSO', discoveryUrl: null }); + vi.spyOn(oidc, 'discover').mockResolvedValue({ authorization_endpoint: 'https://idp/auth', userinfo_endpoint: 'https://idp/ui', issuer: 'https://idp' } as never); + vi.spyOn(oidc, 'createState').mockReturnValue({ state: 'st', codeChallenge: 'cc' }); + consumeAuthCode = vi.spyOn(oidc, 'consumeAuthCode').mockReturnValue({ token: 'jwt.value' }); }); beforeEach(() => { toggles.oidc_login = true; }); @@ -102,7 +99,7 @@ describe('OIDC e2e (real cookie service)', () => { }); it('GET /exchange sets the httpOnly trek_session cookie + returns the token', async () => { - oidcSvc.consumeAuthCode.mockReturnValue({ token: 'jwt.value' }); + consumeAuthCode.mockReturnValue({ token: 'jwt.value' }); const res = await request(server).get('/api/auth/oidc/exchange').query({ code: 'good' }); expect(res.status).toBe(200); expect(res.body).toEqual({ token: 'jwt.value' }); diff --git a/server/tests/integration/oidc.test.ts b/server/tests/integration/oidc.test.ts index dd66fffdb..2f9cac175 100644 --- a/server/tests/integration/oidc.test.ts +++ b/server/tests/integration/oidc.test.ts @@ -1,8 +1,11 @@ /** - * OIDC integration tests — OIDC-001 through OIDC-010. + * OIDC integration tests — OIDC-001 through OIDC-014. * Covers /api/auth/oidc/login, /callback, /exchange. - * HTTP calls (discover, exchangeCodeForToken, getUserInfo) are mocked. - * State management, auth codes, and findOrCreateUser run against the real test DB. + * HTTP calls (discover, exchangeCodeForToken, getUserInfo, verifyIdToken) are + * stubbed with vi.spyOn on the container's OidcService instance (the domain is + * DI-native since the oidc fold — a services/oidcService path mock would + * silently miss). State management, auth codes, and findOrCreateUser run for + * real on that same instance against the real test DB. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; import request from 'supertest'; @@ -42,33 +45,13 @@ vi.mock('../../src/config', () => ({ })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.fn() })); -// ── Mock only the HTTP-calling functions from oidcService ──────────────────── -vi.mock('../../src/services/oidcService', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - discover: vi.fn(), - exchangeCodeForToken: vi.fn(), - getUserInfo: vi.fn(), - // Bypass real JWKS fetch + signature verification in tests. Callers - // that exercise the security of verifyIdToken should unit-test the - // function directly instead; integration tests here focus on the - // callback flow, not the crypto. - verifyIdToken: vi.fn(), - }; -}); - +import type { MockInstance } from 'vitest'; import { buildApp } from '../../src/bootstrap'; import { createTables } from '../../src/db/schema'; import { runMigrations } from '../../src/db/migrations'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser } from '../helpers/factories'; -import * as oidcService from '../../src/services/oidcService'; - -const mockDiscover = vi.mocked(oidcService.discover); -const mockExchangeCode = vi.mocked(oidcService.exchangeCodeForToken); -const mockGetUserInfo = vi.mocked(oidcService.getUserInfo); -const mockVerifyIdToken = vi.mocked(oidcService.verifyIdToken); +import { OidcService } from '../../src/nest/oidc/oidc.service'; const MOCK_DISCOVERY_DOC = { authorization_endpoint: 'https://oidc.example.com/auth', @@ -78,12 +61,29 @@ const MOCK_DISCOVERY_DOC = { let nestApp: INestApplication; let app: Application; +let oidcSvc: OidcService; +let mockDiscover: MockInstance; +let mockExchangeCode: MockInstance; +let mockGetUserInfo: MockInstance; +let mockVerifyIdToken: MockInstance; beforeAll(async () => { createTables(testDb); runMigrations(testDb); nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); + // Stub only the HTTP-calling methods on the container's OidcService — the + // default implementation resolves undefined so an unexpected call fails the + // flow (matching the old vi.fn() mocks) instead of hitting the network. + // Bypasses real JWKS fetch + signature verification: callers that exercise + // the security of verifyIdToken unit-test the method directly; integration + // tests here focus on the callback flow, not the crypto. State management, + // auth codes, and findOrCreateUser run for real on this same instance. + oidcSvc = nestApp.get(OidcService); + mockDiscover = vi.spyOn(oidcSvc, 'discover').mockImplementation(async () => undefined as never); + mockExchangeCode = vi.spyOn(oidcSvc, 'exchangeCodeForToken').mockImplementation(async () => undefined as never); + mockGetUserInfo = vi.spyOn(oidcSvc, 'getUserInfo').mockImplementation(async () => undefined as never); + mockVerifyIdToken = vi.spyOn(oidcSvc, 'verifyIdToken').mockImplementation(async () => undefined as never); }); beforeEach(() => { @@ -169,7 +169,7 @@ describe('GET /api/auth/oidc/callback', () => { }); // Create a valid state token - const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=authcode123&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -187,7 +187,7 @@ describe('GET /api/auth/oidc/callback', () => { name: 'New User', }); - const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=code999&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -224,7 +224,7 @@ describe('GET /api/auth/oidc/callback', () => { mockDiscover.mockResolvedValueOnce(MOCK_DISCOVERY_DOC); mockExchangeCode.mockResolvedValueOnce({ _ok: false, _status: 400 }); - const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=badcode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -236,7 +236,7 @@ describe('GET /api/auth/oidc/callback', () => { mockDiscover.mockResolvedValueOnce(MOCK_DISCOVERY_DOC); mockExchangeCode.mockResolvedValueOnce({ access_token: 'tok', _ok: true, _status: 200 }); // no id_token - const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -249,7 +249,7 @@ describe('GET /api/auth/oidc/callback', () => { mockExchangeCode.mockResolvedValueOnce({ access_token: 'tok', id_token: 'bad.id.token', _ok: true, _status: 200 }); mockVerifyIdToken.mockResolvedValueOnce({ ok: false, error: 'signature_or_claim_mismatch: invalid signature' }); - const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -267,7 +267,7 @@ describe('GET /api/auth/oidc/callback', () => { name: 'Alice', }); - const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -290,7 +290,7 @@ describe('GET /api/auth/oidc/callback', () => { name: 'Blocked', }); - const { state } = oidcService.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -304,7 +304,7 @@ describe('GET /api/auth/oidc/callback', () => { describe('GET /api/auth/oidc/exchange', () => { it('OIDC-011: valid auth code returns JWT and sets cookie', async () => { const fakeToken = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.test.sig'; - const code = oidcService.createAuthCode(fakeToken); + const code = oidcSvc.createAuthCode(fakeToken); const res = await request(app).get(`/api/auth/oidc/exchange?code=${code}`); @@ -331,7 +331,7 @@ describe('GET /api/auth/oidc/exchange', () => { it('OIDC-014: auth code is single-use (second use returns 400)', async () => { const fakeToken = 'test.token.here'; - const code = oidcService.createAuthCode(fakeToken); + const code = oidcSvc.createAuthCode(fakeToken); // First use: success const res1 = await request(app).get(`/api/auth/oidc/exchange?code=${code}`); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index 28fdb7201..83b462d44 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -1,163 +1,739 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import type { Request, Response } from 'express'; +/** + * Unit tests for the DI-native OidcService — OIDC-SVC-001 through OIDC-SVC-048 + * (moved 1:1 from the legacy tests/unit/services/oidcService.test.ts with case + * IDs preserved; 046–048 carry over the wrapper cases from the superseded + * delegation-shim suite). Covers state management, auth codes, role + * resolution, findOrCreateUser, discover caching, and the ReDoS-sensitive + * issuer trailing-slash regex. Constructed directly (no TestingModule, repo + * convention) over a real in-memory SQLite database. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; +import { generateKeyPairSync } from 'crypto'; +import jwtLib from 'jsonwebtoken'; -// The Nest service is a thin pass-through to the legacy OIDC helpers plus a few -// adjacent service modules. Mock each one and assert the wrapper forwards every -// argument and returns whatever the legacy function hands back. -const { oidc } = vi.hoisted(() => ({ - oidc: { - getOidcConfig: vi.fn(), - discover: vi.fn(), - createState: vi.fn(), - consumeState: vi.fn(), - exchangeCodeForToken: vi.fn(), - verifyIdToken: vi.fn(), - getUserInfo: vi.fn(), - findOrCreateUser: vi.fn(), - touchLastLogin: vi.fn(), - generateToken: vi.fn(), - createAuthCode: vi.fn(), - consumeAuthCode: vi.fn(), - frontendUrl: vi.fn(), - }, +// ── DB setup ────────────────────────────────────────────────────────────────── + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: (tripId: any, userId: number) => + db.prepare(` + SELECT t.id, t.user_id FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) + `).get(userId, tripId, userId), + isOwner: (tripId: any, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + SESSION_DURATION_SECONDS: 86400, + SESSION_DURATION_REMEMBER_SECONDS: 2592000, + updateJwtSecret: () => {}, })); -vi.mock('../../../src/services/oidcService', () => oidc); -const { getAppUrl } = vi.hoisted(() => ({ getAppUrl: vi.fn() })); +// Construction insurance for the injected AuthService's import graph (same set +// as auth.service.test.ts) — services/tripMembership is deliberately NOT +// mocked: OIDC-SVC-045 asserts the real trip_members row a trip-bound invite +// creates. +vi.mock('../../../src/services/mfaCrypto', () => ({ + encryptMfaSecret: vi.fn((s) => `enc:${s}`), + decryptMfaSecret: vi.fn((s: string) => s.replace('enc:', '')), +})); +vi.mock('../../../src/services/apiKeyCrypto', () => ({ + decrypt_api_key: vi.fn((v) => v), + maybe_encrypt_api_key: vi.fn((v) => v), + mask_stored_api_key: vi.fn((v: string | null | undefined) => (v ? '••••••••' : null)), + encrypt_api_key: vi.fn((v) => v), +})); +vi.mock('../../../src/services/ephemeralTokens', () => ({ createEphemeralToken: vi.fn() })); +vi.mock('../../../src/services/notifications', () => ({ sendPasswordResetEmail: vi.fn() })); +vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn() })); +vi.mock('../../../src/scheduler', () => ({ + startTripReminders: vi.fn(), + buildCronExpression: vi.fn(), + loadSettings: vi.fn(() => ({ enabled: false })), + VALID_INTERVALS: ['daily', 'weekly', 'monthly'], +})); + +const { getAppUrlMock } = vi.hoisted(() => ({ getAppUrlMock: vi.fn() })); vi.mock('../../../src/app-config', async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, getAppUrl }; + return { ...actual, getAppUrl: getAppUrlMock }; }); -// OidcService injects AuthService since the auth DI fold; a stub instance -// replaces the old services/authService path mock. -const resolveAuthToggles = vi.fn(); - -const { setAuthCookie } = vi.hoisted(() => ({ setAuthCookie: vi.fn() })); -vi.mock('../../../src/services/cookie', () => ({ setAuthCookie })); +const { setAuthCookieMock } = vi.hoisted(() => ({ setAuthCookieMock: vi.fn() })); +vi.mock('../../../src/services/cookie', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, setAuthCookie: setAuthCookieMock }; +}); +import type { Request, Response } from 'express'; +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { AtlasService } from '../../../src/nest/atlas/atlas.service'; +import { AuthService } from '../../../src/nest/auth/auth.service'; import { OidcService } from '../../../src/nest/oidc/oidc.service'; -import type { AuthService } from '../../../src/nest/auth/auth.service'; -let s: OidcService; -beforeEach(() => { - vi.clearAllMocks(); - s = new OidcService({ resolveAuthToggles } as unknown as AuthService); +const auth = new AuthService( + new DatabaseService(testDb), + new PermissionsService(new DatabaseService(testDb)), + new AtlasService(new DatabaseService(testDb)), +); +const svc = new OidcService(new DatabaseService(testDb), auth); + +const MOCK_CONFIG = { + issuer: 'https://oidc.example.com', + clientId: 'client-id', + clientSecret: 'client-secret', + displayName: 'SSO', + discoveryUrl: null, +}; + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); }); -describe('OidcService', () => { - it('oidcLoginEnabled reads the resolved auth toggle', () => { - resolveAuthToggles.mockReturnValue({ oidc_login: true }); - expect(s.oidcLoginEnabled()).toBe(true); - resolveAuthToggles.mockReturnValue({ oidc_login: false }); - expect(s.oidcLoginEnabled()).toBe(false); +beforeEach(() => { + resetTestDb(testDb); + delete process.env.OIDC_ADMIN_VALUE; + delete process.env.OIDC_ADMIN_CLAIM; + delete process.env.NODE_ENV; +}); + +afterAll(() => { + vi.unstubAllGlobals(); + svc.onModuleDestroy(); + testDb.close(); +}); + +// ── createState / consumeState ──────────────────────────────────────────────── + +describe('createState / consumeState', () => { + it('OIDC-SVC-001: createState returns a hex token + PKCE S256 challenge', () => { + const { state, codeChallenge } = svc.createState('https://example.com/callback'); + expect(state).toMatch(/^[0-9a-f]{64}$/); + expect(codeChallenge).toMatch(/^[A-Za-z0-9_-]{43}$/); // base64url SHA-256, no padding }); - it('getOidcConfig delegates to the legacy helper', () => { - const cfg = { issuer: 'https://idp' }; - oidc.getOidcConfig.mockReturnValue(cfg); - expect(s.getOidcConfig()).toBe(cfg); + it('OIDC-SVC-002: consumeState returns stored data (incl. verifier) and deletes state', () => { + const { state } = svc.createState('https://example.com/callback', 'invite-abc'); + const data = svc.consumeState(state); + expect(data).not.toBeNull(); + expect(data!.redirectUri).toBe('https://example.com/callback'); + expect(data!.inviteToken).toBe('invite-abc'); + expect(typeof data!.codeVerifier).toBe('string'); + expect(data!.codeVerifier.length).toBeGreaterThan(20); + // State is consumed — second call returns null + expect(svc.consumeState(state)).toBeNull(); }); - it('getAppUrl delegates to app-config getAppUrl', () => { - getAppUrl.mockReturnValue('https://app'); - expect(s.getAppUrl()).toBe('https://app'); + it('OIDC-SVC-003: consumeState returns null for unknown state', () => { + expect(svc.consumeState('not-a-real-state')).toBeNull(); }); - it('discover forwards the issuer and discovery url', () => { - const doc = { authorization_endpoint: 'https://idp/auth' }; - oidc.discover.mockReturnValue(doc); - expect(s.discover('https://idp', 'https://idp/.well-known')).toBe(doc); - expect(oidc.discover).toHaveBeenCalledWith('https://idp', 'https://idp/.well-known'); + it('OIDC-SVC-004: two different states do not conflict', () => { + const { state: s1 } = svc.createState('http://a.example.com'); + const { state: s2 } = svc.createState('http://b.example.com'); + expect(s1).not.toBe(s2); + expect(svc.consumeState(s1)!.redirectUri).toBe('http://a.example.com'); + expect(svc.consumeState(s2)!.redirectUri).toBe('http://b.example.com'); + }); +}); + +// ── createAuthCode / consumeAuthCode ───────────────────────────────────────── + +describe('createAuthCode / consumeAuthCode', () => { + it('OIDC-SVC-005: createAuthCode returns a UUID-like string', () => { + const code = svc.createAuthCode('my.jwt.token'); + expect(typeof code).toBe('string'); + expect(code.length).toBeGreaterThan(0); }); - it('discover works without a discovery url', () => { - oidc.discover.mockReturnValue('doc'); - expect(s.discover('https://idp')).toBe('doc'); - expect(oidc.discover).toHaveBeenCalledWith('https://idp', undefined); + it('OIDC-SVC-006: consumeAuthCode returns the stored token', () => { + const code = svc.createAuthCode('real.jwt.here'); + const result = svc.consumeAuthCode(code); + expect('token' in result).toBe(true); + expect((result as { token: string }).token).toBe('real.jwt.here'); }); - it('createState forwards the redirect uri and invite token', () => { - const st = { state: 'st', codeChallenge: 'cc' }; - oidc.createState.mockReturnValue(st); - expect(s.createState('https://app/cb', 'inv')).toBe(st); - expect(oidc.createState).toHaveBeenCalledWith('https://app/cb', 'inv'); + it('OIDC-SVC-007: auth code is single-use (second consume returns error)', () => { + const code = svc.createAuthCode('single.use.token'); + svc.consumeAuthCode(code); // first use + const second = svc.consumeAuthCode(code); + expect('error' in second).toBe(true); }); - it('createState works without an invite token', () => { - oidc.createState.mockReturnValue({ state: 'st', codeChallenge: 'cc' }); - s.createState('https://app/cb'); - expect(oidc.createState).toHaveBeenCalledWith('https://app/cb', undefined); + it('OIDC-SVC-008: consumeAuthCode returns error for unknown code', () => { + const result = svc.consumeAuthCode('not-a-real-code'); + expect('error' in result).toBe(true); + }); +}); + +// ── resolveOidcRole ─────────────────────────────────────────────────────────── + +describe('resolveOidcRole', () => { + it('OIDC-SVC-009: returns admin when isFirstUser is true', () => { + expect(svc.resolveOidcRole({ sub: 'x' }, true)).toBe('admin'); }); - it('consumeState forwards the state', () => { - oidc.consumeState.mockReturnValue({ redirectUri: 'r', codeVerifier: 'v' }); - expect(s.consumeState('st')).toEqual({ redirectUri: 'r', codeVerifier: 'v' }); - expect(oidc.consumeState).toHaveBeenCalledWith('st'); + it('OIDC-SVC-010: returns user when no OIDC_ADMIN_VALUE is set', () => { + delete process.env.OIDC_ADMIN_VALUE; + expect(svc.resolveOidcRole({ sub: 'x', groups: ['admins'] }, false)).toBe('user'); }); - it('exchangeCodeForToken spreads all arguments through', () => { - oidc.exchangeCodeForToken.mockReturnValue({ _ok: true }); - const doc = { token_endpoint: 'https://idp/token' } as never; - expect(s.exchangeCodeForToken(doc, 'code', 'redir', 'cid', 'secret', 'verifier')).toEqual({ _ok: true }); - expect(oidc.exchangeCodeForToken).toHaveBeenCalledWith(doc, 'code', 'redir', 'cid', 'secret', 'verifier'); + it('OIDC-SVC-011: returns admin when groups array contains OIDC_ADMIN_VALUE', () => { + process.env.OIDC_ADMIN_VALUE = 'trek-admins'; + expect(svc.resolveOidcRole({ sub: 'x', groups: ['trek-users', 'trek-admins'] }, false)).toBe('admin'); }); - it('verifyIdToken spreads all arguments through', () => { - oidc.verifyIdToken.mockReturnValue({ ok: true }); - const doc = { issuer: 'https://idp' } as never; - expect(s.verifyIdToken('id_token', doc, 'cid', 'https://idp')).toEqual({ ok: true }); - expect(oidc.verifyIdToken).toHaveBeenCalledWith('id_token', doc, 'cid', 'https://idp'); + it('OIDC-SVC-012: returns user when groups array does not contain OIDC_ADMIN_VALUE', () => { + process.env.OIDC_ADMIN_VALUE = 'trek-admins'; + expect(svc.resolveOidcRole({ sub: 'x', groups: ['trek-users'] }, false)).toBe('user'); }); - it('getUserInfo forwards the endpoint and access token', () => { - oidc.getUserInfo.mockReturnValue({ email: 'a@b.c' }); - expect(s.getUserInfo('https://idp/ui', 'at')).toEqual({ email: 'a@b.c' }); - expect(oidc.getUserInfo).toHaveBeenCalledWith('https://idp/ui', 'at'); + it('OIDC-SVC-013: uses custom OIDC_ADMIN_CLAIM when set', () => { + process.env.OIDC_ADMIN_VALUE = 'superadmin'; + process.env.OIDC_ADMIN_CLAIM = 'roles'; + expect(svc.resolveOidcRole({ sub: 'x', roles: ['superadmin', 'editor'] }, false)).toBe('admin'); }); - it('findOrCreateUser spreads all arguments through', () => { - const result = { user: { id: 1 } }; - oidc.findOrCreateUser.mockReturnValue(result); - const info = { email: 'a@b.c' } as never; - const cfg = { issuer: 'https://idp' } as never; - expect(s.findOrCreateUser(info, cfg, 'inv')).toBe(result); - expect(oidc.findOrCreateUser).toHaveBeenCalledWith(info, cfg, 'inv'); + it('OIDC-SVC-014: handles string claim (exact match)', () => { + process.env.OIDC_ADMIN_VALUE = 'admin'; + process.env.OIDC_ADMIN_CLAIM = 'role'; + expect(svc.resolveOidcRole({ sub: 'x', role: 'admin' }, false)).toBe('admin'); + expect(svc.resolveOidcRole({ sub: 'x', role: 'editor' }, false)).toBe('user'); + }); +}); + +// ── frontendUrl ─────────────────────────────────────────────────────────────── + +describe('frontendUrl', () => { + it('OIDC-SVC-015: prepends localhost:5173 in non-production', () => { + delete process.env.NODE_ENV; + expect(svc.frontendUrl('/login?oidc_code=abc')).toBe('http://localhost:5173/login?oidc_code=abc'); }); - it('touchLastLogin forwards the user id', () => { - s.touchLastLogin(42); - expect(oidc.touchLastLogin).toHaveBeenCalledWith(42); + it('OIDC-SVC-016: returns bare path in production', () => { + process.env.NODE_ENV = 'production'; + expect(svc.frontendUrl('/login?oidc_code=abc')).toBe('/login?oidc_code=abc'); + delete process.env.NODE_ENV; + }); +}); + +// ── discover ────────────────────────────────────────────────────────────────── + +describe('discover', () => { + afterEach(() => { + vi.unstubAllGlobals(); }); - it('generateToken forwards the user', () => { - oidc.generateToken.mockReturnValue('jwt'); - expect(s.generateToken({ id: 7 })).toBe('jwt'); - expect(oidc.generateToken).toHaveBeenCalledWith({ id: 7 }); + it('OIDC-SVC-017: fetches and returns discovery document', async () => { + const doc = { + authorization_endpoint: 'https://oidc.example.com/auth', + token_endpoint: 'https://oidc.example.com/token', + userinfo_endpoint: 'https://oidc.example.com/userinfo', + }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: async () => doc, + })); + + // Use unique issuer to bypass the instance-level cache from other tests + const result = await svc.discover('https://unique-1.example.com'); + expect(result.authorization_endpoint).toBe(doc.authorization_endpoint); + expect(result.token_endpoint).toBe(doc.token_endpoint); }); - it('createAuthCode forwards the token', () => { - oidc.createAuthCode.mockReturnValue('ac'); - expect(s.createAuthCode('jwt')).toBe('ac'); - expect(oidc.createAuthCode).toHaveBeenCalledWith('jwt'); + it('OIDC-SVC-018: throws when provider returns non-ok response', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false })); + await expect(svc.discover('https://bad-issuer.example.com')).rejects.toThrow(); }); - it('consumeAuthCode forwards the code', () => { - oidc.consumeAuthCode.mockReturnValue({ token: 'jwt' }); - expect(s.consumeAuthCode('ac')).toEqual({ token: 'jwt' }); - expect(oidc.consumeAuthCode).toHaveBeenCalledWith('ac'); + it('OIDC-SVC-037: accepts mismatched doc issuer when discoveryUrl is explicit', async () => { + const doc = { + issuer: 'https://auth.example.com/application/o/myapp/', + authorization_endpoint: 'https://auth.example.com/application/o/myapp/authorize/', + token_endpoint: 'https://auth.example.com/application/o/token/', + userinfo_endpoint: 'https://auth.example.com/application/o/userinfo/', + }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const result = await svc.discover( + 'https://auth.example.com', + 'https://auth.example.com/application/o/myapp/.well-known/openid-configuration', + ); + + expect(result.issuer).toBe(doc.issuer); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('differs from configured OIDC_ISSUER')); + warnSpy.mockRestore(); }); - it('frontendUrl forwards the path', () => { - oidc.frontendUrl.mockReturnValue('https://app/login'); - expect(s.frontendUrl('/login')).toBe('https://app/login'); - expect(oidc.frontendUrl).toHaveBeenCalledWith('/login'); + it('OIDC-SVC-038: throws on mismatched doc issuer when discoveryUrl is omitted', async () => { + const doc = { + issuer: 'https://evil.example.com', + authorization_endpoint: 'https://unique-2.example.com/auth', + token_endpoint: 'https://unique-2.example.com/token', + userinfo_endpoint: 'https://unique-2.example.com/userinfo', + }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); + + await expect(svc.discover('https://unique-2.example.com')).rejects.toThrow( + 'OIDC discovery issuer mismatch', + ); }); - it('setAuthCookie forwards res, token and req to the cookie helper', () => { + it('OIDC-SVC-039: trailing-slash-only mismatch with explicit discoveryUrl does not warn', async () => { + const doc = { + issuer: 'https://auth.example.com/', + authorization_endpoint: 'https://auth.example.com/auth', + token_endpoint: 'https://auth.example.com/token', + userinfo_endpoint: 'https://auth.example.com/userinfo', + }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await svc.discover( + 'https://auth.example.com', + 'https://auth.example.com/.well-known/openid-configuration', + ); + + expect(warnSpy).not.toHaveBeenCalled(); + warnSpy.mockRestore(); + }); +}); + +// ── issuer trailing-slash regex (ReDoS guard) ───────────────────────────────── + +describe('getOidcConfig issuer trailing-slash regex', () => { + it('OIDC-SVC-019: /\\/+$/ strips trailing slashes in < 5ms', () => { + // The regex /\/+$/ in getOidcConfig: issuer.replace(/\/+$/, '') + // Adversarial input: many trailing slashes — should not backtrack catastrophically + const adversarial = 'https://oidc.example.com' + '/'.repeat(10000); + const start = Date.now(); + const result = adversarial.replace(/\/+$/, ''); + const elapsed = Date.now() - start; + expect(result).toBe('https://oidc.example.com'); + expect(elapsed).toBeLessThan(100); + }); +}); + +// ── findOrCreateUser ────────────────────────────────────────────────────────── + +describe('findOrCreateUser', () => { + it('OIDC-SVC-020: finds existing user by oidc_sub', () => { + const { user } = createUser(testDb, { email: 'alice@example.com' }); + // Link the sub manually + testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?') + .run('sub-alice-123', MOCK_CONFIG.issuer, user.id); + + const result = svc.findOrCreateUser( + { sub: 'sub-alice-123', email: 'alice@example.com', name: 'Alice' }, + MOCK_CONFIG + ); + expect('user' in result).toBe(true); + expect((result as { user: any }).user.id).toBe(user.id); + }); + + it('OIDC-SVC-021: finds existing user by email when no sub match', () => { + const { user } = createUser(testDb, { email: 'bob@example.com' }); + + const result = svc.findOrCreateUser( + { sub: 'sub-bob-new', email: 'bob@example.com', name: 'Bob', email_verified: true }, + MOCK_CONFIG + ); + expect('user' in result).toBe(true); + expect((result as { user: any }).user.id).toBe(user.id); + }); + + it('OIDC-SVC-022: creates new user when registration is open', () => { + const result = svc.findOrCreateUser( + { sub: 'sub-new-1', email: 'newuser@example.com', name: 'New User' }, + MOCK_CONFIG + ); + expect('user' in result).toBe(true); + const newUser = testDb.prepare("SELECT * FROM users WHERE email = 'newuser@example.com'").get(); + expect(newUser).toBeDefined(); + }); + + it('OIDC-SVC-023: first user gets admin role', () => { + // DB is empty after resetTestDb + const result = svc.findOrCreateUser( + { sub: 'sub-first', email: 'first@example.com', name: 'First' }, + MOCK_CONFIG + ); + expect('user' in result).toBe(true); + expect((result as { user: any }).user.role).toBe('admin'); + }); + + it('OIDC-SVC-024: returns registration_disabled error when registration is off', () => { + createUser(testDb, { email: 'existing@example.com' }); + testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('allow_registration', 'false')").run(); + + const result = svc.findOrCreateUser( + { sub: 'sub-blocked', email: 'blocked@example.com', name: 'Blocked' }, + MOCK_CONFIG + ); + expect('error' in result).toBe(true); + expect((result as { error: string }).error).toBe('registration_disabled'); + }); + + it('OIDC-SVC-025: links oidc_sub when existing user has none (verified email)', () => { + const { user } = createUser(testDb, { email: 'charlie@example.com' }); + // Ensure no oidc_sub set + testDb.prepare('UPDATE users SET oidc_sub = NULL, oidc_issuer = NULL WHERE id = ?').run(user.id); + + svc.findOrCreateUser( + { sub: 'sub-charlie-linked', email: 'charlie@example.com', name: 'Charlie', email_verified: true }, + MOCK_CONFIG + ); + + const updated = testDb.prepare('SELECT oidc_sub FROM users WHERE id = ?').get(user.id) as any; + expect(updated.oidc_sub).toBe('sub-charlie-linked'); + }); + + it('OIDC-SVC-025b: refuses to link an unverified email to an existing local account', () => { + const { user } = createUser(testDb, { email: 'dora@example.com' }); + testDb.prepare('UPDATE users SET oidc_sub = NULL, oidc_issuer = NULL WHERE id = ?').run(user.id); + + // No email_verified claim — an IdP that lets users set arbitrary emails must + // not be able to take over a pre-existing password account. + const result = svc.findOrCreateUser( + { sub: 'sub-dora-attacker', email: 'dora@example.com', name: 'Dora' }, + MOCK_CONFIG + ); + + expect('error' in result).toBe(true); + expect((result as { error: string }).error).toBe('email_not_verified'); + const updated = testDb.prepare('SELECT oidc_sub FROM users WHERE id = ?').get(user.id) as any; + expect(updated.oidc_sub).toBeNull(); // account not linked / not hijacked + }); + + it('OIDC-SVC-026: existing user role is updated when OIDC claim mapping changes it', () => { + const { user } = createUser(testDb, { email: 'diana@example.com', role: 'user' }); + // Link oidc_sub manually so the user is found by sub lookup + testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?') + .run('sub-diana-role', MOCK_CONFIG.issuer, user.id); + + process.env.OIDC_ADMIN_VALUE = 'admins'; + + const result = svc.findOrCreateUser( + { sub: 'sub-diana-role', email: 'diana@example.com', name: 'Diana', groups: ['admins'] }, + MOCK_CONFIG + ); + + expect('user' in result).toBe(true); + expect((result as { user: any }).user.role).toBe('admin'); + + const dbUser = testDb.prepare('SELECT role FROM users WHERE id = ?').get(user.id) as any; + expect(dbUser.role).toBe('admin'); + }); + + it('OIDC-SVC-027: new user with valid invite token increments used_count', () => { + const { user: creator } = createUser(testDb, { email: 'creator@example.com' }); + testDb.prepare( + "INSERT INTO invite_tokens (token, max_uses, used_count, created_by) VALUES ('tok-valid', 5, 0, ?)" + ).run(creator.id); + + const result = svc.findOrCreateUser( + { sub: 'sub-invite-user', email: 'invitee@example.com', name: 'Invitee' }, + MOCK_CONFIG, + 'tok-valid' + ); + + expect('user' in result).toBe(true); + + const token = testDb.prepare("SELECT used_count FROM invite_tokens WHERE token = 'tok-valid'").get() as any; + expect(token.used_count).toBe(1); + }); + + it('OIDC-SVC-028: new user with expired invite token is created but invite is ignored', () => { + const { user: creator } = createUser(testDb, { email: 'creator2@example.com' }); + testDb.prepare( + "INSERT INTO invite_tokens (token, max_uses, used_count, expires_at, created_by) VALUES ('tok-expired', 5, 0, '2000-01-01T00:00:00.000Z', ?)" + ).run(creator.id); + + const result = svc.findOrCreateUser( + { sub: 'sub-expired-invite', email: 'expired-invitee@example.com', name: 'ExpiredInvitee' }, + MOCK_CONFIG, + 'tok-expired' + ); + + // User is still created because open registration is allowed + expect('user' in result).toBe(true); + const newUser = testDb.prepare("SELECT id FROM users WHERE email = 'expired-invitee@example.com'").get(); + expect(newUser).toBeDefined(); + + // Invite used_count must remain 0 (token was treated as invalid) + const token = testDb.prepare("SELECT used_count FROM invite_tokens WHERE token = 'tok-expired'").get() as any; + expect(token.used_count).toBe(0); + }); + + it('OIDC-SVC-029: new user with max_uses exceeded invite token is created but invite is ignored', () => { + const { user: creator } = createUser(testDb, { email: 'creator3@example.com' }); + testDb.prepare( + "INSERT INTO invite_tokens (token, max_uses, used_count, created_by) VALUES ('tok-full', 1, 1, ?)" + ).run(creator.id); + + const result = svc.findOrCreateUser( + { sub: 'sub-full-invite', email: 'full-invitee@example.com', name: 'FullInvitee' }, + MOCK_CONFIG, + 'tok-full' + ); + + // User is still created because open registration is allowed + expect('user' in result).toBe(true); + const newUser = testDb.prepare("SELECT id FROM users WHERE email = 'full-invitee@example.com'").get(); + expect(newUser).toBeDefined(); + + // Invite used_count must remain 1 (token was treated as invalid) + const token = testDb.prepare("SELECT used_count FROM invite_tokens WHERE token = 'tok-full'").get() as any; + expect(token.used_count).toBe(1); + }); + + // ── OIDC picture claim → avatar (#1399) ────────────────────────────────── + + it('OIDC-SVC-040: new user stores the https picture claim as their avatar', () => { + const result = svc.findOrCreateUser( + { sub: 'sub-pic-1', email: 'pic1@example.com', name: 'Pic One', picture: 'https://idp.example.com/u/pic1.png' }, + MOCK_CONFIG + ); + expect('user' in result).toBe(true); + const row = testDb.prepare("SELECT avatar FROM users WHERE email = 'pic1@example.com'").get() as any; + expect(row.avatar).toBe('https://idp.example.com/u/pic1.png'); + }); + + it('OIDC-SVC-041: new user with a non-https picture claim stores no avatar', () => { + svc.findOrCreateUser( + { sub: 'sub-pic-2', email: 'pic2@example.com', name: 'Pic Two', picture: 'http://idp.example.com/u/pic2.png' }, + MOCK_CONFIG + ); + const row = testDb.prepare("SELECT avatar FROM users WHERE email = 'pic2@example.com'").get() as any; + expect(row.avatar).toBeNull(); + }); + + it('OIDC-SVC-042: existing user with no avatar gets the OIDC picture', () => { + const { user } = createUser(testDb, { email: 'pic3@example.com' }); + testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = NULL WHERE id = ?') + .run('sub-pic-3', MOCK_CONFIG.issuer, user.id); + svc.findOrCreateUser( + { sub: 'sub-pic-3', email: 'pic3@example.com', name: 'Pic Three', picture: 'https://idp.example.com/u/pic3.png' }, + MOCK_CONFIG + ); + const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any; + expect(row.avatar).toBe('https://idp.example.com/u/pic3.png'); + }); + + it('OIDC-SVC-043: a custom uploaded avatar is never overwritten by the OIDC picture', () => { + const { user } = createUser(testDb, { email: 'pic4@example.com' }); + testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?') + .run('sub-pic-4', MOCK_CONFIG.issuer, 'uploaded-abc.jpg', user.id); + svc.findOrCreateUser( + { sub: 'sub-pic-4', email: 'pic4@example.com', name: 'Pic Four', picture: 'https://idp.example.com/u/pic4.png' }, + MOCK_CONFIG + ); + const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any; + expect(row.avatar).toBe('uploaded-abc.jpg'); + }); + + it('OIDC-SVC-044: a previously stored OIDC picture URL is refreshed on next login', () => { + const { user } = createUser(testDb, { email: 'pic5@example.com' }); + testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?') + .run('sub-pic-5', MOCK_CONFIG.issuer, 'https://idp.example.com/u/old.png', user.id); + svc.findOrCreateUser( + { sub: 'sub-pic-5', email: 'pic5@example.com', name: 'Pic Five', picture: 'https://idp.example.com/u/new.png' }, + MOCK_CONFIG + ); + const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any; + expect(row.avatar).toBe('https://idp.example.com/u/new.png'); + }); + + it('OIDC-SVC-045: a trip-bound invite auto-adds the new SSO user as a trip member (#1402)', () => { + const { user: admin } = createUser(testDb, { role: 'admin' }); + const trip = createTrip(testDb, admin.id); + testDb.prepare( + 'INSERT INTO invite_tokens (token, max_uses, used_count, expires_at, created_by, trip_id) VALUES (?, 5, 0, NULL, ?, ?)' + ).run('inv-trip-join', admin.id, trip.id); + + const result = svc.findOrCreateUser( + { sub: 'sub-trip-join', email: 'joiner@example.com', name: 'Joiner' }, + MOCK_CONFIG, + 'inv-trip-join' + ); + expect('user' in result).toBe(true); + const uid = (result as { user: any }).user.id; + const member = testDb.prepare('SELECT * FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, uid); + expect(member).toBeTruthy(); + }); +}); + +// ── exchangeCodeForToken ────────────────────────────────────────────────────── + +describe('exchangeCodeForToken', () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('OIDC-SVC-030: sends correct POST body and returns token data', async () => { + const mockTokenData = { access_token: 'tok', token_type: 'Bearer' }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => mockTokenData, + })); + + const doc = { token_endpoint: 'https://oidc.example.com/token' } as any; + const result = await svc.exchangeCodeForToken(doc, 'auth-code-123', 'https://app/callback', 'client-id', 'client-secret'); + + expect(result.access_token).toBe('tok'); + expect(result._ok).toBe(true); + expect(result._status).toBe(200); + + const fetchCall = (fetch as ReturnType).mock.calls[0]; + expect(fetchCall[0]).toBe('https://oidc.example.com/token'); + expect(fetchCall[1].method).toBe('POST'); + }); + + it('OIDC-SVC-031: reflects _ok=false when provider returns error status', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, + status: 400, + json: async () => ({ error: 'invalid_grant' }), + })); + + const doc = { token_endpoint: 'https://oidc.example.com/token' } as any; + const result = await svc.exchangeCodeForToken(doc, 'bad-code', 'https://app/callback', 'c', 's'); + + expect(result._ok).toBe(false); + expect(result._status).toBe(400); + }); +}); + +// ── getUserInfo ─────────────────────────────────────────────────────────────── + +describe('getUserInfo', () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('OIDC-SVC-032: fetches userinfo with Bearer token and returns parsed JSON', async () => { + const userInfoData = { sub: 'user-sub', email: 'user@example.com', name: 'User Name' }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + json: async () => userInfoData, + })); + + const result = await svc.getUserInfo('https://oidc.example.com/userinfo', 'access-token-123'); + + expect(result.sub).toBe('user-sub'); + expect(result.email).toBe('user@example.com'); + + const fetchCall = (fetch as ReturnType).mock.calls[0]; + expect(fetchCall[1].headers.Authorization).toBe('Bearer access-token-123'); + }); +}); + +// ── verifyIdToken ───────────────────────────────────────────────────────────── + +describe('verifyIdToken', () => { + const { privateKey, publicKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); + const jwk = publicKey.export({ format: 'jwk' }) as Record; + const ISSUER = 'https://auth.example.com/application/o/trek'; + const CLIENT_ID = 'trek-client'; + const JWKS_URI = 'https://auth.example.com/.well-known/jwks.json'; + + function mockJwks() { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ keys: [jwk] }), + })); + } + + function makeToken(iss: string, overrides: object = {}) { + return jwtLib.sign( + { sub: 'user-sub', email: 'user@example.com', ...overrides }, + privateKey, + { algorithm: 'RS256', audience: CLIENT_ID, issuer: iss, expiresIn: '1h' } + ); + } + + const doc = { jwks_uri: JWKS_URI } as any; + + afterEach(() => { vi.unstubAllGlobals(); }); + + it('OIDC-SVC-033: accepts token whose iss matches expectedIssuer exactly', async () => { + mockJwks(); + const token = makeToken(ISSUER); + const result = await svc.verifyIdToken(token, doc, CLIENT_ID, ISSUER); + expect(result.ok).toBe(true); + }); + + it('OIDC-SVC-034: accepts token whose iss has a trailing slash (Authentik)', async () => { + mockJwks(); + const token = makeToken(ISSUER + '/'); + const result = await svc.verifyIdToken(token, doc, CLIENT_ID, ISSUER); + expect(result.ok).toBe(true); + }); + + it('OIDC-SVC-035: rejects token with wrong issuer', async () => { + mockJwks(); + const token = makeToken('https://evil.example.com'); + const result = await svc.verifyIdToken(token, doc, CLIENT_ID, ISSUER); + expect(result.ok).toBe(false); + expect((result as any).error).toMatch('jwt issuer invalid'); + }); + + it('OIDC-SVC-036: rejects token with wrong audience', async () => { + mockJwks(); + const wrongAudToken = jwtLib.sign( + { sub: 'user-sub', iss: ISSUER }, + privateKey, + { algorithm: 'RS256', audience: 'wrong-client', expiresIn: '1h' } + ); + const result = await svc.verifyIdToken(wrongAudToken, doc, CLIENT_ID, ISSUER); + expect(result.ok).toBe(false); + }); +}); + +// ── wrapper methods (carried over from the delegation-shim suite) ──────────── + +describe('wrapper methods', () => { + it('OIDC-SVC-046: oidcLoginEnabled reads the resolved auth toggle', () => { + const spy = vi.spyOn(auth, 'resolveAuthToggles'); + spy.mockReturnValue({ oidc_login: true } as ReturnType); + expect(svc.oidcLoginEnabled()).toBe(true); + spy.mockReturnValue({ oidc_login: false } as ReturnType); + expect(svc.oidcLoginEnabled()).toBe(false); + spy.mockRestore(); + }); + + it('OIDC-SVC-047: getAppUrl delegates to app-config getAppUrl', () => { + getAppUrlMock.mockReturnValue('https://app'); + expect(svc.getAppUrl()).toBe('https://app'); + }); + + it('OIDC-SVC-048: setAuthCookie forwards res, token and req to the cookie helper', () => { const res = {} as Response; const req = {} as Request; - s.setAuthCookie(res, 'jwt', req); - expect(setAuthCookie).toHaveBeenCalledWith(res, 'jwt', req); + svc.setAuthCookie(res, 'jwt', req); + expect(setAuthCookieMock).toHaveBeenCalledWith(res, 'jwt', req); }); }); diff --git a/server/tests/unit/services/oidcService.test.ts b/server/tests/unit/services/oidcService.test.ts deleted file mode 100644 index 45885d396..000000000 --- a/server/tests/unit/services/oidcService.test.ts +++ /dev/null @@ -1,676 +0,0 @@ -/** - * Unit tests for oidcService — OIDC-SVC-001 through OIDC-SVC-025. - * Covers state management, auth codes, role resolution, findOrCreateUser, - * discover caching, and the ReDoS-sensitive issuer trailing-slash regex. - */ -import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; -import { generateKeyPairSync } from 'crypto'; -import jwtLib from 'jsonwebtoken'; - -// ── DB setup ────────────────────────────────────────────────────────────────── - -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); - const mock = { - db, - closeDb: () => {}, - reinitialize: () => {}, - getPlaceWithTags: () => null, - canAccessTrip: (tripId: any, userId: number) => - db.prepare(` - SELECT t.id, t.user_id FROM trips t - LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? - WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) - `).get(userId, tripId, userId), - isOwner: (tripId: any, userId: number) => - !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), - }; - return { testDb: db, dbMock: mock }; -}); - -vi.mock('../../../src/db/database', () => dbMock); -vi.mock('../../../src/config', () => ({ - JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', - ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', - updateJwtSecret: () => {}, -})); - -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; -import { resetTestDb } from '../../helpers/test-db'; -import { createUser, createTrip } from '../../helpers/factories'; -import { - createState, - consumeState, - createAuthCode, - consumeAuthCode, - resolveOidcRole, - frontendUrl, - findOrCreateUser, - discover, - verifyIdToken, -} from '../../../src/services/oidcService'; - -const MOCK_CONFIG = { - issuer: 'https://oidc.example.com', - clientId: 'client-id', - clientSecret: 'client-secret', - displayName: 'SSO', - discoveryUrl: null, -}; - -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - -beforeEach(() => { - resetTestDb(testDb); - delete process.env.OIDC_ADMIN_VALUE; - delete process.env.OIDC_ADMIN_CLAIM; - delete process.env.NODE_ENV; -}); - -afterAll(() => { - vi.unstubAllGlobals(); - testDb.close(); -}); - -// ── createState / consumeState ──────────────────────────────────────────────── - -describe('createState / consumeState', () => { - it('OIDC-SVC-001: createState returns a hex token + PKCE S256 challenge', () => { - const { state, codeChallenge } = createState('https://example.com/callback'); - expect(state).toMatch(/^[0-9a-f]{64}$/); - expect(codeChallenge).toMatch(/^[A-Za-z0-9_-]{43}$/); // base64url SHA-256, no padding - }); - - it('OIDC-SVC-002: consumeState returns stored data (incl. verifier) and deletes state', () => { - const { state } = createState('https://example.com/callback', 'invite-abc'); - const data = consumeState(state); - expect(data).not.toBeNull(); - expect(data!.redirectUri).toBe('https://example.com/callback'); - expect(data!.inviteToken).toBe('invite-abc'); - expect(typeof data!.codeVerifier).toBe('string'); - expect(data!.codeVerifier.length).toBeGreaterThan(20); - // State is consumed — second call returns null - expect(consumeState(state)).toBeNull(); - }); - - it('OIDC-SVC-003: consumeState returns null for unknown state', () => { - expect(consumeState('not-a-real-state')).toBeNull(); - }); - - it('OIDC-SVC-004: two different states do not conflict', () => { - const { state: s1 } = createState('http://a.example.com'); - const { state: s2 } = createState('http://b.example.com'); - expect(s1).not.toBe(s2); - expect(consumeState(s1)!.redirectUri).toBe('http://a.example.com'); - expect(consumeState(s2)!.redirectUri).toBe('http://b.example.com'); - }); -}); - -// ── createAuthCode / consumeAuthCode ───────────────────────────────────────── - -describe('createAuthCode / consumeAuthCode', () => { - it('OIDC-SVC-005: createAuthCode returns a UUID-like string', () => { - const code = createAuthCode('my.jwt.token'); - expect(typeof code).toBe('string'); - expect(code.length).toBeGreaterThan(0); - }); - - it('OIDC-SVC-006: consumeAuthCode returns the stored token', () => { - const code = createAuthCode('real.jwt.here'); - const result = consumeAuthCode(code); - expect('token' in result).toBe(true); - expect((result as { token: string }).token).toBe('real.jwt.here'); - }); - - it('OIDC-SVC-007: auth code is single-use (second consume returns error)', () => { - const code = createAuthCode('single.use.token'); - consumeAuthCode(code); // first use - const second = consumeAuthCode(code); - expect('error' in second).toBe(true); - }); - - it('OIDC-SVC-008: consumeAuthCode returns error for unknown code', () => { - const result = consumeAuthCode('not-a-real-code'); - expect('error' in result).toBe(true); - }); -}); - -// ── resolveOidcRole ─────────────────────────────────────────────────────────── - -describe('resolveOidcRole', () => { - it('OIDC-SVC-009: returns admin when isFirstUser is true', () => { - expect(resolveOidcRole({ sub: 'x' }, true)).toBe('admin'); - }); - - it('OIDC-SVC-010: returns user when no OIDC_ADMIN_VALUE is set', () => { - delete process.env.OIDC_ADMIN_VALUE; - expect(resolveOidcRole({ sub: 'x', groups: ['admins'] }, false)).toBe('user'); - }); - - it('OIDC-SVC-011: returns admin when groups array contains OIDC_ADMIN_VALUE', () => { - process.env.OIDC_ADMIN_VALUE = 'trek-admins'; - expect(resolveOidcRole({ sub: 'x', groups: ['trek-users', 'trek-admins'] }, false)).toBe('admin'); - }); - - it('OIDC-SVC-012: returns user when groups array does not contain OIDC_ADMIN_VALUE', () => { - process.env.OIDC_ADMIN_VALUE = 'trek-admins'; - expect(resolveOidcRole({ sub: 'x', groups: ['trek-users'] }, false)).toBe('user'); - }); - - it('OIDC-SVC-013: uses custom OIDC_ADMIN_CLAIM when set', () => { - process.env.OIDC_ADMIN_VALUE = 'superadmin'; - process.env.OIDC_ADMIN_CLAIM = 'roles'; - expect(resolveOidcRole({ sub: 'x', roles: ['superadmin', 'editor'] }, false)).toBe('admin'); - }); - - it('OIDC-SVC-014: handles string claim (exact match)', () => { - process.env.OIDC_ADMIN_VALUE = 'admin'; - process.env.OIDC_ADMIN_CLAIM = 'role'; - expect(resolveOidcRole({ sub: 'x', role: 'admin' }, false)).toBe('admin'); - expect(resolveOidcRole({ sub: 'x', role: 'editor' }, false)).toBe('user'); - }); -}); - -// ── frontendUrl ─────────────────────────────────────────────────────────────── - -describe('frontendUrl', () => { - it('OIDC-SVC-015: prepends localhost:5173 in non-production', () => { - delete process.env.NODE_ENV; - expect(frontendUrl('/login?oidc_code=abc')).toBe('http://localhost:5173/login?oidc_code=abc'); - }); - - it('OIDC-SVC-016: returns bare path in production', () => { - process.env.NODE_ENV = 'production'; - expect(frontendUrl('/login?oidc_code=abc')).toBe('/login?oidc_code=abc'); - delete process.env.NODE_ENV; - }); -}); - -// ── discover ────────────────────────────────────────────────────────────────── - -describe('discover', () => { - afterEach(() => { - vi.unstubAllGlobals(); - }); - - it('OIDC-SVC-017: fetches and returns discovery document', async () => { - const doc = { - authorization_endpoint: 'https://oidc.example.com/auth', - token_endpoint: 'https://oidc.example.com/token', - userinfo_endpoint: 'https://oidc.example.com/userinfo', - }; - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ - ok: true, - json: async () => doc, - })); - - // Use unique issuer to bypass module-level cache from other tests - const result = await discover('https://unique-1.example.com'); - expect(result.authorization_endpoint).toBe(doc.authorization_endpoint); - expect(result.token_endpoint).toBe(doc.token_endpoint); - }); - - it('OIDC-SVC-018: throws when provider returns non-ok response', async () => { - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false })); - await expect(discover('https://bad-issuer.example.com')).rejects.toThrow(); - }); - - it('OIDC-SVC-037: accepts mismatched doc issuer when discoveryUrl is explicit', async () => { - const doc = { - issuer: 'https://auth.example.com/application/o/myapp/', - authorization_endpoint: 'https://auth.example.com/application/o/myapp/authorize/', - token_endpoint: 'https://auth.example.com/application/o/token/', - userinfo_endpoint: 'https://auth.example.com/application/o/userinfo/', - }; - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); - const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - - const result = await discover( - 'https://auth.example.com', - 'https://auth.example.com/application/o/myapp/.well-known/openid-configuration', - ); - - expect(result.issuer).toBe(doc.issuer); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('differs from configured OIDC_ISSUER')); - warnSpy.mockRestore(); - }); - - it('OIDC-SVC-038: throws on mismatched doc issuer when discoveryUrl is omitted', async () => { - const doc = { - issuer: 'https://evil.example.com', - authorization_endpoint: 'https://unique-2.example.com/auth', - token_endpoint: 'https://unique-2.example.com/token', - userinfo_endpoint: 'https://unique-2.example.com/userinfo', - }; - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); - - await expect(discover('https://unique-2.example.com')).rejects.toThrow( - 'OIDC discovery issuer mismatch', - ); - }); - - it('OIDC-SVC-039: trailing-slash-only mismatch with explicit discoveryUrl does not warn', async () => { - const doc = { - issuer: 'https://auth.example.com/', - authorization_endpoint: 'https://auth.example.com/auth', - token_endpoint: 'https://auth.example.com/token', - userinfo_endpoint: 'https://auth.example.com/userinfo', - }; - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); - const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - - await discover( - 'https://auth.example.com', - 'https://auth.example.com/.well-known/openid-configuration', - ); - - expect(warnSpy).not.toHaveBeenCalled(); - warnSpy.mockRestore(); - }); -}); - -// ── issuer trailing-slash regex (ReDoS guard) ───────────────────────────────── - -describe('getOidcConfig issuer trailing-slash regex', () => { - it('OIDC-SVC-019: /\\/+$/ strips trailing slashes in < 5ms', () => { - // The regex /\/+$/ in getOidcConfig: issuer.replace(/\/+$/, '') - // Adversarial input: many trailing slashes — should not backtrack catastrophically - const adversarial = 'https://oidc.example.com' + '/'.repeat(10000); - const start = Date.now(); - const result = adversarial.replace(/\/+$/, ''); - const elapsed = Date.now() - start; - expect(result).toBe('https://oidc.example.com'); - expect(elapsed).toBeLessThan(100); - }); -}); - -// ── findOrCreateUser ────────────────────────────────────────────────────────── - -describe('findOrCreateUser', () => { - it('OIDC-SVC-020: finds existing user by oidc_sub', () => { - const { user } = createUser(testDb, { email: 'alice@example.com' }); - // Link the sub manually - testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?') - .run('sub-alice-123', MOCK_CONFIG.issuer, user.id); - - const result = findOrCreateUser( - { sub: 'sub-alice-123', email: 'alice@example.com', name: 'Alice' }, - MOCK_CONFIG - ); - expect('user' in result).toBe(true); - expect((result as { user: any }).user.id).toBe(user.id); - }); - - it('OIDC-SVC-021: finds existing user by email when no sub match', () => { - const { user } = createUser(testDb, { email: 'bob@example.com' }); - - const result = findOrCreateUser( - { sub: 'sub-bob-new', email: 'bob@example.com', name: 'Bob', email_verified: true }, - MOCK_CONFIG - ); - expect('user' in result).toBe(true); - expect((result as { user: any }).user.id).toBe(user.id); - }); - - it('OIDC-SVC-022: creates new user when registration is open', () => { - const result = findOrCreateUser( - { sub: 'sub-new-1', email: 'newuser@example.com', name: 'New User' }, - MOCK_CONFIG - ); - expect('user' in result).toBe(true); - const newUser = testDb.prepare("SELECT * FROM users WHERE email = 'newuser@example.com'").get(); - expect(newUser).toBeDefined(); - }); - - it('OIDC-SVC-023: first user gets admin role', () => { - // DB is empty after resetTestDb - const result = findOrCreateUser( - { sub: 'sub-first', email: 'first@example.com', name: 'First' }, - MOCK_CONFIG - ); - expect('user' in result).toBe(true); - expect((result as { user: any }).user.role).toBe('admin'); - }); - - it('OIDC-SVC-024: returns registration_disabled error when registration is off', () => { - createUser(testDb, { email: 'existing@example.com' }); - testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('allow_registration', 'false')").run(); - - const result = findOrCreateUser( - { sub: 'sub-blocked', email: 'blocked@example.com', name: 'Blocked' }, - MOCK_CONFIG - ); - expect('error' in result).toBe(true); - expect((result as { error: string }).error).toBe('registration_disabled'); - }); - - it('OIDC-SVC-025: links oidc_sub when existing user has none (verified email)', () => { - const { user } = createUser(testDb, { email: 'charlie@example.com' }); - // Ensure no oidc_sub set - testDb.prepare('UPDATE users SET oidc_sub = NULL, oidc_issuer = NULL WHERE id = ?').run(user.id); - - findOrCreateUser( - { sub: 'sub-charlie-linked', email: 'charlie@example.com', name: 'Charlie', email_verified: true }, - MOCK_CONFIG - ); - - const updated = testDb.prepare('SELECT oidc_sub FROM users WHERE id = ?').get(user.id) as any; - expect(updated.oidc_sub).toBe('sub-charlie-linked'); - }); - - it('OIDC-SVC-025b: refuses to link an unverified email to an existing local account', () => { - const { user } = createUser(testDb, { email: 'dora@example.com' }); - testDb.prepare('UPDATE users SET oidc_sub = NULL, oidc_issuer = NULL WHERE id = ?').run(user.id); - - // No email_verified claim — an IdP that lets users set arbitrary emails must - // not be able to take over a pre-existing password account. - const result = findOrCreateUser( - { sub: 'sub-dora-attacker', email: 'dora@example.com', name: 'Dora' }, - MOCK_CONFIG - ); - - expect('error' in result).toBe(true); - expect((result as { error: string }).error).toBe('email_not_verified'); - const updated = testDb.prepare('SELECT oidc_sub FROM users WHERE id = ?').get(user.id) as any; - expect(updated.oidc_sub).toBeNull(); // account not linked / not hijacked - }); - - it('OIDC-SVC-026: existing user role is updated when OIDC claim mapping changes it', () => { - const { user } = createUser(testDb, { email: 'diana@example.com', role: 'user' }); - // Link oidc_sub manually so the user is found by sub lookup - testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?') - .run('sub-diana-role', MOCK_CONFIG.issuer, user.id); - - process.env.OIDC_ADMIN_VALUE = 'admins'; - - const result = findOrCreateUser( - { sub: 'sub-diana-role', email: 'diana@example.com', name: 'Diana', groups: ['admins'] }, - MOCK_CONFIG - ); - - expect('user' in result).toBe(true); - expect((result as { user: any }).user.role).toBe('admin'); - - const dbUser = testDb.prepare('SELECT role FROM users WHERE id = ?').get(user.id) as any; - expect(dbUser.role).toBe('admin'); - }); - - it('OIDC-SVC-027: new user with valid invite token increments used_count', () => { - const { user: creator } = createUser(testDb, { email: 'creator@example.com' }); - testDb.prepare( - "INSERT INTO invite_tokens (token, max_uses, used_count, created_by) VALUES ('tok-valid', 5, 0, ?)" - ).run(creator.id); - - const result = findOrCreateUser( - { sub: 'sub-invite-user', email: 'invitee@example.com', name: 'Invitee' }, - MOCK_CONFIG, - 'tok-valid' - ); - - expect('user' in result).toBe(true); - - const token = testDb.prepare("SELECT used_count FROM invite_tokens WHERE token = 'tok-valid'").get() as any; - expect(token.used_count).toBe(1); - }); - - it('OIDC-SVC-028: new user with expired invite token is created but invite is ignored', () => { - const { user: creator } = createUser(testDb, { email: 'creator2@example.com' }); - testDb.prepare( - "INSERT INTO invite_tokens (token, max_uses, used_count, expires_at, created_by) VALUES ('tok-expired', 5, 0, '2000-01-01T00:00:00.000Z', ?)" - ).run(creator.id); - - const result = findOrCreateUser( - { sub: 'sub-expired-invite', email: 'expired-invitee@example.com', name: 'ExpiredInvitee' }, - MOCK_CONFIG, - 'tok-expired' - ); - - // User is still created because open registration is allowed - expect('user' in result).toBe(true); - const newUser = testDb.prepare("SELECT id FROM users WHERE email = 'expired-invitee@example.com'").get(); - expect(newUser).toBeDefined(); - - // Invite used_count must remain 0 (token was treated as invalid) - const token = testDb.prepare("SELECT used_count FROM invite_tokens WHERE token = 'tok-expired'").get() as any; - expect(token.used_count).toBe(0); - }); - - it('OIDC-SVC-029: new user with max_uses exceeded invite token is created but invite is ignored', () => { - const { user: creator } = createUser(testDb, { email: 'creator3@example.com' }); - testDb.prepare( - "INSERT INTO invite_tokens (token, max_uses, used_count, created_by) VALUES ('tok-full', 1, 1, ?)" - ).run(creator.id); - - const result = findOrCreateUser( - { sub: 'sub-full-invite', email: 'full-invitee@example.com', name: 'FullInvitee' }, - MOCK_CONFIG, - 'tok-full' - ); - - // User is still created because open registration is allowed - expect('user' in result).toBe(true); - const newUser = testDb.prepare("SELECT id FROM users WHERE email = 'full-invitee@example.com'").get(); - expect(newUser).toBeDefined(); - - // Invite used_count must remain 1 (token was treated as invalid) - const token = testDb.prepare("SELECT used_count FROM invite_tokens WHERE token = 'tok-full'").get() as any; - expect(token.used_count).toBe(1); - }); - - // ── OIDC picture claim → avatar (#1399) ────────────────────────────────── - - it('OIDC-SVC-040: new user stores the https picture claim as their avatar', () => { - const result = findOrCreateUser( - { sub: 'sub-pic-1', email: 'pic1@example.com', name: 'Pic One', picture: 'https://idp.example.com/u/pic1.png' }, - MOCK_CONFIG - ); - expect('user' in result).toBe(true); - const row = testDb.prepare("SELECT avatar FROM users WHERE email = 'pic1@example.com'").get() as any; - expect(row.avatar).toBe('https://idp.example.com/u/pic1.png'); - }); - - it('OIDC-SVC-041: new user with a non-https picture claim stores no avatar', () => { - findOrCreateUser( - { sub: 'sub-pic-2', email: 'pic2@example.com', name: 'Pic Two', picture: 'http://idp.example.com/u/pic2.png' }, - MOCK_CONFIG - ); - const row = testDb.prepare("SELECT avatar FROM users WHERE email = 'pic2@example.com'").get() as any; - expect(row.avatar).toBeNull(); - }); - - it('OIDC-SVC-042: existing user with no avatar gets the OIDC picture', () => { - const { user } = createUser(testDb, { email: 'pic3@example.com' }); - testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = NULL WHERE id = ?') - .run('sub-pic-3', MOCK_CONFIG.issuer, user.id); - findOrCreateUser( - { sub: 'sub-pic-3', email: 'pic3@example.com', name: 'Pic Three', picture: 'https://idp.example.com/u/pic3.png' }, - MOCK_CONFIG - ); - const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any; - expect(row.avatar).toBe('https://idp.example.com/u/pic3.png'); - }); - - it('OIDC-SVC-043: a custom uploaded avatar is never overwritten by the OIDC picture', () => { - const { user } = createUser(testDb, { email: 'pic4@example.com' }); - testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?') - .run('sub-pic-4', MOCK_CONFIG.issuer, 'uploaded-abc.jpg', user.id); - findOrCreateUser( - { sub: 'sub-pic-4', email: 'pic4@example.com', name: 'Pic Four', picture: 'https://idp.example.com/u/pic4.png' }, - MOCK_CONFIG - ); - const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any; - expect(row.avatar).toBe('uploaded-abc.jpg'); - }); - - it('OIDC-SVC-044: a previously stored OIDC picture URL is refreshed on next login', () => { - const { user } = createUser(testDb, { email: 'pic5@example.com' }); - testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?') - .run('sub-pic-5', MOCK_CONFIG.issuer, 'https://idp.example.com/u/old.png', user.id); - findOrCreateUser( - { sub: 'sub-pic-5', email: 'pic5@example.com', name: 'Pic Five', picture: 'https://idp.example.com/u/new.png' }, - MOCK_CONFIG - ); - const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any; - expect(row.avatar).toBe('https://idp.example.com/u/new.png'); - }); - - it('OIDC-SVC-045: a trip-bound invite auto-adds the new SSO user as a trip member (#1402)', () => { - const { user: admin } = createUser(testDb, { role: 'admin' }); - const trip = createTrip(testDb, admin.id); - testDb.prepare( - 'INSERT INTO invite_tokens (token, max_uses, used_count, expires_at, created_by, trip_id) VALUES (?, 5, 0, NULL, ?, ?)' - ).run('inv-trip-join', admin.id, trip.id); - - const result = findOrCreateUser( - { sub: 'sub-trip-join', email: 'joiner@example.com', name: 'Joiner' }, - MOCK_CONFIG, - 'inv-trip-join' - ); - expect('user' in result).toBe(true); - const uid = (result as { user: any }).user.id; - const member = testDb.prepare('SELECT * FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, uid); - expect(member).toBeTruthy(); - }); -}); - -// ── exchangeCodeForToken ────────────────────────────────────────────────────── - -describe('exchangeCodeForToken', () => { - afterEach(() => { - vi.unstubAllGlobals(); - }); - - it('OIDC-SVC-030: sends correct POST body and returns token data', async () => { - const { exchangeCodeForToken } = await import('../../../src/services/oidcService'); - - const mockTokenData = { access_token: 'tok', token_type: 'Bearer' }; - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ - ok: true, - status: 200, - json: async () => mockTokenData, - })); - - const doc = { token_endpoint: 'https://oidc.example.com/token' } as any; - const result = await exchangeCodeForToken(doc, 'auth-code-123', 'https://app/callback', 'client-id', 'client-secret'); - - expect(result.access_token).toBe('tok'); - expect(result._ok).toBe(true); - expect(result._status).toBe(200); - - const fetchCall = (fetch as ReturnType).mock.calls[0]; - expect(fetchCall[0]).toBe('https://oidc.example.com/token'); - expect(fetchCall[1].method).toBe('POST'); - }); - - it('OIDC-SVC-031: reflects _ok=false when provider returns error status', async () => { - const { exchangeCodeForToken } = await import('../../../src/services/oidcService'); - - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ - ok: false, - status: 400, - json: async () => ({ error: 'invalid_grant' }), - })); - - const doc = { token_endpoint: 'https://oidc.example.com/token' } as any; - const result = await exchangeCodeForToken(doc, 'bad-code', 'https://app/callback', 'c', 's'); - - expect(result._ok).toBe(false); - expect(result._status).toBe(400); - }); -}); - -// ── getUserInfo ─────────────────────────────────────────────────────────────── - -describe('getUserInfo', () => { - afterEach(() => { - vi.unstubAllGlobals(); - }); - - it('OIDC-SVC-032: fetches userinfo with Bearer token and returns parsed JSON', async () => { - const { getUserInfo } = await import('../../../src/services/oidcService'); - - const userInfoData = { sub: 'user-sub', email: 'user@example.com', name: 'User Name' }; - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ - json: async () => userInfoData, - })); - - const result = await getUserInfo('https://oidc.example.com/userinfo', 'access-token-123'); - - expect(result.sub).toBe('user-sub'); - expect(result.email).toBe('user@example.com'); - - const fetchCall = (fetch as ReturnType).mock.calls[0]; - expect(fetchCall[1].headers.Authorization).toBe('Bearer access-token-123'); - }); -}); - -// ── verifyIdToken ───────────────────────────────────────────────────────────── - -describe('verifyIdToken', () => { - const { privateKey, publicKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); - const jwk = publicKey.export({ format: 'jwk' }) as Record; - const ISSUER = 'https://auth.example.com/application/o/trek'; - const CLIENT_ID = 'trek-client'; - const JWKS_URI = 'https://auth.example.com/.well-known/jwks.json'; - - function mockJwks() { - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ - ok: true, - json: async () => ({ keys: [jwk] }), - })); - } - - function makeToken(iss: string, overrides: object = {}) { - return jwtLib.sign( - { sub: 'user-sub', email: 'user@example.com', ...overrides }, - privateKey, - { algorithm: 'RS256', audience: CLIENT_ID, issuer: iss, expiresIn: '1h' } - ); - } - - const doc = { jwks_uri: JWKS_URI } as any; - - afterEach(() => { vi.unstubAllGlobals(); }); - - it('OIDC-SVC-033: accepts token whose iss matches expectedIssuer exactly', async () => { - mockJwks(); - const token = makeToken(ISSUER); - const result = await verifyIdToken(token, doc, CLIENT_ID, ISSUER); - expect(result.ok).toBe(true); - }); - - it('OIDC-SVC-034: accepts token whose iss has a trailing slash (Authentik)', async () => { - mockJwks(); - const token = makeToken(ISSUER + '/'); - const result = await verifyIdToken(token, doc, CLIENT_ID, ISSUER); - expect(result.ok).toBe(true); - }); - - it('OIDC-SVC-035: rejects token with wrong issuer', async () => { - mockJwks(); - const token = makeToken('https://evil.example.com'); - const result = await verifyIdToken(token, doc, CLIENT_ID, ISSUER); - expect(result.ok).toBe(false); - expect((result as any).error).toMatch('jwt issuer invalid'); - }); - - it('OIDC-SVC-036: rejects token with wrong audience', async () => { - mockJwks(); - const token = makeToken(ISSUER, {}); - const wrongAudToken = jwtLib.sign( - { sub: 'user-sub', iss: ISSUER }, - privateKey, - { algorithm: 'RS256', audience: 'wrong-client', expiresIn: '1h' } - ); - const result = await verifyIdToken(wrongAudToken, doc, CLIENT_ID, ISSUER); - expect(result.ok).toBe(false); - }); -});