From 1d2467049db932900934cc84915718377225dfc5 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Mon, 17 Aug 2026 17:33:55 +0200 Subject: [PATCH] feat: fully backend-driven OIDC session flow (session cookie, not bearer token) Replace the hybrid flow (frontend PKCE + POST /auth/session token exchange, access token in sessionStorage) with a classic backend-driven BFF: the browser only ever navigates to GET /api/v1/auth/login and is redirected straight to the IdP; PKCE verifier/state live server-side in Redis (SessionStoreService); GET /api/v1/auth/callback (now the registered IdP redirect URI, replacing the frontend's /auth/callback route, which is deleted) verifies the id_token, JIT-provisions the user, creates a Redis-backed session, and sets one httpOnly SameSite=Lax cookie before redirecting into the app. No token material of any kind ever reaches the browser. OidcAuthGuard (per-request bearer JWT verification) is replaced by SessionAuthGuard (cookie -> Redis session lookup) across every controller that used it. cookie-parser is now wired into main.ts. Frontend AuthService shrinks to login()/logout()/ensureSessionChecked(); pkce.ts, auth.interceptor.ts, and the callback component/route are all removed as dead code under this model. New required env var: APP_BASE_URL (source of truth for the OIDC redirect_uri and the post-login redirect target). Verified end-to-end against the real API, Redis, and a mocked IdP: login redirect shape, callback cookie + redirect, state-replay rejection, /users/me 401<->200 around the cookie, and logout. --- .env.example | 4 + README.md | 23 ++- .../src/auth/auth-login.controller.spec.ts | 90 +++++++++++ .../api/src/auth/auth-login.controller.ts | 64 ++++++++ .../src/auth/auth-session.controller.spec.ts | 23 --- .../api/src/auth/auth-session.controller.ts | 18 --- backend/apps/api/src/auth/auth.module.ts | 4 +- .../api/src/auth/current-user.decorator.ts | 8 +- backend/apps/api/src/main.ts | 2 + .../src/trips/travelers.controller.spec.ts | 4 +- .../api/src/trips/travelers.controller.ts | 8 +- .../trips/trip-invitations.controller.spec.ts | 4 +- .../src/trips/trip-invitations.controller.ts | 16 +- .../api/src/trips/trip-members.controller.ts | 4 +- ...ip-preference-overrides.controller.spec.ts | 4 +- .../trip-preference-overrides.controller.ts | 8 +- .../api/src/trips/trips.controller.spec.ts | 4 +- .../apps/api/src/trips/trips.controller.ts | 10 +- .../api/src/users/users.controller.spec.ts | 4 +- .../apps/api/src/users/users.controller.ts | 12 +- .../libs/auth/src/auth-flow.service.spec.ts | 148 ++++++++++++++++++ backend/libs/auth/src/auth-flow.service.ts | 106 +++++++++++++ backend/libs/auth/src/auth.module.ts | 19 ++- backend/libs/auth/src/index.ts | 4 +- backend/libs/auth/src/oidc-auth.guard.spec.ts | 123 --------------- backend/libs/auth/src/oidc-auth.guard.ts | 66 -------- .../auth/src/oidc-discovery.service.spec.ts | 12 +- .../libs/auth/src/oidc-discovery.service.ts | 10 ++ .../libs/auth/src}/pkce.spec.ts | 13 +- backend/libs/auth/src/pkce.ts | 9 ++ .../libs/auth/src/session-auth.guard.spec.ts | 56 +++++++ backend/libs/auth/src/session-auth.guard.ts | 33 ++++ .../auth/src/session-store.service.spec.ts | 76 +++++++++ .../libs/auth/src/session-store.service.ts | 64 ++++++++ .../auth/src/token-exchange.service.spec.ts | 20 ++- .../libs/auth/src/token-exchange.service.ts | 2 + .../configuration/src/environment.spec.ts | 2 + backend/libs/configuration/src/environment.ts | 2 + backend/package.json | 2 + backend/test/setup-env.ts | 1 + compose.yml | 2 + ...-planner-phase-02-users-oidc-trips-plan.md | 22 +++ frontend/angular.json | 3 +- frontend/src/app/app.config.ts | 5 +- frontend/src/app/app.html | 3 + frontend/src/app/app.routes.ts | 2 - frontend/src/app/app.ts | 11 +- frontend/src/app/auth/auth.guard.spec.ts | 26 +++ frontend/src/app/auth/auth.guard.ts | 7 +- .../src/app/auth/auth.interceptor.spec.ts | 36 ----- frontend/src/app/auth/auth.interceptor.ts | 19 --- frontend/src/app/auth/auth.service.spec.ts | 87 ++++------ frontend/src/app/auth/auth.service.ts | 115 ++++---------- frontend/src/app/auth/callback/callback.html | 1 - .../src/app/auth/callback/callback.spec.ts | 27 ---- frontend/src/app/auth/callback/callback.ts | 17 -- frontend/src/app/auth/pkce.ts | 16 -- pnpm-lock.yaml | 29 ++++ 58 files changed, 936 insertions(+), 574 deletions(-) create mode 100644 backend/apps/api/src/auth/auth-login.controller.spec.ts create mode 100644 backend/apps/api/src/auth/auth-login.controller.ts delete mode 100644 backend/apps/api/src/auth/auth-session.controller.spec.ts delete mode 100644 backend/apps/api/src/auth/auth-session.controller.ts create mode 100644 backend/libs/auth/src/auth-flow.service.spec.ts create mode 100644 backend/libs/auth/src/auth-flow.service.ts delete mode 100644 backend/libs/auth/src/oidc-auth.guard.spec.ts delete mode 100644 backend/libs/auth/src/oidc-auth.guard.ts rename {frontend/src/app/auth => backend/libs/auth/src}/pkce.spec.ts (50%) create mode 100644 backend/libs/auth/src/pkce.ts create mode 100644 backend/libs/auth/src/session-auth.guard.spec.ts create mode 100644 backend/libs/auth/src/session-auth.guard.ts create mode 100644 backend/libs/auth/src/session-store.service.spec.ts create mode 100644 backend/libs/auth/src/session-store.service.ts create mode 100644 frontend/src/app/auth/auth.guard.spec.ts delete mode 100644 frontend/src/app/auth/auth.interceptor.spec.ts delete mode 100644 frontend/src/app/auth/auth.interceptor.ts delete mode 100644 frontend/src/app/auth/callback/callback.html delete mode 100644 frontend/src/app/auth/callback/callback.spec.ts delete mode 100644 frontend/src/app/auth/callback/callback.ts delete mode 100644 frontend/src/app/auth/pkce.ts diff --git a/.env.example b/.env.example index b530464..d4412bb 100644 --- a/.env.example +++ b/.env.example @@ -20,3 +20,7 @@ OIDC_CLIENT_ID=travel-planner-web # 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 +# APP_BASE_URL: the public origin end users load the app from (used to build the +# OIDC redirect_uri and the post-login redirect target). Must match a redirect +# URI registered with the IdP client, e.g. https://travel-planner.example.com +APP_BASE_URL=http://localhost:4200 diff --git a/README.md b/README.md index 382c1a7..716338a 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ OIDC_ISSUER=https://auth.forgecore.work OIDC_CLIENT_ID=client_a297fd8d9c1f47a79d3600ea0c96984 OIDC_AUDIENCE=client_a297fd8d9c1f47a79d3600ea0c96984 OIDC_CLIENT_SECRET= +APP_BASE_URL=http://localhost:4200 ``` Run pending migrations against the dev database (first time only), then start the API and frontend: @@ -43,9 +44,9 @@ 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`. +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/api/v1/auth/callback` — note this is a **backend** URL (proxied through the same origin), not the frontend's `/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. +This IdP client is **confidential** (it has a client secret), so the entire Authorization Code + PKCE flow — including the callback and token exchange — runs server-side (see "OIDC client type" below). The browser only ever sees an httpOnly session cookie, never an access token. ## Quality gates @@ -85,11 +86,16 @@ 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. 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). +- **OIDC client type & session model:** this deployment's IdP client is confidential (has a client secret), not a public/PKCE-only SPA client. A client secret must never be embedded in a browser bundle, so the **entire** Authorization Code + PKCE flow runs server-side, not just the token exchange: + - `GET /api/v1/auth/login` (`AuthLoginController`) generates the PKCE verifier/challenge and `state`, stores the verifier in Redis keyed by `state` (`SessionStoreService`, short TTL), and 302-redirects the browser straight to the IdP's `authorization_endpoint`. The frontend only ever navigates to this URL (`AuthService.login()`); it holds no PKCE state at all. + - The IdP redirects back to `GET /api/v1/auth/callback` (a **backend** URL, registered as the client's redirect URI) with `code`+`state`. The backend consumes the matching verifier from Redis (single-use — replaying a `state` returns 400), exchanges the code using `OIDC_CLIENT_SECRET` (`TokenExchangeService`), verifies the returned `id_token`'s signature via the IdP's JWKS, JIT-provisions the local `User` from its claims, and stores a server-side session in Redis (`SessionStoreService`, TTL = access-token lifetime). + - The callback sets **one** cookie — `travel_planner_session` (httpOnly, `SameSite=Lax`, `Secure` when `APP_BASE_URL` is https) — containing only an opaque session id, then redirects the browser into the app (`${APP_BASE_URL}/trips`). The browser never receives an access token, ID token, or refresh token; `refresh_token` is never even stored. + - Every subsequent request to a protected route is authenticated by `SessionAuthGuard`, which reads the cookie and looks up the session in Redis — no per-request JWT verification, no `Authorization` header, and (since the frontend and API share an origin via the edge/dev-proxy) no CORS configuration needed. + - `POST /api/v1/auth/logout` deletes the Redis session and clears the cookie. The frontend's `AuthService.ensureSessionChecked()` simply calls `GET /api/v1/users/me` on demand to ask "is there a valid session?" — it holds no token/session state of its own beyond a boolean signal. +- New required backend env vars: `OIDC_ISSUER`, `OIDC_AUDIENCE`, `OIDC_CLIENT_ID`, `OIDC_CLIENT_SECRET`, `APP_BASE_URL` (validated fail-fast like `DATABASE_URL`/`REDIS_URL`; `OIDC_CLIENT_SECRET` is a real secret, never committed). `APP_BASE_URL` is the public origin used to build the OIDC `redirect_uri` and the post-login redirect target — it must match a redirect URI registered with the IdP client. 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 — used only to know where to send the user, since the actual flow is backend-driven). - 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. +- New routes: `GET /api/v1/auth/login`, `GET /api/v1/auth/callback`, `POST /api/v1/auth/logout`, `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 `SessionAuthGuard` (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. - `Trip.version` optimistic locking: a stale `PATCH` (mismatched `version`) returns HTTP 409 and never silently overwrites; proven by both a mocked unit test and a real-database integration test. - Run backend integration tests (migration idempotency + optimistic-locking conflict) against `compose.dev.yml`: @@ -99,7 +105,10 @@ Production Docker Compose (`compose.yml`) publishes **exactly one** host port, o REDIS_URL=redis://localhost:6379 \ OIDC_ISSUER=https://idp.example.invalid/realms/travel-planner \ OIDC_AUDIENCE=travel-planner-api \ + OIDC_CLIENT_ID=test-client \ + OIDC_CLIENT_SECRET=test-secret \ + APP_BASE_URL=http://localhost:4200 \ pnpm --filter backend test:integration ``` -- Verified end-to-end against the real running API and a mocked IdP (local JWKS + discovery document): missing token → 401, valid token → `/users/me` succeeds and JIT-provisions the user, trip create/read, first `PATCH` with the correct version succeeds, a second `PATCH` reusing the stale version → 409, and a user with no `trip_members` row for the trip → 403. +- Verified end-to-end against the real running API, Redis, and a mocked IdP (local JWKS + discovery document): `/auth/login` redirects with a well-formed PKCE authorization URL, `/auth/callback` verifies the ID token, JIT-provisions the user, sets the httpOnly session cookie, and redirects into the app; replaying a consumed `state` is rejected (400); `/users/me` is 401 without the cookie and 200 with it; `/auth/logout` clears the session so `/users/me` returns 401 again. Trip flows verified: create/read, first `PATCH` with the correct version succeeds, a second `PATCH` reusing the stale version → 409, and a user with no `trip_members` row for the trip → 403. diff --git a/backend/apps/api/src/auth/auth-login.controller.spec.ts b/backend/apps/api/src/auth/auth-login.controller.spec.ts new file mode 100644 index 0000000..8644746 --- /dev/null +++ b/backend/apps/api/src/auth/auth-login.controller.spec.ts @@ -0,0 +1,90 @@ +import { AuthLoginController } from './auth-login.controller'; + +function fakeResponse() { + return { + redirect: jest.fn(), + cookie: jest.fn(), + clearCookie: jest.fn(), + status: jest.fn().mockReturnThis(), + send: jest.fn(), + }; +} + +describe('AuthLoginController', () => { + const environment = { appBaseUrl: 'http://localhost:4200' }; + + it('GET /auth/login redirects to the authorization URL built by AuthFlowService', async () => { + const authFlow = { + buildAuthorizationRedirect: jest + .fn() + .mockResolvedValue({ url: 'https://idp.example.test/oidc/auth?...' }), + }; + const sessionStore = { deleteSession: jest.fn() }; + const controller = new AuthLoginController( + environment as never, + authFlow as never, + sessionStore as never, + ); + const res = fakeResponse(); + + await controller.login(res as never); + + expect(res.redirect).toHaveBeenCalledWith( + 'https://idp.example.test/oidc/auth?...', + ); + }); + + it('GET /auth/callback sets an httpOnly session cookie and redirects into the app', async () => { + const authFlow = { + handleCallback: jest + .fn() + .mockResolvedValue({ sessionId: 'session-1', expiresIn: 3600 }), + }; + const sessionStore = { deleteSession: jest.fn() }; + const controller = new AuthLoginController( + environment as never, + authFlow as never, + sessionStore as never, + ); + const res = fakeResponse(); + + await controller.callback('code-1', 'state-1', res as never); + + expect(authFlow.handleCallback).toHaveBeenCalledWith('code-1', 'state-1'); + expect(res.cookie).toHaveBeenCalledWith( + 'travel_planner_session', + 'session-1', + expect.objectContaining({ + httpOnly: true, + sameSite: 'lax', + maxAge: 3600 * 1000, + }), + ); + expect(res.redirect).toHaveBeenCalledWith('http://localhost:4200/trips'); + }); + + it('POST /auth/logout deletes the session and clears the cookie', async () => { + const authFlow = {}; + const sessionStore = { + deleteSession: jest.fn().mockResolvedValue(undefined), + }; + const controller = new AuthLoginController( + environment as never, + authFlow as never, + sessionStore as never, + ); + const res = fakeResponse(); + + await controller.logout( + { cookies: { travel_planner_session: 'session-1' } } as never, + res as never, + ); + + expect(sessionStore.deleteSession).toHaveBeenCalledWith('session-1'); + expect(res.clearCookie).toHaveBeenCalledWith( + 'travel_planner_session', + expect.objectContaining({ path: '/' }), + ); + expect(res.status).toHaveBeenCalledWith(204); + }); +}); diff --git a/backend/apps/api/src/auth/auth-login.controller.ts b/backend/apps/api/src/auth/auth-login.controller.ts new file mode 100644 index 0000000..369b14d --- /dev/null +++ b/backend/apps/api/src/auth/auth-login.controller.ts @@ -0,0 +1,64 @@ +import { + Controller, + Get, + HttpCode, + Inject, + Post, + Query, + Req, + Res, +} from '@nestjs/common'; +import type { Request, Response } from 'express'; +import { APP_ENVIRONMENT } from '../../../../libs/configuration/src'; +import type { AppEnvironment } from '../../../../libs/configuration/src'; +import { + AuthFlowService, + SessionStoreService, + SESSION_COOKIE_NAME, +} from '../../../../libs/auth/src'; + +@Controller('auth') +export class AuthLoginController { + constructor( + @Inject(APP_ENVIRONMENT) private readonly environment: AppEnvironment, + private readonly authFlow: AuthFlowService, + private readonly sessionStore: SessionStoreService, + ) {} + + @Get('login') + async login(@Res() res: Response): Promise { + const { url } = await this.authFlow.buildAuthorizationRedirect(); + res.redirect(url); + } + + @Get('callback') + async callback( + @Query('code') code: string, + @Query('state') state: string, + @Res() res: Response, + ): Promise { + const { sessionId, expiresIn } = await this.authFlow.handleCallback( + code, + state, + ); + res.cookie(SESSION_COOKIE_NAME, sessionId, { + httpOnly: true, + sameSite: 'lax', + secure: this.environment.appBaseUrl.startsWith('https://'), + maxAge: expiresIn * 1000, + path: '/', + }); + res.redirect(`${this.environment.appBaseUrl}/trips`); + } + + @Post('logout') + @HttpCode(204) + async logout(@Req() req: Request, @Res() res: Response): Promise { + const sessionId = (req.cookies as Record | undefined)?.[ + SESSION_COOKIE_NAME + ]; + if (sessionId) await this.sessionStore.deleteSession(sessionId); + res.clearCookie(SESSION_COOKIE_NAME, { path: '/' }); + res.status(204).send(); + } +} diff --git a/backend/apps/api/src/auth/auth-session.controller.spec.ts b/backend/apps/api/src/auth/auth-session.controller.spec.ts deleted file mode 100644 index 44964b1..0000000 --- a/backend/apps/api/src/auth/auth-session.controller.spec.ts +++ /dev/null @@ -1,23 +0,0 @@ -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 deleted file mode 100644 index deb6fb3..0000000 --- a/backend/apps/api/src/auth/auth-session.controller.ts +++ /dev/null @@ -1,18 +0,0 @@ -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 index c1d5fe8..69b4bc6 100644 --- a/backend/apps/api/src/auth/auth.module.ts +++ b/backend/apps/api/src/auth/auth.module.ts @@ -1,7 +1,7 @@ import { Module } from '@nestjs/common'; -import { AuthSessionController } from './auth-session.controller'; +import { AuthLoginController } from './auth-login.controller'; @Module({ - controllers: [AuthSessionController], + controllers: [AuthLoginController], }) export class AuthApiModule {} diff --git a/backend/apps/api/src/auth/current-user.decorator.ts b/backend/apps/api/src/auth/current-user.decorator.ts index 9a5b095..cab2587 100644 --- a/backend/apps/api/src/auth/current-user.decorator.ts +++ b/backend/apps/api/src/auth/current-user.decorator.ts @@ -1,11 +1,9 @@ import { createParamDecorator, ExecutionContext } from '@nestjs/common'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; export const CurrentUser = createParamDecorator( - (_data: unknown, ctx: ExecutionContext): AuthenticatedUser => { - const request = ctx - .switchToHttp() - .getRequest<{ user: AuthenticatedUser }>(); + (_data: unknown, ctx: ExecutionContext): SessionUser => { + const request = ctx.switchToHttp().getRequest<{ user: SessionUser }>(); return request.user; }, ); diff --git a/backend/apps/api/src/main.ts b/backend/apps/api/src/main.ts index e5a0b4b..a9e57bb 100644 --- a/backend/apps/api/src/main.ts +++ b/backend/apps/api/src/main.ts @@ -1,10 +1,12 @@ import { RequestMethod } from '@nestjs/common'; import { NestFactory } from '@nestjs/core'; +import cookieParser from 'cookie-parser'; import { ApiModule } from './api.module'; export async function bootstrapApi(): Promise { const app = await NestFactory.create(ApiModule); app.enableShutdownHooks(); + app.use(cookieParser()); app.setGlobalPrefix('api/v1', { exclude: [ { path: 'health/live', method: RequestMethod.GET }, diff --git a/backend/apps/api/src/trips/travelers.controller.spec.ts b/backend/apps/api/src/trips/travelers.controller.spec.ts index ba89ebf..b2d1088 100644 --- a/backend/apps/api/src/trips/travelers.controller.spec.ts +++ b/backend/apps/api/src/trips/travelers.controller.spec.ts @@ -1,8 +1,8 @@ import { TravelersController } from './travelers.controller'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; describe('TravelersController', () => { - const currentUser: AuthenticatedUser = { + const currentUser: SessionUser = { id: 'u1', externalSubjectId: 'sub-1', displayName: 'Alex', diff --git a/backend/apps/api/src/trips/travelers.controller.ts b/backend/apps/api/src/trips/travelers.controller.ts index 62e2b48..eb3e2e4 100644 --- a/backend/apps/api/src/trips/travelers.controller.ts +++ b/backend/apps/api/src/trips/travelers.controller.ts @@ -8,8 +8,8 @@ import { Post, UseGuards, } from '@nestjs/common'; -import { OidcAuthGuard } from '../../../../libs/auth/src'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import { SessionAuthGuard } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; import { TravelersService, TripMembershipGuard, @@ -22,7 +22,7 @@ import type { import { CurrentUser } from '../auth/current-user.decorator'; @Controller('trips/:tripId/travelers') -@UseGuards(OidcAuthGuard, TripMembershipGuard) +@UseGuards(SessionAuthGuard, TripMembershipGuard) export class TravelersController { constructor(private readonly travelersService: TravelersService) {} @@ -34,7 +34,7 @@ export class TravelersController { @Post() create( @Param('tripId') tripId: string, - @CurrentUser() currentUser: AuthenticatedUser, + @CurrentUser() currentUser: SessionUser, @Body() dto: CreateTravelerDto, ): Promise { return this.travelersService.createTraveler(tripId, dto, currentUser.id); diff --git a/backend/apps/api/src/trips/trip-invitations.controller.spec.ts b/backend/apps/api/src/trips/trip-invitations.controller.spec.ts index 5ff7bf2..a8e7442 100644 --- a/backend/apps/api/src/trips/trip-invitations.controller.spec.ts +++ b/backend/apps/api/src/trips/trip-invitations.controller.spec.ts @@ -1,8 +1,8 @@ import { TripInvitationsController } from './trip-invitations.controller'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; describe('TripInvitationsController', () => { - const currentUser: AuthenticatedUser = { + const currentUser: SessionUser = { id: 'owner-1', externalSubjectId: 'sub-1', displayName: 'Owner', diff --git a/backend/apps/api/src/trips/trip-invitations.controller.ts b/backend/apps/api/src/trips/trip-invitations.controller.ts index f286197..b07d551 100644 --- a/backend/apps/api/src/trips/trip-invitations.controller.ts +++ b/backend/apps/api/src/trips/trip-invitations.controller.ts @@ -7,8 +7,8 @@ import { Post, UseGuards, } from '@nestjs/common'; -import { OidcAuthGuard } from '../../../../libs/auth/src'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import { SessionAuthGuard } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; import { TripInvitationsService, TripMembershipGuard, @@ -26,11 +26,11 @@ export class TripInvitationsController { constructor(private readonly invitationsService: TripInvitationsService) {} @Post('trips/:tripId/invitations') - @UseGuards(OidcAuthGuard, TripMembershipGuard) + @UseGuards(SessionAuthGuard, TripMembershipGuard) @TripRoles('OWNER') create( @Param('tripId') tripId: string, - @CurrentUser() currentUser: AuthenticatedUser, + @CurrentUser() currentUser: SessionUser, @Body() dto: CreateTripInvitationDto, ): Promise<{ invitation: TripInvitation; rawToken: string }> { return this.invitationsService.createInvitation( @@ -41,14 +41,14 @@ export class TripInvitationsController { } @Get('trips/:tripId/invitations') - @UseGuards(OidcAuthGuard, TripMembershipGuard) + @UseGuards(SessionAuthGuard, TripMembershipGuard) @TripRoles('OWNER') list(@Param('tripId') tripId: string): Promise { return this.invitationsService.listInvitations(tripId); } @Delete('trips/:tripId/invitations/:invitationId') - @UseGuards(OidcAuthGuard, TripMembershipGuard) + @UseGuards(SessionAuthGuard, TripMembershipGuard) @TripRoles('OWNER') remove( @Param('tripId') tripId: string, @@ -58,10 +58,10 @@ export class TripInvitationsController { } @Post('invitations/:token/accept') - @UseGuards(OidcAuthGuard) + @UseGuards(SessionAuthGuard) accept( @Param('token') token: string, - @CurrentUser() currentUser: AuthenticatedUser, + @CurrentUser() currentUser: SessionUser, ): Promise { return this.invitationsService.acceptInvitation(token, currentUser.id); } diff --git a/backend/apps/api/src/trips/trip-members.controller.ts b/backend/apps/api/src/trips/trip-members.controller.ts index 7d3ebf5..4717acf 100644 --- a/backend/apps/api/src/trips/trip-members.controller.ts +++ b/backend/apps/api/src/trips/trip-members.controller.ts @@ -7,7 +7,7 @@ import { Patch, UseGuards, } from '@nestjs/common'; -import { OidcAuthGuard } from '../../../../libs/auth/src'; +import { SessionAuthGuard } from '../../../../libs/auth/src'; import { TripMembersService, TripMembershipGuard, @@ -25,7 +25,7 @@ interface UpdateTripMemberDto { } @Controller('trips/:tripId/members') -@UseGuards(OidcAuthGuard, TripMembershipGuard) +@UseGuards(SessionAuthGuard, TripMembershipGuard) export class TripMembersController { constructor(private readonly tripMembersService: TripMembersService) {} diff --git a/backend/apps/api/src/trips/trip-preference-overrides.controller.spec.ts b/backend/apps/api/src/trips/trip-preference-overrides.controller.spec.ts index 70e1447..8c080be 100644 --- a/backend/apps/api/src/trips/trip-preference-overrides.controller.spec.ts +++ b/backend/apps/api/src/trips/trip-preference-overrides.controller.spec.ts @@ -1,9 +1,9 @@ import { ForbiddenException } from '@nestjs/common'; import { TripPreferenceOverridesController } from './trip-preference-overrides.controller'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; describe('TripPreferenceOverridesController', () => { - const member: AuthenticatedUser = { + const member: SessionUser = { id: 'u1', externalSubjectId: 'sub-1', displayName: 'Alex', diff --git a/backend/apps/api/src/trips/trip-preference-overrides.controller.ts b/backend/apps/api/src/trips/trip-preference-overrides.controller.ts index 21af7c1..355e677 100644 --- a/backend/apps/api/src/trips/trip-preference-overrides.controller.ts +++ b/backend/apps/api/src/trips/trip-preference-overrides.controller.ts @@ -8,8 +8,8 @@ import { Put, UseGuards, } from '@nestjs/common'; -import { OidcAuthGuard } from '../../../../libs/auth/src'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import { SessionAuthGuard } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; import { TripMembershipGuard, TripPreferenceOverridesService, @@ -23,7 +23,7 @@ import { CurrentUser } from '../auth/current-user.decorator'; import { CurrentTripRole } from './current-trip-role.decorator'; @Controller('trips/:tripId/preference-overrides') -@UseGuards(OidcAuthGuard, TripMembershipGuard) +@UseGuards(SessionAuthGuard, TripMembershipGuard) export class TripPreferenceOverridesController { constructor(private readonly service: TripPreferenceOverridesService) {} @@ -35,7 +35,7 @@ export class TripPreferenceOverridesController { @Put() async upsert( @Param('tripId') tripId: string, - @CurrentUser() currentUser: AuthenticatedUser, + @CurrentUser() currentUser: SessionUser, @CurrentTripRole() role: TripMemberRole, @Body() dto: UpsertPreferenceOverrideDto, ): Promise { diff --git a/backend/apps/api/src/trips/trips.controller.spec.ts b/backend/apps/api/src/trips/trips.controller.spec.ts index c711813..4eb0a34 100644 --- a/backend/apps/api/src/trips/trips.controller.spec.ts +++ b/backend/apps/api/src/trips/trips.controller.spec.ts @@ -1,8 +1,8 @@ import { TripsController } from './trips.controller'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; describe('TripsController', () => { - const currentUser: AuthenticatedUser = { + const currentUser: SessionUser = { id: 'u1', externalSubjectId: 'sub-1', displayName: 'Alex', diff --git a/backend/apps/api/src/trips/trips.controller.ts b/backend/apps/api/src/trips/trips.controller.ts index b2ce469..297123e 100644 --- a/backend/apps/api/src/trips/trips.controller.ts +++ b/backend/apps/api/src/trips/trips.controller.ts @@ -9,8 +9,8 @@ import { Put, UseGuards, } from '@nestjs/common'; -import { OidcAuthGuard } from '../../../../libs/auth/src'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import { SessionAuthGuard } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; import { TripMembershipGuard, TripRoles, @@ -27,7 +27,7 @@ import type { import { CurrentUser } from '../auth/current-user.decorator'; @Controller('trips') -@UseGuards(OidcAuthGuard) +@UseGuards(SessionAuthGuard) export class TripsController { constructor( private readonly tripsService: TripsService, @@ -35,13 +35,13 @@ export class TripsController { ) {} @Get() - list(@CurrentUser() currentUser: AuthenticatedUser): Promise { + list(@CurrentUser() currentUser: SessionUser): Promise { return this.tripsService.listTripsForUser(currentUser.id); } @Post() create( - @CurrentUser() currentUser: AuthenticatedUser, + @CurrentUser() currentUser: SessionUser, @Body() dto: CreateTripDto, ): Promise { return this.tripsService.createTrip(currentUser.id, dto); diff --git a/backend/apps/api/src/users/users.controller.spec.ts b/backend/apps/api/src/users/users.controller.spec.ts index e836f81..1fd8247 100644 --- a/backend/apps/api/src/users/users.controller.spec.ts +++ b/backend/apps/api/src/users/users.controller.spec.ts @@ -1,8 +1,8 @@ import { UsersController } from './users.controller'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; describe('UsersController', () => { - const currentUser: AuthenticatedUser = { + const currentUser: SessionUser = { id: 'u1', externalSubjectId: 'sub-1', displayName: 'Alex', diff --git a/backend/apps/api/src/users/users.controller.ts b/backend/apps/api/src/users/users.controller.ts index 3bf3115..50dbda5 100644 --- a/backend/apps/api/src/users/users.controller.ts +++ b/backend/apps/api/src/users/users.controller.ts @@ -6,8 +6,8 @@ import { Put, UseGuards, } from '@nestjs/common'; -import { OidcAuthGuard } from '../../../../libs/auth/src'; -import type { AuthenticatedUser } from '../../../../libs/auth/src'; +import { SessionAuthGuard } from '../../../../libs/auth/src'; +import type { SessionUser } from '../../../../libs/auth/src'; import { UsersService, UserPreferencesService, @@ -27,7 +27,7 @@ interface UserProfileResponse { } @Controller('users/me') -@UseGuards(OidcAuthGuard) +@UseGuards(SessionAuthGuard) export class UsersController { constructor( private readonly usersService: UsersService, @@ -36,7 +36,7 @@ export class UsersController { @Get() async me( - @CurrentUser() currentUser: AuthenticatedUser, + @CurrentUser() currentUser: SessionUser, ): Promise { const user = await this.usersService.findById(currentUser.id); if (!user) throw new NotFoundException('User not found'); @@ -51,14 +51,14 @@ export class UsersController { @Get('preferences') getPreferences( - @CurrentUser() currentUser: AuthenticatedUser, + @CurrentUser() currentUser: SessionUser, ): Promise { return this.preferencesService.getOrDefault(currentUser.id); } @Put('preferences') updatePreferences( - @CurrentUser() currentUser: AuthenticatedUser, + @CurrentUser() currentUser: SessionUser, @Body() dto: UpdateUserPreferenceDto, ): Promise { return this.preferencesService.upsert(currentUser.id, dto); diff --git a/backend/libs/auth/src/auth-flow.service.spec.ts b/backend/libs/auth/src/auth-flow.service.spec.ts new file mode 100644 index 0000000..9c9bff5 --- /dev/null +++ b/backend/libs/auth/src/auth-flow.service.spec.ts @@ -0,0 +1,148 @@ +import { BadRequestException } from '@nestjs/common'; +import { generateKeyPair, exportJWK, SignJWT, createLocalJWKSet } from 'jose'; +import { AuthFlowService } from './auth-flow.service'; +import type { AppEnvironment } from '../../configuration/src'; + +describe('AuthFlowService', () => { + 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', + appBaseUrl: 'http://localhost:4200', + appVersion: 'dev', + teamCityBuildNumber: 'local', + sourceRevision: 'local', + }; + + function fakeDiscovery() { + return { + getAuthorizationEndpoint: jest + .fn() + .mockReturnValue('https://idp.example.test/oidc/auth'), + }; + } + + describe('buildAuthorizationRedirect', () => { + it('persists a login attempt and returns a well-formed authorization URL', async () => { + const sessionStore = { createLoginAttempt: jest.fn() }; + const service = new AuthFlowService( + environment, + fakeDiscovery() as never, + {} as never, + sessionStore as never, + {} as never, + ); + + const { url, state } = await service.buildAuthorizationRedirect(); + + expect(sessionStore.createLoginAttempt).toHaveBeenCalledWith( + state, + expect.any(String), + ); + const parsed = new URL(url); + expect(parsed.origin + parsed.pathname).toBe( + 'https://idp.example.test/oidc/auth', + ); + expect(parsed.searchParams.get('response_type')).toBe('code'); + expect(parsed.searchParams.get('client_id')).toBe('client-1'); + expect(parsed.searchParams.get('redirect_uri')).toBe( + 'http://localhost:4200/api/v1/auth/callback', + ); + expect(parsed.searchParams.get('code_challenge_method')).toBe('S256'); + expect(parsed.searchParams.get('state')).toBe(state); + }); + }); + + describe('handleCallback', () => { + it('rejects when the state does not match a stored login attempt', async () => { + const sessionStore = { + consumeLoginAttempt: jest.fn().mockResolvedValue(undefined), + }; + const service = new AuthFlowService( + environment, + fakeDiscovery() as never, + {} as never, + sessionStore as never, + {} as never, + ); + + await expect( + service.handleCallback('code-1', 'unknown-state'), + ).rejects.toThrow(BadRequestException); + }); + + it('verifies the id_token, jit-provisions the user, and creates a session', async () => { + const issuer = environment.oidcIssuer; + const { publicKey, privateKey } = await generateKeyPair('RS256'); + const jwk = (await exportJWK(publicKey)) as Record; + jwk.kid = 'flow-key'; + const jwks = createLocalJWKSet({ keys: [jwk as never] }); + + const idToken = await new SignJWT({ + sub: 'idp-sub-1', + email: 'a@example.com', + name: 'A', + }) + .setProtectedHeader({ alg: 'RS256', kid: 'flow-key' }) + .setIssuer(issuer) + .setAudience(environment.oidcClientId) + .setIssuedAt() + .setExpirationTime('5m') + .sign(privateKey); + + const sessionStore = { + consumeLoginAttempt: jest.fn().mockResolvedValue('verifier-1'), + createSession: jest.fn().mockResolvedValue('session-1'), + }; + const tokenExchange = { + exchangeAuthorizationCode: jest + .fn() + .mockResolvedValue({ accessToken: 'at-1', expiresIn: 3600, idToken }), + }; + const usersService = { + findOrCreateByExternalSubjectId: jest.fn().mockResolvedValue({ + id: 'local-1', + externalSubjectId: 'idp-sub-1', + displayName: 'A', + email: 'a@example.com', + }), + }; + const discovery = { + getAuthorizationEndpoint: jest.fn(), + getVerificationKeySet: jest.fn().mockReturnValue(jwks), + getIssuer: jest.fn().mockReturnValue(issuer), + }; + + const service = new AuthFlowService( + environment, + discovery as never, + tokenExchange as never, + sessionStore as never, + usersService as never, + ); + + const result = await service.handleCallback('code-1', 'state-1'); + + expect(usersService.findOrCreateByExternalSubjectId).toHaveBeenCalledWith( + 'idp-sub-1', + { + email: 'a@example.com', + displayName: 'A', + }, + ); + expect(sessionStore.createSession).toHaveBeenCalledWith( + { + id: 'local-1', + externalSubjectId: 'idp-sub-1', + displayName: 'A', + email: 'a@example.com', + }, + 3600, + ); + expect(result).toEqual({ sessionId: 'session-1', expiresIn: 3600 }); + }); + }); +}); diff --git a/backend/libs/auth/src/auth-flow.service.ts b/backend/libs/auth/src/auth-flow.service.ts new file mode 100644 index 0000000..26e302e --- /dev/null +++ b/backend/libs/auth/src/auth-flow.service.ts @@ -0,0 +1,106 @@ +import { + BadRequestException, + Inject, + Injectable, + UnauthorizedException, +} from '@nestjs/common'; +import { jwtVerify } from 'jose'; +import { APP_ENVIRONMENT } from '../../configuration/src'; +import type { AppEnvironment } from '../../configuration/src'; +import { UsersService } from '../../users/src'; +import { OidcDiscoveryService } from './oidc-discovery.service'; +import { TokenExchangeService } from './token-exchange.service'; +import { SessionStoreService } from './session-store.service'; +import type { SessionUser } from './session-store.service'; +import { generateCodeChallenge, generateRandomString } from './pkce'; + +export interface AuthorizationRedirect { + url: string; + state: string; +} + +export interface CallbackResult { + sessionId: string; + expiresIn: number; +} + +const SCOPE = 'openid profile email'; + +@Injectable() +export class AuthFlowService { + constructor( + @Inject(APP_ENVIRONMENT) private readonly environment: AppEnvironment, + private readonly discovery: OidcDiscoveryService, + private readonly tokenExchange: TokenExchangeService, + private readonly sessionStore: SessionStoreService, + private readonly usersService: UsersService, + ) {} + + private getRedirectUri(): string { + return `${this.environment.appBaseUrl}/api/v1/auth/callback`; + } + + async buildAuthorizationRedirect(): Promise { + const codeVerifier = generateRandomString(); + const state = generateRandomString(); + const codeChallenge = generateCodeChallenge(codeVerifier); + + await this.sessionStore.createLoginAttempt(state, codeVerifier); + + const url = new URL(this.discovery.getAuthorizationEndpoint()); + url.searchParams.set('response_type', 'code'); + url.searchParams.set('client_id', this.environment.oidcClientId); + url.searchParams.set('redirect_uri', this.getRedirectUri()); + url.searchParams.set('scope', SCOPE); + url.searchParams.set('state', state); + url.searchParams.set('code_challenge', codeChallenge); + url.searchParams.set('code_challenge_method', 'S256'); + + return { url: url.toString(), state }; + } + + async handleCallback(code: string, state: string): Promise { + const codeVerifier = await this.sessionStore.consumeLoginAttempt(state); + if (!codeVerifier) { + throw new BadRequestException('Invalid or expired login attempt'); + } + + const tokenResult = await this.tokenExchange.exchangeAuthorizationCode({ + code, + codeVerifier, + redirectUri: this.getRedirectUri(), + }); + + const { payload } = await jwtVerify( + tokenResult.idToken, + this.discovery.getVerificationKeySet(), + { + issuer: this.discovery.getIssuer(), + audience: this.environment.oidcClientId, + }, + ).catch(() => { + throw new UnauthorizedException('Invalid ID token'); + }); + + const sub = payload.sub; + if (!sub) throw new UnauthorizedException('ID token has no subject claim'); + + const user = await this.usersService.findOrCreateByExternalSubjectId(sub, { + email: (payload.email as string) ?? '', + displayName: (payload.name as string) ?? (payload.email as string) ?? sub, + }); + + const sessionUser: SessionUser = { + id: user.id, + externalSubjectId: user.externalSubjectId, + displayName: user.displayName, + email: user.email, + }; + const sessionId = await this.sessionStore.createSession( + sessionUser, + tokenResult.expiresIn, + ); + + return { sessionId, expiresIn: tokenResult.expiresIn }; + } +} diff --git a/backend/libs/auth/src/auth.module.ts b/backend/libs/auth/src/auth.module.ts index 72b7386..c355a8a 100644 --- a/backend/libs/auth/src/auth.module.ts +++ b/backend/libs/auth/src/auth.module.ts @@ -1,17 +1,28 @@ import { Global, Module } from '@nestjs/common'; +import { RedisModule } from '../../infrastructure/src'; import { UsersLibModule } from '../../users/src'; import { OidcDiscoveryService } from './oidc-discovery.service'; -import { OidcAuthGuard } from './oidc-auth.guard'; import { TokenExchangeService } from './token-exchange.service'; +import { SessionStoreService } from './session-store.service'; +import { SessionAuthGuard } from './session-auth.guard'; +import { AuthFlowService } from './auth-flow.service'; @Global() @Module({ - imports: [UsersLibModule], - providers: [OidcDiscoveryService, OidcAuthGuard, TokenExchangeService], + imports: [RedisModule, UsersLibModule], + providers: [ + OidcDiscoveryService, + TokenExchangeService, + SessionStoreService, + SessionAuthGuard, + AuthFlowService, + ], exports: [ OidcDiscoveryService, - OidcAuthGuard, TokenExchangeService, + SessionStoreService, + SessionAuthGuard, + AuthFlowService, UsersLibModule, ], }) diff --git a/backend/libs/auth/src/index.ts b/backend/libs/auth/src/index.ts index 7165225..9a828e9 100644 --- a/backend/libs/auth/src/index.ts +++ b/backend/libs/auth/src/index.ts @@ -1,4 +1,6 @@ export * from './oidc-discovery.service'; -export * from './oidc-auth.guard'; export * from './token-exchange.service'; +export * from './session-store.service'; +export * from './session-auth.guard'; +export * from './auth-flow.service'; export * from './auth.module'; diff --git a/backend/libs/auth/src/oidc-auth.guard.spec.ts b/backend/libs/auth/src/oidc-auth.guard.spec.ts deleted file mode 100644 index f58dc5d..0000000 --- a/backend/libs/auth/src/oidc-auth.guard.spec.ts +++ /dev/null @@ -1,123 +0,0 @@ -import { generateKeyPair, exportJWK, SignJWT, createLocalJWKSet } from 'jose'; -import type { KeyLike } from 'jose'; -import { ExecutionContext, UnauthorizedException } from '@nestjs/common'; -import { OidcAuthGuard } from './oidc-auth.guard'; - -const issuer = 'https://idp.example.test/'; -const audience = 'travel-planner-api'; - -function contextWithHeader(authorization?: string): ExecutionContext { - const req: Record = authorization - ? { headers: { authorization } } - : { headers: {} }; - return { - switchToHttp: () => ({ getRequest: () => req }), - } as unknown as ExecutionContext; -} - -describe('OidcAuthGuard', () => { - let privateKey: KeyLike; - let discovery: { - getVerificationKeySet: jest.Mock; - getIssuer: jest.Mock; - getAudience: jest.Mock; - }; - let usersService: { findOrCreateByExternalSubjectId: jest.Mock }; - - beforeAll(async () => { - const { publicKey, privateKey: pk } = await generateKeyPair('RS256'); - privateKey = pk; - const jwk = await exportJWK(publicKey); - (jwk as Record).kid = 'test-key'; - const jwks = createLocalJWKSet({ keys: [jwk as never] }); - discovery = { - getVerificationKeySet: jest.fn().mockReturnValue(jwks), - getIssuer: jest.fn().mockReturnValue(issuer), - getAudience: jest.fn().mockReturnValue(audience), - }; - }); - - beforeEach(() => { - usersService = { - findOrCreateByExternalSubjectId: jest.fn().mockResolvedValue({ - id: 'local-1', - externalSubjectId: 'idp-sub-1', - displayName: 'A', - email: 'a@example.com', - }), - }; - }); - - async function sign(claims: Record, expires = '5m') { - return new SignJWT(claims) - .setProtectedHeader({ alg: 'RS256', kid: 'test-key' }) - .setIssuer(issuer) - .setAudience(audience) - .setIssuedAt() - .setExpirationTime(expires) - .sign(privateKey); - } - - it('rejects a request with no Authorization header', async () => { - const guard = new OidcAuthGuard(discovery as never, usersService as never); - await expect(guard.canActivate(contextWithHeader())).rejects.toThrow( - UnauthorizedException, - ); - }); - - it('rejects an expired token', async () => { - const token = await sign( - { sub: 'idp-sub-1', email: 'a@example.com', name: 'A' }, - '-10s', - ); - const guard = new OidcAuthGuard(discovery as never, usersService as never); - await expect( - guard.canActivate(contextWithHeader(`Bearer ${token}`)), - ).rejects.toThrow(UnauthorizedException); - }); - - it('rejects a token issued for a different audience', async () => { - const token = await new SignJWT({ sub: 'idp-sub-1' }) - .setProtectedHeader({ alg: 'RS256', kid: 'test-key' }) - .setIssuer(issuer) - .setAudience('some-other-api') - .setIssuedAt() - .setExpirationTime('5m') - .sign(privateKey); - const guard = new OidcAuthGuard(discovery as never, usersService as never); - await expect( - guard.canActivate(contextWithHeader(`Bearer ${token}`)), - ).rejects.toThrow(UnauthorizedException); - }); - - it('provisions the local user and attaches req.user on a valid token', async () => { - const token = await sign({ - sub: 'idp-sub-1', - email: 'a@example.com', - name: 'A', - }); - const req: Record = { - headers: { authorization: `Bearer ${token}` }, - }; - const context = { - switchToHttp: () => ({ getRequest: () => req }), - } as unknown as ExecutionContext; - - const guard = new OidcAuthGuard(discovery as never, usersService as never); - await expect(guard.canActivate(context)).resolves.toBe(true); - - expect(usersService.findOrCreateByExternalSubjectId).toHaveBeenCalledWith( - 'idp-sub-1', - { - email: 'a@example.com', - displayName: 'A', - }, - ); - expect(req.user).toEqual({ - id: 'local-1', - externalSubjectId: 'idp-sub-1', - displayName: 'A', - email: 'a@example.com', - }); - }); -}); diff --git a/backend/libs/auth/src/oidc-auth.guard.ts b/backend/libs/auth/src/oidc-auth.guard.ts deleted file mode 100644 index afd966a..0000000 --- a/backend/libs/auth/src/oidc-auth.guard.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { - CanActivate, - ExecutionContext, - Injectable, - UnauthorizedException, -} from '@nestjs/common'; -import { jwtVerify } from 'jose'; -import type { JWTPayload } from 'jose'; -import { UsersService } from '../../users/src'; -import { OidcDiscoveryService } from './oidc-discovery.service'; - -export interface AuthenticatedUser { - id: string; - externalSubjectId: string; - displayName: string; - email: string; -} - -@Injectable() -export class OidcAuthGuard implements CanActivate { - constructor( - private readonly discovery: OidcDiscoveryService, - private readonly users: UsersService, - ) {} - - async canActivate(context: ExecutionContext): Promise { - const request = context.switchToHttp().getRequest<{ - headers: Record; - user?: AuthenticatedUser; - }>(); - const header = request.headers?.authorization; - const token = header?.startsWith('Bearer ') ? header.slice(7) : undefined; - if (!token) throw new UnauthorizedException('Missing bearer token'); - - let payload: JWTPayload; - try { - const result = await jwtVerify( - token, - this.discovery.getVerificationKeySet(), - { - issuer: this.discovery.getIssuer(), - audience: this.discovery.getAudience(), - }, - ); - payload = result.payload; - } catch { - throw new UnauthorizedException('Invalid or expired token'); - } - - const sub = payload.sub; - if (!sub) throw new UnauthorizedException('Token has no subject claim'); - - const user = await this.users.findOrCreateByExternalSubjectId(sub, { - email: (payload.email as string) ?? '', - displayName: (payload.name as string) ?? (payload.email as string) ?? sub, - }); - - request.user = { - id: user.id, - externalSubjectId: user.externalSubjectId, - displayName: user.displayName, - email: user.email, - }; - return true; - } -} diff --git a/backend/libs/auth/src/oidc-discovery.service.spec.ts b/backend/libs/auth/src/oidc-discovery.service.spec.ts index 64ef8d0..e80f90a 100644 --- a/backend/libs/auth/src/oidc-discovery.service.spec.ts +++ b/backend/libs/auth/src/oidc-discovery.service.spec.ts @@ -9,18 +9,20 @@ describe('OidcDiscoveryService', () => { oidcAudience: 'client-1', oidcClientId: 'client-1', oidcClientSecret: 'secret-1', + appBaseUrl: 'http://localhost:4200', appVersion: 'dev', teamCityBuildNumber: 'local', sourceRevision: 'local', }; - it('fetches the discovery document once and exposes the token endpoint', async () => { + it('fetches the discovery document once and exposes its endpoints', 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', + authorization_endpoint: 'https://idp.example.test/oidc/auth', }), }); (globalThis as { fetch: typeof fetch }).fetch = fetchMock as never; @@ -34,14 +36,20 @@ describe('OidcDiscoveryService', () => { expect(service.getTokenEndpoint()).toBe( 'https://idp.example.test/oidc/token', ); + expect(service.getAuthorizationEndpoint()).toBe( + 'https://idp.example.test/oidc/auth', + ); 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', () => { + it('throws when asked for an endpoint before discovery has completed', () => { const service = new OidcDiscoveryService(environment); expect(() => service.getTokenEndpoint()).toThrow( 'OIDC discovery has not completed yet', ); + expect(() => service.getAuthorizationEndpoint()).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 bbc4aca..2bc2337 100644 --- a/backend/libs/auth/src/oidc-discovery.service.ts +++ b/backend/libs/auth/src/oidc-discovery.service.ts @@ -7,12 +7,14 @@ import type { AppEnvironment } from '../../configuration/src'; interface OidcDiscoveryDocument { jwks_uri: string; token_endpoint: string; + authorization_endpoint: string; } @Injectable() export class OidcDiscoveryService implements OnModuleInit { private verificationKeySet: JWTVerifyGetKey | undefined; private tokenEndpoint: string | undefined; + private authorizationEndpoint: string | undefined; constructor( @Inject(APP_ENVIRONMENT) private readonly environment: AppEnvironment, @@ -29,6 +31,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; + this.authorizationEndpoint = document.authorization_endpoint; } getIssuer(): string { @@ -52,4 +55,11 @@ export class OidcDiscoveryService implements OnModuleInit { } return this.tokenEndpoint; } + + getAuthorizationEndpoint(): string { + if (!this.authorizationEndpoint) { + throw new Error('OIDC discovery has not completed yet'); + } + return this.authorizationEndpoint; + } } diff --git a/frontend/src/app/auth/pkce.spec.ts b/backend/libs/auth/src/pkce.spec.ts similarity index 50% rename from frontend/src/app/auth/pkce.spec.ts rename to backend/libs/auth/src/pkce.spec.ts index 856aa24..d4b7b22 100644 --- a/frontend/src/app/auth/pkce.spec.ts +++ b/backend/libs/auth/src/pkce.spec.ts @@ -1,15 +1,14 @@ -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 +describe('pkce (backend)', () => { + it('computes the RFC 7636 Appendix B S256 test vector', () => { const codeVerifier = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'; - const challenge = await generateCodeChallenge(codeVerifier); - expect(challenge).toBe('E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM'); + expect(generateCodeChallenge(codeVerifier)).toBe( + 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM', + ); }); - it('generates a URL-safe random string of the requested length family', () => { + it('generates a URL-safe random string', () => { const value = generateRandomString(); expect(value).toMatch(/^[A-Za-z0-9_-]+$/); expect(value.length).toBeGreaterThanOrEqual(43); diff --git a/backend/libs/auth/src/pkce.ts b/backend/libs/auth/src/pkce.ts new file mode 100644 index 0000000..b6a5954 --- /dev/null +++ b/backend/libs/auth/src/pkce.ts @@ -0,0 +1,9 @@ +import { createHash, randomBytes } from 'node:crypto'; + +export function generateRandomString(byteLength = 32): string { + return randomBytes(byteLength).toString('base64url'); +} + +export function generateCodeChallenge(codeVerifier: string): string { + return createHash('sha256').update(codeVerifier).digest('base64url'); +} diff --git a/backend/libs/auth/src/session-auth.guard.spec.ts b/backend/libs/auth/src/session-auth.guard.spec.ts new file mode 100644 index 0000000..7faa7bd --- /dev/null +++ b/backend/libs/auth/src/session-auth.guard.spec.ts @@ -0,0 +1,56 @@ +import { ExecutionContext, UnauthorizedException } from '@nestjs/common'; +import { SessionAuthGuard } from './session-auth.guard'; + +describe('SessionAuthGuard', () => { + function contextWithCookies( + cookies?: Record, + ): ExecutionContext { + const req: Record = { cookies }; + return { + switchToHttp: () => ({ getRequest: () => req }), + } as unknown as ExecutionContext; + } + + it('rejects a request with no session cookie', async () => { + const sessionStore = { getSession: jest.fn() }; + const guard = new SessionAuthGuard(sessionStore as never); + + await expect(guard.canActivate(contextWithCookies())).rejects.toThrow( + UnauthorizedException, + ); + expect(sessionStore.getSession).not.toHaveBeenCalled(); + }); + + it('rejects a session cookie that does not match a stored session', async () => { + const sessionStore = { getSession: jest.fn().mockResolvedValue(undefined) }; + const guard = new SessionAuthGuard(sessionStore as never); + + await expect( + guard.canActivate( + contextWithCookies({ travel_planner_session: 'unknown-session' }), + ), + ).rejects.toThrow(UnauthorizedException); + }); + + it('attaches the stored session user to the request on a valid session', async () => { + const user = { + id: 'u1', + externalSubjectId: 'sub-1', + displayName: 'Alex', + email: 'a@example.com', + }; + const sessionStore = { getSession: jest.fn().mockResolvedValue(user) }; + const guard = new SessionAuthGuard(sessionStore as never); + + const req: Record = { + cookies: { travel_planner_session: 'session-1' }, + }; + const context = { + switchToHttp: () => ({ getRequest: () => req }), + } as unknown as ExecutionContext; + + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(sessionStore.getSession).toHaveBeenCalledWith('session-1'); + expect(req.user).toEqual(user); + }); +}); diff --git a/backend/libs/auth/src/session-auth.guard.ts b/backend/libs/auth/src/session-auth.guard.ts new file mode 100644 index 0000000..4c299bc --- /dev/null +++ b/backend/libs/auth/src/session-auth.guard.ts @@ -0,0 +1,33 @@ +import { + CanActivate, + ExecutionContext, + Injectable, + UnauthorizedException, +} from '@nestjs/common'; +import { + SessionStoreService, + SESSION_COOKIE_NAME, +} from './session-store.service'; +import type { SessionUser } from './session-store.service'; + +interface RequestWithSession { + cookies?: Record; + user?: SessionUser; +} + +@Injectable() +export class SessionAuthGuard implements CanActivate { + constructor(private readonly sessionStore: SessionStoreService) {} + + async canActivate(context: ExecutionContext): Promise { + const request = context.switchToHttp().getRequest(); + const sessionId = request.cookies?.[SESSION_COOKIE_NAME]; + if (!sessionId) throw new UnauthorizedException('Missing session cookie'); + + const user = await this.sessionStore.getSession(sessionId); + if (!user) throw new UnauthorizedException('Session expired or invalid'); + + request.user = user; + return true; + } +} diff --git a/backend/libs/auth/src/session-store.service.spec.ts b/backend/libs/auth/src/session-store.service.spec.ts new file mode 100644 index 0000000..1aa5135 --- /dev/null +++ b/backend/libs/auth/src/session-store.service.spec.ts @@ -0,0 +1,76 @@ +import { SessionStoreService } from './session-store.service'; + +function fakeRedis() { + const store = new Map(); + return { + store, + set: jest.fn((key: string, value: string) => { + store.set(key, value); + return Promise.resolve('OK'); + }), + get: jest.fn((key: string) => Promise.resolve(store.get(key) ?? null)), + del: jest.fn((key: string) => { + const existed = store.delete(key); + return Promise.resolve(existed ? 1 : 0); + }), + }; +} + +describe('SessionStoreService', () => { + describe('login attempts', () => { + it('stores and consumes a code verifier for a given state exactly once', async () => { + const redis = fakeRedis(); + const service = new SessionStoreService(redis as never); + + await service.createLoginAttempt('state-1', 'verifier-1'); + + await expect(service.consumeLoginAttempt('state-1')).resolves.toBe( + 'verifier-1', + ); + await expect( + service.consumeLoginAttempt('state-1'), + ).resolves.toBeUndefined(); + }); + + it('returns undefined for an unknown state', async () => { + const redis = fakeRedis(); + const service = new SessionStoreService(redis as never); + + await expect( + service.consumeLoginAttempt('never-seen'), + ).resolves.toBeUndefined(); + }); + }); + + describe('sessions', () => { + const user = { + id: 'u1', + externalSubjectId: 'sub-1', + displayName: 'Alex', + email: 'a@example.com', + }; + + it('creates a session and retrieves the stored user by session id', async () => { + const redis = fakeRedis(); + const service = new SessionStoreService(redis as never); + + const sessionId = await service.createSession(user, 3600); + expect(sessionId).toEqual(expect.any(String)); + + await expect(service.getSession(sessionId)).resolves.toEqual(user); + }); + + it('returns undefined for an unknown or deleted session', async () => { + const redis = fakeRedis(); + const service = new SessionStoreService(redis as never); + + const sessionId = await service.createSession(user, 3600); + await service.deleteSession(sessionId); + + await expect(service.getSession(sessionId)).resolves.toBeUndefined(); + await expect( + service.getSession('does-not-exist'), + ).resolves.toBeUndefined(); + }); + }); +}); diff --git a/backend/libs/auth/src/session-store.service.ts b/backend/libs/auth/src/session-store.service.ts new file mode 100644 index 0000000..fd702b4 --- /dev/null +++ b/backend/libs/auth/src/session-store.service.ts @@ -0,0 +1,64 @@ +import { randomBytes } from 'node:crypto'; +import { Inject, Injectable } from '@nestjs/common'; +import type Redis from 'ioredis'; +import { REDIS_CLIENT } from '../../infrastructure/src'; + +export interface SessionUser { + id: string; + externalSubjectId: string; + displayName: string; + email: string; +} + +export const SESSION_COOKIE_NAME = 'travel_planner_session'; + +const LOGIN_ATTEMPT_TTL_SECONDS = 10 * 60; + +function loginAttemptKey(state: string): string { + return `oidc:login-attempt:${state}`; +} + +function sessionKey(sessionId: string): string { + return `session:${sessionId}`; +} + +@Injectable() +export class SessionStoreService { + constructor(@Inject(REDIS_CLIENT) private readonly redis: Redis) {} + + async createLoginAttempt(state: string, codeVerifier: string): Promise { + await this.redis.set( + loginAttemptKey(state), + codeVerifier, + 'EX', + LOGIN_ATTEMPT_TTL_SECONDS, + ); + } + + async consumeLoginAttempt(state: string): Promise { + const key = loginAttemptKey(state); + const codeVerifier = await this.redis.get(key); + if (codeVerifier) await this.redis.del(key); + return codeVerifier ?? undefined; + } + + async createSession(user: SessionUser, ttlSeconds: number): Promise { + const sessionId = randomBytes(32).toString('base64url'); + await this.redis.set( + sessionKey(sessionId), + JSON.stringify(user), + 'EX', + ttlSeconds, + ); + return sessionId; + } + + async getSession(sessionId: string): Promise { + const raw = await this.redis.get(sessionKey(sessionId)); + return raw ? (JSON.parse(raw) as SessionUser) : undefined; + } + + async deleteSession(sessionId: string): Promise { + await this.redis.del(sessionKey(sessionId)); + } +} diff --git a/backend/libs/auth/src/token-exchange.service.spec.ts b/backend/libs/auth/src/token-exchange.service.spec.ts index cd11491..a6791b9 100644 --- a/backend/libs/auth/src/token-exchange.service.spec.ts +++ b/backend/libs/auth/src/token-exchange.service.spec.ts @@ -10,6 +10,7 @@ describe('TokenExchangeService.exchangeAuthorizationCode', () => { oidcAudience: 'client-1', oidcClientId: 'client-1', oidcClientSecret: 'super-secret', + appBaseUrl: 'http://localhost:4200', appVersion: 'dev', teamCityBuildNumber: 'local', sourceRevision: 'local', @@ -21,7 +22,7 @@ describe('TokenExchangeService.exchangeAuthorizationCode', () => { return { getTokenEndpoint: jest.fn().mockReturnValue(tokenEndpoint) }; } - it('posts a client-secret-authenticated request and returns only the safe fields', async () => { + it('posts a client-secret-authenticated request and returns the access + id token', async () => { const fetchMock = jest.fn().mockResolvedValue({ ok: true, json: () => @@ -41,10 +42,14 @@ describe('TokenExchangeService.exchangeAuthorizationCode', () => { const result = await service.exchangeAuthorizationCode({ code: 'auth-code-1', codeVerifier: 'verifier-1', - redirectUri: 'http://localhost:4200/auth/callback', + redirectUri: 'http://localhost:4200/api/v1/auth/callback', }); - expect(result).toEqual({ accessToken: 'at-1', expiresIn: 3600 }); + expect(result).toEqual({ + accessToken: 'at-1', + expiresIn: 3600, + idToken: 'idt-1', + }); const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; expect(url).toBe('https://idp.example.test/oidc/token'); @@ -55,11 +60,11 @@ describe('TokenExchangeService.exchangeAuthorizationCode', () => { 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', + 'http://localhost:4200/api/v1/auth/callback', ); }); - it('never leaks the refresh_token or id_token to the caller', async () => { + it('never exposes the refresh_token, which is not needed by this MVP (no silent refresh)', async () => { const fetchMock = jest.fn().mockResolvedValue({ ok: true, json: () => @@ -79,11 +84,10 @@ describe('TokenExchangeService.exchangeAuthorizationCode', () => { const result = await service.exchangeAuthorizationCode({ code: 'auth-code-1', codeVerifier: 'verifier-1', - redirectUri: 'http://localhost:4200/auth/callback', + redirectUri: 'http://localhost:4200/api/v1/auth/callback', }); expect(result).not.toHaveProperty('refreshToken'); - expect(result).not.toHaveProperty('idToken'); }); it('rejects with BadRequestException when the identity provider rejects the code', async () => { @@ -103,7 +107,7 @@ describe('TokenExchangeService.exchangeAuthorizationCode', () => { service.exchangeAuthorizationCode({ code: 'bad-code', codeVerifier: 'verifier-1', - redirectUri: 'http://localhost:4200/auth/callback', + redirectUri: 'http://localhost:4200/api/v1/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 index 6909519..64923ff 100644 --- a/backend/libs/auth/src/token-exchange.service.ts +++ b/backend/libs/auth/src/token-exchange.service.ts @@ -12,6 +12,7 @@ export interface AuthorizationCodeExchangeRequest { export interface AuthorizationCodeExchangeResult { accessToken: string; expiresIn: number; + idToken: string; } interface TokenEndpointResponse { @@ -63,6 +64,7 @@ export class TokenExchangeService { return { accessToken: tokenResponse.access_token, expiresIn: tokenResponse.expires_in, + idToken: tokenResponse.id_token ?? '', }; } } diff --git a/backend/libs/configuration/src/environment.spec.ts b/backend/libs/configuration/src/environment.spec.ts index 962e6c5..dffdeea 100644 --- a/backend/libs/configuration/src/environment.spec.ts +++ b/backend/libs/configuration/src/environment.spec.ts @@ -14,6 +14,7 @@ describe('loadEnvironment', () => { OIDC_AUDIENCE: 'travel-planner-api', OIDC_CLIENT_ID: 'test-client', OIDC_CLIENT_SECRET: 'test-secret', + APP_BASE_URL: 'http://localhost:4200/', APP_VERSION: '1.2.3', TEAMCITY_BUILD_NUMBER: '42', SOURCE_REVISION: 'abc123', @@ -25,6 +26,7 @@ describe('loadEnvironment', () => { oidcAudience: 'travel-planner-api', oidcClientId: 'test-client', oidcClientSecret: 'test-secret', + appBaseUrl: 'http://localhost:4200', 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 763d825..95fc429 100644 --- a/backend/libs/configuration/src/environment.ts +++ b/backend/libs/configuration/src/environment.ts @@ -5,6 +5,7 @@ export interface AppEnvironment { oidcAudience: string; oidcClientId: string; oidcClientSecret: string; + appBaseUrl: string; appVersion: string; teamCityBuildNumber: string; sourceRevision: string; @@ -24,6 +25,7 @@ export function loadEnvironment(env: NodeJS.ProcessEnv): AppEnvironment { oidcAudience: required(env, 'OIDC_AUDIENCE'), oidcClientId: required(env, 'OIDC_CLIENT_ID'), oidcClientSecret: required(env, 'OIDC_CLIENT_SECRET'), + appBaseUrl: required(env, 'APP_BASE_URL').replace(/\/$/, ''), appVersion: env.APP_VERSION?.trim() || 'dev', teamCityBuildNumber: env.TEAMCITY_BUILD_NUMBER?.trim() || 'local', sourceRevision: env.SOURCE_REVISION?.trim() || 'local', diff --git a/backend/package.json b/backend/package.json index 114e96f..9b12c19 100644 --- a/backend/package.json +++ b/backend/package.json @@ -28,6 +28,7 @@ "@nestjs/common": "^11.0.1", "@nestjs/core": "^11.0.1", "@nestjs/platform-express": "^11.0.1", + "cookie-parser": "^1.4.7", "ioredis": "^6.0.0", "jose": "^5.10.0", "kysely": "0.28.17", @@ -42,6 +43,7 @@ "@nestjs/cli": "^11.0.0", "@nestjs/schematics": "^11.0.0", "@nestjs/testing": "^11.0.1", + "@types/cookie-parser": "^1.4.10", "@types/express": "^5.0.0", "@types/jest": "^30.0.0", "@types/node": "^24.0.0", diff --git a/backend/test/setup-env.ts b/backend/test/setup-env.ts index 63c65d7..e339acd 100644 --- a/backend/test/setup-env.ts +++ b/backend/test/setup-env.ts @@ -4,3 +4,4 @@ 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'; +process.env.APP_BASE_URL ??= 'http://localhost:4200'; diff --git a/compose.yml b/compose.yml index e83d9ba..f9e59e3 100644 --- a/compose.yml +++ b/compose.yml @@ -32,6 +32,7 @@ services: OIDC_AUDIENCE: "${OIDC_AUDIENCE}" OIDC_CLIENT_ID: "${OIDC_CLIENT_ID}" OIDC_CLIENT_SECRET: "${OIDC_CLIENT_SECRET}" + APP_BASE_URL: "${APP_BASE_URL}" APP_VERSION: "${APP_VERSION:-dev}" TEAMCITY_BUILD_NUMBER: "${TEAMCITY_BUILD_NUMBER:-local}" SOURCE_REVISION: "${SOURCE_REVISION:-local}" @@ -61,6 +62,7 @@ services: OIDC_AUDIENCE: "${OIDC_AUDIENCE}" OIDC_CLIENT_ID: "${OIDC_CLIENT_ID}" OIDC_CLIENT_SECRET: "${OIDC_CLIENT_SECRET}" + APP_BASE_URL: "${APP_BASE_URL}" 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 e006191..db0fb6a 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 @@ -1340,3 +1340,25 @@ The plan's Task 10 assumed a public, PKCE-only SPA client and used `oidc-client- - 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. + +--- + +## Addendum 2: Full Backend-Driven Session Flow, Not Just the Token Exchange (2026-08-17) + +Shortly after Addendum 1 shipped, a review question ("why does OIDC discovery run in the frontend at all — shouldn't everything run in the backend?") led to a further, user-confirmed architecture change. Addendum 1 still had the frontend fetch IdP discovery itself, generate its own PKCE verifier/state, store the access token in `sessionStorage`, and POST the code+verifier to a `POST /api/v1/auth/session` endpoint. That is a workable "hybrid" BFF, but it (a) duplicated IdP knowledge between frontend and backend, and (b) still put the raw access token in JS-reachable browser storage, which is unnecessary exposure to XSS given the backend already brokers everything else. + +The flow shipped instead is a classic, fully backend-driven BFF: + +- `GET /api/v1/auth/login` (`AuthLoginController` → `AuthFlowService.buildAuthorizationRedirect`) generates the PKCE verifier/challenge and `state` **server-side**, persists `state → codeVerifier` in Redis (`SessionStoreService.createLoginAttempt`, 10-minute TTL), and issues an HTTP 302 straight to the IdP's `authorization_endpoint`. The frontend's `AuthService.login()` is now a one-line `window.location.href` navigation; it holds no PKCE state and never calls the IdP directly. +- The registered redirect URI is now the **backend's** `${APP_BASE_URL}/api/v1/auth/callback`, not a frontend route. `frontend/src/app/auth/callback/` (the Angular callback component) was deleted entirely — there is nothing for the frontend to do on callback, since the backend redirects straight into `/trips` once the session is established. A new required env var, `APP_BASE_URL`, is the single source of truth for both the redirect URI sent to the IdP and the post-login redirect target. +- `GET /api/v1/auth/callback` (`AuthFlowService.handleCallback`) consumes the one-time `state` (a replay returns 400), performs the token exchange (`TokenExchangeService`, unchanged from Addendum 1), verifies the response `id_token`'s signature/issuer/audience via `jose` against the IdP's JWKS, JIT-provisions the `User` from its claims, and creates a Redis-backed session (`SessionStoreService.createSession`, TTL = access-token lifetime) holding `{id, externalSubjectId, displayName, email}`. `TokenExchangeService.exchangeAuthorizationCode` now also returns `idToken` (previously deliberately omitted) since it's needed here — the guarantee that tokens never reach the browser is now structural (the callback response is a redirect with a Set-Cookie header, never a JSON body), not just a return-type convention. +- The response sets exactly one cookie, `travel_planner_session` (httpOnly, `SameSite=Lax`, `Secure` when `APP_BASE_URL` is `https://`), containing only an opaque session id — never a JWT or any token material. +- `OidcAuthGuard` (bearer-JWT verification per request) was deleted and replaced by `SessionAuthGuard`, which reads the cookie and looks up the session in Redis. Every controller that referenced `OidcAuthGuard`/`AuthenticatedUser` was updated to `SessionAuthGuard`/`SessionUser` (mechanical rename, same guard-composition pattern with `TripMembershipGuard`). +- `POST /api/v1/auth/logout` deletes the Redis session and clears the cookie. +- `main.ts` now registers Express's `cookie-parser` middleware globally (new dependency), since `SessionAuthGuard` reads `req.cookies`. +- Frontend `pkce.ts`, `auth.interceptor.ts` (Bearer-header attachment — no longer needed since cookies are attached automatically by the browser for same-origin requests), and the `oidc-client-ts`-free-but-still-manual `POST /api/v1/auth/session` call are all gone. `AuthService` is now ~40 lines: `login()` navigates, `logout()` POSTs and clears local state, `ensureSessionChecked()` asks `GET /api/v1/users/me` and caches the in-flight promise so route guards don't trigger duplicate checks. +- `provideHttpClient(withInterceptors([authInterceptor]))` reverted to plain `provideHttpClient()`. + +Net effect: the browser holds zero token material at any point — not in `sessionStorage`, not in a JS-readable cookie, not in memory beyond the lifetime of the login redirect itself. This closes the XSS-exfiltration surface that Addendum 1's `sessionStorage`-held access token still had, at the cost of session state now living in Redis (already a hard dependency of this app) and one more required env var (`APP_BASE_URL`). + +Verified end-to-end with the same style of mocked-IdP smoke test used in Task 13, extended to cover: login redirect shape (PKCE params present, targets the IdP), callback setting the httpOnly cookie and redirecting to `${APP_BASE_URL}/trips`, `state` replay rejection (400), `/users/me` 401→200 transition around the cookie, and logout returning `/users/me` to 401. diff --git a/frontend/angular.json b/frontend/angular.json index d6267a9..4f089f7 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -2,7 +2,8 @@ "$schema": "./node_modules/@angular/cli/lib/config/schema.json", "version": 1, "cli": { - "packageManager": "npm" + "packageManager": "npm", + "analytics": false }, "newProjectRoot": "projects", "projects": { diff --git a/frontend/src/app/app.config.ts b/frontend/src/app/app.config.ts index 3535203..502d052 100644 --- a/frontend/src/app/app.config.ts +++ b/frontend/src/app/app.config.ts @@ -1,16 +1,15 @@ import { ApplicationConfig, provideBrowserGlobalErrorListeners, isDevMode } from '@angular/core'; import { provideRouter } from '@angular/router'; -import { provideHttpClient, withInterceptors } from '@angular/common/http'; +import { provideHttpClient } from '@angular/common/http'; import { routes } from './app.routes'; import { provideServiceWorker } from '@angular/service-worker'; -import { authInterceptor } from './auth/auth.interceptor'; export const appConfig: ApplicationConfig = { providers: [ provideBrowserGlobalErrorListeners(), provideRouter(routes), - provideHttpClient(withInterceptors([authInterceptor])), + provideHttpClient(), provideServiceWorker('ngsw-worker.js', { enabled: !isDevMode(), registrationStrategy: 'registerWhenStable:30000', diff --git a/frontend/src/app/app.html b/frontend/src/app/app.html index 3da6811..4b8eb92 100644 --- a/frontend/src/app/app.html +++ b/frontend/src/app/app.html @@ -3,6 +3,9 @@

Reisen planen, gemeinsam entscheiden.

diff --git a/frontend/src/app/app.routes.ts b/frontend/src/app/app.routes.ts index 9c31733..dc1fb0e 100644 --- a/frontend/src/app/app.routes.ts +++ b/frontend/src/app/app.routes.ts @@ -1,9 +1,7 @@ import { Routes } from '@angular/router'; -import { Callback } from './auth/callback/callback'; import { authGuard } from './auth/auth.guard'; export const routes: Routes = [ - { path: 'auth/callback', component: Callback }, { path: 'trips', canActivate: [authGuard], diff --git a/frontend/src/app/app.ts b/frontend/src/app/app.ts index 597f18f..77f67c2 100644 --- a/frontend/src/app/app.ts +++ b/frontend/src/app/app.ts @@ -1,5 +1,6 @@ -import { Component } from '@angular/core'; +import { Component, inject } from '@angular/core'; import { RouterLink, RouterOutlet } from '@angular/router'; +import { AuthService } from './auth/auth.service'; @Component({ selector: 'app-root', @@ -7,4 +8,10 @@ import { RouterLink, RouterOutlet } from '@angular/router'; templateUrl: './app.html', styleUrl: './app.scss', }) -export class App {} +export class App { + protected readonly authService = inject(AuthService); + + logout(): void { + void this.authService.logout(); + } +} diff --git a/frontend/src/app/auth/auth.guard.spec.ts b/frontend/src/app/auth/auth.guard.spec.ts new file mode 100644 index 0000000..939213c --- /dev/null +++ b/frontend/src/app/auth/auth.guard.spec.ts @@ -0,0 +1,26 @@ +import { TestBed } from '@angular/core/testing'; +import { describe, expect, it, vi } from 'vitest'; +import { authGuard } from './auth.guard'; +import { AuthService } from './auth.service'; + +describe('authGuard', () => { + it('allows activation when a session already exists', async () => { + const authService = { ensureSessionChecked: vi.fn().mockResolvedValue(true), login: vi.fn() }; + TestBed.configureTestingModule({ providers: [{ provide: AuthService, useValue: authService }] }); + + const result = await TestBed.runInInjectionContext(() => authGuard({} as never, {} as never)); + + expect(result).toBe(true); + expect(authService.login).not.toHaveBeenCalled(); + }); + + it('triggers login and denies activation when no session exists', async () => { + const authService = { ensureSessionChecked: vi.fn().mockResolvedValue(false), login: vi.fn() }; + TestBed.configureTestingModule({ providers: [{ provide: AuthService, useValue: authService }] }); + + const result = await TestBed.runInInjectionContext(() => authGuard({} as never, {} as never)); + + expect(result).toBe(false); + expect(authService.login).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/app/auth/auth.guard.ts b/frontend/src/app/auth/auth.guard.ts index 33cf68c..4c248ea 100644 --- a/frontend/src/app/auth/auth.guard.ts +++ b/frontend/src/app/auth/auth.guard.ts @@ -2,11 +2,12 @@ import { inject } from '@angular/core'; import { CanActivateFn } from '@angular/router'; import { AuthService } from './auth.service'; -export const authGuard: CanActivateFn = () => { +export const authGuard: CanActivateFn = async () => { const authService = inject(AuthService); - if (authService.isAuthenticated()) { + const authenticated = await authService.ensureSessionChecked(); + if (authenticated) { return true; } - void authService.login(); + authService.login(); return false; }; diff --git a/frontend/src/app/auth/auth.interceptor.spec.ts b/frontend/src/app/auth/auth.interceptor.spec.ts deleted file mode 100644 index bf0a6e4..0000000 --- a/frontend/src/app/auth/auth.interceptor.spec.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { HttpHandlerFn, HttpRequest, HttpResponse } from '@angular/common/http'; -import { TestBed } from '@angular/core/testing'; -import { firstValueFrom, of } from 'rxjs'; -import { describe, expect, it, vi } from 'vitest'; -import { authInterceptor } from './auth.interceptor'; -import { AuthService } from './auth.service'; - -describe('authInterceptor', () => { - it('attaches a bearer token to API requests', async () => { - const authService = { getAccessToken: vi.fn().mockResolvedValue('token-123') }; - TestBed.configureTestingModule({ providers: [{ provide: AuthService, useValue: authService }] }); - - const req = new HttpRequest('GET', '/api/v1/trips'); - const next = vi.fn().mockReturnValue(of(new HttpResponse())) as unknown as HttpHandlerFn; - - await firstValueFrom(TestBed.runInInjectionContext(() => authInterceptor(req, next))); - - expect(next).toHaveBeenCalledTimes(1); - const forwarded = vi.mocked(next).mock.calls[0][0] as HttpRequest; - expect(forwarded.headers.get('Authorization')).toBe('Bearer token-123'); - }); - - it('does not attach a token to non-API requests', async () => { - const authService = { getAccessToken: vi.fn().mockResolvedValue('token-123') }; - TestBed.configureTestingModule({ providers: [{ provide: AuthService, useValue: authService }] }); - - const req = new HttpRequest('GET', 'https://example.com/unrelated'); - const next = vi.fn().mockReturnValue(of(new HttpResponse())) as unknown as HttpHandlerFn; - - await firstValueFrom(TestBed.runInInjectionContext(() => authInterceptor(req, next))); - - expect(authService.getAccessToken).not.toHaveBeenCalled(); - const forwarded = vi.mocked(next).mock.calls[0][0] as HttpRequest; - expect(forwarded.headers.get('Authorization')).toBeNull(); - }); -}); diff --git a/frontend/src/app/auth/auth.interceptor.ts b/frontend/src/app/auth/auth.interceptor.ts deleted file mode 100644 index 65290fc..0000000 --- a/frontend/src/app/auth/auth.interceptor.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { HttpHandlerFn, HttpRequest } from '@angular/common/http'; -import { inject } from '@angular/core'; -import { from, switchMap } from 'rxjs'; -import { environment } from '../../environments/environment'; -import { AuthService } from './auth.service'; - -export function authInterceptor(req: HttpRequest, next: HttpHandlerFn) { - if (!req.url.startsWith(environment.apiBaseUrl)) { - return next(req); - } - - const authService = inject(AuthService); - return from(authService.getAccessToken()).pipe( - switchMap((token) => { - const authorizedReq = token ? req.clone({ setHeaders: { Authorization: `Bearer ${token}` } }) : req; - return next(authorizedReq); - }), - ); -} diff --git a/frontend/src/app/auth/auth.service.spec.ts b/frontend/src/app/auth/auth.service.spec.ts index a7e0459..2c31cb1 100644 --- a/frontend/src/app/auth/auth.service.spec.ts +++ b/frontend/src/app/auth/auth.service.spec.ts @@ -1,87 +1,62 @@ import { TestBed } from '@angular/core/testing'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { AuthService } from './auth.service'; describe('AuthService', () => { - beforeEach(() => { - sessionStorage.clear(); - }); - afterEach(() => { vi.unstubAllGlobals(); }); - it('starts unauthenticated when no session is stored', () => { + it('starts with an unknown authentication state until checked', () => { const service = TestBed.inject(AuthService); - expect(service.isAuthenticated()).toBe(false); + expect(service.isAuthenticated()).toBeUndefined(); }); - 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); + it('login() navigates to the backend login endpoint', () => { vi.stubGlobal('location', { ...window.location, href: '' }); const service = TestBed.inject(AuthService); - await service.login(); + 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'); + expect(window.location.href).toBe('/api/v1/auth/login'); }); - 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 }), - }); + it('ensureSessionChecked() reports authenticated when /users/me succeeds, and only fetches once', async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: true }); vi.stubGlobal('fetch', fetchMock); const service = TestBed.inject(AuthService); - await service.completeLogin(); + await expect(service.ensureSessionChecked()).resolves.toBe(true); + await expect(service.ensureSessionChecked()).resolves.toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledWith('/api/v1/users/me'); 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)); + it('ensureSessionChecked() reports unauthenticated when /users/me returns 401', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false })); + const service = TestBed.inject(AuthService); - - service.logout(); - + await expect(service.ensureSessionChecked()).resolves.toBe(false); 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); + it('ensureSessionChecked() reports unauthenticated when the request itself fails', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network down'))); - await expect(service.getAccessToken()).resolves.toBeUndefined(); + const service = TestBed.inject(AuthService); + await expect(service.ensureSessionChecked()).resolves.toBe(false); + }); + + it('logout() posts to the backend and clears the authenticated state', async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: true }); + vi.stubGlobal('fetch', fetchMock); + + const service = TestBed.inject(AuthService); + await service.logout(); + + expect(fetchMock).toHaveBeenCalledWith('/api/v1/auth/logout', { method: 'POST' }); + expect(service.isAuthenticated()).toBe(false); }); }); diff --git a/frontend/src/app/auth/auth.service.ts b/frontend/src/app/auth/auth.service.ts index c66a202..570f2f9 100644 --- a/frontend/src/app/auth/auth.service.ts +++ b/frontend/src/app/auth/auth.service.ts @@ -1,103 +1,46 @@ import { Injectable, signal } from '@angular/core'; 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. + * the entire Authorization Code + PKCE dance — including the PKCE verifier and + * the resulting access token — is handled server-side (see + * `GET /api/v1/auth/login`, `GET /api/v1/auth/callback`). The backend sets an + * httpOnly session cookie; the browser never sees an access token at all. + * This service therefore only triggers navigation and asks the backend + * "is there a valid session?" — it holds no tokens or PKCE state itself. */ @Injectable({ providedIn: 'root' }) export class AuthService { - private discoveryPromise: Promise | undefined; + readonly isAuthenticated = signal(undefined); - readonly isAuthenticated = signal(this.hasValidAccessToken()); + private sessionCheck: Promise | undefined; - private hasValidAccessToken(): boolean { - const expiresAt = Number(sessionStorage.getItem(EXPIRES_AT_KEY) ?? 0); - return !!sessionStorage.getItem(ACCESS_TOKEN_KEY) && Date.now() < expiresAt; + login(): void { + window.location.href = `${environment.apiBaseUrl}/auth/login`; } - 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 { - 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(): void { - sessionStorage.removeItem(ACCESS_TOKEN_KEY); - sessionStorage.removeItem(EXPIRES_AT_KEY); + async logout(): Promise { + await fetch(`${environment.apiBaseUrl}/auth/logout`, { method: 'POST' }); + this.sessionCheck = undefined; this.isAuthenticated.set(false); } - async getAccessToken(): Promise { - return this.hasValidAccessToken() ? (sessionStorage.getItem(ACCESS_TOKEN_KEY) ?? undefined) : undefined; + ensureSessionChecked(): Promise { + if (!this.sessionCheck) { + this.sessionCheck = this.checkSession(); + } + return this.sessionCheck; + } + + private async checkSession(): Promise { + try { + const response = await fetch(`${environment.apiBaseUrl}/users/me`); + this.isAuthenticated.set(response.ok); + return response.ok; + } catch { + this.isAuthenticated.set(false); + return false; + } } } diff --git a/frontend/src/app/auth/callback/callback.html b/frontend/src/app/auth/callback/callback.html deleted file mode 100644 index 2098280..0000000 --- a/frontend/src/app/auth/callback/callback.html +++ /dev/null @@ -1 +0,0 @@ -

Signing you in…

diff --git a/frontend/src/app/auth/callback/callback.spec.ts b/frontend/src/app/auth/callback/callback.spec.ts deleted file mode 100644 index 399ba43..0000000 --- a/frontend/src/app/auth/callback/callback.spec.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { TestBed } from '@angular/core/testing'; -import { Router } from '@angular/router'; -import { describe, expect, it, vi } from 'vitest'; -import { Callback } from './callback'; -import { AuthService } from '../auth.service'; - -describe('Callback', () => { - it('completes the OIDC login and navigates to /trips', async () => { - const authService = { completeLogin: vi.fn().mockResolvedValue(undefined) }; - const router = { navigateByUrl: vi.fn().mockResolvedValue(true) }; - - await TestBed.configureTestingModule({ - imports: [Callback], - providers: [ - { provide: AuthService, useValue: authService }, - { provide: Router, useValue: router }, - ], - }).compileComponents(); - - const fixture = TestBed.createComponent(Callback); - fixture.detectChanges(); - await fixture.whenStable(); - - expect(authService.completeLogin).toHaveBeenCalledTimes(1); - expect(router.navigateByUrl).toHaveBeenCalledWith('/trips'); - }); -}); diff --git a/frontend/src/app/auth/callback/callback.ts b/frontend/src/app/auth/callback/callback.ts deleted file mode 100644 index 43041a7..0000000 --- a/frontend/src/app/auth/callback/callback.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Component, inject, OnInit } from '@angular/core'; -import { Router } from '@angular/router'; -import { AuthService } from '../auth.service'; - -@Component({ - selector: 'app-callback', - templateUrl: './callback.html', -}) -export class Callback implements OnInit { - private readonly authService = inject(AuthService); - private readonly router = inject(Router); - - async ngOnInit(): Promise { - await this.authService.completeLogin(); - await this.router.navigateByUrl('/trips'); - } -} diff --git a/frontend/src/app/auth/pkce.ts b/frontend/src/app/auth/pkce.ts deleted file mode 100644 index 69beb19..0000000 --- a/frontend/src/app/auth/pkce.ts +++ /dev/null @@ -1,16 +0,0 @@ -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/pnpm-lock.yaml b/pnpm-lock.yaml index 69b7404..ee838cd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -23,6 +23,9 @@ importers: '@nestjs/platform-express': specifier: ^11.0.1 version: 11.2.1(@nestjs/common@11.2.1(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.2.1) + cookie-parser: + specifier: ^1.4.7 + version: 1.4.7 ioredis: specifier: ^6.0.0 version: 6.0.0 @@ -60,6 +63,9 @@ importers: '@nestjs/testing': specifier: ^11.0.1 version: 11.2.1(@nestjs/common@11.2.1(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.2.1)(@nestjs/platform-express@11.2.1) + '@types/cookie-parser': + specifier: ^1.4.10 + version: 1.4.10(@types/express@5.0.6) '@types/express': specifier: ^5.0.0 version: 5.0.6 @@ -1864,6 +1870,11 @@ packages: '@types/connect@3.4.38': resolution: {integrity: sha512-K6uROf1LD88uDQqJCktA4yzL1YYAK6NgfsI0v/mTgyPKWsX1CnJ0XPSDhViejru1GcRkLWb8RlzFYJRqGUbaug==} + '@types/cookie-parser@1.4.10': + resolution: {integrity: sha512-B4xqkqfZ8Wek+rCOeRxsjMS9OgvzebEzzLYw7NHYuvzb7IdxOkI0ZHGgeEBX4PUM7QGVvNSK60T3OvWj3YfBRg==} + peerDependencies: + '@types/express': '*' + '@types/cookiejar@2.1.5': resolution: {integrity: sha512-he+DHOWReW0nghN24E1WUqM0efK4kI9oTqDm6XmK8ZPe2djZ90BSNdGnIyCLzCPw7/pogPlGbzI2wHGGmi4O/Q==} @@ -2602,6 +2613,13 @@ packages: convert-source-map@2.0.0: resolution: {integrity: sha512-Kvp459HrV2FEJ1CAsi1Ku+MY3kasH19TFykTz2xWmMeq6bk2NU3XXvfJ+Q61m0xktWwt+1HSYf3JZsTms3aRJg==} + cookie-parser@1.4.7: + resolution: {integrity: sha512-nGUvgXnotP3BsjiLX2ypbQnWoGUPIIfHQNZkkC668ntrzGWEZVW70HDEB1qnNGMicPje6EttlIgzo51YSwNQGw==} + engines: {node: '>= 0.8.0'} + + cookie-signature@1.0.6: + resolution: {integrity: sha512-QADzlaHc8icV8I7vbaJXJwod9HWYp8uCqf1xa4OfNu1T7JVxQIrUgOWtHdNDtPiywmFbiS12VjotIXLrKM3orQ==} + cookie-signature@1.2.2: resolution: {integrity: sha512-D76uU73ulSXrD1UXF4KE2TMxVVwhsnCgfAyTg9k8P6KGZjlXKrOLe4dJQKI3Bxi5wjesZoFXJWElNWBjPZMbhg==} engines: {node: '>=6.6.0'} @@ -6710,6 +6728,10 @@ snapshots: dependencies: '@types/node': 24.13.3 + '@types/cookie-parser@1.4.10(@types/express@5.0.6)': + dependencies: + '@types/express': 5.0.6 + '@types/cookiejar@2.1.5': {} '@types/deep-eql@4.0.2': {} @@ -7509,6 +7531,13 @@ snapshots: convert-source-map@2.0.0: {} + cookie-parser@1.4.7: + dependencies: + cookie: 0.7.2 + cookie-signature: 1.0.6 + + cookie-signature@1.0.6: {} + cookie-signature@1.2.2: {} cookie@0.7.2: {}