From 76147af8f1665fc5603ce6bf6cbab5f3b7147244 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 7 May 2026 19:22:34 -0400 Subject: [PATCH] feat(plugins): Accept keycloak_url + keycloak_realm on jwt-validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Pre-PR-#378 the binary ran `config.Resolve()` → `deriveJWKSURL()` to derive `inbound.jwks_url` from `outbound.token_url` (which itself came from `keycloak_url` + `keycloak_realm` — the INTERNAL service URL). After the per-plugin cutover, each plugin decodes its config in isolation, so jwt-validation no longer sees token-exchange's fields. Its only fallback is to derive jwks_url from its own `issuer` — but `issuer` is the PUBLIC hostname that appears in token `iss` claims, which in split-horizon deployments (Kagenti's typical Kind and OpenShift setups) isn't reachable from inside the sidecar pod. Observed failure (CI run kagenti/kagenti#25525390807, weather-tool- advanced envoy-proxy stderr): msg="JWT validation failed" error="fetching JWKS: Get \"http://keycloak.localtest.me:8080/realms/kagenti /protocol/openid-connect/certs\": dial tcp [::1]:8080: connect: connection refused" Every inbound request → 401. Authbridge Weather (advanced) E2E broke. ## Fix Add `keycloak_url` + `keycloak_realm` fields to jwt-validation's config, symmetric with token-exchange's fields of the same name. `applyDefaults()` now derives `jwks_url` with this priority: 1. Explicit jwks_url wins 2. keycloak_url + keycloak_realm (the internal URL) 3. Issuer (single-horizon fallback — existing behavior) Chart/operator/backend callers pass `keycloak_url` + `keycloak_realm` that they already know; the plugin figures out the JWKS path. No one has to hand-roll the full URL anymore. ## Scope - `jwtvalidation.go` — two new fields, updated applyDefaults comment - `plugins_test.go` — 4 new tests + tightened existing JWKS test to assert the exact derived URL (so a future refactor of the priority chain can't silently fall through to a 404 path). Partial-keycloak fall-through is covered in both directions (url-without-realm and realm-without-url) to catch a future AND-check refactor. - `authbridge-combined.yaml` — pass KEYCLOAK_URL/KEYCLOAK_REALM into inbound jwt-validation as well as outbound token-exchange. Header comment explains where the env vars land from (envFrom on the Deployment, injected by operator/chart). ## Backward compatibility Purely additive: - Existing configs with only `issuer` still parse and work (single-horizon deployments unaffected). - Existing configs with explicit `jwks_url` still win (custom JWKS proxies unaffected). - New fields share namespace with token-exchange but decode independently — plugin configs remain isolated. ## Follow-ups Once this lands and a new extensions tag ships: - kagenti-operator: update `synthesizePipeline` to pass keycloak_url + keycloak_realm through to jwt-validation (it already has them in NamespaceConfig). Release a new operator alpha tag. - kagenti chart: drop the temporary explicit `jwks_url` added in kagenti#1507 once both new pins are available. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/plugins/jwtvalidation.go | 43 ++++++++- authbridge/authlib/plugins/plugins_test.go | 93 ++++++++++++++++++- authbridge/authproxy/authbridge-combined.yaml | 17 +++- 3 files changed, 145 insertions(+), 8 deletions(-) diff --git a/authbridge/authlib/plugins/jwtvalidation.go b/authbridge/authlib/plugins/jwtvalidation.go index 5dc909c77..bee71095d 100644 --- a/authbridge/authlib/plugins/jwtvalidation.go +++ b/authbridge/authlib/plugins/jwtvalidation.go @@ -23,13 +23,37 @@ import ( // validate pattern. type jwtValidationConfig struct { // Issuer is the JWT `iss` claim expected on inbound tokens. + // In split-horizon deployments this is the PUBLIC Keycloak URL + // (whatever Keycloak stamps into the `iss` claim) — it only needs + // to match bit-for-bit, not be reachable from inside the pod. Issuer string `json:"issuer"` // JWKSURL points at the JWKS endpoint used to verify signatures. - // When empty, derived from Issuer using Keycloak's convention - // (/protocol/openid-connect/certs). + // The sidecar actually GETs this URL from inside the cluster, so + // in split-horizon deployments it must be the INTERNAL Keycloak + // URL — not the public hostname from Issuer, which typically + // won't resolve from inside the mesh. + // + // When empty, the URL is derived with this priority: + // 1. KeycloakURL + KeycloakRealm (Keycloak convention; the + // internal URL, when the operator supplies it) + // 2. Issuer (fallback for single-horizon deployments where the + // issuer hostname is reachable from inside the cluster) JWKSURL string `json:"jwks_url"` + // KeycloakURL and KeycloakRealm are a convenience for deriving + // JWKSURL from the internal Keycloak service URL, symmetric with + // token-exchange's fields of the same name. Prefer supplying these + // over JWKSURL when you also want the usual split-horizon behavior + // (public Issuer + internal JWKS fetch). + // + // Pre-PR-#378 the binary derived jwks_url from outbound.token_url + // via a cross-plugin pass; per-plugin configs don't share state, + // so each plugin now carries its own copy of the "where is + // Keycloak internally" hint. + KeycloakURL string `json:"keycloak_url"` + KeycloakRealm string `json:"keycloak_realm"` + // Audience is the literal audience value expected on inbound // tokens. One of {Audience, AudienceFile, AudienceMode:"per-host"} // is required. @@ -57,8 +81,19 @@ type jwtValidationConfig struct { } func (c *jwtValidationConfig) applyDefaults() { - if c.JWKSURL == "" && c.Issuer != "" { - c.JWKSURL = strings.TrimRight(c.Issuer, "/") + "/protocol/openid-connect/certs" + // JWKSURL derivation priority: + // 1. Explicit JWKSURL wins. + // 2. KeycloakURL + KeycloakRealm → internal Keycloak URL (the + // reachable host for JWKS fetching in split-horizon setups). + // 3. Issuer → same host as the token's iss claim (fine for + // single-horizon deployments; breaks split-horizon). + if c.JWKSURL == "" { + if c.KeycloakURL != "" && c.KeycloakRealm != "" { + base := strings.TrimRight(c.KeycloakURL, "/") + "/realms/" + c.KeycloakRealm + c.JWKSURL = base + "/protocol/openid-connect/certs" + } else if c.Issuer != "" { + c.JWKSURL = strings.TrimRight(c.Issuer, "/") + "/protocol/openid-connect/certs" + } } if c.AudienceMode == "" { c.AudienceMode = "static" diff --git a/authbridge/authlib/plugins/plugins_test.go b/authbridge/authlib/plugins/plugins_test.go index 7df77ba15..13932fb16 100644 --- a/authbridge/authlib/plugins/plugins_test.go +++ b/authbridge/authlib/plugins/plugins_test.go @@ -143,14 +143,101 @@ func TestJWTValidation_Configure_DefaultsJWKSFromIssuer(t *testing.T) { if err != nil { t.Fatalf("Configure: %v", err) } - // The derived JWKS URL is applied during Configure — we can't - // inspect it directly because it's buried inside the verifier, but - // if the inner auth handler is nil we know Configure bailed. + // Issuer-derived fallback; verify the exact URL so a future refactor + // of the priority chain can't silently fall through to a 404 path. + if got, want := p.cfg.JWKSURL, "http://keycloak/realms/kagenti/protocol/openid-connect/certs"; got != want { + t.Errorf("JWKSURL = %q, want %q", got, want) + } if p.inner == nil { t.Fatal("Configure produced no inner auth handler") } } +// Split-horizon case: operator supplies a public `issuer` (for iss-claim +// matching) and internal `keycloak_url` + `keycloak_realm` (for reachable +// JWKS fetching). The derivation prefers the internal URL — deriving from +// issuer here would send the request into the public hostname which, in +// Kagenti's Kind/OpenShift setups, resolves to 127.0.0.1 and fails +// "connection refused" (see authbridge CLAUDE.md gotcha #2). +func TestJWTValidation_Configure_DerivesJWKSFromInternalKeycloakURL(t *testing.T) { + p := NewJWTValidation() + raw := []byte(`{ + "issuer": "http://keycloak.localtest.me:8080/realms/kagenti", + "keycloak_url": "http://keycloak-service.keycloak.svc:8080", + "keycloak_realm": "kagenti", + "audience": "a" + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + want := "http://keycloak-service.keycloak.svc:8080/realms/kagenti/protocol/openid-connect/certs" + if got := p.cfg.JWKSURL; got != want { + t.Errorf("JWKSURL = %q, want %q (internal URL from keycloak_url+realm, not issuer)", got, want) + } +} + +// Explicit jwks_url beats every derivation source. Operators who know +// exactly which endpoint to hit (e.g. a custom JWKS proxy) must not have +// it silently overwritten by the keycloak_url/realm derivation. +func TestJWTValidation_Configure_ExplicitJWKSURLWins(t *testing.T) { + p := NewJWTValidation() + raw := []byte(`{ + "issuer": "http://keycloak.public:8080/realms/kagenti", + "jwks_url": "http://custom-jwks-proxy.example/keys", + "keycloak_url": "http://keycloak-internal:8080", + "keycloak_realm": "kagenti", + "audience": "a" + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if got, want := p.cfg.JWKSURL, "http://custom-jwks-proxy.example/keys"; got != want { + t.Errorf("JWKSURL = %q, want %q (explicit jwks_url must win over derivations)", got, want) + } +} + +// Only one of keycloak_url/keycloak_realm is supplied — derivation can't +// complete (Keycloak path needs both). Fall through to issuer-derivation +// rather than crashing or leaving JWKSURL empty. +// +// Covered both directions to catch a future refactor that ANDs the check +// differently (e.g. a short-circuit that treats empty realm as default +// would silently build a bogus URL). +func TestJWTValidation_Configure_PartialKeycloakConfigFallsThroughToIssuer(t *testing.T) { + cases := []struct { + name, raw string + }{ + { + name: "keycloak_url without realm", + raw: `{ + "issuer": "http://keycloak/realms/kagenti", + "keycloak_url": "http://internal:8080", + "audience": "a" + }`, + }, + { + name: "keycloak_realm without url", + raw: `{ + "issuer": "http://keycloak/realms/kagenti", + "keycloak_realm": "kagenti", + "audience": "a" + }`, + }, + } + want := "http://keycloak/realms/kagenti/protocol/openid-connect/certs" + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(tc.raw)); err != nil { + t.Fatalf("Configure: %v", err) + } + if got := p.cfg.JWKSURL; got != want { + t.Errorf("JWKSURL = %q, want %q (partial keycloak_* must not short-circuit issuer-derivation)", got, want) + } + }) + } +} + func TestJWTValidation_Configure_AudienceFromFile(t *testing.T) { dir := t.TempDir() f := filepath.Join(dir, "aud") diff --git a/authbridge/authproxy/authbridge-combined.yaml b/authbridge/authproxy/authbridge-combined.yaml index db889f8e6..a4b89928c 100644 --- a/authbridge/authproxy/authbridge-combined.yaml +++ b/authbridge/authproxy/authbridge-combined.yaml @@ -1,16 +1,29 @@ # Default config for the combined AuthBridge sidecar image. # Env vars are expanded at startup by the unified binary's config loader. # These env vars match the operator's ConfigMap contract (authbridge-config). +# At runtime they land on the sidecar via envFrom on the Deployment — +# injected by the kagenti-operator admission webhook (or by the chart +# template for pre-declared namespaces). Keys are listed in +# authbridge/CLAUDE.md "Required ConfigMaps for Webhook Injection". # # Plugin settings live under pipeline.*.plugins[].config; see # authbridge/authlib/plugins/CONVENTIONS.md for the per-plugin schema. # Fields omitted below fall back to plugin defaults — notably: # jwt-validation: audience_file=/shared/client-id.txt, bypass_paths # = .well-known/* + health/ready/live probes, -# jwks_url derived from issuer +# jwks_url derived from keycloak_url + keycloak_realm +# (internal URL; see split-horizon note below). # token-exchange: identity file paths under /shared/, jwt_svid_path # =/opt/jwt_svid.token, routes.file # =/etc/authproxy/routes.yaml +# +# Split-horizon DNS note: `issuer` is the PUBLIC Keycloak URL (matches +# the `iss` claim Keycloak stamps on tokens); `keycloak_url` is the +# INTERNAL service URL that the sidecar actually reaches. The +# jwt-validation plugin uses the latter to build its JWKS fetch URL, +# which is why KEYCLOAK_URL has to be passed into inbound as well as +# outbound — even though pre-PR-#378 the binary derived jwks_url from +# outbound.token_url via a cross-plugin pass. mode: envoy-sidecar pipeline: @@ -19,6 +32,8 @@ pipeline: - name: jwt-validation config: issuer: "${ISSUER}" + keycloak_url: "${KEYCLOAK_URL}" + keycloak_realm: "${KEYCLOAK_REALM}" outbound: plugins: - name: token-exchange