fix(portal-bff): drop strict amr check — flow blocked when Entra omits the claim #108

Merged
julien merged 1 commits from fix/portal-bff/auth-relax-amr-check into main 2026-05-12 15:31:57 +02:00

1 Commits

Author SHA1 Message Date
Julien Gautier afd9f175bf fix(portal-bff): drop strict amr check — flow blocked when Entra omits the claim
CI / commits (pull_request) Successful in 2m5s
CI / scan (pull_request) Successful in 2m17s
CI / check (pull_request) Successful in 2m19s
CI / a11y (pull_request) Successful in 1m12s
CI / perf (pull_request) Successful in 3m48s
The original `amr-missing` guard in PR #107 misread ADR-0011's
intent. `amr` is an OPTIONAL claim in Entra ID tokens: it's
populated for fresh interactive sign-ins where Conditional Access
asked for an MFA method, and frequently absent for SSO / refresh
flows or in tenants without a CA policy on the app. Rejecting
tokens on empty `amr` blocks every legitimate dev sign-in against a
tenant that hasn't yet had CA configured — observed in practice:

    {"context":"AuthCallback","event":"auth.flow_error",
     "failure":{"kind":"amr-missing"}}

ADR-0011 specifies that MFA enforcement is the Conditional Access
policy's job (org-side) and the `@RequireMfa({ freshness: 600 })`
decorator's job on sensitive routes (designed-in, no v1 consumer).
The BFF's part is to surface `amr` through the audit log and the
future guard, NOT to gate the callback on its presence.

Changes:

- `AuthCodeFlowError` discriminator drops the `amr-missing` variant.
  Three failure modes left: state-mismatch, flow-expired,
  token-exchange-failed. MSAL's own ID-token validation (signature,
  issuer, audience, exp, nbf) is the real gate at this stage.
- `AuthService.toAuthenticatedUser` keeps extracting `amr` and
  passing it through (as a possibly-empty string array) so the
  log line and future `@RequireMfa` guard still see it. The strict
  `if (amr.length === 0) throw` is gone, replaced by a comment
  block explaining the new shape.
- Spec test `'throws amr-missing'` becomes `'returns the user even
  when the ID token has no amr claim'` — asserts the array
  passes through empty rather than blocking the flow.

Verified: 52/52 specs green, lint clean, build green. Smoke test
against the live tenant — sign-in now lands cleanly on the SPA;
Pino's `auth.signed_in` log shows the resolved identity with the
`amr` value (often `[]` until CA is configured on the org side).
2026-05-12 15:25:17 +02:00