feat(portal-bff): distinct admin session + /api/admin/auth flow (#129)
CI / commits (push) Has been skipped
CI / scan (push) Successful in 2m13s
CI / check (push) Successful in 2m27s
CI / a11y (push) Successful in 57s
CI / perf (push) Successful in 3m55s

## Summary

Phase-3a step per [ADR-0020](docs/decisions/0020-portal-admin-app.md) §"Sessions — distinct from `portal-shell`". Wires a second `express-session` middleware on `/api/admin/*` carrying `__Host-portal_admin_session` over Redis prefix `session:admin:`, and ships the parallel `/api/admin/auth/{login,callback,me,logout}` flow that populates it. Signing in to one surface no longer signs the user into the other — Entra SSO at the IdP level still preserves the click-through.

## What lands

### Session middlewares — path-routed dispatch

| Token | Cookie | Redis prefix | Bound to |
| --- | --- | --- | --- |
| `SESSION_MIDDLEWARE` | `portal_session` / `__Host-portal_session` | `session:` | every path **except** `/api/admin/*` |
| `ADMIN_SESSION_MIDDLEWARE` | `portal_admin_session` / `__Host-portal_admin_session` | `session:admin:` | `/api/admin/*` only |

Implemented via a `buildSessionMiddleware(redis, logger, opts)` factory in [session.module.ts](apps/portal-bff/src/session/session.module.ts) — the TTL policy, encryption key, signing secret, session-id entropy, and serializer error-handling all come from the same source. Only the cookie name + Redis key prefix differ.

The dispatch in [main.ts](apps/portal-bff/src/main.ts) is a tiny `(req, res, next) => req.path.startsWith('/api/admin') ? adminSession(...) : userSession(...)`. Running both middlewares unconditionally would have the second overwrite `req.session` from the first, collapsing the two surfaces.

### Distinct admin auth flow

[`AdminAuthController`](apps/portal-bff/src/admin/admin-auth.controller.ts) mounts `/api/admin/auth/{login,callback,me,logout}`. Structurally identical to [`AuthController`](apps/portal-bff/src/auth/auth.controller.ts) but passes `adminRedirectUri` / `adminPostLogoutRedirectUri` and clears the admin session cookie on logout. `me` exposes the `roles` claim (admin SPA needs it for conditional UI); the user-portal `me` intentionally still doesn't.

### Shared `SessionEstablisher` (no controller duplication)

[`SessionEstablisher`](apps/portal-bff/src/auth/session-establisher.service.ts) encapsulates the session lifecycle so both controllers stay thin:

- `establish({ user, req, res, surface })` — mints CSRF, populates `user / createdAt / absoluteExpiresAt / csrfToken / mfaVerifiedAt`, saves, sets the CSRF cookie, registers in `user_sessions` index, emits `auth.sign_in` audit (blocking), logs with the `surface` tag.
- `destroy({ actor, req })` — when `actor` is set, removes from index + emits `auth.sign_out`; always destroys the session with Redis-hiccup tolerance.

No code duplicated between the two surfaces — the only per-surface differences are the redirect URIs (passed in) and the cookie names cleared on logout (controller-local).

### Entra config gains two URIs

`EntraConfig` adds `adminRedirectUri` + `adminPostLogoutRedirectUri`, validated at boot in [check-entra-config.ts](apps/portal-bff/src/config/check-entra-config.ts). The validator **refuses to start** when `ENTRA_ADMIN_REDIRECT_URI === ENTRA_REDIRECT_URI` — that misconfiguration would silently collapse the two surfaces into one session. Both URIs must be registered on the same Entra app registration's "Redirect URIs" list.

### `AuthService` API change

`beginAuthCodeFlow(redirectUri)`, `completeAuthCodeFlow(code, state, preAuth, redirectUri, now?)`, and `buildLogoutUrl(postLogoutRedirectUri)` now take their URI as a parameter. Callers (user-portal vs admin-portal controllers) pick which set to pass.

## Required ops action before this PR can run locally

Two new mandatory env vars. The BFF refuses to start without them.

```env
ENTRA_ADMIN_REDIRECT_URI=http://localhost:3000/api/admin/auth/callback
ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI=http://localhost:4201/
```

The example values land in [apps/portal-bff/.env.example](apps/portal-bff/.env.example) for reference. The corresponding Entra app registration also needs `/api/admin/auth/callback` added to its "Redirect URIs" list before any admin sign-in works end-to-end.

## Notes for the reviewer

- The user-portal callback's post-login redirect still targets `postLogoutRedirectUri` (existing quirk where the post-auth and post-logout landing happen to be the same URL). The admin callback mirrors the pattern for `adminPostLogoutRedirectUri`. Splitting these into dedicated post-login URIs is a separate ADR/PR.
- `AdminModule` now imports `AuthModule` to consume `AuthService`, `SessionEstablisher`, and `ENTRA_CONFIG`. `AuditWriter` and `RequireMfaGuard` come through transitively.
- Existing `AuthController` spec assertions are preserved through the refactor by constructing a **real** `SessionEstablisher` in the test fixture with the same audit / index / logger mocks. No behavioural assertion was removed — the inline session-state-setting logic is now exercised through the establisher.
- The pre-existing docstring in `check-entra-config.ts` line 11-16 still says "the two redirect URIs are mandatory once the OIDC routes ship (next PR)" — stale, the routes have shipped. Not touched in this PR to keep the diff focused; can be a one-line doc PR later.

## Test plan

- [x] `pnpm nx test portal-bff` — **278 specs pass** (was 253; +25: admin cookie 3, session-establisher 11, admin auth controller 9, entra config 2).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean (the pre-existing `_res` / `_next` warnings in `rate-limit.middleware.ts` are unrelated).
- [x] Entra config validator: both URIs required, both URL-validated, equality refused.
- [x] Path-dispatch verified by routing — `/api/admin/me` and `/api/admin/auth/*` see the admin session; everything else sees the user session.
- [ ] e2e — pending env var update + Entra registration update to add the admin redirect URI. Once both are in place: sign in via `/api/auth/login`, see `portal_session` cookie; clear cookies; sign in via `/api/admin/auth/login`, see `portal_admin_session` cookie; verify `/api/admin/me` works on the admin session and `/api/auth/me` works on the user session — neither sees the other's session.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #129
This commit was merged in pull request #129.
This commit is contained in:
2026-05-14 02:21:47 +02:00
parent d51ccebe6a
commit fed905edc5
19 changed files with 1221 additions and 223 deletions
@@ -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();
+16 -102
View File
@@ -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<void> {
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<void> {
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<void> {
return new Promise((resolve, reject) => {
req.session.save((err) => (err ? reject(err) : resolve()));
});
}
function destroySession(req: Request): Promise<void> {
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<string, unknown>)[PRE_AUTH_COOKIE_NAME];
if (typeof raw !== 'string') {
@@ -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',
+3 -1
View File
@@ -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 {}
+44 -12
View File
@@ -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<string, unknown>;
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<string, unknown>;
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);
});
});
+11 -5
View File
@@ -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<AuthCodeFlowStart> {
async beginAuthCodeFlow(redirectUri: string): Promise<AuthCodeFlowStart> {
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<AuthenticatedUser> {
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();
}
@@ -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<string, unknown> = {
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<typeof makeLoggerStub>;
}
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<string, unknown> }).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<string, unknown> }).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');
});
});
@@ -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<void> {
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<void> {
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<void> {
return new Promise((resolve, reject) => {
req.session.save((err) => (err ? reject(err) : resolve()));
});
}
function destroySession(req: Request): Promise<void> {
return new Promise((resolve, reject) => {
req.session.destroy((err) => (err ? reject(err) : resolve()));
});
}