diff --git a/apps/portal-bff/.env.example b/apps/portal-bff/.env.example index ebfd995..2df0dae 100644 --- a/apps/portal-bff/.env.example +++ b/apps/portal-bff/.env.example @@ -56,11 +56,20 @@ ENTRA_INSTANCE_URL=https://login.microsoftonline.com/ ENTRA_TENANT_ID=00000000-0000-0000-0000-000000000000 ENTRA_CLIENT_ID=00000000-0000-0000-0000-000000000000 ENTRA_CLIENT_SECRET=replace_with_real_value -# Redirect URIs registered in Entra alongside the same client id. Both -# `/auth/callback` and `/auth/logout` paths are mounted by the BFF -# once the OIDC routes land in a subsequent PR. +# Redirect URIs registered in Entra alongside the same client id. +# User portal — `/api/auth/callback` is the OIDC return URL; the +# post-logout URL is where Entra sends the browser after RP-initiated +# logout (typically the SPA landing page). ENTRA_REDIRECT_URI=http://localhost:3000/api/auth/callback ENTRA_POST_LOGOUT_REDIRECT_URI=http://localhost:4200/ +# Admin portal — distinct callback per ADR-0020 §"Sessions — distinct +# from `portal-shell`" so Entra routes the response to the matching +# session. Both `ENTRA_REDIRECT_URI` and `ENTRA_ADMIN_REDIRECT_URI` +# must be registered in the same Entra app registration's +# "Redirect URIs" list. Distinct post-logout URL routes admin +# sign-outs to the admin SPA's landing page (port 4201 in dev). +ENTRA_ADMIN_REDIRECT_URI=http://localhost:3000/api/admin/auth/callback +ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI=http://localhost:4201/ # Cookie signing secret (per ADR-0009 §"Cookies"). Used to sign the # transient pre-auth cookie that carries the OIDC `state` + PKCE diff --git a/apps/portal-bff/src/admin/admin-auth.controller.spec.ts b/apps/portal-bff/src/admin/admin-auth.controller.spec.ts new file mode 100644 index 0000000..f5d7870 --- /dev/null +++ b/apps/portal-bff/src/admin/admin-auth.controller.spec.ts @@ -0,0 +1,282 @@ +import type { Request, Response } from 'express'; +import type { Logger } from 'nestjs-pino'; +import type { AuditWriter } from '../audit/audit.service'; +import { PRE_AUTH_COOKIE_NAME } from '../auth/auth.cookie'; +import { AuthCodeFlowException } from '../auth/auth.errors'; +import type { AuthService, PreAuthPayload } from '../auth/auth.service'; +import type { EntraConfig } from '../auth/entra-config.token'; +import type { SessionEstablisher } from '../auth/session-establisher.service'; +import { AdminAuthController } from './admin-auth.controller'; + +const ENTRA: EntraConfig = { + instanceUrl: 'https://login.microsoftonline.com/', + tenantId: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', + clientId: '11111111-2222-3333-4444-555555555555', + clientSecret: 's3cret', + redirectUri: 'http://localhost:3000/api/auth/callback', + postLogoutRedirectUri: 'http://localhost:4200/', + adminRedirectUri: 'http://localhost:3000/api/admin/auth/callback', + adminPostLogoutRedirectUri: 'http://localhost:4201/', + authority: 'https://login.microsoftonline.com/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', +}; + +const USER = { + oid: 'admin-oid', + tid: ENTRA.tenantId, + username: 'admin@apf.example', + displayName: 'Admin Smith', + amr: ['pwd', 'mfa'], + roles: ['admin'], +}; + +const PRE_AUTH: PreAuthPayload = { + state: 'state-nonce', + codeVerifier: 'verifier-secret', + createdAt: 1_000, +}; + +const ADMIN_LOGOUT_URL = `${ENTRA.authority}/oauth2/v2.0/logout?post_logout_redirect_uri=${encodeURIComponent(ENTRA.adminPostLogoutRedirectUri)}`; + +function makeResStub() { + const res = { + cookie: jest.fn(), + clearCookie: jest.fn(), + redirect: jest.fn(), + status: jest.fn(), + json: jest.fn(), + }; + res.cookie.mockReturnValue(res); + res.clearCookie.mockReturnValue(res); + res.redirect.mockReturnValue(res); + res.status.mockReturnValue(res); + res.json.mockReturnValue(res); + return res as unknown as Response & typeof res; +} + +function makeReqStub(opts?: { + signedCookies?: Record; + sessionUser?: typeof USER; +}): Request { + return { + signedCookies: opts?.signedCookies ?? {}, + session: opts?.sessionUser !== undefined ? { user: opts.sessionUser } : {}, + sessionID: 'sid-admin', + } as unknown as Request; +} + +function makeFixture(opts?: { completeAuthCodeFlow?: jest.Mock }) { + const beginAuthCodeFlow = jest.fn().mockResolvedValue({ + authUrl: 'https://entra.example/authorize?state=state-nonce', + preAuthPayload: PRE_AUTH, + }); + const completeAuthCodeFlow = opts?.completeAuthCodeFlow ?? jest.fn().mockResolvedValue(USER); + const buildLogoutUrl = jest.fn().mockReturnValue(ADMIN_LOGOUT_URL); + const authService = { + beginAuthCodeFlow, + completeAuthCodeFlow, + buildLogoutUrl, + } as unknown as AuthService; + const audit = { + signIn: jest.fn().mockResolvedValue(undefined), + signInFailed: jest.fn().mockResolvedValue(undefined), + signOut: jest.fn().mockResolvedValue(undefined), + }; + const establisher = { + establish: jest.fn().mockResolvedValue(undefined), + destroy: jest.fn().mockResolvedValue(undefined), + }; + const logger = { log: jest.fn(), warn: jest.fn(), error: jest.fn() }; + return { + controller: new AdminAuthController( + authService, + logger as unknown as Logger, + ENTRA, + audit as unknown as AuditWriter, + establisher as unknown as SessionEstablisher, + ), + beginAuthCodeFlow, + completeAuthCodeFlow, + buildLogoutUrl, + audit, + establisher, + logger, + }; +} + +describe('AdminAuthController.login', () => { + it('passes the admin redirect URI to beginAuthCodeFlow', async () => { + const { controller, beginAuthCodeFlow } = makeFixture(); + await controller.login(makeResStub()); + expect(beginAuthCodeFlow).toHaveBeenCalledWith(ENTRA.adminRedirectUri); + }); + + it('writes the pre-auth cookie and 302s to the Entra auth URL', async () => { + const { controller } = makeFixture(); + const res = makeResStub(); + await controller.login(res); + expect(res.cookie).toHaveBeenCalledWith( + PRE_AUTH_COOKIE_NAME, + expect.any(String), + expect.objectContaining({ signed: true, httpOnly: true }), + ); + expect(res.redirect).toHaveBeenCalledWith(302, expect.stringContaining('entra.example')); + }); +}); + +describe('AdminAuthController.callback', () => { + it('passes the admin redirect URI to completeAuthCodeFlow and establishes a surface=admin session', async () => { + const { controller, completeAuthCodeFlow, establisher } = makeFixture(); + const res = makeResStub(); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + }); + + await controller.callback(req, res, 'auth-code', PRE_AUTH.state); + + expect(completeAuthCodeFlow).toHaveBeenCalledWith( + 'auth-code', + PRE_AUTH.state, + PRE_AUTH, + ENTRA.adminRedirectUri, + ); + expect(establisher.establish).toHaveBeenCalledWith({ + user: USER, + req, + res, + surface: 'admin', + }); + expect(res.redirect).toHaveBeenCalledWith(302, ENTRA.adminPostLogoutRedirectUri); + }); + + it('redirects with ?auth_error=token-exchange-failed when Entra returns error', async () => { + const { controller } = makeFixture(); + const res = makeResStub(); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + }); + + await controller.callback(req, res, undefined, undefined, 'access_denied', 'consent declined'); + + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining('auth_error=token-exchange-failed'), + ); + // Error redirect lands on the admin post-logout target, not the user-portal one. + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining(ENTRA.adminPostLogoutRedirectUri), + ); + }); + + it('redirects with ?auth_error=flow-expired when the pre-auth cookie is missing', async () => { + const { controller, audit } = makeFixture(); + const res = makeResStub(); + const req = makeReqStub({ signedCookies: {} }); + + await controller.callback(req, res, 'auth-code', PRE_AUTH.state); + + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining('auth_error=flow-expired'), + ); + expect(audit.signInFailed).toHaveBeenCalledWith( + expect.objectContaining({ + failureKind: 'no-pre-auth-cookie', + payload: expect.objectContaining({ surface: 'admin' }), + }), + ); + }); + + it('redirects with the typed error from AuthCodeFlowException', async () => { + const completeAuthCodeFlow = jest + .fn() + .mockRejectedValue(new AuthCodeFlowException({ kind: 'state-mismatch' })); + const { controller, audit } = makeFixture({ completeAuthCodeFlow }); + const res = makeResStub(); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + }); + + await controller.callback(req, res, 'auth-code', PRE_AUTH.state); + + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining('auth_error=state-mismatch'), + ); + expect(audit.signInFailed).toHaveBeenCalledWith( + expect.objectContaining({ + failureKind: 'state-mismatch', + payload: expect.objectContaining({ surface: 'admin' }), + }), + ); + }); +}); + +describe('AdminAuthController.me', () => { + it('returns the public payload with roles when the admin session is populated', () => { + const { controller } = makeFixture(); + const res = makeResStub(); + const req = makeReqStub({ sessionUser: USER }); + controller.me(req, res); + expect(res.json).toHaveBeenCalledWith({ + oid: USER.oid, + tid: USER.tid, + username: USER.username, + displayName: USER.displayName, + roles: USER.roles, + }); + }); + + it('throws UnauthorizedException when no user is on the admin session', () => { + const { controller } = makeFixture(); + const res = makeResStub(); + const req = makeReqStub(); + expect(() => controller.me(req, res)).toThrow(/Unauthenticated/); + }); +}); + +describe('AdminAuthController.logout', () => { + it('destroys the admin session, clears the admin cookie + CSRF cookie, redirects to Entra logout', async () => { + const { controller, establisher, buildLogoutUrl } = makeFixture(); + const res = makeResStub(); + const req = makeReqStub({ sessionUser: USER }); + + await controller.logout(req, res); + + expect(establisher.destroy).toHaveBeenCalledWith({ actor: USER, req }); + expect(buildLogoutUrl).toHaveBeenCalledWith(ENTRA.adminPostLogoutRedirectUri); + expect(res.clearCookie).toHaveBeenCalledWith('portal_admin_session', { path: '/' }); + expect(res.clearCookie).toHaveBeenCalledWith('portal_csrf', { path: '/' }); + expect(res.redirect).toHaveBeenCalledWith(302, ADMIN_LOGOUT_URL); + }); + + it('uses the __Host- prefixed admin cookie name in production', async () => { + const originalNodeEnv = process.env['NODE_ENV']; + try { + process.env['NODE_ENV'] = 'production'; + const { controller } = makeFixture(); + const res = makeResStub(); + const req = makeReqStub({ sessionUser: USER }); + await controller.logout(req, res); + expect(res.clearCookie).toHaveBeenCalledWith('__Host-portal_admin_session', { + path: '/', + }); + } finally { + if (originalNodeEnv === undefined) { + delete process.env['NODE_ENV']; + } else { + process.env['NODE_ENV'] = originalNodeEnv; + } + } + }); + + it('still clears cookies and redirects for an anonymous admin sign-out', async () => { + const { controller, establisher } = makeFixture(); + const res = makeResStub(); + const req = makeReqStub(); + await controller.logout(req, res); + expect(establisher.destroy).toHaveBeenCalledWith({ actor: undefined, req }); + expect(res.clearCookie).toHaveBeenCalledWith('portal_admin_session', { path: '/' }); + expect(res.redirect).toHaveBeenCalled(); + }); +}); diff --git a/apps/portal-bff/src/admin/admin-auth.controller.ts b/apps/portal-bff/src/admin/admin-auth.controller.ts new file mode 100644 index 0000000..518a54c --- /dev/null +++ b/apps/portal-bff/src/admin/admin-auth.controller.ts @@ -0,0 +1,185 @@ +import { Controller, Get, Inject, Query, Req, Res, UnauthorizedException } from '@nestjs/common'; +import type { Request, Response } from 'express'; +import { Logger } from 'nestjs-pino'; +import { AuditWriter } from '../audit/audit.service'; +import { + PRE_AUTH_COOKIE_NAME, + clearPreAuthCookieOptions, + preAuthCookieOptions, +} from '../auth/auth.cookie'; +import { AuthCodeFlowException, type AuthCodeFlowError, authErrorCode } from '../auth/auth.errors'; +import { AuthService, type PreAuthPayload } from '../auth/auth.service'; +import { ENTRA_CONFIG, type EntraConfig } from '../auth/entra-config.token'; +import { SessionEstablisher } from '../auth/session-establisher.service'; +import { csrfCookieName } from '../security/csrf-cookie'; +import { adminSessionCookieName } from '../session/admin-session-cookie'; + +/** + * `/api/admin/auth/*` — admin-portal OIDC routes per ADR-0020. + * + * Structurally identical to the user-portal `AuthController`: + * `GET /login` 302s to Entra with PKCE + state in a signed cookie, + * `GET /callback` exchanges the code for tokens, establishes the + * admin session, and redirects to the admin SPA. `GET /me` reads + * the admin session back. `GET /logout` destroys the admin session + * and redirects to Entra's RP-initiated logout. + * + * The key difference is *which* `req.session` we're looking at: the + * session middleware mounted on `/api/admin/*` in `main.ts` resolves + * to the admin-session Redis namespace (`session:admin:*`) and uses + * the `__Host-portal_admin_session` cookie, distinct from + * `__Host-portal_session` used by the user portal. Per ADR-0020 + * §"Sessions — distinct from `portal-shell`": signing in to one + * surface does NOT sign in to the other. + * + * Routes here are **deliberately not** guarded with `@RequireAdmin` + * or `@RequireMfa`. The whole point of the login flow is to *create* + * the session that those guards check against. Sub-controllers + * (admin business routes) layer the guards on top. + */ +@Controller('admin/auth') +export class AdminAuthController { + constructor( + private readonly authService: AuthService, + private readonly logger: Logger, + @Inject(ENTRA_CONFIG) private readonly entra: EntraConfig, + private readonly audit: AuditWriter, + private readonly sessionEstablisher: SessionEstablisher, + ) {} + + @Get('login') + async login(@Res() res: Response): Promise { + const { authUrl, preAuthPayload } = await this.authService.beginAuthCodeFlow( + this.entra.adminRedirectUri, + ); + res.cookie(PRE_AUTH_COOKIE_NAME, JSON.stringify(preAuthPayload), preAuthCookieOptions()); + res.redirect(302, authUrl); + } + + @Get('callback') + async callback( + @Req() req: Request, + @Res() res: Response, + @Query('code') code?: string, + @Query('state') state?: string, + @Query('error') entraError?: string, + @Query('error_description') entraErrorDescription?: string, + ): Promise { + res.clearCookie(PRE_AUTH_COOKIE_NAME, clearPreAuthCookieOptions()); + + if (entraError) { + this.logger.warn( + { + event: 'auth.entra_error', + surface: 'admin', + entraError, + entraErrorDescription, + }, + 'AdminAuthCallback', + ); + await this.audit.signInFailed({ + failureKind: 'entra-error', + payload: { surface: 'admin', entraError, entraErrorDescription }, + }); + return this.redirectWithError(res, 'token-exchange-failed'); + } + + if (typeof code !== 'string' || typeof state !== 'string') { + await this.audit.signInFailed({ + failureKind: 'missing-code-or-state', + payload: { surface: 'admin' }, + }); + return this.redirectWithError(res, 'token-exchange-failed'); + } + + const preAuth = readPreAuthCookie(req); + if (!preAuth) { + this.logger.warn({ event: 'auth.no_pre_auth_cookie', surface: 'admin' }, 'AdminAuthCallback'); + await this.audit.signInFailed({ + failureKind: 'no-pre-auth-cookie', + payload: { surface: 'admin' }, + }); + return this.redirectWithError(res, 'flow-expired'); + } + + try { + const user = await this.authService.completeAuthCodeFlow( + code, + state, + preAuth, + this.entra.adminRedirectUri, + ); + await this.sessionEstablisher.establish({ user, req, res, surface: 'admin' }); + res.redirect(302, this.entra.adminPostLogoutRedirectUri); + } catch (err) { + if (err instanceof AuthCodeFlowException) { + this.logger.warn( + { event: 'auth.flow_error', surface: 'admin', failure: err.failure }, + 'AdminAuthCallback', + ); + await this.audit.signInFailed({ + failureKind: err.failure.kind, + payload: { surface: 'admin' }, + }); + return this.redirectWithError(res, err.failure.kind); + } + throw err; + } + } + + @Get('me') + me(@Req() req: Request, @Res() res: Response): void { + const user = req.session.user; + if (!user) { + throw new UnauthorizedException({ + code: 'unauthenticated', + message: 'Unauthenticated', + }); + } + res.json({ + oid: user.oid, + tid: user.tid, + username: user.username, + displayName: user.displayName, + // Admins benefit from seeing their role assignment in the + // SPA — drives conditional UI for super-admin features later. + // Other surfaces (`/api/auth/me`) still omit it. + roles: user.roles, + }); + } + + @Get('logout') + async logout(@Req() req: Request, @Res() res: Response): Promise { + const user = req.session.user; + const wasAuthenticated = Boolean(user); + const logoutUrl = this.authService.buildLogoutUrl(this.entra.adminPostLogoutRedirectUri); + + await this.sessionEstablisher.destroy({ actor: user, req }); + + res.clearCookie(adminSessionCookieName(), { path: '/' }); + res.clearCookie(csrfCookieName(), { path: '/' }); + this.logger.log( + { event: 'auth.signed_out', surface: 'admin', wasAuthenticated }, + 'AdminAuthLogout', + ); + res.redirect(302, logoutUrl); + } + + private redirectWithError(res: Response, kind: AuthCodeFlowError['kind']): void { + const url = new URL(this.entra.adminPostLogoutRedirectUri); + url.searchParams.set('auth_error', authErrorCode({ kind } as AuthCodeFlowError)); + res.redirect(302, url.toString()); + } +} + +function readPreAuthCookie(req: Request): PreAuthPayload | null { + const raw = (req.signedCookies as Record)[PRE_AUTH_COOKIE_NAME]; + if (typeof raw !== 'string') { + return null; + } + try { + return JSON.parse(raw) as PreAuthPayload; + } catch { + return null; + } +} diff --git a/apps/portal-bff/src/admin/admin.module.ts b/apps/portal-bff/src/admin/admin.module.ts index d5c765f..b1fb3ab 100644 --- a/apps/portal-bff/src/admin/admin.module.ts +++ b/apps/portal-bff/src/admin/admin.module.ts @@ -1,21 +1,34 @@ import { Module } from '@nestjs/common'; +import { AuthModule } from '../auth/auth.module'; +import { AdminAuthController } from './admin-auth.controller'; import { AdminController } from './admin.controller'; import { AdminRoleGuard } from './admin-role.guard'; /** * `AdminModule` — root of the `/api/admin/*` surface per ADR-0020. * - * v1 ships the self-test endpoint only (`GET /api/admin/me`). The - * functional admin modules (audit log viewer, CMS, menu management, - * user list) land as separate sub-modules consumed from here once - * the distinct admin session + the audit query endpoint are in - * place — see the chantier sequence in `notes/handoff.md`. + * v1 ships: + * - `GET /api/admin/me` (self-test, guarded by `@RequireAdmin`). + * - `/api/admin/auth/{login,callback,me,logout}` — the distinct + * admin auth flow that establishes the `__Host-portal_admin_session` + * per ADR-0020 §"Sessions — distinct from `portal-shell`". + * + * Imports `AuthModule` to consume `AuthService`, `SessionEstablisher`, + * and `ENTRA_CONFIG` — the auth flow itself is shared with the + * user-portal `AuthController`; what differs is the redirect URIs + * passed in (admin-specific) and the session that gets populated + * (resolved by the path-routed session middleware in `main.ts`). + * + * The functional admin modules (audit log viewer, CMS, menu + * management, user list) land as separate sub-modules consumed from + * here in upcoming PRs. * * `AuditWriter` (required by `AdminRoleGuard`) is provided globally - * by `AuditModule`, so no extra import is needed here. + * by `AuditModule`, so no extra import is needed for it. */ @Module({ - controllers: [AdminController], + imports: [AuthModule], + controllers: [AdminController, AdminAuthController], providers: [AdminRoleGuard], }) export class AdminModule {} diff --git a/apps/portal-bff/src/auth/auth.controller.spec.ts b/apps/portal-bff/src/auth/auth.controller.spec.ts index 4e2a338..3e72138 100644 --- a/apps/portal-bff/src/auth/auth.controller.spec.ts +++ b/apps/portal-bff/src/auth/auth.controller.spec.ts @@ -7,6 +7,7 @@ import { PRE_AUTH_COOKIE_NAME, PRE_AUTH_COOKIE_TTL_MS } from './auth.cookie'; import { AuthCodeFlowException } from './auth.errors'; import type { AuthService, AuthenticatedUser, PreAuthPayload } from './auth.service'; import type { EntraConfig } from './entra-config.token'; +import { SessionEstablisher } from './session-establisher.service'; const ENTRA: EntraConfig = { instanceUrl: 'https://login.microsoftonline.com/', @@ -15,6 +16,8 @@ const ENTRA: EntraConfig = { clientSecret: 's3cret', redirectUri: 'http://localhost:3000/api/auth/callback', postLogoutRedirectUri: 'http://localhost:4200/', + adminRedirectUri: 'http://localhost:3000/api/admin/auth/callback', + adminPostLogoutRedirectUri: 'http://localhost:4201/', authority: 'https://login.microsoftonline.com/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', }; @@ -140,13 +143,21 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller sessionExpired: jest.fn().mockResolvedValue(undefined), }; const logger = makeLoggerStub(); + // Real SessionEstablisher with the same mocks the legacy tests + // already wire — keeps the behavioural assertions on session + // fields / audit calls untouched after the controller refactor. + const sessionEstablisher = new SessionEstablisher( + logger as unknown as Logger, + userSessionIndex as unknown as UserSessionIndexService, + audit as unknown as AuditWriter, + ); return { controller: new AuthController( service, logger as unknown as Logger, ENTRA, - userSessionIndex as unknown as UserSessionIndexService, audit as unknown as AuditWriter, + sessionEstablisher, ), beginAuthCodeFlow, completeAuthCodeFlow, @@ -214,7 +225,12 @@ describe('AuthController.callback', () => { await controller.callback(req, res, 'auth-code', PRE_AUTH.state); expect(res.clearCookie).toHaveBeenCalledWith(PRE_AUTH_COOKIE_NAME, expect.any(Object)); - expect(completeAuthCodeFlow).toHaveBeenCalledWith('auth-code', PRE_AUTH.state, PRE_AUTH); + expect(completeAuthCodeFlow).toHaveBeenCalledWith( + 'auth-code', + PRE_AUTH.state, + PRE_AUTH, + ENTRA.redirectUri, + ); // The resolved user must be on the session before the redirect // so the subsequent /me call sees a populated payload. expect(session.user).toEqual(USER); @@ -222,6 +238,7 @@ describe('AuthController.callback', () => { expect(logger.log).toHaveBeenCalledWith( expect.objectContaining({ event: 'auth.signed_in', + surface: 'user', oid: USER.oid, username: USER.username, amr: USER.amr, @@ -565,7 +582,7 @@ describe('AuthController.logout', () => { expect(res.clearCookie).toHaveBeenCalledWith('portal_session', { path: '/' }); expect(res.clearCookie).toHaveBeenCalledWith('portal_csrf', { path: '/' }); expect(logger.log).toHaveBeenCalledWith( - { event: 'auth.signed_out', wasAuthenticated: true }, + { event: 'auth.signed_out', surface: 'user', wasAuthenticated: true }, 'AuthLogout', ); // Authority + /oauth2/v2.0/logout?post_logout_redirect_uri=… @@ -617,7 +634,7 @@ describe('AuthController.logout', () => { expect(audit.signOut).not.toHaveBeenCalled(); expect(session.destroy).toHaveBeenCalledTimes(1); expect(logger.log).toHaveBeenCalledWith( - { event: 'auth.signed_out', wasAuthenticated: false }, + { event: 'auth.signed_out', surface: 'user', wasAuthenticated: false }, 'AuthLogout', ); expect(res.redirect).toHaveBeenCalled(); diff --git a/apps/portal-bff/src/auth/auth.controller.ts b/apps/portal-bff/src/auth/auth.controller.ts index 8e8f5d0..7bb8cda 100644 --- a/apps/portal-bff/src/auth/auth.controller.ts +++ b/apps/portal-bff/src/auth/auth.controller.ts @@ -1,11 +1,9 @@ -import { randomBytes } from 'node:crypto'; import { Controller, Get, Inject, Query, Req, Res, UnauthorizedException } from '@nestjs/common'; import type { Request, Response } from 'express'; import { Logger } from 'nestjs-pino'; import { AuditWriter } from '../audit/audit.service'; -import { csrfCookieName, csrfCookieOptions } from '../security/csrf-cookie'; -import { readSessionTimeouts, sessionCookieName } from '../session/session-cookie'; -import { UserSessionIndexService } from '../session/user-session-index.service'; +import { csrfCookieName } from '../security/csrf-cookie'; +import { sessionCookieName } from '../session/session-cookie'; import { PRE_AUTH_COOKIE_NAME, clearPreAuthCookieOptions, @@ -14,6 +12,7 @@ import { import { AuthCodeFlowException, type AuthCodeFlowError, authErrorCode } from './auth.errors'; import { AuthService, type AuthenticatedUser, type PreAuthPayload } from './auth.service'; import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token'; +import { SessionEstablisher } from './session-establisher.service'; /** * OIDC routes mounted under `/api/auth/` per ADR-0009. @@ -31,8 +30,8 @@ export class AuthController { private readonly authService: AuthService, private readonly logger: Logger, @Inject(ENTRA_CONFIG) private readonly entra: EntraConfig, - private readonly userSessionIndex: UserSessionIndexService, private readonly audit: AuditWriter, + private readonly sessionEstablisher: SessionEstablisher, ) {} /** @@ -47,7 +46,9 @@ export class AuthController { */ @Get('login') async login(@Res() res: Response): Promise { - const { authUrl, preAuthPayload } = await this.authService.beginAuthCodeFlow(); + const { authUrl, preAuthPayload } = await this.authService.beginAuthCodeFlow( + this.entra.redirectUri, + ); res.cookie(PRE_AUTH_COOKIE_NAME, JSON.stringify(preAuthPayload), preAuthCookieOptions()); res.redirect(302, authUrl); } @@ -117,60 +118,13 @@ export class AuthController { } try { - const user = await this.authService.completeAuthCodeFlow(code, state, preAuth); - const now = Date.now(); - const { idleSeconds, absoluteSeconds } = readSessionTimeouts(); - const csrfToken = randomBytes(32).toString('base64url'); - req.session.user = user; - req.session.createdAt = now; - // Hard ceiling per ADR-0010 §"TTL policy" — checked on every - // request by the absolute-timeout middleware, independent of - // idle TTL. - req.session.absoluteExpiresAt = now + absoluteSeconds * 1000; - // CSRF token per ADR-0009 §"Double-submit CSRF". Server-side - // source of truth lives on the session; the cookie below is - // the SPA's read-only mirror used to echo the value in the - // X-CSRF-Token header. - req.session.csrfToken = csrfToken; - // MFA freshness anchor per ADR-0011 §"Confirmation". Entra's - // CA policy decides whether MFA actually happened — the BFF - // does not re-validate factors. The stamp reflects "the - // session was just established with whatever assurance Entra - // returned"; `RequireMfaGuard` measures freshness against it. - // Refreshed by future step-up re-auth flows. - req.session.mfaVerifiedAt = now; - // Force the save before the redirect: express-session writes - // on response end, but the 302 we're about to emit closes the - // response before the async store-write would otherwise - // complete. Without this, the browser hits the SPA before - // Redis carries the new payload. - await saveSession(req); - // Mirror the CSRF token to a JS-readable cookie. maxAge - // matches the session's idle TTL so the cookie expires at - // the same time as the session (rolling, refreshed on each - // request alongside express-session's own cookie). - res.cookie(csrfCookieName(), csrfToken, csrfCookieOptions(idleSeconds * 1000)); - // Register the freshly-minted session id in the per-user - // index so a future admin "logout everywhere" can enumerate - // and revoke. Best-effort: a Redis hiccup here doesn't fail - // sign-in (the service swallows + logs). - await this.userSessionIndex.add(user.oid, req.sessionID); - // First write to the audit trail — blocking, per ADR-0013. - // If this throws the user does NOT see a successful sign-in: - // the exception propagates and the controller emits a 5xx via - // Nest's default exception filter. Same posture as the - // session.save() above. - await this.audit.signIn({ actor: user, sessionId: req.sessionID }); - this.logger.log( - { - event: 'auth.signed_in', - oid: user.oid, - tid: user.tid, - username: user.username, - amr: user.amr, - }, - 'AuthCallback', + const user = await this.authService.completeAuthCodeFlow( + code, + state, + preAuth, + this.entra.redirectUri, ); + await this.sessionEstablisher.establish({ user, req, res, surface: 'user' }); res.redirect(302, this.entra.postLogoutRedirectUri); } catch (err) { if (err instanceof AuthCodeFlowException) { @@ -226,41 +180,13 @@ export class AuthController { async logout(@Req() req: Request, @Res() res: Response): Promise { const user = req.session.user; const wasAuthenticated = Boolean(user); - const sessionId = req.sessionID; - const logoutUrl = this.authService.buildLogoutUrl(); + const logoutUrl = this.authService.buildLogoutUrl(this.entra.postLogoutRedirectUri); - // Drop the user_sessions index entry before destroy() removes - // the session id from `req`. Skipped on already-anonymous - // requests (nothing was ever added). - if (user) { - await this.userSessionIndex.remove(user.oid, sessionId); - // Audit the sign-out before tearing the session down — once - // destroy() runs we lose the actor id. Blocking per ADR-0013: - // if the audit row can't be written, the user does NOT get a - // "you're logged out" experience, because we can't certify - // the sign-out happened. - await this.audit.signOut({ actor: user, sessionId }); - } - - try { - await destroySession(req); - } catch (err) { - // The Redis DEL failed — log and continue. Clearing the - // cookie still gets the user effectively logged out from the - // BFF's point of view; the orphan Redis key will hit its idle - // TTL on its own. - this.logger.error( - { - event: 'session.destroy_failed', - message: err instanceof Error ? err.message : String(err), - }, - 'AuthLogout', - ); - } + await this.sessionEstablisher.destroy({ actor: user, req }); res.clearCookie(sessionCookieName(), { path: '/' }); res.clearCookie(csrfCookieName(), { path: '/' }); - this.logger.log({ event: 'auth.signed_out', wasAuthenticated }, 'AuthLogout'); + this.logger.log({ event: 'auth.signed_out', surface: 'user', wasAuthenticated }, 'AuthLogout'); res.redirect(302, logoutUrl); } @@ -287,18 +213,6 @@ function toPublicUser(user: AuthenticatedUser): PublicUser { }; } -function saveSession(req: Request): Promise { - return new Promise((resolve, reject) => { - req.session.save((err) => (err ? reject(err) : resolve())); - }); -} - -function destroySession(req: Request): Promise { - return new Promise((resolve, reject) => { - req.session.destroy((err) => (err ? reject(err) : resolve())); - }); -} - function readPreAuthCookie(req: Request): PreAuthPayload | null { const raw = (req.signedCookies as Record)[PRE_AUTH_COOKIE_NAME]; if (typeof raw !== 'string') { diff --git a/apps/portal-bff/src/auth/auth.module.spec.ts b/apps/portal-bff/src/auth/auth.module.spec.ts index 4a24c6b..9490a6c 100644 --- a/apps/portal-bff/src/auth/auth.module.spec.ts +++ b/apps/portal-bff/src/auth/auth.module.spec.ts @@ -42,6 +42,8 @@ const VALID = { ENTRA_CLIENT_SECRET: 's3cret-value-from-entra', ENTRA_REDIRECT_URI: 'http://localhost:3000/api/auth/callback', ENTRA_POST_LOGOUT_REDIRECT_URI: 'http://localhost:4200/', + ENTRA_ADMIN_REDIRECT_URI: 'http://localhost:3000/api/admin/auth/callback', + ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI: 'http://localhost:4201/', // Well-formed but unreachable Redis URL — `ioredis` opens the // socket lazily so the module compiles without any network access. REDIS_URL: 'redis://default:test-pass@127.0.0.1:65535/0', diff --git a/apps/portal-bff/src/auth/auth.module.ts b/apps/portal-bff/src/auth/auth.module.ts index fdb5a6e..0d574c8 100644 --- a/apps/portal-bff/src/auth/auth.module.ts +++ b/apps/portal-bff/src/auth/auth.module.ts @@ -8,6 +8,7 @@ import { AuthService } from './auth.service'; import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token'; import { MSAL_CLIENT } from './msal-client.token'; import { RequireMfaGuard } from './require-mfa.guard'; +import { SessionEstablisher } from './session-establisher.service'; /** * Auth module — owns the Entra ID configuration, the MSAL Node @@ -45,6 +46,7 @@ import { RequireMfaGuard } from './require-mfa.guard'; providers: [ AuthService, RequireMfaGuard, + SessionEstablisher, { provide: ENTRA_CONFIG, useFactory: () => assertEntraConfig(), @@ -90,6 +92,6 @@ import { RequireMfaGuard } from './require-mfa.guard'; }), }, ], - exports: [ENTRA_CONFIG, MSAL_CLIENT, RequireMfaGuard], + exports: [ENTRA_CONFIG, MSAL_CLIENT, RequireMfaGuard, AuthService, SessionEstablisher], }) export class AuthModule {} diff --git a/apps/portal-bff/src/auth/auth.service.spec.ts b/apps/portal-bff/src/auth/auth.service.spec.ts index 46c57ab..09a3f2e 100644 --- a/apps/portal-bff/src/auth/auth.service.spec.ts +++ b/apps/portal-bff/src/auth/auth.service.spec.ts @@ -11,6 +11,8 @@ const ENTRA: EntraConfig = { clientSecret: 's3cret', redirectUri: 'http://localhost:3000/api/auth/callback', postLogoutRedirectUri: 'http://localhost:4200/', + adminRedirectUri: 'http://localhost:3000/api/admin/auth/callback', + adminPostLogoutRedirectUri: 'http://localhost:4201/', authority: 'https://login.microsoftonline.com/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', }; @@ -71,7 +73,7 @@ const PRE_AUTH_OK: PreAuthPayload = { describe('AuthService.beginAuthCodeFlow', () => { it('builds the auth URL with the configured redirect, OIDC scopes, S256 challenge', async () => { const { service, getAuthCodeUrl } = makeService(); - await service.beginAuthCodeFlow(); + await service.beginAuthCodeFlow(ENTRA.redirectUri); expect(getAuthCodeUrl).toHaveBeenCalledTimes(1); const arg = getAuthCodeUrl.mock.calls[0]?.[0] as Record; expect(arg['redirectUri']).toBe(ENTRA.redirectUri); @@ -83,7 +85,7 @@ describe('AuthService.beginAuthCodeFlow', () => { it('returns the pre-auth payload with state + codeVerifier matching what MSAL was called with', async () => { const { service, getAuthCodeUrl } = makeService(); - const { preAuthPayload } = await service.beginAuthCodeFlow(); + const { preAuthPayload } = await service.beginAuthCodeFlow(ENTRA.redirectUri); const arg = getAuthCodeUrl.mock.calls[0]?.[0] as Record; expect(preAuthPayload.state).toBe(arg['state']); expect(preAuthPayload.codeVerifier).not.toBe(arg['codeChallenge']); @@ -92,8 +94,8 @@ describe('AuthService.beginAuthCodeFlow', () => { it('generates a fresh state + verifier on every call', async () => { const { service } = makeService(); - const a = await service.beginAuthCodeFlow(); - const b = await service.beginAuthCodeFlow(); + const a = await service.beginAuthCodeFlow(ENTRA.redirectUri); + const b = await service.beginAuthCodeFlow(ENTRA.redirectUri); expect(a.preAuthPayload.state).not.toBe(b.preAuthPayload.state); expect(a.preAuthPayload.codeVerifier).not.toBe(b.preAuthPayload.codeVerifier); }); @@ -106,6 +108,7 @@ describe('AuthService.completeAuthCodeFlow', () => { 'auth-code', PRE_AUTH_OK.state, PRE_AUTH_OK, + ENTRA.redirectUri, PRE_AUTH_OK.createdAt + 1_000, ); expect(acquireTokenByCode).toHaveBeenCalledWith({ @@ -127,7 +130,13 @@ describe('AuthService.completeAuthCodeFlow', () => { it('throws state-mismatch when the query state differs from the cookie state', async () => { const { service } = makeService(); await expect( - service.completeAuthCodeFlow('code', 'other-state', PRE_AUTH_OK, PRE_AUTH_OK.createdAt), + service.completeAuthCodeFlow( + 'code', + 'other-state', + PRE_AUTH_OK, + ENTRA.redirectUri, + PRE_AUTH_OK.createdAt, + ), ).rejects.toMatchObject({ failure: { kind: 'state-mismatch' } }); }); @@ -135,7 +144,7 @@ describe('AuthService.completeAuthCodeFlow', () => { const { service } = makeService(); const now = PRE_AUTH_OK.createdAt + PRE_AUTH_COOKIE_TTL_MS + 1; await expect( - service.completeAuthCodeFlow('code', PRE_AUTH_OK.state, PRE_AUTH_OK, now), + service.completeAuthCodeFlow('code', PRE_AUTH_OK.state, PRE_AUTH_OK, ENTRA.redirectUri, now), ).rejects.toMatchObject({ failure: { kind: 'flow-expired' } }); }); @@ -146,6 +155,7 @@ describe('AuthService.completeAuthCodeFlow', () => { 'code', PRE_AUTH_OK.state, PRE_AUTH_OK, + ENTRA.redirectUri, PRE_AUTH_OK.createdAt, ); // `amr` flows through as an empty array; MFA enforcement is @@ -163,6 +173,7 @@ describe('AuthService.completeAuthCodeFlow', () => { 'code', PRE_AUTH_OK.state, PRE_AUTH_OK, + ENTRA.redirectUri, PRE_AUTH_OK.createdAt, ); expect(user.roles).toEqual(['admin', 'editor']); @@ -175,6 +186,7 @@ describe('AuthService.completeAuthCodeFlow', () => { 'code', PRE_AUTH_OK.state, PRE_AUTH_OK, + ENTRA.redirectUri, PRE_AUTH_OK.createdAt, ); expect(user.roles).toEqual([]); @@ -187,6 +199,7 @@ describe('AuthService.completeAuthCodeFlow', () => { 'code', PRE_AUTH_OK.state, PRE_AUTH_OK, + ENTRA.redirectUri, PRE_AUTH_OK.createdAt, ); expect(user.roles).toEqual([]); @@ -201,6 +214,7 @@ describe('AuthService.completeAuthCodeFlow', () => { 'code', PRE_AUTH_OK.state, PRE_AUTH_OK, + ENTRA.redirectUri, PRE_AUTH_OK.createdAt, ); expect(user.roles).toEqual(['admin', 'editor']); @@ -210,7 +224,13 @@ describe('AuthService.completeAuthCodeFlow', () => { const acquireTokenByCode = jest.fn().mockRejectedValue(new Error('AADSTS70008')); const { service } = makeService({ acquireTokenByCode }); await expect( - service.completeAuthCodeFlow('code', PRE_AUTH_OK.state, PRE_AUTH_OK, PRE_AUTH_OK.createdAt), + service.completeAuthCodeFlow( + 'code', + PRE_AUTH_OK.state, + PRE_AUTH_OK, + ENTRA.redirectUri, + PRE_AUTH_OK.createdAt, + ), ).rejects.toMatchObject({ failure: { kind: 'token-exchange-failed', cause: 'AADSTS70008' }, }); @@ -220,7 +240,13 @@ describe('AuthService.completeAuthCodeFlow', () => { const acquireTokenByCode = jest.fn().mockResolvedValue(null); const { service } = makeService({ acquireTokenByCode }); await expect( - service.completeAuthCodeFlow('code', PRE_AUTH_OK.state, PRE_AUTH_OK, PRE_AUTH_OK.createdAt), + service.completeAuthCodeFlow( + 'code', + PRE_AUTH_OK.state, + PRE_AUTH_OK, + ENTRA.redirectUri, + PRE_AUTH_OK.createdAt, + ), ).rejects.toBeInstanceOf(AuthCodeFlowException); }); @@ -231,7 +257,13 @@ describe('AuthService.completeAuthCodeFlow', () => { } as unknown as AuthenticationResult); const { service } = makeService({ acquireTokenByCode }); await expect( - service.completeAuthCodeFlow('code', PRE_AUTH_OK.state, PRE_AUTH_OK, PRE_AUTH_OK.createdAt), + service.completeAuthCodeFlow( + 'code', + PRE_AUTH_OK.state, + PRE_AUTH_OK, + ENTRA.redirectUri, + PRE_AUTH_OK.createdAt, + ), ).rejects.toMatchObject({ failure: { kind: 'token-exchange-failed', cause: /oid/ }, }); @@ -241,19 +273,19 @@ describe('AuthService.completeAuthCodeFlow', () => { describe('AuthService.buildLogoutUrl', () => { it('targets the v2.0 logout endpoint on the configured authority', () => { const { service } = makeService(); - const url = new URL(service.buildLogoutUrl()); + const url = new URL(service.buildLogoutUrl(ENTRA.postLogoutRedirectUri)); expect(url.origin + url.pathname).toBe(`${ENTRA.authority}/oauth2/v2.0/logout`); }); it('includes the post_logout_redirect_uri query param', () => { const { service } = makeService(); - const url = new URL(service.buildLogoutUrl()); + const url = new URL(service.buildLogoutUrl(ENTRA.postLogoutRedirectUri)); expect(url.searchParams.get('post_logout_redirect_uri')).toBe(ENTRA.postLogoutRedirectUri); }); it('does not pass id_token_hint (v1 does not persist the id_token yet)', () => { const { service } = makeService(); - const url = new URL(service.buildLogoutUrl()); + const url = new URL(service.buildLogoutUrl(ENTRA.postLogoutRedirectUri)); expect(url.searchParams.has('id_token_hint')).toBe(false); }); }); diff --git a/apps/portal-bff/src/auth/auth.service.ts b/apps/portal-bff/src/auth/auth.service.ts index 9c068b3..c2aa9b4 100644 --- a/apps/portal-bff/src/auth/auth.service.ts +++ b/apps/portal-bff/src/auth/auth.service.ts @@ -81,13 +81,18 @@ export class AuthService { * MSAL Node owns the PKCE code generation (`CryptoProvider`) so * the verifier / challenge pair is canonical. The state nonce is * a fresh GUID per flow. + * + * `redirectUri` is passed by the caller (user-portal or + * admin-portal controller) per ADR-0020 §"Sessions — distinct + * from `portal-shell`". Same MSAL client, distinct redirect URI → + * Entra routes the callback to the matching session. */ - async beginAuthCodeFlow(): Promise { + async beginAuthCodeFlow(redirectUri: string): Promise { const { verifier, challenge } = await this.crypto.generatePkceCodes(); const state = this.crypto.createNewGuid(); const authUrl = await this.msal.getAuthCodeUrl({ - redirectUri: this.config.redirectUri, + redirectUri, scopes: [...SCOPES], state, codeChallenge: challenge, @@ -121,6 +126,7 @@ export class AuthService { code: string, state: string, preAuth: PreAuthPayload, + redirectUri: string, now: number = Date.now(), ): Promise { if (state !== preAuth.state) { @@ -135,7 +141,7 @@ export class AuthService { result = await this.msal.acquireTokenByCode({ code, codeVerifier: preAuth.codeVerifier, - redirectUri: this.config.redirectUri, + redirectUri, scopes: [...SCOPES], }); } catch (err) { @@ -167,9 +173,9 @@ export class AuthService { * lands with downstream API support per ADR-0014. The account * picker is the safer default in the interim. */ - buildLogoutUrl(): string { + buildLogoutUrl(postLogoutRedirectUri: string): string { const url = new URL(`${this.config.authority}/oauth2/v2.0/logout`); - url.searchParams.set('post_logout_redirect_uri', this.config.postLogoutRedirectUri); + url.searchParams.set('post_logout_redirect_uri', postLogoutRedirectUri); return url.toString(); } diff --git a/apps/portal-bff/src/auth/session-establisher.service.spec.ts b/apps/portal-bff/src/auth/session-establisher.service.spec.ts new file mode 100644 index 0000000..7645083 --- /dev/null +++ b/apps/portal-bff/src/auth/session-establisher.service.spec.ts @@ -0,0 +1,198 @@ +import type { Request, Response } from 'express'; +import type { Logger } from 'nestjs-pino'; +import type { AuditWriter } from '../audit/audit.service'; +import type { UserSessionIndexService } from '../session/user-session-index.service'; +import type { AuthenticatedUser } from './auth.service'; +import { SessionEstablisher } from './session-establisher.service'; + +const USER: AuthenticatedUser = { + oid: 'user-oid', + tid: 'tenant-1', + username: 'jane@apf.example', + displayName: 'Jane Doe', + amr: ['pwd', 'mfa'], + roles: [], +}; + +function makeReqStub(opts?: { sessionID?: string; sessionUser?: AuthenticatedUser }): Request { + const session: Record = { + save: jest.fn((cb: (err?: Error) => void) => cb()), + destroy: jest.fn((cb: (err?: Error) => void) => cb()), + }; + if (opts?.sessionUser !== undefined) { + session['user'] = opts.sessionUser; + } + return { + session, + sessionID: opts?.sessionID ?? 'sid-test', + } as unknown as Request; +} + +function makeResStub(): Response & { cookie: jest.Mock; clearCookie: jest.Mock } { + const res = { + cookie: jest.fn(), + clearCookie: jest.fn(), + }; + res.cookie.mockReturnValue(res); + res.clearCookie.mockReturnValue(res); + return res as unknown as Response & typeof res; +} + +function makeLoggerStub() { + return { + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }; +} + +interface Fixture { + est: SessionEstablisher; + index: { add: jest.Mock; remove: jest.Mock; list: jest.Mock }; + audit: { signIn: jest.Mock; signOut: jest.Mock }; + logger: ReturnType; +} + +function makeFixture(): Fixture { + const index = { + add: jest.fn().mockResolvedValue(undefined), + remove: jest.fn().mockResolvedValue(undefined), + list: jest.fn().mockResolvedValue([]), + }; + const audit = { + signIn: jest.fn().mockResolvedValue(undefined), + signOut: jest.fn().mockResolvedValue(undefined), + }; + const logger = makeLoggerStub(); + const est = new SessionEstablisher( + logger as unknown as Logger, + index as unknown as UserSessionIndexService, + audit as unknown as AuditWriter, + ); + return { est, index, audit, logger }; +} + +describe('SessionEstablisher.establish', () => { + it('writes user + createdAt + absoluteExpiresAt + csrfToken + mfaVerifiedAt on the session', async () => { + const { est } = makeFixture(); + const req = makeReqStub(); + const res = makeResStub(); + const before = Date.now(); + await est.establish({ user: USER, req, res, surface: 'user' }); + const after = Date.now(); + const sess = (req as unknown as { session: Record }).session; + expect(sess['user']).toEqual(USER); + expect(sess['createdAt']).toBeGreaterThanOrEqual(before); + expect(sess['createdAt']).toBeLessThanOrEqual(after); + // createdAt + mfaVerifiedAt share a clock reading. + expect(sess['mfaVerifiedAt']).toBe(sess['createdAt']); + expect(sess['absoluteExpiresAt']).toBeGreaterThan(sess['createdAt'] as number); + expect(typeof sess['csrfToken']).toBe('string'); + expect((sess['csrfToken'] as string).length).toBeGreaterThan(20); + }); + + it('saves the session before returning (no race with the controller redirect)', async () => { + const { est } = makeFixture(); + const req = makeReqStub(); + const res = makeResStub(); + await est.establish({ user: USER, req, res, surface: 'user' }); + const sess = (req as unknown as { session: { save: jest.Mock } }).session; + expect(sess.save).toHaveBeenCalledTimes(1); + }); + + it('mirrors the CSRF token to a JS-readable cookie', async () => { + const { est } = makeFixture(); + const req = makeReqStub(); + const res = makeResStub(); + await est.establish({ user: USER, req, res, surface: 'user' }); + const sess = (req as unknown as { session: Record }).session; + expect(res.cookie).toHaveBeenCalledWith( + expect.stringMatching(/csrf/), + sess['csrfToken'], + expect.objectContaining({ httpOnly: false }), + ); + }); + + it('adds the session id to the user_sessions index', async () => { + const { est, index } = makeFixture(); + const req = makeReqStub({ sessionID: 'sid-7' }); + const res = makeResStub(); + await est.establish({ user: USER, req, res, surface: 'user' }); + expect(index.add).toHaveBeenCalledWith(USER.oid, 'sid-7'); + }); + + it('emits the auth.sign_in audit row', async () => { + const { est, audit } = makeFixture(); + const req = makeReqStub({ sessionID: 'sid-7' }); + const res = makeResStub(); + await est.establish({ user: USER, req, res, surface: 'user' }); + expect(audit.signIn).toHaveBeenCalledWith({ actor: USER, sessionId: 'sid-7' }); + }); + + it('logs the success event with the surface tag', async () => { + const { est, logger } = makeFixture(); + const req = makeReqStub(); + const res = makeResStub(); + await est.establish({ user: USER, req, res, surface: 'admin' }); + expect(logger.log).toHaveBeenCalledWith( + expect.objectContaining({ + event: 'auth.signed_in', + surface: 'admin', + oid: USER.oid, + }), + 'AuthCallback', + ); + }); + + it('propagates audit failures (blocking per ADR-0013)', async () => { + const { est, audit } = makeFixture(); + audit.signIn.mockRejectedValueOnce(new Error('audit_writer denied')); + const req = makeReqStub(); + const res = makeResStub(); + await expect(est.establish({ user: USER, req, res, surface: 'user' })).rejects.toThrow( + 'audit_writer denied', + ); + }); +}); + +describe('SessionEstablisher.destroy', () => { + it('removes from index + audits signOut + destroys session when actor is set', async () => { + const { est, index, audit } = makeFixture(); + const req = makeReqStub({ sessionID: 'sid-9', sessionUser: USER }); + await est.destroy({ actor: USER, req }); + expect(index.remove).toHaveBeenCalledWith(USER.oid, 'sid-9'); + expect(audit.signOut).toHaveBeenCalledWith({ actor: USER, sessionId: 'sid-9' }); + const sess = (req as unknown as { session: { destroy: jest.Mock } }).session; + expect(sess.destroy).toHaveBeenCalledTimes(1); + }); + + it('still destroys the session for anonymous sign-outs but skips index + audit', async () => { + const { est, index, audit } = makeFixture(); + const req = makeReqStub(); + await est.destroy({ actor: undefined, req }); + expect(index.remove).not.toHaveBeenCalled(); + expect(audit.signOut).not.toHaveBeenCalled(); + const sess = (req as unknown as { session: { destroy: jest.Mock } }).session; + expect(sess.destroy).toHaveBeenCalledTimes(1); + }); + + it('logs but does not propagate when req.session.destroy fails (Redis hiccup)', async () => { + const { est, logger } = makeFixture(); + const req = makeReqStub(); + (req as unknown as { session: { destroy: jest.Mock } }).session.destroy = jest.fn( + (cb: (err?: Error) => void) => cb(new Error('redis down')), + ); + await expect(est.destroy({ actor: USER, req })).resolves.toBeUndefined(); + expect(logger.error).toHaveBeenCalledWith( + expect.objectContaining({ event: 'session.destroy_failed' }), + 'AuthLogout', + ); + }); + + it('propagates audit failures from signOut (blocking per ADR-0013)', async () => { + const { est, audit } = makeFixture(); + audit.signOut.mockRejectedValueOnce(new Error('audit_writer denied')); + const req = makeReqStub({ sessionUser: USER }); + await expect(est.destroy({ actor: USER, req })).rejects.toThrow('audit_writer denied'); + }); +}); diff --git a/apps/portal-bff/src/auth/session-establisher.service.ts b/apps/portal-bff/src/auth/session-establisher.service.ts new file mode 100644 index 0000000..7fbb936 --- /dev/null +++ b/apps/portal-bff/src/auth/session-establisher.service.ts @@ -0,0 +1,159 @@ +import { randomBytes } from 'node:crypto'; +import { Injectable } from '@nestjs/common'; +import type { Request, Response } from 'express'; +import { Logger } from 'nestjs-pino'; +import { AuditWriter } from '../audit/audit.service'; +import { csrfCookieName, csrfCookieOptions } from '../security/csrf-cookie'; +import { readSessionTimeouts } from '../session/session-cookie'; +import { UserSessionIndexService } from '../session/user-session-index.service'; +import type { AuthenticatedUser } from './auth.service'; + +export type AuthSurface = 'user' | 'admin'; + +/** + * Shared session-establishment recipe used by both `AuthController` + * (user-portal) and `AdminAuthController` (admin-portal). Per + * ADR-0020 §"Sessions — distinct from `portal-shell`", the two + * surfaces use distinct cookies / Redis namespaces — but the + * *recipe* for persisting an authenticated user into a session is + * identical: + * + * 1. Mint a CSRF token (per ADR-0009 §"Double-submit CSRF"). + * 2. Populate the session fields (`user`, `createdAt`, + * `absoluteExpiresAt`, `csrfToken`, `mfaVerifiedAt`). + * 3. Force `req.session.save()` before the 302 — `express-session` + * writes on response end, but the redirect closes the response + * before the async store write would otherwise complete. + * 4. Mirror the CSRF token to the JS-readable cookie. + * 5. Register the session id in the per-user index for future + * "logout everywhere" — best-effort, a Redis hiccup does NOT + * fail the sign-in. + * 6. Emit the `auth.sign_in` audit row (blocking per ADR-0013). + * 7. Log the success event. + * + * Surface-specific concerns — the redirect destination, the pre-auth + * cookie clearing, the error paths — stay in the controllers. + * + * The middleware in `main.ts` has already resolved `req.session` to + * the correct surface (user vs admin) by the time the controller's + * callback handler runs; this service therefore writes to whichever + * session was loaded without needing to know which one. + */ +@Injectable() +export class SessionEstablisher { + constructor( + private readonly logger: Logger, + private readonly userSessionIndex: UserSessionIndexService, + private readonly audit: AuditWriter, + ) {} + + async establish(opts: { + user: AuthenticatedUser; + req: Request; + res: Response; + /** + * Tag forwarded into the success log so dashboards can split + * user-portal sign-ins from admin-portal sign-ins. Audit rows + * keep the same `auth.sign_in` event type in both cases — + * adding a surface field to the audit catalogue is a follow-up + * decision (current ADR-0013 catalogue is single-tier). + */ + surface: AuthSurface; + }): Promise { + const { user, req, res, surface } = opts; + const now = Date.now(); + const { idleSeconds, absoluteSeconds } = readSessionTimeouts(); + const csrfToken = randomBytes(32).toString('base64url'); + + req.session.user = user; + req.session.createdAt = now; + // Hard ceiling per ADR-0010 §"TTL policy" — checked on every + // request by the absolute-timeout middleware, independent of + // idle TTL. + req.session.absoluteExpiresAt = now + absoluteSeconds * 1000; + req.session.csrfToken = csrfToken; + // MFA freshness anchor per ADR-0011 §"Confirmation". Entra's + // CA policy decides whether MFA actually happened — the BFF + // does not re-validate factors. Refreshed by future step-up + // re-auth flows. + req.session.mfaVerifiedAt = now; + + await saveSession(req); + + // Cookie maxAge matches the session's idle TTL so the CSRF + // cookie expires alongside the session itself (rolling, + // refreshed on each request). + res.cookie(csrfCookieName(), csrfToken, csrfCookieOptions(idleSeconds * 1000)); + + // Best-effort: a Redis hiccup here doesn't fail sign-in. + await this.userSessionIndex.add(user.oid, req.sessionID); + + // Blocking audit per ADR-0013. If this throws the user does + // NOT see a successful sign-in: the exception propagates and + // the controller emits a 5xx via the StructuredErrorFilter. + await this.audit.signIn({ actor: user, sessionId: req.sessionID }); + + this.logger.log( + { + event: 'auth.signed_in', + surface, + oid: user.oid, + tid: user.tid, + username: user.username, + amr: user.amr, + }, + 'AuthCallback', + ); + } + + /** + * Symmetric helper for the sign-out path. When `actor` is set, + * removes the session id from the per-user index and emits the + * `auth.sign_out` audit row (blocking per ADR-0013 — if the audit + * row can't be written, the user does NOT get a "you're logged + * out" experience). Then tears the session down unconditionally + * — anonymous sign-outs still benefit from a `req.session.destroy()` + * call to clear any orphan state in the store. A Redis hiccup on + * `destroy()` is logged but non-fatal: clearing the cookie at the + * HTTP layer (the controller's job) is sufficient to log the user + * out from the BFF's point of view; the orphan Redis key will hit + * its idle TTL on its own. + * + * The controller is responsible for HTTP-layer cleanup — the + * session-cookie name is surface-specific (`portal_session` vs + * `portal_admin_session`) and lives on the controller. + */ + async destroy(opts: { actor: AuthenticatedUser | undefined; req: Request }): Promise { + const { actor, req } = opts; + + if (actor !== undefined) { + const sessionId = req.sessionID; + await this.userSessionIndex.remove(actor.oid, sessionId); + await this.audit.signOut({ actor, sessionId }); + } + + try { + await destroySession(req); + } catch (err) { + this.logger.error( + { + event: 'session.destroy_failed', + message: err instanceof Error ? err.message : String(err), + }, + 'AuthLogout', + ); + } + } +} + +function saveSession(req: Request): Promise { + return new Promise((resolve, reject) => { + req.session.save((err) => (err ? reject(err) : resolve())); + }); +} + +function destroySession(req: Request): Promise { + return new Promise((resolve, reject) => { + req.session.destroy((err) => (err ? reject(err) : resolve())); + }); +} diff --git a/apps/portal-bff/src/config/check-entra-config.spec.ts b/apps/portal-bff/src/config/check-entra-config.spec.ts index 01047e2..4c36260 100644 --- a/apps/portal-bff/src/config/check-entra-config.spec.ts +++ b/apps/portal-bff/src/config/check-entra-config.spec.ts @@ -7,6 +7,8 @@ const VALID = { ENTRA_CLIENT_SECRET: 's3cret-value-from-entra', ENTRA_REDIRECT_URI: 'http://localhost:3000/api/auth/callback', ENTRA_POST_LOGOUT_REDIRECT_URI: 'http://localhost:4200/', + ENTRA_ADMIN_REDIRECT_URI: 'http://localhost:3000/api/admin/auth/callback', + ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI: 'http://localhost:4201/', }; describe('assertEntraConfig', () => { @@ -38,9 +40,21 @@ describe('assertEntraConfig', () => { expect(config.clientSecret).toBe(VALID.ENTRA_CLIENT_SECRET); expect(config.redirectUri).toBe(VALID.ENTRA_REDIRECT_URI); expect(config.postLogoutRedirectUri).toBe(VALID.ENTRA_POST_LOGOUT_REDIRECT_URI); + expect(config.adminRedirectUri).toBe(VALID.ENTRA_ADMIN_REDIRECT_URI); + expect(config.adminPostLogoutRedirectUri).toBe(VALID.ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI); expect(config.authority).toBe(`${VALID.ENTRA_INSTANCE_URL}${VALID.ENTRA_TENANT_ID}`); }); + it('throws when ENTRA_ADMIN_REDIRECT_URI equals ENTRA_REDIRECT_URI (would collapse the two surfaces)', () => { + process.env['ENTRA_ADMIN_REDIRECT_URI'] = VALID.ENTRA_REDIRECT_URI; + expect(() => assertEntraConfig()).toThrow(/must differ from ENTRA_REDIRECT_URI/); + }); + + it('rejects ENTRA_ADMIN_REDIRECT_URI when not a valid URL', () => { + process.env['ENTRA_ADMIN_REDIRECT_URI'] = 'nope'; + expect(() => assertEntraConfig()).toThrow(/ENTRA_ADMIN_REDIRECT_URI is not a valid URL/); + }); + it('throws listing every missing required key', () => { delete process.env['ENTRA_CLIENT_ID']; delete process.env['ENTRA_CLIENT_SECRET']; diff --git a/apps/portal-bff/src/config/check-entra-config.ts b/apps/portal-bff/src/config/check-entra-config.ts index 7992374..c057146 100644 --- a/apps/portal-bff/src/config/check-entra-config.ts +++ b/apps/portal-bff/src/config/check-entra-config.ts @@ -28,10 +28,25 @@ export interface EntraConfig { readonly clientId: string; /** Confidential client secret. */ readonly clientSecret: string; - /** Where Entra sends the user after authentication — `/auth/callback`. */ + /** Where Entra sends the user after authentication — `/api/auth/callback`. */ readonly redirectUri: string; /** Where Entra sends the user after RP-initiated logout. */ readonly postLogoutRedirectUri: string; + /** + * Admin-surface callback URL — `/api/admin/auth/callback`. Distinct + * from {@link redirectUri} per ADR-0020 §"Sessions — distinct from + * `portal-shell`" so Entra knows which callback handler (and + * therefore which session) the auth flow is establishing. + * Both URIs must be registered on the same Entra app registration. + */ + readonly adminRedirectUri: string; + /** + * Where Entra sends the admin user after RP-initiated logout from + * the admin surface. Typically the admin SPA's landing page — kept + * distinct from {@link postLogoutRedirectUri} so an operator can + * route admin sign-outs to a different page than user sign-outs. + */ + readonly adminPostLogoutRedirectUri: string; /** * Convenience: the full authority URL passed to MSAL Node * (`${instanceUrl}${tenantId}`). Computed once so the rest of the @@ -47,6 +62,8 @@ const REQUIRED_KEYS = [ 'ENTRA_CLIENT_SECRET', 'ENTRA_REDIRECT_URI', 'ENTRA_POST_LOGOUT_REDIRECT_URI', + 'ENTRA_ADMIN_REDIRECT_URI', + 'ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI', ] as const; const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; @@ -70,6 +87,8 @@ export function assertEntraConfig(): EntraConfig { const clientSecret = process.env['ENTRA_CLIENT_SECRET'] as string; const redirectUri = process.env['ENTRA_REDIRECT_URI'] as string; const postLogoutRedirectUri = process.env['ENTRA_POST_LOGOUT_REDIRECT_URI'] as string; + const adminRedirectUri = process.env['ENTRA_ADMIN_REDIRECT_URI'] as string; + const adminPostLogoutRedirectUri = process.env['ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI'] as string; assertUrl('ENTRA_INSTANCE_URL', instanceUrl, ['https:']); if (!instanceUrl.endsWith('/')) { @@ -87,6 +106,24 @@ export function assertEntraConfig(): EntraConfig { assertUrl('ENTRA_REDIRECT_URI', redirectUri, ['http:', 'https:']); assertUrl('ENTRA_POST_LOGOUT_REDIRECT_URI', postLogoutRedirectUri, ['http:', 'https:']); + assertUrl('ENTRA_ADMIN_REDIRECT_URI', adminRedirectUri, ['http:', 'https:']); + assertUrl('ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI', adminPostLogoutRedirectUri, [ + 'http:', + 'https:', + ]); + + // Admin and user redirect URIs must differ — they're the only + // way Entra (and therefore the BFF) tells the two flows apart. + // A misconfiguration that points both to the same handler would + // silently collapse the two surfaces into one session. + if (adminRedirectUri === redirectUri) { + throw new Error( + `ENTRA_ADMIN_REDIRECT_URI must differ from ENTRA_REDIRECT_URI ` + + `so Entra can route the callback to the matching session ` + + `(see ADR-0020 §"Sessions — distinct from portal-shell"). ` + + `Both are: ${redirectUri}`, + ); + } return { instanceUrl, @@ -95,6 +132,8 @@ export function assertEntraConfig(): EntraConfig { clientSecret, redirectUri, postLogoutRedirectUri, + adminRedirectUri, + adminPostLogoutRedirectUri, authority: `${instanceUrl}${tenantId}`, }; } diff --git a/apps/portal-bff/src/main.ts b/apps/portal-bff/src/main.ts index ce4f8c6..74eb5a7 100644 --- a/apps/portal-bff/src/main.ts +++ b/apps/portal-bff/src/main.ts @@ -19,7 +19,9 @@ import { assertSessionSecret } from './config/check-session-secret'; import { createRateLimitMiddleware, readRateLimitConfig } from './security/rate-limit.middleware'; import { CSRF_MIDDLEWARE } from './security/security.token'; import { StructuredErrorFilter } from './security/structured-error.filter'; +import type { NextFunction, Request, Response } from 'express'; import { + ADMIN_SESSION_MIDDLEWARE, SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE, SESSION_MIDDLEWARE, type RequestHandler, @@ -125,12 +127,27 @@ async function bootstrap() { // `req.cookies`. app.use(cookieParser(sessionSecret)); - // Session middleware (per ADR-0010). Resolved from the Nest - // container so it sits on the same `ioredis` client the rest of - // the BFF uses, with AES-256-GCM applied to the payload before it - // lands in Redis. Mounted after `cookieParser` so the session id - // cookie is parsed by the time `express-session` reads it. - app.use(app.get(SESSION_MIDDLEWARE)); + // Session middlewares (per ADR-0010 + ADR-0020 §"Sessions — distinct + // from `portal-shell`"). Two parallel express-session instances: + // + // - `SESSION_MIDDLEWARE` carries `portal_session` / Redis prefix + // `session:` and binds to every path EXCEPT `/api/admin/*`. + // - `ADMIN_SESSION_MIDDLEWARE` carries `portal_admin_session` / + // Redis prefix `session:admin:` and binds to `/api/admin/*` only. + // + // The dispatch is a tiny wrapper that picks one or the other per + // request — running both would have the second overwrite `req.session` + // from the first, collapsing the two surfaces. Mounted after + // `cookieParser` so the session-id cookie is parsed by the time the + // selected middleware reads it. + const userSession = app.get(SESSION_MIDDLEWARE); + const adminSession = app.get(ADMIN_SESSION_MIDDLEWARE); + app.use((req: Request, res: Response, next: NextFunction) => { + if (req.path.startsWith('/api/admin')) { + return adminSession(req, res, next); + } + return userSession(req, res, next); + }); // Absolute-timeout enforcement (ADR-0010 §"TTL policy"). Runs on // every request that survives `express-session`; if the session diff --git a/apps/portal-bff/src/session/admin-session-cookie.spec.ts b/apps/portal-bff/src/session/admin-session-cookie.spec.ts new file mode 100644 index 0000000..3c5e98a --- /dev/null +++ b/apps/portal-bff/src/session/admin-session-cookie.spec.ts @@ -0,0 +1,28 @@ +import { adminSessionCookieName } from './admin-session-cookie'; + +describe('adminSessionCookieName', () => { + const originalNodeEnv = process.env['NODE_ENV']; + + afterEach(() => { + if (originalNodeEnv === undefined) { + delete process.env['NODE_ENV']; + } else { + process.env['NODE_ENV'] = originalNodeEnv; + } + }); + + it('uses the `__Host-` prefixed name in production', () => { + process.env['NODE_ENV'] = 'production'; + expect(adminSessionCookieName()).toBe('__Host-portal_admin_session'); + }); + + it('uses the unprefixed name in development', () => { + process.env['NODE_ENV'] = 'development'; + expect(adminSessionCookieName()).toBe('portal_admin_session'); + }); + + it('defaults to development naming when NODE_ENV is unset', () => { + delete process.env['NODE_ENV']; + expect(adminSessionCookieName()).toBe('portal_admin_session'); + }); +}); diff --git a/apps/portal-bff/src/session/admin-session-cookie.ts b/apps/portal-bff/src/session/admin-session-cookie.ts new file mode 100644 index 0000000..d3d188c --- /dev/null +++ b/apps/portal-bff/src/session/admin-session-cookie.ts @@ -0,0 +1,21 @@ +/** + * Admin-session cookie name per ADR-0020 §"Sessions — distinct from + * `portal-shell`". The admin SPA carries `__Host-portal_admin_session` + * (prod) so a browser holding both cookies can switch between + * portal-shell and portal-admin without one signing the user in to + * the other. + * + * Same `__Host-` posture as the user-portal session: `Secure` + no + * `Domain` + `Path=/`. The dev variant drops the prefix because + * local servers are plain HTTP. + * + * The cookie scoping is the only thing keeping the two sessions + * apart at the browser level — the BFF further isolates them in + * Redis under a distinct key prefix (`session:admin:`). + */ +const PRODUCTION_NAME = '__Host-portal_admin_session'; +const DEVELOPMENT_NAME = 'portal_admin_session'; + +export function adminSessionCookieName(): string { + return process.env['NODE_ENV'] === 'production' ? PRODUCTION_NAME : DEVELOPMENT_NAME; +} diff --git a/apps/portal-bff/src/session/session.module.ts b/apps/portal-bff/src/session/session.module.ts index 2e7343a..df8db14 100644 --- a/apps/portal-bff/src/session/session.module.ts +++ b/apps/portal-bff/src/session/session.module.ts @@ -2,6 +2,7 @@ import { randomBytes } from 'node:crypto'; import { Module } from '@nestjs/common'; import { RedisStore } from 'connect-redis'; import expressSession from 'express-session'; +import type { CookieOptions } from 'express'; import { Logger } from 'nestjs-pino'; import { assertSessionEncryptionKey } from '../config/check-session-encryption-key'; import { assertSessionSecret } from '../config/check-session-secret'; @@ -9,10 +10,12 @@ import { RedisModule } from '../redis/redis.module'; import { REDIS_CLIENT, type Redis } from '../redis/redis.token'; import { AuditWriter } from '../audit/audit.service'; import { createAbsoluteTimeoutMiddleware } from './absolute-timeout.middleware'; +import { adminSessionCookieName } from './admin-session-cookie'; import { adaptIoredisForConnectRedis } from './ioredis-connect-redis-adapter'; import { SessionDecryptError, decrypt, encrypt } from './session-crypto'; import { readSessionTimeouts, sessionCookieName, sessionCookieOptions } from './session-cookie'; import { + ADMIN_SESSION_MIDDLEWARE, SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE, SESSION_MIDDLEWARE, type RequestHandler, @@ -22,35 +25,108 @@ import { import './session.types'; import { UserSessionIndexService } from './user-session-index.service'; +/** + * Build a configured `express-session` middleware. Shared between + * the user-portal middleware ({@link SESSION_MIDDLEWARE}) and the + * admin-portal middleware ({@link ADMIN_SESSION_MIDDLEWARE}) per + * ADR-0020 §"Sessions — distinct from `portal-shell`". + * + * The two surfaces differ only on the cookie name and Redis key + * prefix; the TTL policy, encryption key, signing secret, session-id + * entropy, and error-handling all come from the same source so a + * future hardening (e.g. tighter idle TTL on admin) is a parameter + * change rather than a divergent code path. + */ +function buildSessionMiddleware( + redis: Redis, + logger: Logger, + opts: { cookieName: string; redisKeyPrefix: string; surfaceTag: 'user' | 'admin' }, +): RequestHandler { + const secret = assertSessionSecret(); + const key = assertSessionEncryptionKey(); + const timeouts = readSessionTimeouts(); + + const store = new RedisStore({ + // `connect-redis` v9 was rewritten against the `node-redis` v4 + // command surface; the adapter shims `ioredis` to look the same + // for the handful of commands the store actually calls. + client: adaptIoredisForConnectRedis(redis) as unknown as never, + prefix: opts.redisKeyPrefix, + ttl: timeouts.idleSeconds, + serializer: { + stringify: (sess) => encrypt(JSON.stringify(sess), key), + parse: (payload) => { + try { + return JSON.parse(decrypt(payload, key)); + } catch (err) { + // Tamper, wrong key, or unknown version. The phase-2 + // audit pipeline (ADR-0013) will turn this into a + // first-class audit event; for now we surface a + // structured Pino log so ops can spot it. The + // `surface` tag lets dashboards split user-portal + // decrypt failures from admin-portal ones. + logger.warn( + { + event: 'session.decrypt_failed', + surface: opts.surfaceTag, + reason: err instanceof SessionDecryptError ? err.message : String(err), + }, + 'session', + ); + throw err; + } + }, + }, + }); + + const cookie: CookieOptions = sessionCookieOptions(timeouts.idleSeconds); + + return expressSession({ + name: opts.cookieName, + secret, + // `crypto.randomBytes(32).toString('base64url')` ⇒ 256 + // bits of entropy in the session id, per ADR-0010 + // §"Confirmation". + genid: () => randomBytes(32).toString('base64url'), + store, + // `resave: false` — RedisStore.touch refreshes the TTL on + // every request; no need to rewrite the payload. + resave: false, + // `saveUninitialized: false` — don't create empty session + // keys for unauthenticated visitors browsing public + // routes. The session is born when the OIDC callback + // populates it. + saveUninitialized: false, + // `rolling: true` — the cookie's `expires` slides forward + // on every response, matching the sliding-idle policy. + rolling: true, + cookie, + }); +} + /** * Session module — wires `express-session` with a `connect-redis` * store on top of the shared `ioredis` client, with AES-256-GCM * encryption applied to the JSON payload before it lands in Redis. * - * The configured middleware is exposed as a NestJS provider under - * the {@link SESSION_MIDDLEWARE} token. `main.ts` resolves it from - * the application context and mounts it once via `app.use(...)` - * after `cookie-parser`. Mounting the express-session middleware - * through DI (rather than constructing it in `main.ts`) keeps it on - * the same Redis client the rest of the BFF uses, instead of - * spinning up a second connection at the bootstrap layer. + * Two middlewares are provided, path-routed in `main.ts`: * - * What's wired today: - * - Express-session middleware on every request (`SESSION_MIDDLEWARE`). - * - Absolute-timeout middleware (`SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE`) - * that enforces the 12 h hard ceiling per ADR-0010. - * - `UserSessionIndexService`: maintains the - * `user_sessions:{userId}` Redis set so a future admin endpoint - * can "log out user X everywhere" — write-side hooks live in - * the auth controller (create on `/auth/callback`, drop on - * `/auth/logout` or absolute-timeout). + * - {@link SESSION_MIDDLEWARE} — user-portal. Cookie + * `portal_session` / `__Host-portal_session`. Redis prefix + * `session:`. Bound to every path except `/api/admin/*`. * - * Out of scope, landing in follow-ups: - * - The admin "logout everywhere" route that consumes the - * `UserSessionIndexService` (waits on the admin module + the - * `@RequireAdmin` / `@RequireMfa` guards). - * - Audit-pipeline wiring for `session.decrypt_failed` and - * `session.absolute_timeout` (ADR-0013). + * - {@link ADMIN_SESSION_MIDDLEWARE} — admin-portal per ADR-0020. + * Cookie `portal_admin_session` / `__Host-portal_admin_session`. + * Redis prefix `session:admin:`. Bound to `/api/admin/*` only. + * + * The {@link SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE} (ADR-0010 §"TTL + * policy") and {@link UserSessionIndexService} are shared across + * both surfaces — the absolute-timeout check reads `req.session` + * which the dispatch has already resolved to one surface or the + * other; the user-session index is keyed by Entra `oid`, so an + * admin sign-in and a user-portal sign-in by the same person + * naturally land in different Redis sets only because the session + * id namespaces differ. */ @Module({ imports: [RedisModule], @@ -68,66 +144,29 @@ import { UserSessionIndexService } from './user-session-index.service'; { provide: SESSION_MIDDLEWARE, inject: [REDIS_CLIENT, Logger], - useFactory: (redis: Redis, logger: Logger): RequestHandler => { - const secret = assertSessionSecret(); - const key = assertSessionEncryptionKey(); - const timeouts = readSessionTimeouts(); - - const store = new RedisStore({ - // `connect-redis` v9 was rewritten against the - // `node-redis` v4 command surface; the adapter shims - // `ioredis` to look the same for the handful of commands - // the store actually calls. - client: adaptIoredisForConnectRedis(redis) as unknown as never, - prefix: 'session:', - ttl: timeouts.idleSeconds, - serializer: { - stringify: (sess) => encrypt(JSON.stringify(sess), key), - parse: (payload) => { - try { - return JSON.parse(decrypt(payload, key)); - } catch (err) { - // Tamper, wrong key, or unknown version. The phase-2 - // audit pipeline (ADR-0013) will turn this into a - // first-class audit event; for now we surface a - // structured Pino log so ops can spot it. - logger.warn( - { - event: 'session.decrypt_failed', - reason: err instanceof SessionDecryptError ? err.message : String(err), - }, - 'session', - ); - throw err; - } - }, - }, - }); - - return expressSession({ - name: sessionCookieName(), - secret, - // `crypto.randomBytes(32).toString('base64url')` ⇒ 256 - // bits of entropy in the session id, per ADR-0010 - // §"Confirmation". - genid: () => randomBytes(32).toString('base64url'), - store, - // `resave: false` — RedisStore.touch refreshes the TTL on - // every request; no need to rewrite the payload. - resave: false, - // `saveUninitialized: false` — don't create empty session - // keys for unauthenticated visitors browsing public - // routes. The session is born when `/auth/callback` - // populates it. - saveUninitialized: false, - // `rolling: true` — the cookie's `expires` slides forward - // on every response, matching the sliding-idle policy. - rolling: true, - cookie: sessionCookieOptions(timeouts.idleSeconds), - }); - }, + useFactory: (redis: Redis, logger: Logger): RequestHandler => + buildSessionMiddleware(redis, logger, { + cookieName: sessionCookieName(), + redisKeyPrefix: 'session:', + surfaceTag: 'user', + }), + }, + { + provide: ADMIN_SESSION_MIDDLEWARE, + inject: [REDIS_CLIENT, Logger], + useFactory: (redis: Redis, logger: Logger): RequestHandler => + buildSessionMiddleware(redis, logger, { + cookieName: adminSessionCookieName(), + redisKeyPrefix: 'session:admin:', + surfaceTag: 'admin', + }), }, ], - exports: [SESSION_MIDDLEWARE, SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE, UserSessionIndexService], + exports: [ + SESSION_MIDDLEWARE, + ADMIN_SESSION_MIDDLEWARE, + SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE, + UserSessionIndexService, + ], }) export class SessionModule {} diff --git a/apps/portal-bff/src/session/session.token.ts b/apps/portal-bff/src/session/session.token.ts index 622f35a..42d8459 100644 --- a/apps/portal-bff/src/session/session.token.ts +++ b/apps/portal-bff/src/session/session.token.ts @@ -1,17 +1,38 @@ import type { RequestHandler } from 'express'; /** - * DI token for the configured `express-session` middleware. Resolved + * DI token for the user-portal `express-session` middleware. Resolved * once at bootstrap (`main.ts`) and mounted with `app.use(...)` * after `cookie-parser` so cookies are already parsed when the * session middleware reads the session id. * + * Carries the `portal_session` / `__Host-portal_session` cookie and + * persists payloads under the `session:` Redis prefix. Bound to + * every path EXCEPT `/api/admin/*` by the dispatch in `main.ts`. + * * Usage: * const session = app.get(SESSION_MIDDLEWARE); * app.use(session); */ export const SESSION_MIDDLEWARE = 'SESSION_MIDDLEWARE'; +/** + * DI token for the admin-portal `express-session` middleware. Same + * underlying `express-session` + `connect-redis` plumbing as + * {@link SESSION_MIDDLEWARE}, but configured for the admin surface + * per ADR-0020 §"Sessions — distinct from `portal-shell`": + * + * - Cookie name: `portal_admin_session` / `__Host-portal_admin_session`. + * - Redis key prefix: `session:admin:`. + * - Same idle / absolute TTL policy (ADR-0010); admin tightening + * can come later through env without code changes. + * + * Bound to `/api/admin/*` only by the dispatch in `main.ts`. Signing + * into one surface does NOT sign the user into the other — Entra + * SSO at the IdP level still preserves a click-through experience. + */ +export const ADMIN_SESSION_MIDDLEWARE = 'ADMIN_SESSION_MIDDLEWARE'; + /** * DI token for the absolute-timeout middleware (per ADR-0010 §"TTL * policy"). Resolved at bootstrap and mounted in `main.ts`