fix: Keycloak 26 import compatibility + DB-backed runtime federation registry - #18
fix: Keycloak 26 import compatibility + DB-backed runtime federation registry#18seonghobae wants to merge 12 commits into
Conversation
…irst RP Real-world bring-up on Keycloak 26.3.2 surfaced four import/runtime failures in the committed realm, each reproduced and fixed: - RealmRepresentation rejects unknown fields, so the $-prefixed annotation keys aborted --import-realm and crash-looped the container. Annotations moved to deploy/keycloak/README.md; the validator now fails on any $ key. - SAML IdP URLs are URL-validated at import: the bare __set_from_kv__ placeholder aborted the import. Placeholders are now URL-shaped (https://set-from-kv.invalid/__set_from_kv__) and still patched from KV. - An ENABLED committed LDAP source with placeholder DNs breaks every realm user operation (Invalid DN). The committed source now ships disabled and kcadm-bootstrap.sh enables it only after patching real values from KV. - The default Infinispan jdbc-ping stack crash-loops single-node compose restarts (each aborted boot leaves a stale jgroups_ping coordinator row the next boot fatally tries to join). The standalone compose now defaults to KC_CACHE=local via IDP_CACHE_MODE (clustered deployments set ispn). Imported realms also lack the standard client scopes, and without `basic` Keycloak 26 lightweight access tokens omit `sub`, breaking any RP that authenticates by subject. The realm now commits basic/profile/email scopes as realm defaults, adds an audience mapper to the RP template, and registers naruon-web as the first concrete RP (public PKCE S256 client with the sub/aud/role/org/workspace claims naruon's session contract requires). Verified: scripts/validate_realm.py passes; the realm imports cleanly into quay.io/keycloak/keycloak:26.3.2 with no sanitization; a live naruon authorization-code login (PKCE, RS256, JWKS) established a backend session end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…time API
Employer-specific federation (the hssmartdev ADFS SAML IdP, corporate LDAP)
was committed into realm-cwl.json with KV placeholders. That hardcodes one
employer into the ecosystem IdP's code and also breaks bring-up mechanically:
SAML IdP URLs are validated at import (placeholders abort it) and an enabled
LDAP source with placeholder DNs fails every realm user operation
(Invalid DN: __set_from_kv__).
External IdPs are deployment data, so they now live behind a runtime admin
API on the account-unification service:
- GET/PUT/DELETE /federation/identity-providers[/{alias}] and
POST /federation/identity-providers:apply. Desired state persists in the
KV/DB config store (source of truth) and is converged into Keycloak via
the Admin REST API, so a realm rebuild re-converges with one apply call.
- AdminApi gains identity-provider CRUD (HttpAdminApi + mock); KvStore gains
delete() on the protocol and both backends.
- realm-cwl.json commits no identityProviders and no user-storage federation;
scripts/validate_realm.py now fails closed on committed federation instead
of requiring it, and kcadm-bootstrap.sh drops the ADFS/LDAP patch steps and
additionally grants manage-identity-providers to the service account.
- deploy/templates/ remain as ready-made payload references for the API.
Verified: scripts/validate_realm.py passes; the account-unification suite
passes (58 tests) including new federation registry tests that register the
employer ADFS as runtime data; the realm still imports cleanly and a live
naruon OIDC login flow reaches the passwordless form after removing the
hardcoded federation from a running realm.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Semgrep (dynamic-urllib-use-detected) flags urllib.request.urlopen because it accepts file:// and other schemes. The probe only ever talks to the local listener, so it now validates the scheme and issues the request over an explicit plain-HTTP http.client connection; non-http probe URLs are refused before any connection is opened, with a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ret handling Strix flagged the account-unification service (which holds realm-management privileges) as reachable unauthenticated, plus path-traversal and secret-in-argv issues. Remediated without suppression: - Operator bearer auth (app/auth.py) now gates every privileged router — merge, identity reads, SCIM provisioning/deactivation, and the federation registry — via a shared operator token from the KV/DB config store (constant-time compare, fails closed when unconfigured). /healthz stays open for probes. (VULN-0001 merge, VULN-0002 federation, VULN-0003 SCIM, VULN-0007 reads) - Path-segment identifier validation (app/identifiers.py) rejects '/', '\\', '.', '..', percent-encoding, and control characters. Applied at the API/SCIM boundary (400) and as a centralized chokepoint inside the Admin REST client before any URL is built, so a user_id like '../users/victim' or '%2e%2e' cannot escape the intended resource. (VULN-0006) - kcadm bootstrap no longer passes the admin password on argv: it fetches a short-lived admin token via curl with the password sourced from a 0600 temp file (--data-urlencode "@file"), then configures kcadm with that bearer token. (VULN-0005) - The Helm chart supports and documents an immutable image digest for the privileged account-unification image, rendered as tag@sha256 when set. (VULN-0004) New tests: operator-auth gating (missing/wrong/valid token, fail-closed, /healthz open), path-segment validation + Admin-client traversal rejection via MockTransport. Full suite passes (73 tests); ruff clean; interrogate 97.4%; validate_realm.py OK. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ot-review dispatch allowlist Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pull request was closed
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head8cf239ffdc53c5297acdc01197c7babcf6860de7. -
Head SHA:
8cf239ffdc53c5297acdc01197c7babcf6860de7 -
Workflow run: 29877526040
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (20 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (20 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test (8 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (8 files)"]
R2 --> V2["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (20 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (20 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test (8 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (8 files)"]
R2 --> V2["targeted test run"]
|
The login page offered no way to create an account, so a fresh deployment could onboard nobody without operator kcadm surgery. Registration is now allowed with the email address as the account identity; the throwaway registration password never becomes a usable credential because the browser flow has no password authenticator and the default webauthn-register-passwordless required action enrolls a passkey in the first session. verifyEmail stays false while the realm has no smtpServer — the validator now enforces both pairings fail-closed (email-first + default passkey enrollment when registration is on; verifyEmail only with SMTP), each proven by mutation tests. Evidence: scripts/validate_realm.py OK on the shipped realm and rejects all three mutations; account-unification pytest 77 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Product frontends now own the signup UX: POST /registration/accounts on the account-unification service (own bearer token, distinct from the operator credential) creates the Keycloak account via the Admin API with an initial password and the webauthn-register-passwordless required action. The IdP-hosted registration form goes back off (registrationAllowed:false). The browser flow gains a browser-passwordless-credentials subflow where the passkey and the credential form are ALTERNATIVE siblings: the form is offered only while the account has no passkey, and the in-process password janitor (plus /registration/password-janitor:run) revokes the bootstrap password after enrollment, keeping the steady state passwordless. The validator enforces this exact bootstrap shape and still bans every other credential-form authenticator. Fresh-bring-up fixes found while wiring this live: the audit sink wrote into the read-only /bootstrap mount (now a separate writable path with a Dockerfile-owned directory), and kcadm-bootstrap.sh granted realm-management roles without scope mappings or a client-role protocol mapper, so the fullScopeAllowed:false service-account token never carried them and every Admin call failed 403. Evidence: validate_realm.py OK plus REQUIRED-password-form and username-password-form mutations rejected; service pytest 90 passed; live compose bring-up serves /healthz ok and POST /registration/accounts returns 201 with the account id. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
HttpAdminApi cached the service-account token for the connection lifetime, so after the token lifespan every Admin REST call failed 401 until a process restart — first observed as registration 500s minutes after bring-up. Each verb now routes through _send_with_reauth, which refreshes the token exactly once on a 401 and then fails honestly. Regression test drives a MockTransport that rejects the stale token and asserts the retry carries a freshly issued one. Evidence: service pytest 91 passed; live compose registration returns 201 again after the previous token aged out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (34)
Comment |
|
Superseded by #42, which integrates this Keycloak 26/runtime federation work with the overlapping SCIM serialization, SQLite worker-safety, credential redaction, registration rollback, lifecycle, and current dependency/CI state. Further fixes and verification will continue on the consolidated current-head PR. |
2 participants
Summary
Real-world bring-up of cwl-idp on Keycloak 26.3.2 (as the OIDC login IdP for naruon) surfaced import/runtime failures in the committed realm, and operator review rejected hardcoding employer federation into realm code. Two commits:
1.
fix:make the realm importable on Keycloak 26 and onboard naruon as first RP$-annotation keys abort--import-realm(Unrecognized field "$comment"— RealmRepresentation rejects unknown fields) and crash-loop the container. Notes moved todeploy/keycloak/README.md;scripts/validate_realm.pynow fails on any$key.jgroups_pingcoordinator row the next boot fatally tries to join. Standalone compose now defaultsKC_CACHE=localviaIDP_CACHE_MODE(clustered deployments setispn).subwithout thebasicscope (imported realms get no standard scopes auto-created), which 401s any subject-authenticating RP. The realm now commitsbasic/profile/emailscopes as realm defaults.naruon-webas the first concrete RP (public PKCE S256 client) with the audience +role/org/workspaceclaims naruon's session contract requires; RP template gains an audience mapper.2.
feat:move external federation out of realm code into a DB-backed runtime APIPer operator direction, employer-specific federation (hssmartdev ADFS, corporate LDAP) must be runtime-configurable data, not committed realm code (committed placeholders also broke import: SAML URLs are URL-validated, and an enabled LDAP source with placeholder DNs fails every realm user operation with
Invalid DN).GET/PUT/DELETE /federation/identity-providers[/{alias}]+POST /federation/identity-providers:apply. Desired state persists in the KV/DB config store (source of truth) and is converged into Keycloak via the Admin REST API — a realm rebuild re-converges with oneapply.AdminApigains identity-provider CRUD (HttpAdminApi+ test mock);KvStoregainsdelete()(protocol + InMemory + SQLite).identityProviders/ user-storage federation; the validator now fails closed on committed federation;kcadm-bootstrap.shdrops the ADFS/LDAP patch steps and grantsmanage-identity-providersto the service account.deploy/templates/remain as payload references.Verification
Live evidence: the fixed realm imports cleanly into
quay.io/keycloak/keycloak:26.3.2with no sanitization; a live naruon authorization-code login (PKCE, RS256, JWKS over the compose network) established a backend session end-to-end, re-verified after removing the hardcoded federation from the running realm. The new federation tests register the employer ADFS as runtime data (store → converge → simulated realm rebuild → re-converge → delete).🤖 Generated with Claude Code