diff --git a/apps/portal-bff/src/auth/auth.controller.spec.ts b/apps/portal-bff/src/auth/auth.controller.spec.ts index b24c67f..3ce2a02 100644 --- a/apps/portal-bff/src/auth/auth.controller.spec.ts +++ b/apps/portal-bff/src/auth/auth.controller.spec.ts @@ -31,19 +31,50 @@ const USER: AuthenticatedUser = { }; function makeResStub() { + const res = { + cookie: jest.fn(), + clearCookie: jest.fn(), + redirect: jest.fn(), + status: jest.fn(), + json: jest.fn(), + }; + // status/json/cookie/clearCookie/redirect all return the response + // in Express for chaining. Wire `mockReturnThis` after the object + // exists so `this` resolves to the same stub. + 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; +} + +interface SessionStub { + user?: AuthenticatedUser; + save: jest.Mock; + destroy: jest.Mock; +} + +function makeSessionStub(opts?: { + user?: AuthenticatedUser; + saveError?: Error; + destroyError?: Error; +}): SessionStub { return { - cookie: jest.fn().mockReturnThis(), - clearCookie: jest.fn().mockReturnThis(), - redirect: jest.fn().mockReturnThis(), - } as unknown as Response & { - cookie: jest.Mock; - clearCookie: jest.Mock; - redirect: jest.Mock; + ...(opts?.user !== undefined ? { user: opts.user } : {}), + save: jest.fn((cb: (err?: Error) => void) => cb(opts?.saveError)), + destroy: jest.fn((cb: (err?: Error) => void) => cb(opts?.destroyError)), }; } -function makeReqStub(signedCookies: Record = {}): Request { - return { signedCookies } as unknown as Request; +function makeReqStub(opts?: { + signedCookies?: Record; + session?: SessionStub; +}): Request & { session: SessionStub } { + return { + signedCookies: opts?.signedCookies ?? {}, + session: opts?.session ?? makeSessionStub(), + } as unknown as Request & { session: SessionStub }; } function makeLoggerStub() { @@ -52,7 +83,12 @@ function makeLoggerStub() { warn: jest.fn(), error: jest.fn(), debug: jest.fn(), - } as unknown as Logger & { log: jest.Mock; warn: jest.Mock }; + } as unknown as Logger & { + log: jest.Mock; + warn: jest.Mock; + error: jest.Mock; + debug: jest.Mock; + }; } interface ControllerFixture { @@ -62,13 +98,20 @@ interface ControllerFixture { logger: ReturnType; } +const LOGOUT_URL = `${ENTRA.authority}/oauth2/v2.0/logout?post_logout_redirect_uri=${encodeURIComponent(ENTRA.postLogoutRedirectUri)}`; + function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): ControllerFixture { 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 service = { beginAuthCodeFlow, completeAuthCodeFlow } as unknown as AuthService; + const buildLogoutUrl = jest.fn().mockReturnValue(LOGOUT_URL); + const service = { + beginAuthCodeFlow, + completeAuthCodeFlow, + buildLogoutUrl, + } as unknown as AuthService; const logger = makeLoggerStub(); return { controller: new AuthController(service, logger as unknown as Logger, ENTRA), @@ -124,15 +167,23 @@ describe('AuthController.login', () => { }); describe('AuthController.callback', () => { - it('clears the pre-auth cookie, logs success, redirects to the SPA on the happy path', async () => { + it('clears the pre-auth cookie, persists the user in the session, logs success, redirects to the SPA on the happy path', async () => { const { controller, completeAuthCodeFlow, logger } = makeController(); const res = makeResStub(); - const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }); + const session = makeSessionStub(); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + session, + }); 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); + // 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); + expect(session.save).toHaveBeenCalledTimes(1); expect(logger.log).toHaveBeenCalledWith( expect.objectContaining({ event: 'auth.signed_in', @@ -145,10 +196,27 @@ describe('AuthController.callback', () => { expect(res.redirect).toHaveBeenCalledWith(302, ENTRA.postLogoutRedirectUri); }); + it('propagates a session.save() error as an exception (so the SPA never sees a successful redirect with no session)', async () => { + const { controller } = makeController(); + const res = makeResStub(); + const session = makeSessionStub({ saveError: new Error('redis down') }); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + session, + }); + + await expect(controller.callback(req, res, 'auth-code', PRE_AUTH.state)).rejects.toThrow( + /redis down/, + ); + expect(res.redirect).not.toHaveBeenCalledWith(302, ENTRA.postLogoutRedirectUri); + }); + it('redirects with ?auth_error=token-exchange-failed when Entra returns error', async () => { const { controller, completeAuthCodeFlow } = makeController(); const res = makeResStub(); - const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + }); await controller.callback(req, res, undefined, undefined, 'access_denied', 'user cancelled'); @@ -163,7 +231,7 @@ describe('AuthController.callback', () => { it('redirects with ?auth_error=flow-expired when the pre-auth cookie is missing', async () => { const { controller, completeAuthCodeFlow, logger } = makeController(); const res = makeResStub(); - const req = makeReqStub({}); + const req = makeReqStub(); await controller.callback(req, res, 'code', 'state'); @@ -184,7 +252,9 @@ describe('AuthController.callback', () => { .mockRejectedValue(new AuthCodeFlowException({ kind: 'state-mismatch' })); const { controller, logger } = makeController({ completeAuthCodeFlow }); const res = makeResStub(); - const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + }); await controller.callback(req, res, 'code', 'state'); @@ -204,7 +274,9 @@ describe('AuthController.callback', () => { it('redirects with ?auth_error=token-exchange-failed when query is missing code/state', async () => { const { controller } = makeController(); const res = makeResStub(); - const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }); + const req = makeReqStub({ + signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) }, + }); await controller.callback(req, res); // no code, no state @@ -217,7 +289,7 @@ describe('AuthController.callback', () => { it('rejects a malformed pre-auth cookie as flow-expired', async () => { const { controller, completeAuthCodeFlow } = makeController(); const res = makeResStub(); - const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: 'not-json' }); + const req = makeReqStub({ signedCookies: { [PRE_AUTH_COOKIE_NAME]: 'not-json' } }); await controller.callback(req, res, 'code', 'state'); @@ -228,3 +300,127 @@ describe('AuthController.callback', () => { ); }); }); + +describe('AuthController.me', () => { + it('returns the curated public user when the session is populated', () => { + const { controller } = makeController(); + const res = makeResStub(); + const req = makeReqStub({ session: makeSessionStub({ user: USER }) }); + + controller.me(req, res); + + expect(res.status).not.toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ + oid: USER.oid, + tid: USER.tid, + username: USER.username, + displayName: USER.displayName, + }); + }); + + it('strips amr (internal claim) from the response payload', () => { + const { controller } = makeController(); + const res = makeResStub(); + const req = makeReqStub({ session: makeSessionStub({ user: USER }) }); + + controller.me(req, res); + + const payload = res.json.mock.calls[0]?.[0] as Record; + expect(payload).not.toHaveProperty('amr'); + }); + + it('returns 401 when no user is on the session', () => { + const { controller } = makeController(); + const res = makeResStub(); + const req = makeReqStub(); + + controller.me(req, res); + + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ error: 'unauthenticated' }); + }); +}); + +describe('AuthController.logout', () => { + it('destroys the session, clears the session cookie, logs success, redirects to Entra logout', async () => { + const { controller, logger } = makeController(); + const res = makeResStub(); + const session = makeSessionStub({ user: USER }); + const req = makeReqStub({ session }); + + await controller.logout(req, res); + + expect(session.destroy).toHaveBeenCalledTimes(1); + expect(res.clearCookie).toHaveBeenCalledWith('portal_session', { path: '/' }); + expect(logger.log).toHaveBeenCalledWith( + { event: 'auth.signed_out', wasAuthenticated: true }, + 'AuthLogout', + ); + // Authority + /oauth2/v2.0/logout?post_logout_redirect_uri=… + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining(`${ENTRA.authority}/oauth2/v2.0/logout`), + ); + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining( + `post_logout_redirect_uri=${encodeURIComponent(ENTRA.postLogoutRedirectUri)}`, + ), + ); + }); + + it('uses the __Host- prefixed cookie name in production', async () => { + const originalNodeEnv = process.env['NODE_ENV']; + try { + process.env['NODE_ENV'] = 'production'; + const { controller } = makeController(); + const res = makeResStub(); + const req = makeReqStub({ session: makeSessionStub({ user: USER }) }); + + await controller.logout(req, res); + + expect(res.clearCookie).toHaveBeenCalledWith('__Host-portal_session', { path: '/' }); + } finally { + if (originalNodeEnv === undefined) { + delete process.env['NODE_ENV']; + } else { + process.env['NODE_ENV'] = originalNodeEnv; + } + } + }); + + it('still clears the cookie and redirects when the user was already anonymous', async () => { + const { controller, logger } = makeController(); + const res = makeResStub(); + const session = makeSessionStub(); + const req = makeReqStub({ session }); + + await controller.logout(req, res); + + expect(session.destroy).toHaveBeenCalledTimes(1); + expect(logger.log).toHaveBeenCalledWith( + { event: 'auth.signed_out', wasAuthenticated: false }, + 'AuthLogout', + ); + expect(res.redirect).toHaveBeenCalled(); + }); + + it('logs but does not throw when session.destroy fails — still clears cookie and redirects', async () => { + const { controller, logger } = makeController(); + const res = makeResStub(); + const session = makeSessionStub({ user: USER, destroyError: new Error('redis down') }); + const req = makeReqStub({ session }); + + await controller.logout(req, res); + + expect(logger.error).toHaveBeenCalledWith( + expect.objectContaining({ + event: 'session.destroy_failed', + message: 'redis down', + }), + 'AuthLogout', + ); + expect(res.clearCookie).toHaveBeenCalled(); + 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 4552ee7..0789c00 100644 --- a/apps/portal-bff/src/auth/auth.controller.ts +++ b/apps/portal-bff/src/auth/auth.controller.ts @@ -1,21 +1,25 @@ import { Controller, Get, Inject, Query, Req, Res } from '@nestjs/common'; import type { Request, Response } from 'express'; import { Logger } from 'nestjs-pino'; +import { sessionCookieName } from '../session/session-cookie'; import { PRE_AUTH_COOKIE_NAME, clearPreAuthCookieOptions, preAuthCookieOptions, } from './auth.cookie'; import { AuthCodeFlowException, type AuthCodeFlowError, authErrorCode } from './auth.errors'; -import { AuthService, type PreAuthPayload } from './auth.service'; +import { AuthService, type AuthenticatedUser, type PreAuthPayload } from './auth.service'; import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token'; /** * OIDC routes mounted under `/api/auth/` per ADR-0009. * - * v1 ships two routes — `GET /login` (PR #105) and `GET /callback` - * (this PR). The next PR adds session persistence (Redis, - * ADR-0010); after that, `/me` and `/logout` close the loop. + * Routes shipped here: `GET /login`, `GET /callback`, `GET /me`, + * `GET /logout`. The callback persists the resolved identity into + * the express-session store (ADR-0010); `/me` reads it back; the + * logout endpoint destroys the session, clears the cookie, and + * redirects to Entra's RP-initiated logout endpoint so the user is + * signed out at the IdP too. */ @Controller('auth') export class AuthController { @@ -102,6 +106,13 @@ export class AuthController { try { const user = await this.authService.completeAuthCodeFlow(code, state, preAuth); + req.session.user = user; + // 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); this.logger.log( { event: 'auth.signed_in', @@ -112,8 +123,6 @@ export class AuthController { }, 'AuthCallback', ); - // No session persistence yet — next PR. SPA will see the user - // as anonymous on the landing page. res.redirect(302, this.entra.postLogoutRedirectUri); } catch (err) { if (err instanceof AuthCodeFlowException) { @@ -124,6 +133,65 @@ export class AuthController { } } + /** + * Read-only view of the current session for the SPA. Returns the + * curated subset of the resolved identity — `amr` and other + * auth-internal claims stay server-side. + * + * 401 (with no body beyond a `{error}` tag) on missing session + * lets the SPA distinguish "anonymous" from any other failure; + * the browser's session cookie was either absent, expired, or + * pointed at a Redis key that no longer exists. + */ + @Get('me') + me(@Req() req: Request, @Res() res: Response): void { + const user = req.session.user; + if (!user) { + res.status(401).json({ error: 'unauthenticated' }); + return; + } + res.json(toPublicUser(user)); + } + + /** + * RP-initiated logout per ADR-0009. Destroys the BFF session + * (DEL on the Redis key), clears the session cookie, then + * redirects the browser to Entra's `/oauth2/v2.0/logout` so the + * IdP-side session is killed too — single sign-out behaviour. The + * post-logout redirect on the Entra end lands the user back on + * the SPA root. + * + * Idempotent: if the user was already anonymous, the destroy is + * a no-op and we redirect anyway. CSRF for v1 relies on + * SameSite=Lax (subresource requests don't carry the cookie); a + * dedicated CSRF middleware lands with phase-2 security. + */ + @Get('logout') + async logout(@Req() req: Request, @Res() res: Response): Promise { + const wasAuthenticated = Boolean(req.session.user); + const logoutUrl = this.authService.buildLogoutUrl(); + + 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', + ); + } + + res.clearCookie(sessionCookieName(), { path: '/' }); + this.logger.log({ event: 'auth.signed_out', wasAuthenticated }, 'AuthLogout'); + res.redirect(302, logoutUrl); + } + private redirectWithError(res: Response, kind: AuthCodeFlowError['kind']): void { const url = new URL(this.entra.postLogoutRedirectUri); url.searchParams.set('auth_error', authErrorCode({ kind } as AuthCodeFlowError)); @@ -131,6 +199,34 @@ export class AuthController { } } +interface PublicUser { + oid: string; + tid: string; + username: string; + displayName: string; +} + +function toPublicUser(user: AuthenticatedUser): PublicUser { + return { + oid: user.oid, + tid: user.tid, + username: user.username, + displayName: user.displayName, + }; +} + +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.service.spec.ts b/apps/portal-bff/src/auth/auth.service.spec.ts index 109ad7e..73f82b4 100644 --- a/apps/portal-bff/src/auth/auth.service.spec.ts +++ b/apps/portal-bff/src/auth/auth.service.spec.ts @@ -175,3 +175,23 @@ 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()); + 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()); + 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()); + 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 eb8ad72..6f4e9ef 100644 --- a/apps/portal-bff/src/auth/auth.service.ts +++ b/apps/portal-bff/src/auth/auth.service.ts @@ -148,6 +148,24 @@ export class AuthService { return this.toAuthenticatedUser(result); } + /** + * Build the Entra RP-initiated logout URL. Hits the v2.0 logout + * endpoint on the configured authority and passes the + * `post_logout_redirect_uri` so the browser lands back on the SPA + * after Entra finishes its side of the sign-out. + * + * We deliberately do *not* pass `id_token_hint` (which would skip + * the account-picker on multi-account devices) because v1 doesn't + * persist the id_token in the session yet — token persistence + * lands with downstream API support per ADR-0014. The account + * picker is the safer default in the interim. + */ + buildLogoutUrl(): string { + const url = new URL(`${this.config.authority}/oauth2/v2.0/logout`); + url.searchParams.set('post_logout_redirect_uri', this.config.postLogoutRedirectUri); + return url.toString(); + } + private toAuthenticatedUser(result: AuthenticationResult): AuthenticatedUser { const claims = result.idTokenClaims as Record; diff --git a/apps/portal-bff/src/session/session.module.ts b/apps/portal-bff/src/session/session.module.ts index 886129e..f24f34e 100644 --- a/apps/portal-bff/src/session/session.module.ts +++ b/apps/portal-bff/src/session/session.module.ts @@ -11,6 +11,9 @@ import { adaptIoredisForConnectRedis } from './ioredis-connect-redis-adapter'; import { SessionDecryptError, decrypt, encrypt } from './session-crypto'; import { readSessionTimeouts, sessionCookieName, sessionCookieOptions } from './session-cookie'; import { SESSION_MIDDLEWARE, type RequestHandler } from './session.token'; +// Side-effect import: brings the `req.session.user` declaration +// merging into the compile graph for every consumer of this module. +import './session.types'; /** * Session module — wires `express-session` with a `connect-redis` diff --git a/apps/portal-bff/src/session/session.types.ts b/apps/portal-bff/src/session/session.types.ts new file mode 100644 index 0000000..eb4efb2 --- /dev/null +++ b/apps/portal-bff/src/session/session.types.ts @@ -0,0 +1,26 @@ +import type { AuthenticatedUser } from '../auth/auth.service'; + +/** + * Module augmentation: extends `express-session`'s `SessionData` + * with the BFF-specific fields we persist on the session. + * + * Today: just `user` — the identity resolved by `/auth/callback`. + * Later (per ADR-0010 §"Session payload"): the encrypted `tokens` + * blob, `audience`, `createdAt`, `absoluteExpiresAt`, etc. Each new + * field lands as a new entry in this interface so consumers get + * compile-time visibility. + * + * The file is imported (for its side-effect) by `session.module.ts` + * so the augmentation is always in the TypeScript compile graph — + * `req.session.user` would otherwise be `unknown` at every call + * site. + */ +declare module 'express-session' { + interface SessionData { + user?: AuthenticatedUser; + } +} + +// The empty export turns this file into a module, which is what +// declaration merging on `express-session` requires. +export {};