diff --git a/.env.example b/.env.example index f51f6f1..b530464 100644 --- a/.env.example +++ b/.env.example @@ -16,3 +16,7 @@ TLS_KEY_FILE=/etc/travel-planner/tls/tls.key OIDC_ISSUER=https://idp.example.invalid/realms/travel-planner OIDC_AUDIENCE=travel-planner-api OIDC_CLIENT_ID=travel-planner-web +# OIDC_CLIENT_SECRET: this client is confidential (holds a secret). Never commit +# a real value here; supply it only via the deployment host's secret store / +# the developer's own shell environment. +OIDC_CLIENT_SECRET=change-me-outside-source-control diff --git a/README.md b/README.md index f2cf9cc..4d105d6 100644 --- a/README.md +++ b/README.md @@ -19,25 +19,32 @@ docs/ Specs, plans, and architecture documentation ```bash pnpm install pnpm dev:infra -DATABASE_URL=postgresql://travel_planner:travel_planner_dev@localhost:5432/travel_planner \ -REDIS_URL=redis://localhost:6379 \ -OIDC_ISSUER=https://idp.example.invalid/realms/travel-planner \ -OIDC_AUDIENCE=travel-planner-api \ - pnpm --filter backend start:api -pnpm --filter frontend start ``` -Run pending migrations against the dev database before starting the API for the first time: +`pnpm dev:infra` starts local PostgreSQL/Redis (see `compose.dev.yml`); `pnpm dev:infra:down` stops them. + +Set the required OIDC env vars once per shell before running the backend. **`OIDC_CLIENT_SECRET` is a real secret — set it directly in your own shell, never commit it, never paste it into a shared/logged terminal.** `OIDC_AUDIENCE` should be set to the same value as `OIDC_CLIENT_ID` unless your IdP issues a distinct API audience. ```bash export DATABASE_URL=postgresql://travel_planner:travel_planner_dev@localhost:5432/travel_planner export REDIS_URL=redis://localhost:6379 -export OIDC_ISSUER=https://idp.example.invalid/realms/travel-planner -export OIDC_AUDIENCE=travel-planner-api -pnpm --filter backend build:api && node backend/dist/apps/api/src/migration.js +export OIDC_ISSUER=https://auth.forgecore.work +export OIDC_CLIENT_ID=client_a297fd8d9c1f47a79d3600ea0c96984 +export OIDC_AUDIENCE=$OIDC_CLIENT_ID +export OIDC_CLIENT_SECRET= # never commit this value ``` -`pnpm dev:infra` starts local PostgreSQL/Redis (see `compose.dev.yml`); `pnpm dev:infra:down` stops them. +Run pending migrations against the dev database (first time only), then start the API and frontend: + +```bash +pnpm --filter backend build:api && node backend/dist/apps/api/src/migration.js +pnpm --filter backend start:api +pnpm --filter frontend start +``` + +Open `http://localhost:4200`. The Angular dev server proxies `/api/*` and `/health/*` to the API on `localhost:3000` (see `frontend/proxy.conf.json`), so no CORS configuration is needed locally. Make sure the IdP client `client_a297fd8d9c1f47a79d3600ea0c96984` allows the redirect URI `http://localhost:4200/auth/callback`. + +This IdP client is **confidential** (it has a client secret), so the Authorization Code + PKCE token exchange happens server-side via `POST /api/v1/auth/session` (see "OIDC client type" below) — the secret never reaches the browser. ## Quality gates @@ -76,8 +83,9 @@ Production Docker Compose (`compose.yml`) publishes **exactly one** host port, o ## Phase 02 status: OIDC auth, users, and trip core complete -- Authentication: OIDC Authorization Code + PKCE against an external IdP (`oidc-client-ts` on the frontend, `jose`-based bearer-token verification on the backend). No local password storage; users are keyed by the OIDC `sub` claim and just-in-time provisioned on first login. -- New required backend env vars: `OIDC_ISSUER`, `OIDC_AUDIENCE` (validated fail-fast like `DATABASE_URL`/`REDIS_URL`). New frontend build-time values: `OIDC_ISSUER`, `OIDC_CLIENT_ID` (baked into the production bundle by `docker/edge.Dockerfile`, never read from the container at runtime). +- Authentication: OIDC Authorization Code + PKCE against an external IdP. No local password storage; users are keyed by the OIDC `sub` claim and just-in-time provisioned on first login. +- **OIDC client type:** this deployment's IdP client is confidential (has a client secret), not a plain public/PKCE-only SPA client. A client secret must never be embedded in a browser bundle, so the frontend performs only the browser-side Authorization Code + PKCE redirect (hand-rolled PKCE in `frontend/src/app/auth/pkce.ts`, no `oidc-client-ts` dependency); the resulting `code` + PKCE `code_verifier` are then POSTed to the backend's `POST /api/v1/auth/session` (unauthenticated by design — there is no token yet), which performs the actual code-for-tokens exchange using `OIDC_CLIENT_SECRET` server-side (`TokenExchangeService`) and returns only `{ accessToken, expiresIn }` to the frontend — `refresh_token`/`id_token` are never forwarded. If a future deployment instead uses a public PKCE-only client, this proxy step could be skipped in favor of a direct frontend-to-IdP exchange, but the current IdP requires it. +- New required backend env vars: `OIDC_ISSUER`, `OIDC_AUDIENCE`, `OIDC_CLIENT_ID`, `OIDC_CLIENT_SECRET` (validated fail-fast like `DATABASE_URL`/`REDIS_URL`; `OIDC_CLIENT_SECRET` is a real secret, never committed). New frontend build-time values: `OIDC_ISSUER`, `OIDC_CLIENT_ID` (non-secret; baked into the production bundle by `docker/edge.Dockerfile`, never read from the container at runtime). - Real, versioned database migrations (`node-pg-migrate`, files under `backend/migrations/`) replace the Phase 01 no-op `migration.ts` body; the container command contract (`node backend/dist/apps/api/src/migration.js`) is unchanged. Database access goes through `kysely` (a type-safe query builder, not an ORM) over the existing `pg.Pool`; there is no schema auto-sync anywhere. - New routes: `GET/PUT /api/v1/users/me`(`/preferences`), `GET/POST /api/v1/trips`, `GET/PATCH/DELETE /api/v1/trips/:tripId`, `GET/PUT /api/v1/trips/:tripId/settings`, `GET/PATCH/DELETE /api/v1/trips/:tripId/members(/:memberId)`, `POST/GET/DELETE /api/v1/trips/:tripId/invitations(/:invitationId)`, `POST /api/v1/invitations/:token/accept`, `GET/POST/PATCH/DELETE /api/v1/trips/:tripId/travelers(/:travelerId)`, `GET/PUT/DELETE /api/v1/trips/:tripId/preference-overrides(/:overrideId)`. - Authorization is enforced backend-side by `OidcAuthGuard` (who) and `TripMembershipGuard` (trip access + `@TripRoles('OWNER')`), never only in the frontend. `TripMember` and `Traveler` are independent tables — a `Traveler` never implies or requires trip membership. diff --git a/backend/apps/api/src/api.module.ts b/backend/apps/api/src/api.module.ts index 4699d94..841334e 100644 --- a/backend/apps/api/src/api.module.ts +++ b/backend/apps/api/src/api.module.ts @@ -6,12 +6,14 @@ import { HealthModule } from './health/health.module'; import { VersionModule } from './version/version.module'; import { UsersApiModule } from './users/users.module'; import { TripsApiModule } from './trips/trips.module'; +import { AuthApiModule } from './auth/auth.module'; @Module({ imports: [ ConfigurationModule, HealthModule, VersionModule, + AuthApiModule, UsersApiModule, TripsApiModule, ], diff --git a/backend/apps/api/src/auth/auth-session.controller.spec.ts b/backend/apps/api/src/auth/auth-session.controller.spec.ts new file mode 100644 index 0000000..44964b1 --- /dev/null +++ b/backend/apps/api/src/auth/auth-session.controller.spec.ts @@ -0,0 +1,23 @@ +import { AuthSessionController } from './auth-session.controller'; + +describe('AuthSessionController', () => { + it('POST /auth/session exchanges the authorization code via the token exchange service', async () => { + const tokenExchange = { + exchangeAuthorizationCode: jest + .fn() + .mockResolvedValue({ accessToken: 'at-1', expiresIn: 3600 }), + }; + const controller = new AuthSessionController(tokenExchange as never); + + const dto = { + code: 'code-1', + codeVerifier: 'verifier-1', + redirectUri: 'http://localhost:4200/auth/callback', + }; + await expect(controller.createSession(dto)).resolves.toEqual({ + accessToken: 'at-1', + expiresIn: 3600, + }); + expect(tokenExchange.exchangeAuthorizationCode).toHaveBeenCalledWith(dto); + }); +}); diff --git a/backend/apps/api/src/auth/auth-session.controller.ts b/backend/apps/api/src/auth/auth-session.controller.ts new file mode 100644 index 0000000..d78eb71 --- /dev/null +++ b/backend/apps/api/src/auth/auth-session.controller.ts @@ -0,0 +1,15 @@ +import { Body, Controller, Post } from '@nestjs/common'; +import { TokenExchangeService } from '../../../../libs/auth/src'; +import type { AuthorizationCodeExchangeRequest, AuthorizationCodeExchangeResult } from '../../../../libs/auth/src'; + +@Controller('auth') +export class AuthSessionController { + constructor(private readonly tokenExchange: TokenExchangeService) {} + + @Post('session') + createSession( + @Body() dto: AuthorizationCodeExchangeRequest, + ): Promise { + return this.tokenExchange.exchangeAuthorizationCode(dto); + } +} diff --git a/backend/apps/api/src/auth/auth.module.ts b/backend/apps/api/src/auth/auth.module.ts new file mode 100644 index 0000000..c1d5fe8 --- /dev/null +++ b/backend/apps/api/src/auth/auth.module.ts @@ -0,0 +1,7 @@ +import { Module } from '@nestjs/common'; +import { AuthSessionController } from './auth-session.controller'; + +@Module({ + controllers: [AuthSessionController], +}) +export class AuthApiModule {} diff --git a/backend/libs/auth/src/auth.module.ts b/backend/libs/auth/src/auth.module.ts index 23f9a47..72b7386 100644 --- a/backend/libs/auth/src/auth.module.ts +++ b/backend/libs/auth/src/auth.module.ts @@ -2,11 +2,17 @@ import { Global, Module } from '@nestjs/common'; import { UsersLibModule } from '../../users/src'; import { OidcDiscoveryService } from './oidc-discovery.service'; import { OidcAuthGuard } from './oidc-auth.guard'; +import { TokenExchangeService } from './token-exchange.service'; @Global() @Module({ imports: [UsersLibModule], - providers: [OidcDiscoveryService, OidcAuthGuard], - exports: [OidcDiscoveryService, OidcAuthGuard, UsersLibModule], + providers: [OidcDiscoveryService, OidcAuthGuard, TokenExchangeService], + exports: [ + OidcDiscoveryService, + OidcAuthGuard, + TokenExchangeService, + UsersLibModule, + ], }) export class AuthModule {} diff --git a/backend/libs/auth/src/index.ts b/backend/libs/auth/src/index.ts index 03eba96..7165225 100644 --- a/backend/libs/auth/src/index.ts +++ b/backend/libs/auth/src/index.ts @@ -1,3 +1,4 @@ export * from './oidc-discovery.service'; export * from './oidc-auth.guard'; +export * from './token-exchange.service'; export * from './auth.module'; diff --git a/backend/libs/auth/src/oidc-discovery.service.spec.ts b/backend/libs/auth/src/oidc-discovery.service.spec.ts new file mode 100644 index 0000000..64ef8d0 --- /dev/null +++ b/backend/libs/auth/src/oidc-discovery.service.spec.ts @@ -0,0 +1,47 @@ +import { OidcDiscoveryService } from './oidc-discovery.service'; +import type { AppEnvironment } from '../../configuration/src'; + +describe('OidcDiscoveryService', () => { + const environment: AppEnvironment = { + databaseUrl: 'postgresql://u:p@postgres:5432/db', + redisUrl: 'redis://redis:6379', + oidcIssuer: 'https://idp.example.test', + oidcAudience: 'client-1', + oidcClientId: 'client-1', + oidcClientSecret: 'secret-1', + appVersion: 'dev', + teamCityBuildNumber: 'local', + sourceRevision: 'local', + }; + + it('fetches the discovery document once and exposes the token endpoint', async () => { + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + jwks_uri: 'https://idp.example.test/oidc/jwks', + token_endpoint: 'https://idp.example.test/oidc/token', + }), + }); + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as never; + + const service = new OidcDiscoveryService(environment); + await service.onModuleInit(); + + expect(fetchMock).toHaveBeenCalledWith( + 'https://idp.example.test/.well-known/openid-configuration', + ); + expect(service.getTokenEndpoint()).toBe( + 'https://idp.example.test/oidc/token', + ); + expect(service.getIssuer()).toBe('https://idp.example.test'); + expect(service.getAudience()).toBe('client-1'); + }); + + it('throws when asked for the token endpoint before discovery has completed', () => { + const service = new OidcDiscoveryService(environment); + expect(() => service.getTokenEndpoint()).toThrow( + 'OIDC discovery has not completed yet', + ); + }); +}); diff --git a/backend/libs/auth/src/oidc-discovery.service.ts b/backend/libs/auth/src/oidc-discovery.service.ts index 43c66f4..bbc4aca 100644 --- a/backend/libs/auth/src/oidc-discovery.service.ts +++ b/backend/libs/auth/src/oidc-discovery.service.ts @@ -6,11 +6,13 @@ import type { AppEnvironment } from '../../configuration/src'; interface OidcDiscoveryDocument { jwks_uri: string; + token_endpoint: string; } @Injectable() export class OidcDiscoveryService implements OnModuleInit { private verificationKeySet: JWTVerifyGetKey | undefined; + private tokenEndpoint: string | undefined; constructor( @Inject(APP_ENVIRONMENT) private readonly environment: AppEnvironment, @@ -26,6 +28,7 @@ export class OidcDiscoveryService implements OnModuleInit { } const document = (await response.json()) as OidcDiscoveryDocument; this.verificationKeySet = createRemoteJWKSet(new URL(document.jwks_uri)); + this.tokenEndpoint = document.token_endpoint; } getIssuer(): string { @@ -42,4 +45,11 @@ export class OidcDiscoveryService implements OnModuleInit { } return this.verificationKeySet; } + + getTokenEndpoint(): string { + if (!this.tokenEndpoint) { + throw new Error('OIDC discovery has not completed yet'); + } + return this.tokenEndpoint; + } } diff --git a/backend/libs/auth/src/token-exchange.service.spec.ts b/backend/libs/auth/src/token-exchange.service.spec.ts new file mode 100644 index 0000000..cd11491 --- /dev/null +++ b/backend/libs/auth/src/token-exchange.service.spec.ts @@ -0,0 +1,110 @@ +import { BadRequestException } from '@nestjs/common'; +import { TokenExchangeService } from './token-exchange.service'; +import type { AppEnvironment } from '../../configuration/src'; + +describe('TokenExchangeService.exchangeAuthorizationCode', () => { + const environment: AppEnvironment = { + databaseUrl: 'postgresql://u:p@postgres:5432/db', + redisUrl: 'redis://redis:6379', + oidcIssuer: 'https://idp.example.test', + oidcAudience: 'client-1', + oidcClientId: 'client-1', + oidcClientSecret: 'super-secret', + appVersion: 'dev', + teamCityBuildNumber: 'local', + sourceRevision: 'local', + }; + + function fakeDiscovery( + tokenEndpoint = 'https://idp.example.test/oidc/token', + ) { + return { getTokenEndpoint: jest.fn().mockReturnValue(tokenEndpoint) }; + } + + it('posts a client-secret-authenticated request and returns only the safe fields', async () => { + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + access_token: 'at-1', + expires_in: 3600, + refresh_token: 'rt-1', + id_token: 'idt-1', + }), + }); + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as never; + + const service = new TokenExchangeService( + environment, + fakeDiscovery() as never, + ); + const result = await service.exchangeAuthorizationCode({ + code: 'auth-code-1', + codeVerifier: 'verifier-1', + redirectUri: 'http://localhost:4200/auth/callback', + }); + + expect(result).toEqual({ accessToken: 'at-1', expiresIn: 3600 }); + + const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toBe('https://idp.example.test/oidc/token'); + const body = new URLSearchParams(init.body as string); + expect(body.get('grant_type')).toBe('authorization_code'); + expect(body.get('client_id')).toBe('client-1'); + expect(body.get('client_secret')).toBe('super-secret'); + expect(body.get('code')).toBe('auth-code-1'); + expect(body.get('code_verifier')).toBe('verifier-1'); + expect(body.get('redirect_uri')).toBe( + 'http://localhost:4200/auth/callback', + ); + }); + + it('never leaks the refresh_token or id_token to the caller', async () => { + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + access_token: 'at-1', + expires_in: 3600, + refresh_token: 'rt-1', + id_token: 'idt-1', + }), + }); + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as never; + + const service = new TokenExchangeService( + environment, + fakeDiscovery() as never, + ); + const result = await service.exchangeAuthorizationCode({ + code: 'auth-code-1', + codeVerifier: 'verifier-1', + redirectUri: 'http://localhost:4200/auth/callback', + }); + + expect(result).not.toHaveProperty('refreshToken'); + expect(result).not.toHaveProperty('idToken'); + }); + + it('rejects with BadRequestException when the identity provider rejects the code', async () => { + const fetchMock = jest.fn().mockResolvedValue({ + ok: false, + status: 400, + json: () => Promise.resolve({ error: 'invalid_grant' }), + }); + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as never; + + const service = new TokenExchangeService( + environment, + fakeDiscovery() as never, + ); + + await expect( + service.exchangeAuthorizationCode({ + code: 'bad-code', + codeVerifier: 'verifier-1', + redirectUri: 'http://localhost:4200/auth/callback', + }), + ).rejects.toThrow(BadRequestException); + }); +}); diff --git a/backend/libs/auth/src/token-exchange.service.ts b/backend/libs/auth/src/token-exchange.service.ts new file mode 100644 index 0000000..6909519 --- /dev/null +++ b/backend/libs/auth/src/token-exchange.service.ts @@ -0,0 +1,68 @@ +import { BadRequestException, Inject, Injectable } from '@nestjs/common'; +import { APP_ENVIRONMENT } from '../../configuration/src'; +import type { AppEnvironment } from '../../configuration/src'; +import { OidcDiscoveryService } from './oidc-discovery.service'; + +export interface AuthorizationCodeExchangeRequest { + code: string; + codeVerifier: string; + redirectUri: string; +} + +export interface AuthorizationCodeExchangeResult { + accessToken: string; + expiresIn: number; +} + +interface TokenEndpointResponse { + access_token: string; + expires_in: number; + refresh_token?: string; + id_token?: string; +} + +/** + * Exchanges an OIDC authorization code for tokens on behalf of the (confidential, + * client-secret-bearing) SPA client. The secret never reaches the browser: the + * frontend performs the Authorization Code + PKCE redirect itself, then hands the + * resulting code + PKCE verifier to this service, which is the only place the + * client secret is used. + */ +@Injectable() +export class TokenExchangeService { + constructor( + @Inject(APP_ENVIRONMENT) private readonly environment: AppEnvironment, + private readonly discovery: OidcDiscoveryService, + ) {} + + async exchangeAuthorizationCode( + request: AuthorizationCodeExchangeRequest, + ): Promise { + const body = new URLSearchParams({ + grant_type: 'authorization_code', + client_id: this.environment.oidcClientId, + client_secret: this.environment.oidcClientSecret, + code: request.code, + code_verifier: request.codeVerifier, + redirect_uri: request.redirectUri, + }); + + const response = await fetch(this.discovery.getTokenEndpoint(), { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: body.toString(), + }); + + if (!response.ok) { + throw new BadRequestException( + 'The identity provider rejected the authorization code', + ); + } + + const tokenResponse = (await response.json()) as TokenEndpointResponse; + return { + accessToken: tokenResponse.access_token, + expiresIn: tokenResponse.expires_in, + }; + } +} diff --git a/backend/libs/configuration/src/environment.spec.ts b/backend/libs/configuration/src/environment.spec.ts index 6d38d48..962e6c5 100644 --- a/backend/libs/configuration/src/environment.spec.ts +++ b/backend/libs/configuration/src/environment.spec.ts @@ -12,6 +12,8 @@ describe('loadEnvironment', () => { REDIS_URL: 'redis://redis:6379', OIDC_ISSUER: 'https://idp.example.test/', OIDC_AUDIENCE: 'travel-planner-api', + OIDC_CLIENT_ID: 'test-client', + OIDC_CLIENT_SECRET: 'test-secret', APP_VERSION: '1.2.3', TEAMCITY_BUILD_NUMBER: '42', SOURCE_REVISION: 'abc123', @@ -21,6 +23,8 @@ describe('loadEnvironment', () => { redisUrl: 'redis://redis:6379', oidcIssuer: 'https://idp.example.test/', oidcAudience: 'travel-planner-api', + oidcClientId: 'test-client', + oidcClientSecret: 'test-secret', appVersion: '1.2.3', teamCityBuildNumber: '42', sourceRevision: 'abc123', diff --git a/backend/libs/configuration/src/environment.ts b/backend/libs/configuration/src/environment.ts index 8324a55..763d825 100644 --- a/backend/libs/configuration/src/environment.ts +++ b/backend/libs/configuration/src/environment.ts @@ -3,6 +3,8 @@ export interface AppEnvironment { redisUrl: string; oidcIssuer: string; oidcAudience: string; + oidcClientId: string; + oidcClientSecret: string; appVersion: string; teamCityBuildNumber: string; sourceRevision: string; @@ -20,6 +22,8 @@ export function loadEnvironment(env: NodeJS.ProcessEnv): AppEnvironment { redisUrl: required(env, 'REDIS_URL'), oidcIssuer: required(env, 'OIDC_ISSUER'), oidcAudience: required(env, 'OIDC_AUDIENCE'), + oidcClientId: required(env, 'OIDC_CLIENT_ID'), + oidcClientSecret: required(env, 'OIDC_CLIENT_SECRET'), appVersion: env.APP_VERSION?.trim() || 'dev', teamCityBuildNumber: env.TEAMCITY_BUILD_NUMBER?.trim() || 'local', sourceRevision: env.SOURCE_REVISION?.trim() || 'local', diff --git a/backend/test/setup-env.ts b/backend/test/setup-env.ts index 560f4ec..63c65d7 100644 --- a/backend/test/setup-env.ts +++ b/backend/test/setup-env.ts @@ -2,3 +2,5 @@ process.env.DATABASE_URL ??= 'postgresql://test:test@localhost:5432/test'; process.env.REDIS_URL ??= 'redis://localhost:6379'; process.env.OIDC_ISSUER ??= 'https://idp.example.test/'; process.env.OIDC_AUDIENCE ??= 'travel-planner-api'; +process.env.OIDC_CLIENT_ID ??= 'test-client'; +process.env.OIDC_CLIENT_SECRET ??= 'test-secret'; diff --git a/compose.yml b/compose.yml index 2f8d556..e83d9ba 100644 --- a/compose.yml +++ b/compose.yml @@ -30,6 +30,8 @@ services: REDIS_URL: "redis://redis:6379" OIDC_ISSUER: "${OIDC_ISSUER}" OIDC_AUDIENCE: "${OIDC_AUDIENCE}" + OIDC_CLIENT_ID: "${OIDC_CLIENT_ID}" + OIDC_CLIENT_SECRET: "${OIDC_CLIENT_SECRET}" APP_VERSION: "${APP_VERSION:-dev}" TEAMCITY_BUILD_NUMBER: "${TEAMCITY_BUILD_NUMBER:-local}" SOURCE_REVISION: "${SOURCE_REVISION:-local}" @@ -57,6 +59,8 @@ services: REDIS_URL: "redis://redis:6379" OIDC_ISSUER: "${OIDC_ISSUER}" OIDC_AUDIENCE: "${OIDC_AUDIENCE}" + OIDC_CLIENT_ID: "${OIDC_CLIENT_ID}" + OIDC_CLIENT_SECRET: "${OIDC_CLIENT_SECRET}" APP_VERSION: "${APP_VERSION:-dev}" TEAMCITY_BUILD_NUMBER: "${TEAMCITY_BUILD_NUMBER:-local}" SOURCE_REVISION: "${SOURCE_REVISION:-local}" diff --git a/docs/superpowers/plans/travel-planner-phase-02-users-oidc-trips-plan.md b/docs/superpowers/plans/travel-planner-phase-02-users-oidc-trips-plan.md index ed5e3f2..e006191 100644 --- a/docs/superpowers/plans/travel-planner-phase-02-users-oidc-trips-plan.md +++ b/docs/superpowers/plans/travel-planner-phase-02-users-oidc-trips-plan.md @@ -1325,3 +1325,18 @@ Do not start any of the following during this phase; each belongs to a later, ex - Payment/booking automation (permanent non-goal). Phase 03 begins only after the Phase 02 acceptance checklist is reviewed and green. + +--- + +## Addendum: Confidential IdP Client Requires Backend Token Exchange (2026-08-17) + +The plan's Task 10 assumed a public, PKCE-only SPA client and used `oidc-client-ts` to perform the full Authorization Code + PKCE token exchange directly in the browser. During manual end-to-end testing against the actual target IdP (`https://auth.forgecore.work`), it turned out the provisioned client is **confidential** (it has a client secret) rather than public. A client secret must never be shipped in a browser bundle, so this required a real, user-confirmed architecture change rather than a workaround: + +- The frontend still performs the Authorization Code + PKCE redirect itself (hand-rolled PKCE generation in `frontend/src/app/auth/pkce.ts`; `oidc-client-ts` was removed as a dependency since its callback handling assumes a direct-to-IdP token exchange that doesn't fit this model). +- The resulting authorization `code` and PKCE `code_verifier` are POSTed to a new, intentionally unauthenticated backend endpoint, `POST /api/v1/auth/session` (`AuthSessionController` → `TokenExchangeService`), which performs the code-for-tokens exchange using `OIDC_CLIENT_SECRET` server-side and returns only `{ accessToken, expiresIn }` — `refresh_token`/`id_token` are deliberately never forwarded to the frontend. +- New required backend env vars: `OIDC_CLIENT_ID`, `OIDC_CLIENT_SECRET` (in addition to the already-planned `OIDC_ISSUER`/`OIDC_AUDIENCE`). +- `OIDC_AUDIENCE` is operationally set equal to `OIDC_CLIENT_ID` for this IdP, since it does not issue a separate API-resource audience; no code change was needed for this, only a configuration convention. +- `OidcDiscoveryService` now also captures `token_endpoint` (in addition to `jwks_uri`), needed for the backend-side token exchange. +- A local dev proxy (`frontend/proxy.conf.json`, wired into `angular.json`'s `serve` target) forwards `/api/*` and `/health/*` from the Angular dev server to the API, avoiding a need for CORS configuration in local development (production already avoids CORS entirely since `edge` serves both origins). + +This is a deployment-specific IdP constraint, not a general Travel Planner requirement — a future public-client IdP could skip the backend proxy — but the BFF pattern is what ships today since it is what the actual configured IdP requires. diff --git a/frontend/angular.json b/frontend/angular.json index 286f0bc..d6267a9 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -64,6 +64,9 @@ }, "serve": { "builder": "@angular/build:dev-server", + "options": { + "proxyConfig": "proxy.conf.json" + }, "configurations": { "production": { "buildTarget": "frontend:build:production" diff --git a/frontend/package.json b/frontend/package.json index b4716ea..5344ef4 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -17,7 +17,6 @@ "@angular/platform-browser": "^21.2.0", "@angular/router": "^21.2.0", "@angular/service-worker": "^21.2.0", - "oidc-client-ts": "^3.5.0", "rxjs": "~7.8.0", "tslib": "^2.3.0" }, diff --git a/frontend/proxy.conf.json b/frontend/proxy.conf.json new file mode 100644 index 0000000..76ed92e --- /dev/null +++ b/frontend/proxy.conf.json @@ -0,0 +1,4 @@ +{ + "/api": { "target": "http://localhost:3000", "secure": false }, + "/health": { "target": "http://localhost:3000", "secure": false } +} diff --git a/frontend/src/app/auth/auth.service.spec.ts b/frontend/src/app/auth/auth.service.spec.ts index 40afd2e..a7e0459 100644 --- a/frontend/src/app/auth/auth.service.spec.ts +++ b/frontend/src/app/auth/auth.service.spec.ts @@ -1,11 +1,87 @@ import { TestBed } from '@angular/core/testing'; -import { describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { AuthService } from './auth.service'; describe('AuthService', () => { - it('starts unauthenticated when no OIDC user is stored', async () => { + beforeEach(() => { + sessionStorage.clear(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('starts unauthenticated when no session is stored', () => { const service = TestBed.inject(AuthService); - await Promise.resolve(); expect(service.isAuthenticated()).toBe(false); }); + + it('starts authenticated when a non-expired access token is already stored', () => { + sessionStorage.setItem('auth.accessToken', 'stored-token'); + sessionStorage.setItem('auth.expiresAt', String(Date.now() + 60_000)); + const service = TestBed.inject(AuthService); + expect(service.isAuthenticated()).toBe(true); + }); + + it('login() persists a PKCE verifier and state before redirecting', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + json: () => Promise.resolve({ authorization_endpoint: 'https://idp.example.test/oidc/auth' }), + }); + vi.stubGlobal('fetch', fetchMock); + vi.stubGlobal('location', { ...window.location, href: '' }); + + const service = TestBed.inject(AuthService); + await service.login(); + + expect(sessionStorage.getItem('auth.codeVerifier')).toBeTruthy(); + expect(sessionStorage.getItem('auth.state')).toBeTruthy(); + expect(window.location.href).toContain('https://idp.example.test/oidc/auth?'); + expect(window.location.href).toContain('code_challenge_method=S256'); + }); + + it('completeLogin() rejects a state that does not match the one stored before redirecting', async () => { + sessionStorage.setItem('auth.codeVerifier', 'verifier-1'); + sessionStorage.setItem('auth.state', 'expected-state'); + vi.stubGlobal('location', { ...window.location, search: '?code=abc&state=wrong-state' }); + + const service = TestBed.inject(AuthService); + await expect(service.completeLogin()).rejects.toThrow(); + }); + + it('completeLogin() exchanges the code via the backend and stores the resulting access token', async () => { + sessionStorage.setItem('auth.codeVerifier', 'verifier-1'); + sessionStorage.setItem('auth.state', 'state-1'); + vi.stubGlobal('location', { ...window.location, search: '?code=abc&state=state-1' }); + + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ accessToken: 'at-1', expiresIn: 3600 }), + }); + vi.stubGlobal('fetch', fetchMock); + + const service = TestBed.inject(AuthService); + await service.completeLogin(); + + expect(service.isAuthenticated()).toBe(true); + await expect(service.getAccessToken()).resolves.toBe('at-1'); + expect(sessionStorage.getItem('auth.codeVerifier')).toBeNull(); + }); + + it('logout() clears the stored session', () => { + sessionStorage.setItem('auth.accessToken', 'at-1'); + sessionStorage.setItem('auth.expiresAt', String(Date.now() + 60_000)); + const service = TestBed.inject(AuthService); + + service.logout(); + + expect(service.isAuthenticated()).toBe(false); + }); + + it('getAccessToken() returns undefined once the token has expired', async () => { + sessionStorage.setItem('auth.accessToken', 'at-1'); + sessionStorage.setItem('auth.expiresAt', String(Date.now() - 1000)); + const service = TestBed.inject(AuthService); + + await expect(service.getAccessToken()).resolves.toBeUndefined(); + }); }); diff --git a/frontend/src/app/auth/auth.service.ts b/frontend/src/app/auth/auth.service.ts index 2a39293..c66a202 100644 --- a/frontend/src/app/auth/auth.service.ts +++ b/frontend/src/app/auth/auth.service.ts @@ -1,39 +1,103 @@ import { Injectable, signal } from '@angular/core'; -import { UserManager } from 'oidc-client-ts'; import { environment } from '../../environments/environment'; +import { generateCodeChallenge, generateRandomString } from './pkce'; +const ACCESS_TOKEN_KEY = 'auth.accessToken'; +const EXPIRES_AT_KEY = 'auth.expiresAt'; +const CODE_VERIFIER_KEY = 'auth.codeVerifier'; +const STATE_KEY = 'auth.state'; + +interface DiscoveryDocument { + authorization_endpoint: string; +} + +interface SessionResponse { + accessToken: string; + expiresIn: number; +} + +/** + * The IdP client backing this app is confidential (holds a client secret), so + * the authorization-code-for-tokens exchange must happen server-side — see + * `POST /api/v1/auth/session`. This service only performs the browser-side + * Authorization Code + PKCE redirect and hands the resulting code + PKCE + * verifier to the backend; it never sees or stores the client secret. + */ @Injectable({ providedIn: 'root' }) export class AuthService { - private readonly userManager = new UserManager({ - authority: environment.oidc.issuer, - client_id: environment.oidc.clientId, - redirect_uri: environment.oidc.redirectUri, - scope: environment.oidc.scope, - response_type: 'code', - }); + private discoveryPromise: Promise | undefined; - readonly isAuthenticated = signal(false); + readonly isAuthenticated = signal(this.hasValidAccessToken()); - constructor() { - this.userManager.events.addUserLoaded(() => this.isAuthenticated.set(true)); - this.userManager.events.addUserUnloaded(() => this.isAuthenticated.set(false)); - void this.userManager.getUser().then((user) => this.isAuthenticated.set(!!user && !user.expired)); + private hasValidAccessToken(): boolean { + const expiresAt = Number(sessionStorage.getItem(EXPIRES_AT_KEY) ?? 0); + return !!sessionStorage.getItem(ACCESS_TOKEN_KEY) && Date.now() < expiresAt; } - login(): Promise { - return this.userManager.signinRedirect(); + private discover(): Promise { + if (!this.discoveryPromise) { + const issuer = environment.oidc.issuer.replace(/\/$/, ''); + this.discoveryPromise = fetch(`${issuer}/.well-known/openid-configuration`).then((response) => response.json()); + } + return this.discoveryPromise; + } + + async login(): Promise { + const codeVerifier = generateRandomString(); + const state = generateRandomString(); + const codeChallenge = await generateCodeChallenge(codeVerifier); + sessionStorage.setItem(CODE_VERIFIER_KEY, codeVerifier); + sessionStorage.setItem(STATE_KEY, state); + + const discovery = await this.discover(); + const url = new URL(discovery.authorization_endpoint); + url.searchParams.set('response_type', 'code'); + url.searchParams.set('client_id', environment.oidc.clientId); + url.searchParams.set('redirect_uri', environment.oidc.redirectUri); + url.searchParams.set('scope', environment.oidc.scope); + url.searchParams.set('state', state); + url.searchParams.set('code_challenge', codeChallenge); + url.searchParams.set('code_challenge_method', 'S256'); + + window.location.href = url.toString(); } async completeLogin(): Promise { - await this.userManager.signinRedirectCallback(); + const params = new URLSearchParams(window.location.search); + const code = params.get('code'); + const state = params.get('state'); + const codeVerifier = sessionStorage.getItem(CODE_VERIFIER_KEY); + const expectedState = sessionStorage.getItem(STATE_KEY); + + if (!code || !state || !codeVerifier || state !== expectedState) { + throw new Error('Invalid or missing OIDC callback parameters'); + } + + const response = await fetch(`${environment.apiBaseUrl}/auth/session`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ code, codeVerifier, redirectUri: environment.oidc.redirectUri }), + }); + + if (!response.ok) { + throw new Error('Failed to exchange the authorization code for a session'); + } + + const session = (await response.json()) as SessionResponse; + sessionStorage.setItem(ACCESS_TOKEN_KEY, session.accessToken); + sessionStorage.setItem(EXPIRES_AT_KEY, String(Date.now() + session.expiresIn * 1000)); + sessionStorage.removeItem(CODE_VERIFIER_KEY); + sessionStorage.removeItem(STATE_KEY); + this.isAuthenticated.set(true); } - logout(): Promise { - return this.userManager.signoutRedirect(); + logout(): void { + sessionStorage.removeItem(ACCESS_TOKEN_KEY); + sessionStorage.removeItem(EXPIRES_AT_KEY); + this.isAuthenticated.set(false); } async getAccessToken(): Promise { - const user = await this.userManager.getUser(); - return user && !user.expired ? user.access_token : undefined; + return this.hasValidAccessToken() ? (sessionStorage.getItem(ACCESS_TOKEN_KEY) ?? undefined) : undefined; } } diff --git a/frontend/src/app/auth/pkce.spec.ts b/frontend/src/app/auth/pkce.spec.ts new file mode 100644 index 0000000..856aa24 --- /dev/null +++ b/frontend/src/app/auth/pkce.spec.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest'; +import { generateCodeChallenge, generateRandomString } from './pkce'; + +describe('pkce', () => { + it('computes the RFC 7636 Appendix B S256 test vector', async () => { + // https://datatracker.ietf.org/doc/html/rfc7636#appendix-B + const codeVerifier = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'; + const challenge = await generateCodeChallenge(codeVerifier); + expect(challenge).toBe('E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM'); + }); + + it('generates a URL-safe random string of the requested length family', () => { + const value = generateRandomString(); + expect(value).toMatch(/^[A-Za-z0-9_-]+$/); + expect(value.length).toBeGreaterThanOrEqual(43); + }); + + it('generates different values on each call', () => { + expect(generateRandomString()).not.toBe(generateRandomString()); + }); +}); diff --git a/frontend/src/app/auth/pkce.ts b/frontend/src/app/auth/pkce.ts new file mode 100644 index 0000000..69beb19 --- /dev/null +++ b/frontend/src/app/auth/pkce.ts @@ -0,0 +1,16 @@ +function base64UrlEncode(bytes: Uint8Array): string { + let binary = ''; + for (const byte of bytes) binary += String.fromCharCode(byte); + return btoa(binary).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); +} + +export function generateRandomString(byteLength = 32): string { + const bytes = new Uint8Array(byteLength); + crypto.getRandomValues(bytes); + return base64UrlEncode(bytes); +} + +export async function generateCodeChallenge(codeVerifier: string): Promise { + const digest = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(codeVerifier)); + return base64UrlEncode(new Uint8Array(digest)); +} diff --git a/frontend/src/environments/environment.ts b/frontend/src/environments/environment.ts index 931e419..7ee5791 100644 --- a/frontend/src/environments/environment.ts +++ b/frontend/src/environments/environment.ts @@ -2,8 +2,8 @@ export const environment = { production: false, apiBaseUrl: '/api/v1', oidc: { - issuer: 'https://idp.example.invalid/realms/travel-planner', - clientId: 'travel-planner-web', + issuer: 'https://auth.forgecore.work', + clientId: 'client_a297fd8d9c1f47a79d3600ea0c96984', redirectUri: `${window.location.origin}/auth/callback`, scope: 'openid profile email', }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0293a9f..69b7404 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -141,9 +141,6 @@ importers: '@angular/service-worker': specifier: ^21.2.0 version: 21.2.20(@angular/core@21.2.20(@angular/compiler@21.2.20)(rxjs@7.8.2))(rxjs@7.8.2) - oidc-client-ts: - specifier: ^3.5.0 - version: 3.5.0 rxjs: specifier: ~7.8.0 version: 7.8.2 @@ -3537,10 +3534,6 @@ packages: resolution: {integrity: sha512-POQXvpdL69+CluYsillJ7SUhKvytYjW9vG/GKpnf+xP8UWgYEM/RaMzHHofbALDiKbbP1W8UEYmgGl39WkPZsg==} engines: {'0': node >= 0.2.0} - jwt-decode@4.0.0: - resolution: {integrity: sha512-+KJGIyHgkGuIq3IEBNftfhW/LfWhXUIY6OmyVWjliu5KH1y0fw7VQ8YndE2O4qZdMSd9SqbnC8GOcZEy0Om7sA==} - engines: {node: '>=18'} - keyv@4.5.4: resolution: {integrity: sha512-oxVHkHR/EJf2CNXnWxRLW6mg7JyCCUcG0DtEGmL2ctUo1PNTin1PUil+r/+4r5MpVgC/fn1kjsx7mjSujKqIpw==} @@ -3876,10 +3869,6 @@ packages: resolution: {integrity: sha512-4a+OsYv9UktOJKE+l1A4OufDgdRF9PifWj+tJnHURo/P+WOxpG4GzUFL9qCalmWauao6ogiG+QvnCovwPoyAWA==} engines: {node: '>=12.20.0'} - oidc-client-ts@3.5.0: - resolution: {integrity: sha512-l2q8l9CTCTOlbX+AnK4p3M+4CEpKpyQhle6blQkdFhm0IsBqsxm15bYaSa11G7pWdsYr6epdsRZxJpCyCRbT8A==} - engines: {node: '>=18'} - on-finished@2.4.1: resolution: {integrity: sha512-oVlzkg3ENAhCk2zdv7IJwd/QUD4z2RxRwpkcGY8psCVcCYZNq4wYnVWALHM+brtuJjePWiYF/ClmuDr8Ch5+kg==} engines: {node: '>= 0.8'} @@ -8690,8 +8679,6 @@ snapshots: jsonparse@1.3.1: {} - jwt-decode@4.0.0: {} - keyv@4.5.4: dependencies: json-buffer: 3.0.1 @@ -9041,10 +9028,6 @@ snapshots: obug@2.1.4: {} - oidc-client-ts@3.5.0: - dependencies: - jwt-decode: 4.0.0 - on-finished@2.4.1: dependencies: ee-first: 1.1.1