diff --git a/apps/portal-bff/src/auth/auth.controller.spec.ts b/apps/portal-bff/src/auth/auth.controller.spec.ts index 90d0cef..b24c67f 100644 --- a/apps/portal-bff/src/auth/auth.controller.spec.ts +++ b/apps/portal-bff/src/auth/auth.controller.spec.ts @@ -1,7 +1,20 @@ -import type { Response } from 'express'; +import type { Request, Response } from 'express'; +import type { Logger } from 'nestjs-pino'; import { AuthController } from './auth.controller'; import { PRE_AUTH_COOKIE_NAME, PRE_AUTH_COOKIE_TTL_MS } from './auth.cookie'; -import type { AuthService, PreAuthPayload } from './auth.service'; +import { AuthCodeFlowException } from './auth.errors'; +import type { AuthService, AuthenticatedUser, PreAuthPayload } from './auth.service'; +import type { EntraConfig } from './entra-config.token'; + +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/', + authority: 'https://login.microsoftonline.com/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', +}; const PRE_AUTH: PreAuthPayload = { state: 'state-nonce', @@ -9,28 +22,65 @@ const PRE_AUTH: PreAuthPayload = { createdAt: 1_000, }; +const USER: AuthenticatedUser = { + oid: 'user-oid', + tid: ENTRA.tenantId, + username: 'jane.doe@apf.example', + displayName: 'Jane Doe', + amr: ['pwd', 'mfa'], +}; + function makeResStub() { return { cookie: jest.fn().mockReturnThis(), + clearCookie: jest.fn().mockReturnThis(), redirect: jest.fn().mockReturnThis(), - } as unknown as Response & { cookie: jest.Mock; redirect: jest.Mock }; + } as unknown as Response & { + cookie: jest.Mock; + clearCookie: jest.Mock; + redirect: jest.Mock; + }; } -describe('AuthController', () => { - let beginAuthCodeFlow: jest.Mock; - let service: AuthService; - let controller: AuthController; +function makeReqStub(signedCookies: Record = {}): Request { + return { signedCookies } as unknown as Request; +} - beforeEach(() => { - beginAuthCodeFlow = jest.fn().mockResolvedValue({ - authUrl: 'https://entra.example/authorize?state=state-nonce', - preAuthPayload: PRE_AUTH, - }); - service = { beginAuthCodeFlow } as unknown as AuthService; - controller = new AuthController(service); +function makeLoggerStub() { + return { + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + } as unknown as Logger & { log: jest.Mock; warn: jest.Mock }; +} + +interface ControllerFixture { + controller: AuthController; + beginAuthCodeFlow: jest.Mock; + completeAuthCodeFlow: jest.Mock; + logger: ReturnType; +} + +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 logger = makeLoggerStub(); + return { + controller: new AuthController(service, logger as unknown as Logger, ENTRA), + beginAuthCodeFlow, + completeAuthCodeFlow, + logger, + }; +} +describe('AuthController.login', () => { it('writes the pre-auth cookie and 302s to the Entra auth URL', async () => { + const { controller } = makeController(); const res = makeResStub(); await controller.login(res); @@ -57,6 +107,7 @@ describe('AuthController', () => { const originalNodeEnv = process.env['NODE_ENV']; try { process.env['NODE_ENV'] = 'production'; + const { controller } = makeController(); const res = makeResStub(); await controller.login(res); const call = res.cookie.mock.calls[0] ?? []; @@ -71,3 +122,109 @@ describe('AuthController', () => { } }); }); + +describe('AuthController.callback', () => { + it('clears the pre-auth cookie, 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) }); + + 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(logger.log).toHaveBeenCalledWith( + expect.objectContaining({ + event: 'auth.signed_in', + oid: USER.oid, + username: USER.username, + amr: USER.amr, + }), + 'AuthCallback', + ); + expect(res.redirect).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) }); + + await controller.callback(req, res, undefined, undefined, 'access_denied', 'user cancelled'); + + expect(completeAuthCodeFlow).not.toHaveBeenCalled(); + expect(res.clearCookie).toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining('auth_error=token-exchange-failed'), + ); + }); + + 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({}); + + await controller.callback(req, res, 'code', 'state'); + + expect(completeAuthCodeFlow).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ event: 'auth.no_pre_auth_cookie' }), + 'AuthCallback', + ); + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining('auth_error=flow-expired'), + ); + }); + + it('redirects with the typed error from AuthCodeFlowException', async () => { + const completeAuthCodeFlow = jest + .fn() + .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) }); + + await controller.callback(req, res, 'code', 'state'); + + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ + event: 'auth.flow_error', + failure: { kind: 'state-mismatch' }, + }), + 'AuthCallback', + ); + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining('auth_error=state-mismatch'), + ); + }); + + 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) }); + + await controller.callback(req, res); // no code, no state + + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining('auth_error=token-exchange-failed'), + ); + }); + + 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' }); + + await controller.callback(req, res, 'code', 'state'); + + expect(completeAuthCodeFlow).not.toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith( + 302, + expect.stringContaining('auth_error=flow-expired'), + ); + }); +}); diff --git a/apps/portal-bff/src/auth/auth.controller.ts b/apps/portal-bff/src/auth/auth.controller.ts index a3deeda..4552ee7 100644 --- a/apps/portal-bff/src/auth/auth.controller.ts +++ b/apps/portal-bff/src/auth/auth.controller.ts @@ -1,19 +1,29 @@ -import { Controller, Get, Res } from '@nestjs/common'; -import type { Response } from 'express'; -import { AuthService } from './auth.service'; -import { PRE_AUTH_COOKIE_NAME, preAuthCookieOptions } from './auth.cookie'; +import { Controller, Get, Inject, Query, Req, Res } from '@nestjs/common'; +import type { Request, Response } from 'express'; +import { Logger } from 'nestjs-pino'; +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 { ENTRA_CONFIG, type EntraConfig } from './entra-config.token'; /** * OIDC routes mounted under `/api/auth/` per ADR-0009. * - * v1 ships one route: `GET /login`, the entry point of the - * Authorization Code + PKCE flow. The follow-up PR adds - * `/callback`, then `/me` and `/logout` once Redis-backed sessions - * land (ADR-0010). + * 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. */ @Controller('auth') export class AuthController { - constructor(private readonly authService: AuthService) {} + constructor( + private readonly authService: AuthService, + private readonly logger: Logger, + @Inject(ENTRA_CONFIG) private readonly entra: EntraConfig, + ) {} /** * Starts the auth flow. Generates a fresh state + PKCE pair via @@ -31,4 +41,116 @@ export class AuthController { res.cookie(PRE_AUTH_COOKIE_NAME, JSON.stringify(preAuthPayload), preAuthCookieOptions()); res.redirect(302, authUrl); } + + /** + * Second leg of the OIDC flow. Entra redirects the user here with + * `?code=…&state=…` (success) or `?error=…&error_description=…` + * (failure). The pre-auth cookie set by `/login` carries the + * matching state + PKCE verifier so we can verify the round-trip + * and finish the token exchange. + * + * On success, the user identity is logged via Pino and the + * browser is redirected to the SPA. No session is persisted yet — + * that's the next PR (Redis-backed sessions per ADR-0010); until + * then the SPA cannot tell the user is "logged in". + * + * On failure (any branch — state mismatch, expired cookie, + * missing `amr` per ADR-0011, MSAL token-exchange error, or + * Entra-side error from the query) the user lands on the SPA + * with `?auth_error=` so the front-end can surface a + * specific message. The pre-auth cookie is cleared on every + * exit path: it is single-use by design. + */ + @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 { + // Drop the single-use cookie regardless of outcome. + res.clearCookie(PRE_AUTH_COOKIE_NAME, clearPreAuthCookieOptions()); + + // Entra signalled a failure (e.g. user cancelled the consent + // screen, MFA challenge failed, app-registration mismatch). + if (entraError) { + this.logger.warn( + { + event: 'auth.entra_error', + entraError, + entraErrorDescription, + }, + 'AuthCallback', + ); + return this.redirectWithError(res, 'token-exchange-failed'); + } + + if (typeof code !== 'string' || typeof state !== 'string') { + return this.redirectWithError(res, 'token-exchange-failed'); + } + + const preAuth = readPreAuthCookie(req); + if (!preAuth) { + // No cookie → either the user opened /callback directly, or + // their browser dropped the cookie (TTL elapsed in + // a 3rd-party-cookie blocker, etc.). Treat as flow-expired. + this.logger.warn({ event: 'auth.no_pre_auth_cookie' }, 'AuthCallback'); + return this.redirectWithError(res, 'flow-expired'); + } + + try { + const user = await this.authService.completeAuthCodeFlow(code, state, preAuth); + this.logger.log( + { + event: 'auth.signed_in', + oid: user.oid, + tid: user.tid, + username: user.username, + amr: user.amr, + }, + '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) { + this.logger.warn({ event: 'auth.flow_error', failure: err.failure }, 'AuthCallback'); + return this.redirectWithError(res, err.failure.kind); + } + throw err; + } + } + + private redirectWithError(res: Response, kind: AuthCodeFlowError['kind']): void { + const url = new URL(this.entra.postLogoutRedirectUri); + 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 { + const parsed = JSON.parse(raw) as Partial; + if ( + typeof parsed.state === 'string' && + typeof parsed.codeVerifier === 'string' && + typeof parsed.createdAt === 'number' + ) { + return { + state: parsed.state, + codeVerifier: parsed.codeVerifier, + createdAt: parsed.createdAt, + }; + } + return null; + } catch { + return null; + } } diff --git a/apps/portal-bff/src/auth/auth.cookie.ts b/apps/portal-bff/src/auth/auth.cookie.ts index a9c8c18..330489e 100644 --- a/apps/portal-bff/src/auth/auth.cookie.ts +++ b/apps/portal-bff/src/auth/auth.cookie.ts @@ -43,3 +43,21 @@ export function preAuthCookieOptions(): CookieOptions { maxAge: PRE_AUTH_COOKIE_TTL_MS, }; } + +/** + * Options for `res.clearCookie(PRE_AUTH_COOKIE_NAME, …)` — must + * mirror everything but `maxAge` from `preAuthCookieOptions()` so + * the browser actually drops the cookie. Browsers match cookies by + * (name, domain, path) — getting path / sameSite / secure wrong + * leaves the old cookie in place. + */ +export function clearPreAuthCookieOptions(): CookieOptions { + const isProduction = process.env['NODE_ENV'] === 'production'; + return { + signed: true, + httpOnly: true, + sameSite: 'lax', + secure: isProduction, + path: '/', + }; +} diff --git a/apps/portal-bff/src/auth/auth.errors.ts b/apps/portal-bff/src/auth/auth.errors.ts new file mode 100644 index 0000000..3a11501 --- /dev/null +++ b/apps/portal-bff/src/auth/auth.errors.ts @@ -0,0 +1,29 @@ +/** + * Typed failure modes of the OIDC callback. The controller maps each + * to a query-param identifier on the SPA redirect so the front-end + * can render an appropriate message without inventing strings. + * + * Keep this list flat — the controller's `switch` is exhaustive + * thanks to TypeScript's narrowing on `kind`. + */ +export type AuthCodeFlowError = + | { kind: 'state-mismatch' } + | { kind: 'flow-expired' } + | { kind: 'amr-missing' } + | { kind: 'token-exchange-failed'; cause: string }; + +export class AuthCodeFlowException extends Error { + constructor(readonly failure: AuthCodeFlowError) { + super(`auth code flow failed: ${failure.kind}`); + this.name = 'AuthCodeFlowException'; + } +} + +/** + * Stable short codes used as `?auth_error=` on the SPA-bound + * redirect. Same identifiers as the discriminant `kind` so log + * grepping correlates trivially. + */ +export function authErrorCode(failure: AuthCodeFlowError): string { + return failure.kind; +} diff --git a/apps/portal-bff/src/auth/auth.service.spec.ts b/apps/portal-bff/src/auth/auth.service.spec.ts index c4b414d..74e3010 100644 --- a/apps/portal-bff/src/auth/auth.service.spec.ts +++ b/apps/portal-bff/src/auth/auth.service.spec.ts @@ -1,5 +1,7 @@ -import type { ConfidentialClientApplication } from '@azure/msal-node'; -import { AuthService } from './auth.service'; +import type { AuthenticationResult, ConfidentialClientApplication } from '@azure/msal-node'; +import { PRE_AUTH_COOKIE_TTL_MS } from './auth.cookie'; +import { AuthCodeFlowException } from './auth.errors'; +import { AuthService, type PreAuthPayload } from './auth.service'; import type { EntraConfig } from './entra-config.token'; const ENTRA: EntraConfig = { @@ -12,17 +14,54 @@ const ENTRA: EntraConfig = { authority: 'https://login.microsoftonline.com/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', }; -describe('AuthService', () => { - let getAuthCodeUrl: jest.Mock, [unknown]>; - let service: AuthService; +interface ServiceFixture { + service: AuthService; + getAuthCodeUrl: jest.Mock; + acquireTokenByCode: jest.Mock; +} - beforeEach(() => { - getAuthCodeUrl = jest.fn().mockResolvedValue('https://entra.example/authorize?…'); - const msalStub = { getAuthCodeUrl } as unknown as ConfidentialClientApplication; - service = new AuthService(msalStub, ENTRA); - }); +function makeService(overrides?: { acquireTokenByCode?: jest.Mock }): ServiceFixture { + const getAuthCodeUrl = jest.fn().mockResolvedValue('https://entra.example/authorize?…'); + const acquireTokenByCode = + overrides?.acquireTokenByCode ?? + jest.fn().mockResolvedValue(makeAuthResult({ amr: ['pwd', 'mfa'] })); + const msalStub = { + getAuthCodeUrl, + acquireTokenByCode, + } as unknown as ConfidentialClientApplication; + return { service: new AuthService(msalStub, ENTRA), getAuthCodeUrl, acquireTokenByCode }; +} +function makeAuthResult( + claims: Partial<{ + amr: string[]; + oid: string; + tid: string; + name: string; + preferred_username: string; + }>, +): AuthenticationResult { + return { + idTokenClaims: { + oid: claims.oid ?? 'user-oid', + tid: claims.tid ?? ENTRA.tenantId, + name: claims.name ?? 'Jane Doe', + preferred_username: claims.preferred_username ?? 'jane.doe@apf.example', + amr: claims.amr ?? ['pwd', 'mfa'], + }, + account: { username: 'jane.doe@apf.example', name: 'Jane Doe' }, + } as unknown as AuthenticationResult; +} + +const PRE_AUTH_OK: PreAuthPayload = { + state: 'state-nonce', + codeVerifier: 'verifier-secret', + createdAt: 1_000_000, +}; + +describe('AuthService.beginAuthCodeFlow', () => { it('builds the auth URL with the configured redirect, OIDC scopes, S256 challenge', async () => { + const { service, getAuthCodeUrl } = makeService(); await service.beginAuthCodeFlow(); expect(getAuthCodeUrl).toHaveBeenCalledTimes(1); const arg = getAuthCodeUrl.mock.calls[0]?.[0] as Record; @@ -30,30 +69,102 @@ describe('AuthService', () => { expect(arg['scopes']).toEqual(['openid', 'profile', 'email']); expect(arg['codeChallengeMethod']).toBe('S256'); expect(typeof arg['codeChallenge']).toBe('string'); - expect((arg['codeChallenge'] as string).length).toBeGreaterThan(20); expect(typeof arg['state']).toBe('string'); - expect((arg['state'] as string).length).toBeGreaterThan(8); }); it('returns the pre-auth payload with state + codeVerifier matching what MSAL was called with', async () => { - const { authUrl, preAuthPayload } = await service.beginAuthCodeFlow(); - expect(authUrl).toBe('https://entra.example/authorize?…'); - + const { service, getAuthCodeUrl } = makeService(); + const { preAuthPayload } = await service.beginAuthCodeFlow(); const arg = getAuthCodeUrl.mock.calls[0]?.[0] as Record; expect(preAuthPayload.state).toBe(arg['state']); - // The verifier returned to the caller is the secret the callback - // will send back to Entra; the challenge sent to Entra is its - // SHA-256-of-verifier transform, so the two must differ. expect(preAuthPayload.codeVerifier).not.toBe(arg['codeChallenge']); - expect(typeof preAuthPayload.codeVerifier).toBe('string'); - expect(preAuthPayload.codeVerifier.length).toBeGreaterThan(40); expect(preAuthPayload.createdAt).toBeLessThanOrEqual(Date.now()); }); - it('generates a fresh state + verifier on every call (replay protection)', async () => { + it('generates a fresh state + verifier on every call', async () => { + const { service } = makeService(); const a = await service.beginAuthCodeFlow(); const b = await service.beginAuthCodeFlow(); expect(a.preAuthPayload.state).not.toBe(b.preAuthPayload.state); expect(a.preAuthPayload.codeVerifier).not.toBe(b.preAuthPayload.codeVerifier); }); }); + +describe('AuthService.completeAuthCodeFlow', () => { + it('returns the authenticated user on the happy path', async () => { + const { service, acquireTokenByCode } = makeService(); + const user = await service.completeAuthCodeFlow( + 'auth-code', + PRE_AUTH_OK.state, + PRE_AUTH_OK, + PRE_AUTH_OK.createdAt + 1_000, + ); + expect(acquireTokenByCode).toHaveBeenCalledWith({ + code: 'auth-code', + codeVerifier: PRE_AUTH_OK.codeVerifier, + redirectUri: ENTRA.redirectUri, + scopes: ['openid', 'profile', 'email'], + }); + expect(user).toEqual({ + oid: 'user-oid', + tid: ENTRA.tenantId, + username: 'jane.doe@apf.example', + displayName: 'Jane Doe', + amr: ['pwd', 'mfa'], + }); + }); + + 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), + ).rejects.toMatchObject({ failure: { kind: 'state-mismatch' } }); + }); + + it('throws flow-expired when the cookie is older than the TTL', async () => { + 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), + ).rejects.toMatchObject({ failure: { kind: 'flow-expired' } }); + }); + + it('throws amr-missing when the ID token has no amr claim', async () => { + const acquireTokenByCode = jest.fn().mockResolvedValue(makeAuthResult({ amr: [] })); + const { service } = makeService({ acquireTokenByCode }); + await expect( + service.completeAuthCodeFlow('code', PRE_AUTH_OK.state, PRE_AUTH_OK, PRE_AUTH_OK.createdAt), + ).rejects.toMatchObject({ failure: { kind: 'amr-missing' } }); + }); + + it('throws token-exchange-failed when MSAL throws', async () => { + 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), + ).rejects.toMatchObject({ + failure: { kind: 'token-exchange-failed', cause: 'AADSTS70008' }, + }); + }); + + it('throws token-exchange-failed when MSAL returns null', async () => { + 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), + ).rejects.toBeInstanceOf(AuthCodeFlowException); + }); + + it('throws token-exchange-failed when oid claim is missing', async () => { + const acquireTokenByCode = jest.fn().mockResolvedValue({ + idTokenClaims: { tid: ENTRA.tenantId, amr: ['mfa'] }, + account: { username: '', name: '' }, + } as unknown as AuthenticationResult); + const { service } = makeService({ acquireTokenByCode }); + await expect( + service.completeAuthCodeFlow('code', PRE_AUTH_OK.state, PRE_AUTH_OK, PRE_AUTH_OK.createdAt), + ).rejects.toMatchObject({ + failure: { kind: 'token-exchange-failed', cause: /oid/ }, + }); + }); +}); diff --git a/apps/portal-bff/src/auth/auth.service.ts b/apps/portal-bff/src/auth/auth.service.ts index 2e082b5..6d00e35 100644 --- a/apps/portal-bff/src/auth/auth.service.ts +++ b/apps/portal-bff/src/auth/auth.service.ts @@ -1,5 +1,11 @@ -import { ConfidentialClientApplication, CryptoProvider } from '@azure/msal-node'; +import { + ConfidentialClientApplication, + CryptoProvider, + type AuthenticationResult, +} from '@azure/msal-node'; import { Inject, Injectable } from '@nestjs/common'; +import { PRE_AUTH_COOKIE_TTL_MS } from './auth.cookie'; +import { AuthCodeFlowException } from './auth.errors'; import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token'; import { MSAL_CLIENT } from './msal-client.token'; @@ -21,6 +27,25 @@ export interface AuthCodeFlowStart { readonly preAuthPayload: PreAuthPayload; } +/** + * Identity resolved by `completeAuthCodeFlow`. Carries the claims + * the rest of the BFF / SPA care about post-authentication: + * - `oid`: stable per-user object id inside the Entra tenant. + * - `tid`: tenant id the user authenticated against (for the + * dual-audience / multi-tenant logic to come). + * - `username` / `displayName`: surfaced in the UI. + * - `amr`: authentication methods reference — the array used by + * the BFF's MFA sanity-check (ADR-0011) and by `@RequireMfa` + * freshness checks once the session lands. + */ +export interface AuthenticatedUser { + readonly oid: string; + readonly tid: string; + readonly username: string; + readonly displayName: string; + readonly amr: readonly string[]; +} + /** * The minimum OIDC scopes — `openid` to get an ID token, `profile` * for the user's display name / preferred_username, `email` for the @@ -71,4 +96,101 @@ export class AuthService { }, }; } + + /** + * Second leg. Verifies the state round-trip, refuses cookies + * older than the flow TTL, asks MSAL to exchange the auth code + * for tokens using the stored PKCE verifier, then runs the + * BFF-side sanity-checks ADR-0009 mandates (state, expiry, `amr`). + * + * MSAL Node performs the heavy ID-token validation itself + * (signature against Entra's JWKS, issuer + audience, exp, nbf). + * What this method adds on top is the application-layer policy: + * - anti-CSRF state binding, + * - flow-TTL replay protection, + * - MFA presence (`amr`) per ADR-0011. + */ + async completeAuthCodeFlow( + code: string, + state: string, + preAuth: PreAuthPayload, + now: number = Date.now(), + ): Promise { + if (state !== preAuth.state) { + throw new AuthCodeFlowException({ kind: 'state-mismatch' }); + } + if (now - preAuth.createdAt > PRE_AUTH_COOKIE_TTL_MS) { + throw new AuthCodeFlowException({ kind: 'flow-expired' }); + } + + let result: AuthenticationResult | null; + try { + result = await this.msal.acquireTokenByCode({ + code, + codeVerifier: preAuth.codeVerifier, + redirectUri: this.config.redirectUri, + scopes: [...SCOPES], + }); + } catch (err) { + throw new AuthCodeFlowException({ + kind: 'token-exchange-failed', + cause: errorCause(err), + }); + } + + if (!result) { + throw new AuthCodeFlowException({ + kind: 'token-exchange-failed', + cause: 'msal returned null result', + }); + } + + return this.toAuthenticatedUser(result); + } + + private toAuthenticatedUser(result: AuthenticationResult): AuthenticatedUser { + const claims = result.idTokenClaims as Record; + const amr = Array.isArray(claims['amr']) + ? (claims['amr'] as unknown[]).filter((v): v is string => typeof v === 'string') + : []; + + // ADR-0011 sanity-check: Conditional Access on the org side is + // the actual enforcement layer; the BFF refuses tokens that + // lack any `amr` evidence (an empty array would indicate a + // policy misconfiguration where MFA never fired). + if (amr.length === 0) { + throw new AuthCodeFlowException({ kind: 'amr-missing' }); + } + + return { + oid: requireString(claims['oid'], 'oid'), + tid: requireString(claims['tid'], 'tid'), + username: + typeof claims['preferred_username'] === 'string' + ? (claims['preferred_username'] as string) + : (result.account?.username ?? ''), + displayName: + typeof claims['name'] === 'string' + ? (claims['name'] as string) + : (result.account?.name ?? ''), + amr, + }; + } +} + +function requireString(value: unknown, claim: string): string { + if (typeof value !== 'string' || value === '') { + throw new AuthCodeFlowException({ + kind: 'token-exchange-failed', + cause: `id token missing required string claim: ${claim}`, + }); + } + return value; +} + +function errorCause(err: unknown): string { + if (err instanceof Error) { + return err.message; + } + return String(err); }