d51ccebe6a
## Summary
Third step in the `portal-admin` audit-log-viewer workstream — ships the `@RequireMfa({ freshness })` decorator + guard called out in [ADR-0011](docs/decisions/0011-mfa-enforcement-entra-conditional-access.md) and referenced as the gate on the admin entry route in [ADR-0020](docs/decisions/0020-portal-admin-app.md). Designed-in, dormant: no v1 route uses the decorator yet. First consumer will be the admin entry route once the distinct admin session lands (next PR).
## What ships
- **[`auth/mfa.ts`](apps/portal-bff/src/auth/mfa.ts)** — `MFA_AMR_VALUES = ['mfa', 'otp', 'fido', 'wia', 'phr']` allow-list and `wasMultiFactor(amr): boolean`. The list mirrors ADR-0011 §"BFF verification"; the spec pins it so an ad-hoc edit can't bypass review.
- **[`config/check-mfa-config.ts`](apps/portal-bff/src/config/check-mfa-config.ts)** — `readMfaConfig()` reads `MFA_FRESHNESS_SECONDS` (default **600 s**, minimum **60 s**). Anything below the floor throws at boot — the floor catches a misconfigured "MFA on every navigation" before the BFF starts.
- **[`auth/require-mfa.guard.ts`](apps/portal-bff/src/auth/require-mfa.guard.ts)** — four branches:
| Branch | HTTP | Code | Audit |
| --- | --- | --- | --- |
| No session | 401 | `unauthenticated` | none (noise) |
| Session, no MFA-class `amr` | 401 | `mfa_required` | `auth.mfa_required reason=no-mfa-in-amr` |
| Session, no `mfaVerifiedAt` | 401 | `mfa_required` | `auth.mfa_required reason=no-mfa-verified-at` |
| Session, stale `mfaVerifiedAt` | 401 | `mfa_required` | `auth.mfa_required reason=mfa-stale, mfaAgeMs=…` |
The `reason` discriminator is **not** surfaced over the wire — only the audit row carries it. An attacker probing for "stale vs no-MFA" can't distinguish the two from the response.
- **[`auth/require-mfa.decorator.ts`](apps/portal-bff/src/auth/require-mfa.decorator.ts)** — `@RequireMfa({ freshness? })` built via `applyDecorators(SetMetadata, UseGuards)`. The per-route `freshness` override wins over the env default. Designed to compose with `@RequireAdmin()` — apply `@RequireMfa` outside `@RequireAdmin` so the freshness gate runs only after role is established.
- **[`AuditWriter.mfaRequired()`](apps/portal-bff/src/audit/audit.service.ts)** — new typed method using `outcome=denied`, captures `reason`, `freshnessSeconds`, and `mfaAgeMs` (when applicable) in the JSONB payload.
- **`session.mfaVerifiedAt: number`** — augmented onto `express-session`'s `SessionData` in [`session.types.ts`](apps/portal-bff/src/session/session.types.ts). Set to `Date.now()` at sign-in by the callback ([`auth.controller.ts`](apps/portal-bff/src/auth/auth.controller.ts)). Entra's CA policy is the authority on whether MFA actually happened; the BFF stamps "now" when persisting a session whose `amr` reflects MFA.
## Deferred — for the SPA-interceptor PR
ADR-0011 §"Step-up MFA — designed-in" step 2 calls for a `WWW-Authenticate` header carrying a **claims challenge** (MSAL-produced blob) on the 401. That requires:
1. MSAL Node integration to mint the challenge — adds wire-format coupling to MSAL we don't have anywhere else yet.
2. The Angular SPA interceptor to consume the header, redirect to `/auth/login?claims=…`, and retry the original request.
Neither side has a consumer in this PR. Shipping a `code: 'mfa_required'` in the structured envelope is sufficient signalling for the SPA interceptor once it lands — the interceptor PR can layer the `WWW-Authenticate` header and the MSAL claims blob without changing the guard's audit contract.
## Composability with `@RequireAdmin`
The admin entry route (next-PR consumer) will read:
```ts
@Controller('admin')
@RequireMfa({ freshness: 600 })
@RequireAdmin()
export class AdminController { … }
```
Apply order matters — Nest runs guards in the order their decorators were applied (innermost first). Putting `@RequireMfa()` outside `@RequireAdmin()` means a non-admin user gets a clean 403 from `AdminRoleGuard` without a spurious `auth.mfa_required` audit row. The decorator's JSDoc spells this out for future consumers.
## Notes for the reviewer
- The `RequireMfaGuard` is registered as a provider in `AuthModule` and re-exported. Per the existing convention ("`AuthModule` stays non-global; modules state 'I depend on auth' by importing it"), any future module using `@RequireMfa()` will need to `imports: [AuthModule]`. The `AdminModule` already does this transitively via shared `AuditWriter`; the explicit import will follow when the decorator is first applied.
- `mfaChallenge(reason)` takes the reason argument deliberately even though it ignores it in the response — keeps the call sites readable (`throw this.mfaChallenge('mfa-stale')`) and parks a hook for the day we want to localise / differentiate the message.
- New env var `MFA_FRESHNESS_SECONDS` is **optional** (default 600). No production env change is required to ship this PR.
## Test plan
- [x] `pnpm nx test portal-bff` — **251 specs pass** (was 214; +37 covering helpers, config reader, guard branches, audit typed method, callback stamp).
- [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] `MFA_FRESHNESS_SECONDS` boot validator: default + valid + below-floor + non-integer + decimal + non-numeric all covered.
- [x] Guard timing-boundary cases covered (age == freshness passes; age == freshness + 1 ms fails — implicitly via the 700-s-vs-600-s test).
- [ ] e2e — pending real Entra session with `amr` carrying an MFA token. Will be exercised when the admin entry route applies the decorator.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #128
213 lines
8.0 KiB
TypeScript
213 lines
8.0 KiB
TypeScript
import { Injectable } from '@nestjs/common';
|
|
import { trace } from '@opentelemetry/api';
|
|
import { ClsService } from 'nestjs-cls';
|
|
import { PrismaService } from 'nestjs-prisma';
|
|
import type {
|
|
AdminAccessDeniedInput,
|
|
AuditEventInput,
|
|
MfaRequiredInput,
|
|
SignInActor,
|
|
SignInFailedInput,
|
|
SignOutInput,
|
|
SessionExpiredInput,
|
|
} from './audit.types';
|
|
import { HashUserIdService } from './hash-user-id.service';
|
|
|
|
/**
|
|
* AuditWriter — single entry point for ADR-0013 audit-log writes.
|
|
*
|
|
* Contract
|
|
* --------
|
|
* - **Append-only at the database level.** Every write runs inside a
|
|
* transaction whose first statement is `SET LOCAL ROLE
|
|
* audit_writer`. That role only has `INSERT` on `audit.events`
|
|
* (per the migration that created the table); `UPDATE`, `DELETE`,
|
|
* `TRUNCATE` all fail at the Postgres level even if the BFF
|
|
* connection is otherwise privileged. The role is reset
|
|
* automatically at transaction end.
|
|
*
|
|
* - **Fail loud, never swallow.** Per ADR-0013 §"Blocking writes":
|
|
* no audit ⇒ no action. Callers must propagate the rejection up
|
|
* so the requested action does not proceed when its audit trail
|
|
* cannot be written. The service throws the underlying Prisma
|
|
* error unchanged; do not wrap it in a catch-and-log block.
|
|
*
|
|
* - **trace_id and actor_id_hash are auto-resolved.** trace_id is
|
|
* read from the active OTel span context (so the audit row joins
|
|
* with the BFF request span and the Pino log lines on the same
|
|
* request). actor_id_hash is read from the CLS context populated
|
|
* by future auth guards (ADR-0009 / ADR-0010); v1 stores `null`
|
|
* when no actor is established. Callers can override either by
|
|
* passing them on `AuditEventInput`.
|
|
*/
|
|
@Injectable()
|
|
export class AuditWriter {
|
|
constructor(
|
|
private readonly prisma: PrismaService,
|
|
private readonly cls: ClsService,
|
|
private readonly hashUserId: HashUserIdService,
|
|
) {}
|
|
|
|
/**
|
|
* Typed event: successful sign-in via the OIDC callback. Per
|
|
* ADR-0013's v1 catalogue (`auth.sign_in`).
|
|
*
|
|
* Hashes the user id internally — callers pass the raw Entra
|
|
* `oid`, never the hash, so the salt stays inside the audit
|
|
* module.
|
|
*/
|
|
async signIn(input: { actor: SignInActor; sessionId: string }): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'auth.sign_in',
|
|
audience: 'workforce',
|
|
outcome: 'success',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
subject: `session:${input.sessionId}`,
|
|
payload: { amr: input.actor.amr },
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: failed sign-in at the callback. The `failureKind`
|
|
* mirrors the discriminator on `AuthCodeFlowError` so the audit
|
|
* row is self-describing without joining anything.
|
|
*
|
|
* `actorIdHash` is left null on purpose: at the moment of
|
|
* failure we may not have resolved an identity yet (state
|
|
* mismatch, expired flow, token-exchange error before any user
|
|
* claim was parsed). Callers can pass an explicit hash when the
|
|
* identity *was* resolved before rejection.
|
|
*/
|
|
async signInFailed(input: SignInFailedInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'auth.sign_in.failed',
|
|
audience: 'workforce',
|
|
outcome: 'failure',
|
|
...(input.actor !== undefined ? { actorIdHash: this.hashUserId.hash(input.actor.oid) } : {}),
|
|
payload: { failureKind: input.failureKind, ...(input.payload ?? {}) },
|
|
});
|
|
}
|
|
|
|
async signOut(input: SignOutInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'auth.sign_out',
|
|
audience: 'workforce',
|
|
outcome: 'success',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
subject: `session:${input.sessionId}`,
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: session destroyed by the absolute-timeout
|
|
* middleware (12 h hard ceiling, ADR-0010 §"TTL policy"). The
|
|
* idle-TTL expiry is *not* surfaced through this method — it
|
|
* happens silently inside Redis with no BFF observation point.
|
|
*/
|
|
async sessionExpired(input: SessionExpiredInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'auth.session.expired',
|
|
audience: 'workforce',
|
|
outcome: 'success',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
subject: `session:${input.sessionId}`,
|
|
payload: { reason: input.reason, ageMs: input.ageMs },
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: a request was rejected by `RequireMfaGuard` because
|
|
* the session did not satisfy the route's MFA freshness gate.
|
|
* Per ADR-0011 §"Step-up MFA — designed-in, dormant", every
|
|
* step-up challenge emitted by the BFF lands here so auditors can
|
|
* track the distribution of step-up prompts (and the rate of stale
|
|
* `mfaVerifiedAt` rejections that would suggest the default
|
|
* freshness is too tight). `outcome=denied` matches the
|
|
* `AdminRoleGuard` posture: the user *is* authenticated, the
|
|
* action is just refused.
|
|
*/
|
|
async mfaRequired(input: MfaRequiredInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'auth.mfa_required',
|
|
audience: 'workforce',
|
|
outcome: 'denied',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
subject: input.attemptedRoute,
|
|
payload: {
|
|
reason: input.reason,
|
|
freshnessSeconds: input.freshnessSeconds,
|
|
...(input.mfaAgeMs !== undefined ? { mfaAgeMs: input.mfaAgeMs } : {}),
|
|
},
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Typed event: `/api/admin/*` request rejected by `AdminRoleGuard`
|
|
* because the session's `roles` claim does not include `admin`.
|
|
* Per ADR-0020 §"Auth — same Entra ID … `admin` role claim", every
|
|
* 403 from the admin surface is captured here with the attempted
|
|
* route and the roles the user actually held — auditors looking
|
|
* for privilege-escalation attempts pivot on `subject` (the route)
|
|
* and `outcome=denied`.
|
|
*/
|
|
async adminAccessDenied(input: AdminAccessDeniedInput): Promise<void> {
|
|
await this.recordEvent({
|
|
eventType: 'admin.access_denied',
|
|
audience: 'workforce',
|
|
outcome: 'denied',
|
|
actorIdHash: this.hashUserId.hash(input.actor.oid),
|
|
subject: input.attemptedRoute,
|
|
payload: { rolesHeld: input.rolesHeld },
|
|
});
|
|
}
|
|
|
|
async recordEvent(input: AuditEventInput): Promise<void> {
|
|
const traceId = trace.getActiveSpan()?.spanContext().traceId ?? null;
|
|
const actorIdHash =
|
|
input.actorIdHash ?? this.cls.get<string | undefined>('actorIdHash') ?? null;
|
|
const payloadJson = input.payload === undefined ? null : JSON.stringify(input.payload);
|
|
|
|
await this.prisma.$transaction(async (tx) => {
|
|
// Lock the connection to audit_writer for the duration of this
|
|
// transaction. SET LOCAL is reset at COMMIT/ROLLBACK so the
|
|
// pool's next consumer sees the original role.
|
|
await tx.$executeRawUnsafe(`SET LOCAL ROLE audit_writer`);
|
|
|
|
// Deliberately NOT `tx.auditEvent.create(...)`. The Prisma ORM
|
|
// create() emits `INSERT … RETURNING *` to hydrate the entity
|
|
// it returns, and Postgres requires SELECT on every column
|
|
// listed in RETURNING. `audit_writer` is granted INSERT only
|
|
// (ADR-0013 §"Append-only by role grants"); RETURNING fails
|
|
// with the deeply misleading "permission denied for table
|
|
// events" error code 42501. Raw parameterised INSERT keeps
|
|
// the role contract strict — audit_writer never needs SELECT.
|
|
//
|
|
// `gen_random_uuid()` is built into Postgres 13+ (the dev /
|
|
// prod target is 17). The enum + jsonb casts are needed
|
|
// because the parameter values are sent as TEXT over the
|
|
// wire.
|
|
await tx.$executeRawUnsafe(
|
|
`INSERT INTO "audit"."events"
|
|
(id, event_type, audience, outcome, subject, actor_id_hash, trace_id, payload)
|
|
VALUES (
|
|
gen_random_uuid(),
|
|
$1,
|
|
$2::"audit"."AuditAudience",
|
|
$3::"audit"."AuditOutcome",
|
|
$4,
|
|
$5,
|
|
$6,
|
|
$7::jsonb
|
|
)`,
|
|
input.eventType,
|
|
input.audience,
|
|
input.outcome,
|
|
input.subject ?? null,
|
|
actorIdHash,
|
|
traceId,
|
|
payloadJson,
|
|
);
|
|
});
|
|
}
|
|
}
|