diff --git a/.github/workflows/keycloak-realm-contract.yml b/.github/workflows/keycloak-realm-contract.yml new file mode 100644 index 000000000..d0a1ef860 --- /dev/null +++ b/.github/workflows/keycloak-realm-contract.yml @@ -0,0 +1,42 @@ +name: Keycloak realm — tenant claim contract + +# Guards the pinned-tenant contract (ADR-0003 / DD-AUTH-04) against the +# canonical broker realm: fail-closed with no pinned attribute, the pin +# emitted verbatim as a single string, tenant-bearing groups inert AND +# flagged as a policy violation. Boots a throwaway Keycloak in docker. + +on: + pull_request: + branches: [main] + paths: + - "deploy/gitops/environments/*/keycloak/realms/**" + - "deploy/compose/keycloak/tests/**" + - ".github/workflows/keycloak-realm-contract.yml" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + tenant-contract: + name: tenant contract guard + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.13" + + - name: Install PyYAML + run: python3 -m pip install --quiet pyyaml + + - name: Tenant translation guard + run: python3 deploy/compose/keycloak/tests/tenant_contract_guard.py diff --git a/charts/insight/templates/secrets.yaml b/charts/insight/templates/secrets.yaml index af694da27..c6367d3ed 100644 --- a/charts/insight/templates/secrets.yaml +++ b/charts/insight/templates/secrets.yaml @@ -201,8 +201,10 @@ stringData: APP__gears__authenticator__config__idp__client_secret: {{ .Values.authenticator.oidc.clientSecret | default "" | quote }} # Browser-facing callback the IdP redirects back to (through the gateway edge). APP__gears__authenticator__config__redirect_uri: {{ tpl (required "authenticator.oidc.redirectUri is required" .Values.authenticator.oidc.redirectUri) . | quote }} - # Tenant resolution. `tenant_claim` = the id_token claim naming the user's - # tenant (Entra=`tid`; fakeidp/Keycloak=`tenant_id`). `default_tenant_id` is + # Tenant resolution. `tenant_claim` defaults to `tenant_id` and is FROZEN + # there (ADR-0003: the broker always emits `tenant_id`); the value remains + # configurable only for third-party consumers wiring a non-broker IdP + # directly (e.g. Entra=`tid`). `default_tenant_id` is # the single-tenant fallback, used ONLY when that claim is absent (Okta and # other claim-less IdPs — see #1853). It is sourced from `global.tenantDefaultId` # — the same single source of truth the rest of the platform keys on — so a diff --git a/deploy/compose/keycloak/tests/tenant_contract_guard.py b/deploy/compose/keycloak/tests/tenant_contract_guard.py new file mode 100755 index 000000000..996a3b95a --- /dev/null +++ b/deploy/compose/keycloak/tests/tenant_contract_guard.py @@ -0,0 +1,248 @@ +#!/usr/bin/env python3 +"""Pinned-tenant contract guard (ADR-0003, insight#2196). + +The tenant is ALWAYS the pinned per-registration value (a user attribute +stamped by each IdP mapper); an IdP's own tenancy assertions are never +consulted, and group-sourced tenants are rejected by policy. Imports the +canonical broker realm into a throwaway Keycloak and asserts: + +1. clean baseline -> the canonical realm ships zero tenant groups +2. no pinned attribute -> token carries NO tenant_id (fail closed) +3. pinned attribute -> token tenant_id == the pin, a single string +4. tenant-bearing group -> must NOT influence the token (no aggregation) + AND the realm scan flags it as a violation + +Requires docker and PyYAML. Boots quay.io/keycloak/keycloak, converts the +canonical realm YAML to a realm representation (env placeholders substituted +with synthetic values), creates it via the admin API, and evaluates example +tokens. Exit 0 = contract holds. +""" + +# ruff: noqa: T201 — stdout IS this script's CI report. + +import json +import re +import subprocess +import sys +import time +import urllib.error +import urllib.parse +import urllib.request +from pathlib import Path + +import yaml + +REPO_ROOT = Path(__file__).resolve().parents[4] +CANONICAL_REALM = REPO_ROOT / "deploy/gitops/environments/local/keycloak/realms/insight-broker.yaml" +KC_IMAGE = "quay.io/keycloak/keycloak:26.4" +KC_PORT = 18086 +BASE = f"http://127.0.0.1:{KC_PORT}" +CONTAINER = "tenant-guard-kc" + +PLACEHOLDER = re.compile(r"\$\(env:([A-Za-z_][A-Za-z0-9_]*)\)") +SYNTHETIC = { + "INSIGHT_AUTHENTICATOR_CLIENT_SECRET": "guard-secret", + "INSIGHT_AUTHENTICATOR_REDIRECT_URI": "http://localhost/auth/callback", + "INSIGHT_TENANT_ID": "00000000-0000-0000-0000-00000000feed", +} + +TENANT_GROUP = "aaaaaaaa-0000-0000-0000-000000000001" +TENANT_PIN = "cccccccc-0000-0000-0000-000000000003" + + +def sh(*args: str) -> None: + subprocess.run(args, check=True, capture_output=True) + + +def substitute(node): + if isinstance(node, str): + return PLACEHOLDER.sub(lambda m: SYNTHETIC.get(m.group(1), f"missing-{m.group(1)}"), node) + if isinstance(node, list): + return [substitute(v) for v in node] + if isinstance(node, dict): + return {k: substitute(v) for k, v in node.items()} + return node + + +class Admin: + def __init__(self) -> None: + self.token = self._call( + "/realms/master/protocol/openid-connect/token", + method="POST", + raw=urllib.parse.urlencode( + {"grant_type": "password", "client_id": "admin-cli", "username": "admin", "password": "admin"} + ).encode(), + )["access_token"] + + def _call(self, path: str, method: str = "GET", body=None, raw: bytes | None = None): + headers = {"Content-Type": "application/x-www-form-urlencoded" if raw else "application/json"} + if hasattr(self, "token"): + headers["Authorization"] = f"Bearer {self.token}" + req = urllib.request.Request( + f"{BASE}{path}", + method=method, + headers=headers, + data=raw if raw is not None else (json.dumps(body).encode() if body is not None else None), + ) + # URLs are module constants targeting the throwaway 127.0.0.1 Keycloak. + # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected + with urllib.request.urlopen(req, timeout=30) as resp: + payload = resp.read() + return json.loads(payload) if payload else None + + def realm(self, path: str, method: str = "GET", body=None): + return self._call(f"/admin/realms/insight-broker{path}", method, body) + + +def wait_for_keycloak(timeout_s: int = 180) -> None: + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + try: + # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected, python.lang.security.audit.insecure-transport.urllib.insecure-urlopen.insecure-urlopen + urllib.request.urlopen(f"{BASE}/realms/master/.well-known/openid-configuration", timeout=3) + return + except (urllib.error.URLError, OSError): + time.sleep(3) + raise TimeoutError("Keycloak did not come up") + + +def tenant_claim(admin: Admin, client_id: str, user_id: str): + token = admin.realm(f"/clients/{client_id}/evaluate-scopes/generate-example-id-token?userId={user_id}&scope=openid") + return token.get("tenant_id") + + +def all_groups(admin: Admin) -> list[dict]: + groups, first, page = [], 0, 100 + while True: + batch = admin.realm(f"/groups?first={first}&max={page}") + groups.extend(batch) + if len(batch) < page: + return groups + first += page + + +def tenant_bearing_groups(admin: Admin) -> list[str]: + """Policy scan: NO group in the realm may carry a tenant_id attribute.""" + offenders = [] + for g in all_groups(admin): + detail = admin.realm(f"/groups/{g['id']}") + if (detail.get("attributes") or {}).get("tenant_id"): + offenders.append(g["name"]) + return offenders + + +def registration_violations(realm_doc: dict) -> list[str]: + """Static contract on a realm file: every IdP registration must pin the + tenant (hardcoded mapper from the INSIGHT_TENANT_ID placeholder) and + stamp idp_sub; no group may carry a tenant_id attribute.""" + problems = [] + mappers = realm_doc.get("identityProviderMappers") or [] + for idp in realm_doc.get("identityProviders") or []: + alias = idp.get("alias", "?") + mine = [m for m in mappers if m.get("identityProviderAlias") == alias] + pins = [ + m + for m in mine + if m.get("identityProviderMapper") == "hardcoded-attribute-idp-mapper" + and (m.get("config") or {}).get("attribute") == "tenant_id" + and (m.get("config") or {}).get("attribute.value") == "$(env:INSIGHT_TENANT_ID)" + ] + if len(pins) != 1: + problems.append(f"idp '{alias}': expected exactly one INSIGHT_TENANT_ID pin mapper, found {len(pins)}") + if not any((m.get("config") or {}).get("user.attribute") == "idp_sub" for m in mine): + problems.append(f"idp '{alias}': no mapper stamps the idp_sub attribute") + for g in realm_doc.get("groups") or []: + if (g.get("attributes") or {}).get("tenant_id"): + problems.append(f"group '{g.get('name')}': carries a tenant_id attribute") + return problems + + +def main() -> int: + failures: list[str] = [] + + def check(name: str, ok: bool, detail: str) -> None: + print(f"{'ok ' if ok else 'FAIL'} {name}: {detail}") + if not ok: + failures.append(name) + + for realm_file in sorted(REPO_ROOT.glob("deploy/gitops/environments/*/keycloak/realms/*.yaml")): + problems = registration_violations(yaml.safe_load(realm_file.read_text())) + check( + f"registrations({realm_file.parent.parent.parent.name})", + not problems, + f"{realm_file.name}: {problems or 'contract holds'}", + ) + + realm = substitute(yaml.safe_load(CANONICAL_REALM.read_text())) + + subprocess.run(["docker", "rm", "-f", CONTAINER], capture_output=True, check=False) + sh( + "docker", + "run", + "-d", + "--name", + CONTAINER, + "-p", + f"127.0.0.1:{KC_PORT}:8080", + "-e", + "KC_BOOTSTRAP_ADMIN_USERNAME=admin", + "-e", + "KC_BOOTSTRAP_ADMIN_PASSWORD=admin", + KC_IMAGE, + "start-dev", + ) + try: + wait_for_keycloak() + admin = Admin() + try: + admin._call("/admin/realms", "POST", realm) + except urllib.error.HTTPError as e: + check("realm-import", False, f"canonical realm rejected: HTTP {e.code} {e.read()[:200]!r}") + return 1 + + profile = admin.realm("/users/profile") + profile["unmanagedAttributePolicy"] = "ADMIN_EDIT" + admin.realm("/users/profile", "PUT", profile) + + check("clean-baseline", not tenant_bearing_groups(admin), "canonical realm ships zero tenant groups") + + admin.realm( + "/users", + "POST", + {"username": "guard@example.com", "email": "guard@example.com", "enabled": True, "emailVerified": True}, + ) + user = admin.realm("/users?username=guard@example.com&exact=true")[0]["id"] + client = admin.realm("/clients?clientId=insight-authenticator")[0]["id"] + + claim = tenant_claim(admin, client, user) + check("fail-closed", claim is None, f"no pinned attribute -> claim {claim!r}") + + u = admin.realm(f"/users/{user}") + u["attributes"] = {"tenant_id": [TENANT_PIN]} + admin.realm(f"/users/{user}", "PUT", u) + claim = tenant_claim(admin, client, user) + check("pinned-tenant", claim == TENANT_PIN, f"pinned attribute -> claim {claim!r}") + check("claim-is-scalar", isinstance(claim, str), f"claim type {type(claim).__name__}") + + admin.realm("/groups", "POST", {"name": "tenant-a", "attributes": {"tenant_id": [TENANT_GROUP]}}) + groups = {g["name"]: g["id"] for g in admin.realm("/groups")} + admin.realm(f"/users/{user}/groups/{groups['tenant-a']}", "PUT") + claim = tenant_claim(admin, client, user) + check("group-inert", claim == TENANT_PIN, f"tenant group must not affect the token -> claim {claim!r}") + check( + "group-policy-detected", + bool(tenant_bearing_groups(admin)), + "a tenant-bearing group exists and the realm scan flags it", + ) + finally: + subprocess.run(["docker", "rm", "-f", CONTAINER], capture_output=True, check=False) + + if failures: + print(f"\ntenant-contract guard FAILED: {failures}") + return 1 + print("\ntenant-contract guard OK") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/deploy/gitops/README.md b/deploy/gitops/README.md index fe3b46747..f1c877e3c 100644 --- a/deploy/gitops/README.md +++ b/deploy/gitops/README.md @@ -254,6 +254,36 @@ so an IdP swap or upstream drift cannot change the tenant. Canonical realm shape to copy: [`environments/local/keycloak/realms/insight-broker.yaml`](environments/local/keycloak/realms/insight-broker.yaml). +Every IdP registration carries the same two mappers — the tenant pin +and the upstream-id stamp (a customer with several IdPs pins the same +tenant on each; an IdP's own tenancy assertions are never consulted): + +```yaml +identityProviderMappers: + - name: tenant-id + identityProviderAlias: + identityProviderMapper: hardcoded-attribute-idp-mapper + config: + syncMode: FORCE + attribute: tenant_id + attribute.value: + - name: idp-sub + identityProviderAlias: + identityProviderMapper: oidc-user-attribute-idp-mapper + config: + syncMode: FORCE + claim: + user.attribute: idp_sub +``` + +CI runs `deploy/compose/keycloak/tests/tenant_contract_guard.py` — a +contract test, not an audit of deployed realms: it statically checks +every committed realm file (each registration has exactly one tenant +pin and an `idp_sub` stamp; no group carries a `tenant_id` attribute) +and imports the canonical realm into a throwaway Keycloak to assert +token behavior (fail closed without the pin, pin emitted verbatim as a +single string, tenant-bearing groups inert). + ## Secret management Sealed secrets ([Bitnami sealed-secrets](https://github.com/bitnami-labs/sealed-secrets)) diff --git a/deploy/gitops/environments/local/keycloak/realms/insight-broker.yaml b/deploy/gitops/environments/local/keycloak/realms/insight-broker.yaml index d07c5ac88..7425a9d83 100644 --- a/deploy/gitops/environments/local/keycloak/realms/insight-broker.yaml +++ b/deploy/gitops/environments/local/keycloak/realms/insight-broker.yaml @@ -10,10 +10,13 @@ realm: insight-broker enabled: true clientScopes: - # The claim allow-list: tokens carry exactly email + single-string tenant_id - # (plus protocol-level sub). tenant_id is a user attribute stamped per IdP by - # a hardcoded-attribute-idp-mapper from the INSIGHT_TENANT_ID placeholder - # (global.tenantDefaultId) — pinned per env, never from an upstream claim. + # The claim allow-list: tokens carry exactly email, single-string tenant_id, + # and idp_sub (plus protocol-level sub). tenant_id is ALWAYS the pinned + # per-registration tenant: every IdP's hardcoded-attribute-idp-mapper stamps + # INSIGHT_TENANT_ID (global.tenantDefaultId); a customer with several IdPs + # pins the same tenant on each. What an IdP asserts about tenancy is never + # consulted. idp_sub carries the upstream directory id (e.g. Entra oid) for + # the login-bootstrap person lookup, stamped per IdP by an importer mapper. - name: insight description: Canonical Insight claim allow-list (email + single-string tenant_id) protocol: openid-connect @@ -44,6 +47,18 @@ clientScopes: id.token.claim: "true" access.token.claim: "true" userinfo.token.claim: "true" + # Upstream directory id passthrough (login-bootstrap external id). + - name: idp_sub + protocol: openid-connect + protocolMapper: oidc-usermodel-attribute-mapper + consentRequired: false + config: + user.attribute: idp_sub + claim.name: idp_sub + jsonType.label: String + id.token.claim: "true" + access.token.claim: "true" + userinfo.token.claim: "true" clients: # Authenticator client: full-scope off, exactly [basic, insight] — `basic` diff --git a/docs/components/backend/authenticator/specs/ADR/0003-keycloak-identity-broker.md b/docs/components/backend/authenticator/specs/ADR/0003-keycloak-identity-broker.md index 695af0545..f9bfa92a6 100644 --- a/docs/components/backend/authenticator/specs/ADR/0003-keycloak-identity-broker.md +++ b/docs/components/backend/authenticator/specs/ADR/0003-keycloak-identity-broker.md @@ -9,6 +9,13 @@ date: 2026-08-04 **Status history**: +- 2026-08-06: AMENDED -- claim-value-to-tenant translation (the advanced claim-to-group mapper + sketched in the Decision Outcome) is REJECTED: the tenant is always the fixed per-registration + pin from environment values, an IdP's own tenancy assertions are never consulted, and a + customer with several IdPs pins the same tenant on each registration. Two customers sharing + an IdP vendor do not intersect: realm-per-customer gives each its own registration, client, + and pin. A CI guard asserts the contract against the canonical realm (fail-closed without the + pin, single-string claim, tenant-bearing groups inert and flagged). - 2026-08-05: AMENDED -- of the instance-deployment options this ADR left open, the umbrella subchart is chosen: the broker runs **in-stack** (production-mode `insight-keycloak`, MariaDB-backed) as part of each environment's auth services, amending ADR-0002's shared @@ -104,19 +111,18 @@ for multi-customer (cloud) installations (see realm selection below). secrets; nothing lands in the repository or an image (the #2163 credentials criterion). The admin UI is read-only in practice: `KeycloakRealmImport` and hand edits are not configuration channels, and config-cli re-applies the versioned realm on every sync, reverting drift. -- **Claims are shaped at the broker.** Per-provider **identity provider mappers** inject or - import the Insight `tenant_id` (`hardcoded-attribute-idp-mapper` for a fixed per-registration - tenant; claim-importer mappers where the upstream carries tenancy, e.g. Entra `tid`). The - client's **protocol mappers / client scopes** are the allow-list: the token contains only what - is explicitly emitted -- `sub`, `email`, one string `tenant_id` -- matching what the compose - realm generator already emits today. Upstream claims never pass through by default. Where a - single upstream registration itself distinguishes tenants by a claim value, the external value - is translated inside the realm: an advanced claim-to-group mapper (`syncMode: FORCE`) puts the - user in a per-tenant group whose `tenant_id` attribute carries the internal UUID, emitted by - the protocol mapper with group-attribute aggregation. The translation table is realm YAML, one - entry per external tenant, and an unmapped value fails closed -- no group, no `tenant_id` - claim, token rejected downstream. Exactly one group per user is an invariant the end-to-end - tests guard. +- **Claims are shaped at the broker.** Every provider registration carries a + `hardcoded-attribute-idp-mapper` pinning the fixed per-registration Insight `tenant_id` from + environment values, plus an attribute-importer stamping `idp_sub` (the upstream's stable + directory id, e.g. Entra `oid` -- the login-bootstrap external id). An upstream's own tenancy + assertions are never consulted (amended 2026-08-06; the claim-to-group translation once + sketched here is rejected): a customer with several IdPs pins the same tenant on each + registration, and two customers sharing an IdP vendor never intersect -- realm-per-customer + gives each its own registration, client, and pin. The client's **protocol mappers / client + scopes** are the allow-list: the token contains only what is explicitly emitted -- `sub`, + `email`, one string `tenant_id`, `idp_sub` -- and deliberately does NOT aggregate attributes + over groups, so a group-sourced tenant is mechanically impossible; a CI guard asserts the + contract on every committed realm file and against a live import. - **Topology: one realm per customer**, holding that customer's brokered IdPs and one confidential client. The single-`tenant_id` rule holds because each provider registration (or upstream tenancy claim) maps to exactly one Insight tenant. Realm-per-tenant remains available