feat(portal-bff): /auth/callback route — token exchange + amr check
Fourth step of ADR-0009 wiring. Closes the OIDC round-trip on the
BFF side (modulo session persistence — that's the next PR per
ADR-0010). Entra redirects the user back to `/api/auth/callback`
with a `code` + `state`; the BFF verifies the state, exchanges the
code for tokens via MSAL Node's `acquireTokenByCode`, runs the
ADR-0011 `amr` sanity-check, logs the resolved identity to Pino,
clears the single-use pre-auth cookie, and 302s the user back to
the SPA.
What lands:
- `apps/portal-bff/src/auth/auth.errors.ts` — discriminated-union
`AuthCodeFlowError` (state-mismatch / flow-expired / amr-missing
/ token-exchange-failed) + `AuthCodeFlowException` wrapper. The
`kind` field doubles as the `?auth_error=<code>` query param on
the SPA-bound redirect, so the front-end can render an exact
message without duplicating the string set.
- `AuthService.completeAuthCodeFlow(code, state, preAuth, now?)` —
verifies state binding, refuses cookies older than the 5-minute
flow TTL, calls MSAL, validates `amr` is non-empty (ADR-0011 BFF
sanity-check; Conditional Access on the org side is the real
enforcement), extracts the four `oid` / `tid` /
`preferred_username` / `name` claims plus the `amr` array into
an `AuthenticatedUser` shape.
- `auth.cookie.ts` gains `clearPreAuthCookieOptions()` mirroring
the set-options minus `maxAge` so the browser actually drops the
cookie (cookies match by name + path + secure; getting any of
those wrong leaves the old cookie in place).
- `AuthController.callback()` — `@Get('callback')`:
1. Always `res.clearCookie(...)` first. The cookie is single-use
by design; a parallel /login overlap should not survive.
2. Bail on Entra-side errors (`?error=`), missing query params,
missing or malformed pre-auth cookie. Each branch logs a
structured Pino warning and redirects with the right
`auth_error` code.
3. Call the service. On `AuthCodeFlowException`, log + redirect
with the typed `kind`.
4. On success, log a `auth.signed_in` event with `oid`, `tid`,
`username`, `amr` (PII-sensitive bits only; no tokens), then
302 to `entra.postLogoutRedirectUri` (reused as the SPA root
URL — a dedicated `SPA_BASE_URL` env var lands if the two
URLs ever need to diverge).
- The controller now takes `Logger` + `ENTRA_CONFIG` via DI
alongside `AuthService`.
Verification:
- `nx run-many -t lint test build --projects=portal-bff` — green.
- 52/52 specs (was 39; +13 across the new service-completeFlow spec
branches and the controller-callback spec branches). Service
spec covers happy path + 6 failure modes (state mismatch, flow
expired, amr missing, MSAL throws, MSAL returns null, oid claim
missing). Controller spec covers happy redirect, Entra error
branch, missing cookie, AuthCodeFlowException branch, missing
query, malformed cookie.
What this PR explicitly does NOT do:
- Persist a session. The user is "authenticated" in the BFF's
point of view (we have their identity) but the next request lands
anonymous. Closes in the Redis sessions PR (ADR-0010).
- Audit log entry. The audit module is in place; wiring the
`auth.signed_in` event into it lands alongside sessions so the
audit row carries a `session_id`.
- Logout / `/me`. Land after sessions.
This commit is contained in:
@@ -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<string, unknown> = {}): Request {
|
||||
return { signedCookies } as unknown as Request;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
beginAuthCodeFlow = jest.fn().mockResolvedValue({
|
||||
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<typeof makeLoggerStub>;
|
||||
}
|
||||
|
||||
function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): ControllerFixture {
|
||||
const 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);
|
||||
});
|
||||
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'),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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=<code>` 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<void> {
|
||||
// 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<string, unknown>)[PRE_AUTH_COOKIE_NAME];
|
||||
if (typeof raw !== 'string') {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const parsed = JSON.parse(raw) as Partial<PreAuthPayload>;
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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: '/',
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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=<code>` 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;
|
||||
}
|
||||
@@ -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<Promise<string>, [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<string, unknown>;
|
||||
@@ -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<string, unknown>;
|
||||
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/ },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<AuthenticatedUser> {
|
||||
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<string, unknown>;
|
||||
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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user