Commit Graph

3 Commits

Author SHA1 Message Date
julien 96339cc99b fix(portal-bff): serve /.well-known/jwks.json via express (path-to-regexp v8 ducks the dot) (#139)
CI / commits (push) Has been skipped
CI / check (push) Successful in 2m23s
CI / scan (push) Successful in 2m29s
CI / a11y (push) Successful in 59s
CI / perf (push) Successful in 4m0s
## Summary

The Nest `@Controller('.well-known/jwks.json')` declared in PR #138 combined with `setGlobalPrefix('api', { exclude: [...] })` landed the JWKS route at **neither** `/.well-known/jwks.json` (intended) **nor** `/api/.well-known/jwks.json` (with-prefix fallback). Both URLs 404'd. The user reported it on the merged PR; this fix reroutes the endpoint so the JWKS lands at the correct RFC 8615 bare-root path.

## Root cause

Nest 11 routes via [path-to-regexp v8.4.2](https://github.com/pillarjs/path-to-regexp/blob/main/Readme.md), whose grammar broke backward compatibility on several leading-character cases. The combination of a leading-dot path segment (`.well-known`) plus the `setGlobalPrefix` `exclude` rewrite falls into one of those cases — the route registers but matches no incoming request. Without the `exclude`, it would register under `/api/.well-known/jwks.json`, which would at least be reachable, but with `exclude` enabled it ends up in a path-to-regexp limbo.

## Fix

Sidestep Nest's router for this one route. The JWKS payload-builder stays in the Nest DI graph (renamed `JwksController` → `JwksPublisher`, just the decorators stripped), and [`main.ts`](apps/portal-bff/src/main.ts) resolves it from the container then registers a plain Express GET handler at `/.well-known/jwks.json`. Express's router accepts the leading dot verbatim and the route lands exactly where RFC 8615 says it should.

```ts
const jwksPublisher = app.get(JwksPublisher);
app.getHttpAdapter().get('/.well-known/jwks.json', (_req, res) => {
  res.json(jwksPublisher.jwks());
});
```

## Touched

- [`jwks.controller.{ts,spec.ts}`](apps/portal-bff/src/downstream/) → [`jwks.publisher.{ts,spec.ts}`](apps/portal-bff/src/downstream/). Same constructor, same `jwks()` method shape — only the `@Controller` / `@Get` decorators are gone. The DI signature is unchanged so the existing tests rename → green without other edits.
- [`downstream.module.ts`](apps/portal-bff/src/downstream/downstream.module.ts): drops the `controllers` array, lists `JwksPublisher` as a provider + export so `main.ts` can resolve it.
- [`main.ts`](apps/portal-bff/src/main.ts): drops the `setGlobalPrefix` `exclude` option, drops the `RequestMethod` import, registers an Express GET handler at the bare-root JWKS path immediately before `app.listen()`.

## Verification

Verified locally against a running BFF (with a generated RSA-3072 key + `BFF_JWKS_KID=bff-2026-05`):

```bash
$ curl -s http://localhost:3000/.well-known/jwks.json | jq .
{
  "keys": [
    {
      "kty": "RSA",
      "n": "ppDvWBUEQTD6sv-7FFG-UfCPALG…",
      "e": "AQAB",
      "kid": "bff-2026-05",
      "alg": "RS256",
      "use": "sig"
    }
  ]
}
```

## Test plan

- [x] `pnpm nx test portal-bff` — **358 specs pass** (unchanged: the publisher's `jwks()` method shape is identical, the rename-only spec delta keeps the existing coverage).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean.
- [x] Manual: `curl http://localhost:3000/.well-known/jwks.json` returns the JWKS with the configured `kid`, `alg=RS256`, `use=sig`. No private RSA components (`d` / `p` / `q` / `dp` / `dq` / `qi`) in the response.

## Notes for the reviewer

- The "use Express directly when path-to-regexp v8 fights you" escape hatch is rare. It's the right move here because the path is fixed by RFC 8615 — we can't compromise on the URL shape. For any other route we'd let Nest's router handle it.
- The publisher class is still injectable, still in the DI graph, still trivially mockable in tests. The only thing that's "outside Nest" is the route binding in `main.ts`. Production behaviour is identical to a Nest-routed controller; only the registration mechanism differs.
- No new specs were added because the routing fix is a wiring change. A controller-spec-style integration test using Nest's `TestingModule` wouldn't exercise the actual Express route binding either, so the manual curl + the publisher's existing unit tests are the right coverage.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #139
2026-05-14 19:12:38 +02:00
julien 282a972346 feat(portal-bff): signed-assertion strategy + /.well-known/jwks.json (#138)
CI / scan (push) Successful in 2m40s
CI / commits (push) Has been skipped
CI / check (push) Successful in 3m38s
CI / a11y (push) Successful in 1m38s
CI / perf (push) Successful in 3m36s
## Summary

Second half of the **DownstreamApiClient + OBO** chantier per [ADR-0014](docs/decisions/0014-downstream-api-access-obo-pattern.md). Ships the **signed-assertion strategy** (non-Entra downstreams) and the **JWKS publishing endpoint** as testable primitives, completing the strategy layer the OBO PR (#137) started. The framework around them (DownstreamApiClientFactory, cockatiel, audience pre-check, error translation) still waits for the first concrete integration per the ADR's own "until then" clause.

After this PR the BFF has, ready to plug into a future integration:

- `OboStrategy` — Entra-protected downstreams (PR #137)
- `SignedAssertionStrategy` — non-Entra downstreams (this PR)
- `DownstreamTokenCache` — encrypted-at-rest OBO token cache (PR #137)
- `GET /.well-known/jwks.json` — public key publication (this PR)

## What lands

### [`assertJwksConfig`](apps/portal-bff/src/config/check-jwks-config.ts)

Boot validator for `BFF_JWKS_PRIVATE_KEY_PATH` + `BFF_JWKS_KID`. Reads the PEM file once at startup, refuses missing / unreadable / weak material (RSA < 2048, Ed25519, unknown key type), derives the JOSE algorithm (`RS256` / `ES256` / `ES384`) from the key shape, and validates the kid against `[A-Za-z0-9_-]{4,128}` so the value lives unescaped in JWT headers + JWKS payloads.

### [`BffSigningKey`](apps/portal-bff/src/downstream/bff-signing-key.ts)

Singleton holding `{ config: JwksConfig, publicJwk: JWK }`. The `publicJwk` is derived from the **public half** of the key (via `jose.exportJWK` on a `createPublicKey`-derived `KeyObject`) so no private material can leak through. Single DI source for both consumers (strategy + JWKS controller) so a key rotation only changes one provider.

### [`SignedAssertionStrategy`](apps/portal-bff/src/downstream/strategies/signed-assertion.strategy.ts)

Wraps `jose.SignJWT` with the ADR-0014 claim shape:

```json
{
  "iss": "portal-bff",
  "sub": "<actor_id_hash>",
  "aud": "<downstream-name>",
  "audience": "workforce" | "customer",
  "claims": { /* curated subset */ },
  "exp": <now + 60s>,
  "iat": <now>,
  "trace_id": "<W3C trace id>"
}
```

- **60 s TTL** hard-coded — the ADR mandates it.
- **No JWT cache** — at 60 s lifetime the savings would be negligible and a cache would let replayed assertions linger past their useful life. The signing operation itself is cheap (~hundreds of µs for RS256 with a 3 KB key).
- **kid in the protected header** matches the JWKS so a downstream picks the right key during rotation.
- Supports **RS256 / ES256 / ES384** transparently — picks the alg the validator derived at boot.

### [`JwksController`](apps/portal-bff/src/downstream/jwks.controller.ts)

`GET /.well-known/jwks.json` returns `{ keys: [<single jwk>] }`. v1 publishes one key; the rotation chantier will add a second entry + window-based eviction so a downstream that cached the previous JWK keeps verifying during cut-over.

[`main.ts`](apps/portal-bff/src/main.ts) excludes `/.well-known/*` from the global `/api` prefix so the route lands at the bare root per RFC 8615. No auth gate — the JWKS is the verification anchor; gating it would defeat the purpose. The CSRF middleware already exempts GET methods, so the route comes out clean.

## Required env update (mandatory at boot)

Generate the key:

```bash
mkdir -p apps/portal-bff/.secrets
openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:3072 \
  -out apps/portal-bff/.secrets/jwks.pem
```

Set in `apps/portal-bff/.env`:

```env
BFF_JWKS_PRIVATE_KEY_PATH=apps/portal-bff/.secrets/jwks.pem
BFF_JWKS_KID=bff-2026-05
```

The repo's existing `*.pem` / `*.key` gitignore patterns cover `.secrets/`.

## Dependency

- **`jose@^6`** added as a direct dep (was transitive via MSAL). Pinned at the workspace root since the BFF is the only consumer today and the package isn't part of the Angular bundle graph.
- `jest.config.cts`: `jose` ships ESM-only, so its `node_modules` path is removed from `transformIgnorePatterns`. The pattern walks pnpm's deep `.pnpm/` layout — anything under `/node_modules/` whose path also contains `jose` somewhere gets transformed by ts-jest.

## Out of scope (deferred until the first concrete integration)

Per ADR-0014's "until then" clause:

- `DownstreamApiClientFactory` + per-service typed `DownstreamApiConfig`.
- `cockatiel` resilience composition (timeout, retry, circuit breaker, bulkhead).
- Audience pre-check at the call site (`audienceConstraint` → `authz.deny` audit).
- Error translation tables per service.
- OTel custom spans `downstream.<service>.<verb>.<path>`.
- The framework code that actually calls `SignedAssertionStrategy.sign()` and attaches `X-User-Assertion` + the `ServiceCredential` auth header to an outbound HTTP request.
- Key rotation (the JWKS lists one key for now; the rotation chantier adds the second entry + eviction policy).

These land alongside the first concrete integration so the framework shape is validated against a real consumer, not speculative needs.

## Test plan

- [x] `pnpm nx test portal-bff` — **358 specs pass** (was 334; +24: env validators 11, signing key 4, strategy 6, controller 3).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean.
- [x] Env validator: missing path, unreadable file, garbage PEM, RSA-1024 (weak), Ed25519 (unsupported), missing kid, illegal kid charset, kid too short.
- [x] Signing key: RSA / EC P-256 / EC P-384 round-trip to public JWK with no private material (`d`, `p`, `q`, `dp`, `dq`, `qi` all absent from the published JWK).
- [x] Strategy: claim shape matches ADR-0014, `exp - iat == 60`, audience mismatch rejected, signature mismatch rejected, EC P-256 signing path (ES256), per-call freshness.
- [x] Controller: returns JWKS with the single public key, no private material leaks.
- [ ] Manual smoke: generate a key locally + set the two env vars + `curl http://localhost:3000/.well-known/jwks.json` should return the JWKS shape with the chosen kid.

## Notes for the reviewer

- The strategy uses `setProtectedHeader({ alg, kid })` — the kid in the protected header is the canonical way to tell a verifier "use the entry with this kid in the JWKS". Without it, a verifier holding two keys during rotation has to try both.
- The `60 s` TTL is intentionally not env-overridable. ADR-0014 mandates it; making it tunable would create a tempting knob to widen the replay window for "performance".
- `jose` was already in the tree transitively (likely via MSAL). Promoting it to a direct dep + pinning means a future hoist deduplication can't silently remove it without our review.

## What's next

The chantier's strategy layer is complete. Open follow-ups on the roadmap:

- **First concrete downstream integration** — when a real consumer arrives, the framework gets built around the two strategies (DownstreamApiClientFactory, cockatiel resilience, audience pre-check, error translation, OTel spans, audit events). Until then the strategies + cache + JWKS sit ready.
- **Strategic security baseline ADR** — RSSI sign-off on ASVS / HDS / GDPR / NIS 2. Paused per [CLAUDE.md](CLAUDE.md) §"Repository status".
- **portal-admin v1 modules** — CMS pages, menu management, user list. Each is its own self-contained chantier.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #138
2026-05-14 18:34:07 +02:00
julien d665c66c4e feat(portal-bff): obo strategy + encrypted downstream token cache (#137)
CI / check (push) Successful in 3m49s
CI / commits (push) Has been skipped
CI / scan (push) Successful in 2m31s
CI / a11y (push) Successful in 1m45s
CI / perf (push) Successful in 3m25s
## Summary

First half of the **DownstreamApiClient + OBO** chantier per [ADR-0014](docs/decisions/0014-downstream-api-access-obo-pattern.md). Ships the OBO auth strategy and its encrypted-at-rest token cache as testable primitives — explicitly **not** the full `DownstreamApiClientFactory` + cockatiel + audience-pre-check framework.

The scope is dictated by ADR-0014 §"Consequences":

> *"Bad, because the framework is forward-looking — there is no concrete v1 caller. Risk of drift between framework and real needs. **Mitigated by writing the framework code only in the same iteration as the first concrete integration; until then, this ADR plus mock-driven unit tests on the strategies (OBO, signed-assertion) keep the design honest.**"*

The framework gets assembled when the first real downstream integration arrives, with that integration as the validation surface. The next PR in this chantier ships the symmetric signed-assertion strategy + the JWKS endpoint.

## What lands

### [`assertOboCacheEncryptionKey`](apps/portal-bff/src/config/check-obo-cache-encryption-key.ts)

Boot validator mirroring `assertSessionEncryptionKey`. AES-256-GCM, 32-byte requirement, placeholder rejection, fail-fast posture. Plus one extra defense in depth:

> *Refuses a value identical to `SESSION_ENCRYPTION_KEY`* — ADR-0014 §"Token cache (for OBO)" mandates dedicated keys; catching the copy-paste regression at boot prevents a silent trust-boundary downgrade.

Wired in [`main.ts`](apps/portal-bff/src/main.ts) alongside the other `assertX()` validators.

### [`DownstreamTokenCache`](apps/portal-bff/src/downstream/downstream-token-cache.service.ts)

Redis-backed cache, key shape `obo:{actorIdHash}:{resource}`. Encrypts each entry via the shared AES-256-GCM helpers from `session-crypto` but under a **dedicated key** (`OBO_CACHE_KEY`).

| Path | Behaviour |
| --- | --- |
| Cache miss | Returns `null`. |
| Tampered ciphertext | Returns `null` + Pino warn `downstream.obo_cache.decrypt_failed`. |
| Wrong-key ciphertext | Returns `null` (GCM auth-tag mismatch). |
| Decrypted but malformed shape | Returns `null` + Pino warn. |
| Redis read failure | Returns `null` + Pino warn `downstream.obo_cache.read_failed`. |
| Write of a token already inside the 60 s buffer | Skipped (TTL would be useless). |
| Redis write failure | Logged, non-fatal. |

Reads never throw — every failure collapses to a miss, the strategy re-acquires from Entra.

### [`OboStrategy`](apps/portal-bff/src/downstream/strategies/obo.strategy.ts)

Wraps MSAL Node's `acquireTokenOnBehalfOf` with the cache.

```
acquire(input):
  cached = cache.get(...)
  if cached && cached.expiresAt - now > 60s → return cached
  result = msal.acquireTokenOnBehalfOf({ oboAssertion, scopes })
  if !result || !result.accessToken || !result.expiresOn → throw OboAcquireError(msal-no-result)
  cache.set(...)
  return result
```

`OboAcquireError` carries a typed `reason` discriminator (`msal-refused` / `msal-no-result`) the future framework will translate to a **502 + `auth.token.validation.failed`** audit event per ADR-0014 — "the BFF does NOT silently fall back to the user's original token".

### One scope nuance from ADR-0014

ADR-0014 §"OBO strategy" says *"uses MSAL Node's `acquireTokenOnBehalfOf` with the user's current Entra access token (read from session via CLS)"*. v1 sessions don't persist the user's access token (ADR-0009 omits `offline_access` deliberately). For now the strategy takes the user access token as an **input parameter** — when the first concrete integration ships, the framework will fetch it from CLS / MSAL's token cache and forward here. That keeps the strategy a testable primitive without coupling to a session shape that doesn't exist yet.

### [`DownstreamModule`](apps/portal-bff/src/downstream/downstream.module.ts)

Provides `OBO_CACHE_KEY` (via the validator at factory time), `DownstreamTokenCache`, `OboStrategy`. Imports `AuthModule` for the shared `MSAL_CLIENT` and `RedisModule` for the shared `ioredis` client. Wired into `AppModule` though no runtime consumer yet — the registration makes the strategy injectable for the future integration without that integration having to also touch the module graph.

## Required env update (mandatory at boot)

```env
OBO_CACHE_ENCRYPTION_KEY=replace_with_32_random_bytes_base64url
```

Generate with `node -e "console.log(require('crypto').randomBytes(32).toString('base64url'))"`. Must differ from `SESSION_ENCRYPTION_KEY` — the boot validator refuses identical values.

## Out of scope (deferred until the first concrete integration)

Per ADR-0014's "until then" clause:

- `DownstreamApiClientFactory` + per-service typed config.
- `cockatiel` resilience composition (timeout, retry, circuit breaker, bulkhead).
- Audience pre-check at the call site (`audienceConstraint` → `authz.deny` audit event).
- Error-translation tables per service.
- OTel custom spans `downstream.<service>.<verb>.<path>`.
- The `auth.token.validation.failed` audit event itself (the discriminator is on `OboAcquireError`, the audit-emission glue lives in the future framework).
- The framework wiring that reads the user access token from CLS instead of accepting it as a parameter.

These land alongside the first concrete integration so the framework shape is validated against a real consumer, not speculative needs.

## Test plan

- [x] `pnpm nx test portal-bff` — **334 specs pass** (was 308; +26: env validator 8, token cache 9, OBO strategy 9).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean.
- [x] Env validator refuses placeholder, wrong length, non-base64url, AND identical-to-`SESSION_ENCRYPTION_KEY`. Boot-order tolerant: accepts the value when `SESSION_ENCRYPTION_KEY` is unset.
- [x] Token cache round-trip verified: written ciphertext starts with `v1.`, never contains the plaintext sentinel.
- [x] Tamper rejection verified: flipping the last char of the GCM-encrypted blob fails decryption and collapses to a miss.
- [x] Wrong-key rejection verified: writing with one key, reading with another, returns `null`.
- [x] TTL math verified: PX TTL = `expiresAt − now − 60 000`. Write skipped when token already inside the buffer.
- [x] OBO strategy: cache-hit short-circuit, stale-cache re-acquire, cold-cache → MSAL → cache.set, MSAL refusal → typed error, MSAL null-result → typed error, empty access token → typed error, null expiresOn → typed error.

## Notes for the reviewer

- The strategy file uses `override readonly cause` on `OboAcquireError` because TS `strict.exactOptionalPropertyTypes + noImplicitOverride` flags shadowing the built-in `Error.cause`. The shadowing is intentional — we want the typed cause property visible in error consumers — so the `override` keyword is the canonical way.
- `DownstreamTokenCache.get`'s "never throws" posture is deliberate. A cache failure must not poison a downstream call: the strategy re-acquires from Entra. The trade-off is that a key-rotation gone wrong shows up as silent re-acquisitions (no errors, just extra MSAL load); the structured Pino warns are the ops signal.
- The `DownstreamModule` is wired into `AppModule` even though nothing consumes the strategy at runtime. Without the wiring, the first integration PR would have to also touch the module graph; with it, the integration is just "inject `OboStrategy` and call `.acquire()`".

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #137
2026-05-14 18:13:30 +02:00