docs(auth): SecurityPolicy issuer stays in-cluster — investigation result for #112, with tests#144
Open
tylerpotts wants to merge 1 commit into
Open
docs(auth): SecurityPolicy issuer stays in-cluster — investigation result for #112, with tests#144tylerpotts wants to merge 1 commit into
tylerpotts wants to merge 1 commit into
Conversation
…the invariant Resolves the investigation in issue #112: SecurityPolicy.spec.oidc.provider.issuer deliberately remains the in-cluster Keycloak realm URL even when Keycloak (frontendUrl configured) emits a public iss claim. Verified against Envoy Gateway v1.6.3 and Envoy oauth2 filter source: - EG uses the issuer only for control-plane OIDC discovery, and skips discovery entirely when authorizationEndpoint and tokenEndpoint are both explicitly set (which the operator does whenever KEYCLOAK_EXTERNAL_URL is configured). The issuer never reaches the data plane. - The Envoy oauth2 filter has no issuer config field and never inspects the token's iss claim; its only JWT decode is an unverified exp read. - EG ignores the discovery document's issuer field, so the OIDC Discovery issuer-match rule is not enforced. - Strict-iss consumers (e.g. downstream spec.jwt providers) read the public issuer from the client Secret's issuer-url key (GetExternalIssuerURL). Changes: - GetIssuerURL/GetEndpointOverrides doc comments record the verified semantics and the invariant that makes the in-cluster issuer safe. - TestKeycloakProvider_GetIssuerURL: new case asserting the issuer stays in-cluster when ExternalURL is set. - TestBuildSecurityPolicySpec_KeycloakIssuerAndEndpointSplit: reconciler-level test through buildSecurityPolicySpec with the real KeycloakProvider, asserting issuer + token endpoint in-cluster and authorization/end-session on the external URL (also satisfies issue #113). - docs/reconcilers/authentication.md: new Issuer Semantics section.
Docker Images BuiltImages pushed to Quay.io for branch
Test the operator: kubectl apply -k https://github.com/nebari-dev/nebari-operator.git/config/default?ref=issue-112-issuer-semantics
kubectl set image deployment/nebari-operator-controller-manager manager=quay.io/nebari/nebari-operator:issue-112-issuer-semantics -n nebari-operator-system |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Answers the investigation in #112:
SecurityPolicy.spec.oidc.provider.issuershould NOT trackKEYCLOAK_EXTERNAL_URL. The in-cluster issuer is correct and the iss-claim mismatch with KeycloakfrontendUrldeployments is harmless by construction. This PR records that decision in code comments and docs, and adds tests locking in the invariant that makes it safe.Investigation findings (verified against Envoy Gateway v1.6.3 and Envoy source)
/.well-known/openid-configuration— and only whenauthorizationEndpointortokenEndpointis not explicitly set (internal/gatewayapi/securitypolicy.go,buildOIDCProvider). WhenKEYCLOAK_EXTERNAL_URLis configured the operator sets both, so EG never contacts the issuer and the IR passed to the data plane does not carry it.oauth2filter never validatesiss. Its config proto (api/envoy/extensions/filters/http/oauth2/v3/oauth.proto) has no issuer field at all; the filter's only JWT decode is an unverifiedexpread for cookie lifetimes (source/extensions/filters/http/oauth2/filter.cc). This holds through Envoy 1.39-dev.issuer. So even the discovery fallback path works when Keycloak returns a public issuer on the internal endpoint.iss—spec.jwtproviders (Envoyjwt_authn), e.g. the one the LLM serving pack's operator generates — already receive the public issuer via the client Secret'sissuer-urlkey, populated fromGetExternalIssuerURL(). That key is empty whenKEYCLOAK_EXTERNAL_URLis unset, so strict-iss consumers require it to be set.Changes
GetIssuerURL/GetEndpointOverridesdoc comments record the verified semantics and the both-endpoints-set invariant.TestKeycloakProvider_GetIssuerURL: new case asserting the issuer stays in-cluster whenExternalURLis set.TestBuildSecurityPolicySpec_KeycloakIssuerAndEndpointSplit: reconciler-level test throughbuildSecurityPolicySpecwith the realKeycloakProvider— issuer and token endpoint on the in-cluster host, authorization/end-session on the external URL, plus the no-ExternalURL fallback. This is the reconciler-level endpoint-split test requested in Add reconciler test asserting SecurityPolicy.spec.oidc.provider has Token on in-cluster URL and Authorization on public URL #113.docs/reconcilers/authentication.md: new Issuer Semantics (Keycloak) section.Closes #112
Closes #113
Test plan
go test ./internal/controller/reconcilers/auth/...passes; new tests verified to execute (-v -run)golangci-lint run— 0 issuesgo test ./...— no regressions vs main (pre-existingTestControllersHTTPRoute scheme failure reproduced on main, unrelated)