From 2b58395f37ae02be7a52609dd12010dcb8fa3e4d Mon Sep 17 00:00:00 2001 From: wangmingzhou1986 Date: Fri, 24 Jul 2026 17:48:24 +0800 Subject: [PATCH 1/2] fix(api): default OIDC tenant fallback so tokens without tenant_id claim do not 401 --- packages/api/src/server.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/api/src/server.ts b/packages/api/src/server.ts index 2465897..1fe7b95 100644 --- a/packages/api/src/server.ts +++ b/packages/api/src/server.ts @@ -361,6 +361,9 @@ async function main(): Promise { // OIDC_JWKS_URI overrides for non-Keycloak issuers (e.g. NHS CIS2). // Default: Keycloak-style path. Set OIDC_JWKS_URI for other providers. jwksUri: process.env['OIDC_JWKS_URI'] ?? `${oidcIssuer}/protocol/openid-connect/certs`, + // Fallback tenant when the IdP token carries no tenant_id claim (e.g. stock + // Keycloak); without it, production auth 401s for every user. See issue #1. + defaultTenantId: process.env['OIDC_DEFAULT_TENANT'] ?? 'default', }); // ── Authorization (OpenFGA) ── From 9d3130877dfef343a5242554abaebaf313c471ae Mon Sep 17 00:00:00 2001 From: wangmingzhou1986 Date: Mon, 27 Jul 2026 08:50:51 +0800 Subject: [PATCH 2/2] fix(api): make OIDC default tenant opt-in (OIDC_DEFAULT_TENANT) + docs + tests Per review on #1: drop the implicit `?? 'default'` (fail-open) in favor of an explicit opt-in env knob. Unset arrives as undefined and configure() normalizes it to null, preserving the documented fail-closed posture (claimless tokens are rejected). Single-tenant deployments set OIDC_DEFAULT_TENANT=default; multi-tenant deployments leave it unset and map real tenants via a tenant_id claim. - packages/api/src/server.ts: opt-in wiring + env doc. - deploy/README.md, .env.example, docker-compose.yaml, Helm values + configmap: document/surface the knob (fail-closed by default). - oidc-authenticator.test.ts: assert unset stays fail-closed and set opts into the fallback tenant. --- deploy/.env.example | 6 ++++ deploy/README.md | 12 +++++--- deploy/docker-compose.yaml | 4 +++ .../helm/openfoundry/templates/configmap.yaml | 3 ++ deploy/helm/openfoundry/values.yaml | 4 +++ packages/api/src/server.ts | 10 +++++-- .../src/auth/oidc-authenticator.test.ts | 29 +++++++++++++++++++ 7 files changed, 61 insertions(+), 7 deletions(-) diff --git a/deploy/.env.example b/deploy/.env.example index 7de311e..8e25811 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -32,6 +32,12 @@ CEL_PORT=50051 # ─── API Gateway ───────────────────────────────────────────────── API_PORT=4000 +# ─── OIDC (production auth; NODE_ENV=production) ───────────────── +# Opt-in fallback tenant for IdP tokens WITHOUT a `tenant_id` claim. Unset by +# default → fail-closed (claimless tokens are rejected); a real `tenant_id` claim +# always takes precedence. Single-tenant deployments can opt in: +# OIDC_DEFAULT_TENANT=default + # ─── Build ────────────────────────────────────────────────────── # Git revision stamped into Docker image labels. # Set automatically by: GIT_REVISION=$(git rev-parse HEAD) docker compose up --build diff --git a/deploy/README.md b/deploy/README.md index 18bac63..dc125b6 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -141,9 +141,13 @@ requirements are easy to miss — each fails with an opaque `401 UNAUTHENTICATED `audience = clientId` (`OIDC_CLIENT_ID`) and validates it. Keycloak access tokens default to `aud: "account"` and will be **rejected**. Add an **audience protocol mapper** that includes your client in the `aud` claim. -2. **A `tenant_id` claim is mandatory.** `extractUser` throws `MISSING_TENANT` - when the tenant claim is absent — and no default tenant is configured. Add a - mapper (hardcoded claim or per-user attribute) that emits `tenant_id`. +2. **A `tenant_id` claim is mandatory (unless a default is opted in).** + `extractUser` throws `MISSING_TENANT` when the tenant claim is absent and no + default tenant is configured. Either add a mapper (hardcoded claim or per-user + attribute) that emits `tenant_id`, or — for a single-tenant deployment — set + `OIDC_DEFAULT_TENANT` to opt into a fallback tenant. Leaving it unset preserves + the fail-closed default (claimless tokens are rejected); a real `tenant_id` + claim always takes precedence when present. Required token claims: @@ -151,7 +155,7 @@ Required token claims: |-------|-------------| | `sub` | Subject — used as the actor/user id | | `aud` | Must equal `OIDC_CLIENT_ID` | -| `tenant_id` | Mandatory — request is rejected without it | +| `tenant_id` | Mandatory unless `OIDC_DEFAULT_TENANT` opts into a fallback tenant — otherwise the request is rejected | | `roles` | **Flat top-level array** — the authenticator reads `claims["roles"]` directly (`role-mapping.ts` `resolveRoles`, `DEFAULT_ROLE_MAPPING.claimName = "roles"`). It does **not** descend into Keycloak's nested `realm_access.roles`. Add a Keycloak realm-role mapper (`oidc-usermodel-realm-role-mapper`, `claim.name=roles`, multivalued) — otherwise the actor has no roles and CEL preconditions like `actor.hasRole('clinician')` fail with `PRECONDITION_FAILED` even after OpenFGA passes. | ### Issuer vs JWKS host split diff --git a/deploy/docker-compose.yaml b/deploy/docker-compose.yaml index a470095..b5fe905 100644 --- a/deploy/docker-compose.yaml +++ b/deploy/docker-compose.yaml @@ -206,6 +206,10 @@ services: OTEL_SERVICE_NAME: api-gateway DOMAIN_PACKS: ${DOMAIN_PACKS:-} DOMAIN_PACKS_EXTRA_DIRS: ${DOMAIN_PACKS_EXTRA_DIRS:-} + # OIDC fallback tenant for tokens without a `tenant_id` claim (production + # auth only — see docker-compose.prod-test.yaml). Opt-in; unset → fail-closed + # (claimless tokens rejected). Single-tenant deployments set OIDC_DEFAULT_TENANT. + OIDC_DEFAULT_TENANT: ${OIDC_DEFAULT_TENANT:-} # Deployment policy: platform roles allowed to grant relationships / record # consent. Generic default is `admin` only — no NHS roles are forced here. # NHS deployments set these in .env (see .env.example): diff --git a/deploy/helm/openfoundry/templates/configmap.yaml b/deploy/helm/openfoundry/templates/configmap.yaml index 0c24a23..fba3fa1 100644 --- a/deploy/helm/openfoundry/templates/configmap.yaml +++ b/deploy/helm/openfoundry/templates/configmap.yaml @@ -22,6 +22,9 @@ data: {{- if .Values.auth.oidc.jwksUri }} OIDC_JWKS_URI: {{ .Values.auth.oidc.jwksUri | quote }} {{- end }} + {{- if .Values.auth.oidc.defaultTenant }} + OIDC_DEFAULT_TENANT: {{ .Values.auth.oidc.defaultTenant | quote }} + {{- end }} OPENFGA_URL: {{ .Values.authz.openfga.url | quote }} {{/* HELM-06: OPENFGA_STORE_ID is infrastructure config, not a secret */}} OPENFGA_STORE_ID: {{ .Values.authz.openfga.storeId | quote }} diff --git a/deploy/helm/openfoundry/values.yaml b/deploy/helm/openfoundry/values.yaml index 6069587..925dd0b 100644 --- a/deploy/helm/openfoundry/values.yaml +++ b/deploy/helm/openfoundry/values.yaml @@ -42,6 +42,10 @@ auth: clientId: "" # Override for non-Keycloak issuers (e.g. NHS CIS2). If empty, uses ${issuer}/protocol/openid-connect/certs jwksUri: "" + # Opt-in fallback tenant for tokens without a `tenant_id` claim. Empty → + # fail-closed (claimless tokens rejected); a real tenant_id claim always wins. + # Single-tenant deployments set this (e.g. "default"); multi-tenant leave empty. + defaultTenant: "" existingSecret: openfoundry-oidc secretKeys: clientSecret: clientSecret diff --git a/packages/api/src/server.ts b/packages/api/src/server.ts index 1fe7b95..1d95906 100644 --- a/packages/api/src/server.ts +++ b/packages/api/src/server.ts @@ -12,6 +12,7 @@ * OIDC_ISSUER — OIDC provider issuer URL (matches Helm configmap) * OIDC_CLIENT_ID — OIDC client ID * OIDC_JWKS_URI — JWKS endpoint override for non-Keycloak issuers + * OIDC_DEFAULT_TENANT — Opt-in fallback tenant for tokens without a tenant_id claim (unset = fail-closed) * OPENFGA_URL — OpenFGA API URL (matches Helm configmap / docker-compose) * OPENFGA_STORE_ID — OpenFGA store ID * POSTGRES_URL — PostgreSQL connection string (with ?sslmode= for TLS) @@ -361,9 +362,12 @@ async function main(): Promise { // OIDC_JWKS_URI overrides for non-Keycloak issuers (e.g. NHS CIS2). // Default: Keycloak-style path. Set OIDC_JWKS_URI for other providers. jwksUri: process.env['OIDC_JWKS_URI'] ?? `${oidcIssuer}/protocol/openid-connect/certs`, - // Fallback tenant when the IdP token carries no tenant_id claim (e.g. stock - // Keycloak); without it, production auth 401s for every user. See issue #1. - defaultTenantId: process.env['OIDC_DEFAULT_TENANT'] ?? 'default', + // Opt-in fallback tenant for IdP tokens that carry no `tenant_id` claim. + // Unset (undefined) is normalized to null by configure(), preserving the + // documented fail-closed posture (MISSING_TENANT -> 401). Single-tenant + // deployments set OIDC_DEFAULT_TENANT=default; multi-tenant deployments leave + // it unset and map real tenants via a tenant_id claim. See issue #1. + defaultTenantId: process.env['OIDC_DEFAULT_TENANT'], }); // ── Authorization (OpenFGA) ── diff --git a/packages/security/src/auth/oidc-authenticator.test.ts b/packages/security/src/auth/oidc-authenticator.test.ts index 92f18b4..dd7bfb2 100644 --- a/packages/security/src/auth/oidc-authenticator.test.ts +++ b/packages/security/src/auth/oidc-authenticator.test.ts @@ -210,6 +210,35 @@ describe("OidcAuthenticator", () => { }); }); + describe("OIDC_DEFAULT_TENANT env wiring (fail-closed posture)", () => { + // server.ts wires `defaultTenantId: process.env['OIDC_DEFAULT_TENANT']`, so an + // unset env var arrives as `undefined`. configure() normalizes it to null, so a + // token without a tenant_id claim must still be rejected (fail-closed). + it("stays fail-closed when OIDC_DEFAULT_TENANT is unset (undefined)", async () => { + const auth = createAuthenticator({ defaultTenantId: undefined }); + const token = await signToken({ + sub: "user-env-unset", + name: "Env Unset", + email: "u@nhs.net", + }); + await expect(auth.authenticate(token)).rejects.toThrow(AuthenticationError); + await expect(auth.authenticate(token)).rejects.toThrow( + "no default tenant is configured", + ); + }); + + it("opts into a fallback tenant when OIDC_DEFAULT_TENANT is set", async () => { + const auth = createAuthenticator({ defaultTenantId: "default" }); + const token = await signToken({ + sub: "user-env-set", + name: "Env Set", + email: "s@nhs.net", + }); + const user = await auth.authenticate(token); + expect(user.tenantId).toBe("default"); + }); + }); + describe("CIS2 role mapping", () => { it("maps CIS2 role codes to platform roles", async () => { const auth = createAuthenticator({