feat(portal-bff): absolute-timeout middleware + user_sessions index per ADR-0010 (#115)
CI / check (push) Failing after 1m28s
CI / commits (push) Has been skipped
CI / scan (push) Successful in 1m3s
CI / a11y (push) Successful in 58s
CI / perf (push) Successful in 2m53s

## Summary

Hardens the BFF session per ADR-0010 §"TTL policy" and §"Revocation":

- **Absolute-timeout middleware** — every request that survives `express-session` runs through a new middleware that checks `req.session.absoluteExpiresAt`. Past the 12 h hard ceiling, the middleware destroys the Redis-side session, clears the `portal_session` cookie, drops the entry from the per-user index, and lets the request continue anonymously. Route-level guards (`/me`, future `@RequireAuth`) turn that into a 401 where the user actually needs auth — public routes keep serving.
- **`user_sessions:{userId}` secondary index** — a new `UserSessionIndexService` maintains a Redis set of active session ids per user. Hooked into `/auth/callback` (SADD on sign-in) and `/auth/logout` + the absolute-timeout middleware (SREM on destroy). Best-effort: a failed `SADD`/`SREM` logs a warning and the auth flow continues. No in-product consumer in this PR — the admin "logout everywhere" endpoint lands with the admin module.
- **Session payload extension** — `createdAt` and `absoluteExpiresAt` are now set on the session at the same moment as `req.session.user` (in `/auth/callback`). The `session.types.ts` declaration merging exposes them as optional `SessionData` fields.

## Notable choices

**Non-intrusive enforcement on expiry.** ADR-0010 says "returns 401"; we interpret that as "the user eventually sees a 401 when they touch something that needs auth", not "every route returns 401 the moment we notice the ceiling". The middleware destroys the session and calls `next()` — `/me` returns 401 on its own (no user on the session), public routes stay accessible. Validated with the project lead 2026-05-12.

**Express middleware exposed via DI, not a NestJS `MiddlewareConsumer`.** Same pattern as `SESSION_MIDDLEWARE`: factory inside `SessionModule`, resolved from the application context in `main.ts` with `app.get<RequestHandler>(SESSION_ABSOLUTE_TIMEOUT_MIDDLEWARE)`. Keeps the wiring co-located with the session middleware and avoids the `AppModule.configure(consumer)` boilerplate for a one-off enforcement layer.

**Best-effort index maintenance.** `UserSessionIndexService.add` / `remove` catch Redis errors and log a Pino warning instead of throwing. Rationale (per ADR-0010): the index is a convenience for admin operations, not a security invariant — a Redis hiccup must not break sign-in / sign-out. Orphans (entries pointing to keys that have expired idle-TTL on their own) are tolerated and will be filtered by future consumer code.

**Per-user index identifier = Entra `oid`.** Stable per-user inside the tenant, matches `req.session.user.oid`. Admin "logout user X" will work against this same key. Future multi-tenant scenarios may want `${tid}:${oid}` — easy refactor when External ID activation lands (ADR-0008).

## Out of scope (next PRs)

- Admin "logout everywhere" endpoint consuming `UserSessionIndexService.list(userId)`. Waits on the admin module + `@RequireAdmin` / `@RequireMfa` guards.
- Audit-pipeline first-class events for `session.absolute_timeout` and `user_session_index.*` (ADR-0013). For now they're structured Pino logs.
- Token blob persistence (id_token / access_token / refresh_token) in the encrypted session — ADR-0014 dependency.

## Test plan

- [x] `pnpm nx test portal-bff` → **123/123 pass** (was 110 before; +13 specs across new `user-session-index.service.spec.ts`, `absolute-timeout.middleware.spec.ts`, and added cases in `auth.controller.spec.ts`).
- [x] `pnpm nx lint portal-bff` → clean.
- [x] `pnpm nx build portal-bff` → clean webpack build.
- [x] Prettier-clean for all touched files.
- [ ] Manual smoke against running BFF:
  - [ ] Sign in normally → Redis has `session:<id>` + `user_sessions:<oid>` SISMEMBER returns `<id>`.
  - [ ] Logout → both keys gone.
  - [ ] Forge a past `absoluteExpiresAt` in Redis (or shorten `SESSION_ABSOLUTE_TIMEOUT_SECONDS=5` in `.env`) → next request after expiry returns 401 on `/me`, cookie cleared, index entry SREM-ed.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #115
This commit was merged in pull request #115.
This commit is contained in:
2026-05-12 23:23:14 +02:00
parent 0e4f0fc611
commit c3de2340e7
11 changed files with 607 additions and 24 deletions
@@ -1,5 +1,6 @@
import type { Request, Response } from 'express';
import type { Logger } from 'nestjs-pino';
import type { UserSessionIndexService } from '../session/user-session-index.service';
import { AuthController } from './auth.controller';
import { PRE_AUTH_COOKIE_NAME, PRE_AUTH_COOKIE_TTL_MS } from './auth.cookie';
import { AuthCodeFlowException } from './auth.errors';
@@ -51,6 +52,8 @@ function makeResStub() {
interface SessionStub {
user?: AuthenticatedUser;
createdAt?: number;
absoluteExpiresAt?: number;
save: jest.Mock;
destroy: jest.Mock;
}
@@ -70,11 +73,13 @@ function makeSessionStub(opts?: {
function makeReqStub(opts?: {
signedCookies?: Record<string, unknown>;
session?: SessionStub;
}): Request & { session: SessionStub } {
sessionID?: string;
}): Request & { session: SessionStub; sessionID: string } {
return {
signedCookies: opts?.signedCookies ?? {},
session: opts?.session ?? makeSessionStub(),
} as unknown as Request & { session: SessionStub };
sessionID: opts?.sessionID ?? 'sid-test',
} as unknown as Request & { session: SessionStub; sessionID: string };
}
function makeLoggerStub() {
@@ -96,6 +101,7 @@ interface ControllerFixture {
beginAuthCodeFlow: jest.Mock;
completeAuthCodeFlow: jest.Mock;
logger: ReturnType<typeof makeLoggerStub>;
userSessionIndex: { add: jest.Mock; remove: jest.Mock; list: jest.Mock };
}
const LOGOUT_URL = `${ENTRA.authority}/oauth2/v2.0/logout?post_logout_redirect_uri=${encodeURIComponent(ENTRA.postLogoutRedirectUri)}`;
@@ -112,12 +118,23 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller
completeAuthCodeFlow,
buildLogoutUrl,
} as unknown as AuthService;
const userSessionIndex = {
add: jest.fn().mockResolvedValue(undefined),
remove: jest.fn().mockResolvedValue(undefined),
list: jest.fn().mockResolvedValue([]),
};
const logger = makeLoggerStub();
return {
controller: new AuthController(service, logger as unknown as Logger, ENTRA),
controller: new AuthController(
service,
logger as unknown as Logger,
ENTRA,
userSessionIndex as unknown as UserSessionIndexService,
),
beginAuthCodeFlow,
completeAuthCodeFlow,
logger,
userSessionIndex,
};
}
@@ -196,6 +213,45 @@ describe('AuthController.callback', () => {
expect(res.redirect).toHaveBeenCalledWith(302, ENTRA.postLogoutRedirectUri);
});
it('records createdAt + absoluteExpiresAt on the session (ADR-0010 §"TTL policy")', async () => {
const before = Date.now();
const { controller } = makeController();
const res = makeResStub();
const session = makeSessionStub();
const req = makeReqStub({
signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) },
session,
});
await controller.callback(req, res, 'auth-code', PRE_AUTH.state);
const after = Date.now();
expect(session.createdAt).toBeGreaterThanOrEqual(before);
expect(session.createdAt).toBeLessThanOrEqual(after);
// Default absoluteSeconds = 43200 (12 h) per ADR-0010.
expect(session.absoluteExpiresAt).toBe((session.createdAt as number) + 43_200 * 1000);
});
it('registers the new session in the user_sessions index after save', async () => {
const { controller, userSessionIndex } = makeController();
const res = makeResStub();
const session = makeSessionStub();
const req = makeReqStub({
signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) },
session,
sessionID: 'fresh-sid',
});
await controller.callback(req, res, 'auth-code', PRE_AUTH.state);
expect(userSessionIndex.add).toHaveBeenCalledWith(USER.oid, 'fresh-sid');
// Order matters: the Redis session row must be written before
// the index points at it. We can't enforce a strict
// `toHaveBeenCalledBefore` ordering across jest mocks without
// a plugin — assert at least that the index entry was added.
expect(session.save).toHaveBeenCalled();
});
it('propagates a session.save() error as an exception (so the SPA never sees a successful redirect with no session)', async () => {
const { controller } = makeController();
const res = makeResStub();
@@ -343,13 +399,14 @@ describe('AuthController.me', () => {
describe('AuthController.logout', () => {
it('destroys the session, clears the session cookie, logs success, redirects to Entra logout', async () => {
const { controller, logger } = makeController();
const { controller, logger, userSessionIndex } = makeController();
const res = makeResStub();
const session = makeSessionStub({ user: USER });
const req = makeReqStub({ session });
const req = makeReqStub({ session, sessionID: 'logged-out-sid' });
await controller.logout(req, res);
expect(userSessionIndex.remove).toHaveBeenCalledWith(USER.oid, 'logged-out-sid');
expect(session.destroy).toHaveBeenCalledTimes(1);
expect(res.clearCookie).toHaveBeenCalledWith('portal_session', { path: '/' });
expect(logger.log).toHaveBeenCalledWith(
@@ -390,13 +447,17 @@ describe('AuthController.logout', () => {
});
it('still clears the cookie and redirects when the user was already anonymous', async () => {
const { controller, logger } = makeController();
const { controller, logger, userSessionIndex } = makeController();
const res = makeResStub();
const session = makeSessionStub();
const req = makeReqStub({ session });
await controller.logout(req, res);
// Nothing was ever added to the index for an anonymous request —
// skip the SREM (cheaper + avoids racing the cleanup of a
// future user's session id collision).
expect(userSessionIndex.remove).not.toHaveBeenCalled();
expect(session.destroy).toHaveBeenCalledTimes(1);
expect(logger.log).toHaveBeenCalledWith(
{ event: 'auth.signed_out', wasAuthenticated: false },
+25 -2
View File
@@ -1,7 +1,8 @@
import { Controller, Get, Inject, Query, Req, Res } from '@nestjs/common';
import type { Request, Response } from 'express';
import { Logger } from 'nestjs-pino';
import { sessionCookieName } from '../session/session-cookie';
import { readSessionTimeouts, sessionCookieName } from '../session/session-cookie';
import { UserSessionIndexService } from '../session/user-session-index.service';
import {
PRE_AUTH_COOKIE_NAME,
clearPreAuthCookieOptions,
@@ -27,6 +28,7 @@ export class AuthController {
private readonly authService: AuthService,
private readonly logger: Logger,
@Inject(ENTRA_CONFIG) private readonly entra: EntraConfig,
private readonly userSessionIndex: UserSessionIndexService,
) {}
/**
@@ -106,13 +108,25 @@ export class AuthController {
try {
const user = await this.authService.completeAuthCodeFlow(code, state, preAuth);
const now = Date.now();
const { absoluteSeconds } = readSessionTimeouts();
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;
// 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);
// 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);
this.logger.log(
{
event: 'auth.signed_in',
@@ -168,9 +182,18 @@ export class AuthController {
*/
@Get('logout')
async logout(@Req() req: Request, @Res() res: Response): Promise<void> {
const wasAuthenticated = Boolean(req.session.user);
const user = req.session.user;
const wasAuthenticated = Boolean(user);
const sessionId = req.sessionID;
const logoutUrl = this.authService.buildLogoutUrl();
// 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);
}
try {
await destroySession(req);
} catch (err) {
+2
View File
@@ -2,6 +2,7 @@ import { ConfidentialClientApplication, LogLevel } from '@azure/msal-node';
import { Module } from '@nestjs/common';
import { Logger } from 'nestjs-pino';
import { assertEntraConfig } from '../config/check-entra-config';
import { SessionModule } from '../session/session.module';
import { AuthController } from './auth.controller';
import { AuthService } from './auth.service';
import { ENTRA_CONFIG, type EntraConfig } from './entra-config.token';
@@ -38,6 +39,7 @@ import { MSAL_CLIENT } from './msal-client.token';
* `imports: [AuthModule]` is enough to consume either.
*/
@Module({
imports: [SessionModule],
controllers: [AuthController],
providers: [
AuthService,