docs(auth): SecurityPolicy issuer stays in-cluster — investigation result for #112, with tests

docs/reconcilers/authentication.md

@@ -193,6 +193,33 @@ spec:
 - `redirectURL`: Defaults to `https://{hostname}/oauth2/callback`
 - `scopes`: Uses `spec.auth.scopes` or defaults to `["openid", "profile", "email"]`
 
+### Issuer Semantics (Keycloak)
+
+For Keycloak, `provider.issuer` is always the **in-cluster** realm URL, even when
+`KEYCLOAK_EXTERNAL_URL` is set and Keycloak (with a public `frontendUrl`) stamps the
+**public** URL into every token's `iss` claim. This mismatch is deliberate and harmless
+([investigation in #112](https://github.com/nebari-dev/nebari-operator/issues/112)):
+
+- Envoy Gateway uses the issuer **only at control-plane time** to fetch
+  `/.well-known/openid-configuration`, and only when `authorizationEndpoint` or
+  `tokenEndpoint` is not explicitly set. When `KEYCLOAK_EXTERNAL_URL` is configured the
+  operator sets both, so the issuer URL is never contacted and never reaches the data
+  plane.
+- The Envoy `oauth2` filter has no issuer field in its configuration and never inspects
+  the token's `iss` claim (its only JWT decode is an unverified `exp` read for cookie
+  lifetimes).
+- Envoy Gateway does not enforce the OIDC Discovery issuer-match rule: it reads only the
+  endpoint fields from the discovery document and ignores its `issuer` field.
+- Consumers that **do** strictly validate `iss` — e.g. a SecurityPolicy `spec.jwt`
+  provider configured by a downstream pack — must use the public issuer. They read it
+  from the client Secret's `issuer-url` key, which is populated from
+  `GetExternalIssuerURL()` (the public URL). That key is empty when
+  `KEYCLOAK_EXTERNAL_URL` is unset, so strict-`iss` consumers require it to be set.
+
+Keeping the issuer in-cluster means the discovery fallback (used when
+`KEYCLOAK_EXTERNAL_URL` is unset) runs from the envoy-gateway controller pod over
+cluster DNS, avoiding public TLS-chain trust and hairpin-routing requirements.
+
 **On Failure:**
 - Event: `Warning` with reason `SecurityPolicyFailed`
 - Condition: `AuthReady=False` with reason `SecurityPolicyFailed`

internal/controller/reconcilers/auth/providers/keycloak.go

@@ -71,7 +71,29 @@ func (p *KeycloakProvider) externalRealmURL() string {
 }
 
 // GetIssuerURL returns the internal cluster URL for the Keycloak realm.
-// Envoy uses this to fetch OIDC configuration from within the cluster.
+//
+// This deliberately stays on the in-cluster URL even when Keycloak issues
+// tokens with a public `iss` claim (frontendUrl / KC_HOSTNAME configured).
+// Investigated in https://github.com/nebari-dev/nebari-operator/issues/112:
+//
+//   - Envoy Gateway uses SecurityPolicy.spec.oidc.provider.issuer only at
+//     control-plane time, to fetch /.well-known/openid-configuration — and
+//     only when authorizationEndpoint or tokenEndpoint is not explicitly set
+//     (gateway v1.6.3 internal/gatewayapi/securitypolicy.go). The issuer is
+//     never passed to the Envoy oauth2 filter; the filter's config proto has
+//     no issuer field and the filter never inspects the token's `iss` claim.
+//   - Envoy Gateway does not enforce the OIDC Discovery issuer-match rule
+//     either: it unmarshals only the endpoint fields from the discovery
+//     document, so a public `issuer` value in the document is ignored.
+//   - Consumers that DO strictly validate `iss` (e.g. SecurityPolicy
+//     spec.jwt providers configured by downstream packs) must use the public
+//     issuer, which they read from the client Secret's issuer-url key —
+//     populated from GetExternalIssuerURL, not from this method.
+//
+// Keeping the issuer in-cluster means the discovery fallback (when endpoint
+// overrides are incomplete) is fetched from the envoy-gateway controller pod
+// over cluster DNS, avoiding public TLS-chain trust and hairpin-routing
+// requirements.
 func (p *KeycloakProvider) GetIssuerURL(ctx context.Context, nebariApp *appsv1.NebariApp) (string, error) {
 	return p.internalRealmURL(), nil
 }
@@ -88,6 +110,11 @@ func (p *KeycloakProvider) GetIssuerURL(ctx context.Context, nebariApp *appsv1.N
 //     the in-cluster URL causes the browser to fail DNS resolution and dead-end
 //     the OAuth2 flow.
 //
+// When KEYCLOAK_EXTERNAL_URL is set, both Authorization and Token endpoints
+// are explicitly provided, so Envoy Gateway skips OIDC discovery entirely and
+// the issuer URL never leaves the control plane (see GetIssuerURL). Tests
+// assert this invariant; do not remove one override without the other.
+//
 // Fallback when KEYCLOAK_EXTERNAL_URL is unset: Authorization and EndSession
 // are returned as nil overrides. Envoy Gateway then triggers OIDC discovery
 // against the (in-cluster) issuer URL and uses whatever Authorization /

internal/controller/reconcilers/auth/providers/keycloak_test.go

@@ -91,6 +91,23 @@ func TestKeycloakProvider_GetIssuerURL(t *testing.T) {
 			},
 			expectedURL: "http://custom-keycloak.auth.svc.cluster.local:9090/realms/custom-realm",
 		},
+		{
+			// Issue #112: the issuer deliberately stays in-cluster even when
+			// KEYCLOAK_EXTERNAL_URL is set. The external URL only affects
+			// browser-facing endpoint overrides and the client Secret's
+			// issuer-url key, never SecurityPolicy.spec.oidc.provider.issuer.
+			name: "ExternalURL set: issuer remains in-cluster",
+			kcConfig: config.KeycloakConfig{
+				URL:                    "http://keycloak-keycloakx-http.keycloak.svc.cluster.local:8080",
+				Realm:                  "nebari",
+				IssuerServiceName:      "keycloak-keycloakx-http",
+				IssuerServiceNamespace: "keycloak",
+				IssuerServicePort:      8080,
+				IssuerContextPath:      "",
+				ExternalURL:            "https://keycloak.example.com",
+			},
+			expectedURL: "http://keycloak-keycloakx-http.keycloak.svc.cluster.local:8080/realms/nebari",
+		},
 	}
 
 	for _, tt := range tests {

internal/controller/reconcilers/auth/reconciler_test.go

@@ -25,6 +25,7 @@ import (
 
 	egv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1"
 	appsv1 "github.com/nebari-dev/nebari-operator/api/v1"
+	"github.com/nebari-dev/nebari-operator/internal/config"
 	"github.com/nebari-dev/nebari-operator/internal/controller/reconcilers/auth/providers"
 	"github.com/nebari-dev/nebari-operator/internal/controller/utils/constants"
 	"github.com/nebari-dev/nebari-operator/internal/controller/utils/naming"
@@ -601,6 +602,105 @@ func TestBuildSecurityPolicySpec(t *testing.T) {
 	}
 }
 
+// TestBuildSecurityPolicySpec_KeycloakIssuerAndEndpointSplit exercises the
+// real KeycloakProvider end-to-end through buildSecurityPolicySpec (issues
+// #112 and #113): the issuer stays on the in-cluster URL, the token endpoint
+// stays on the in-cluster host (Envoy back-channel), and the browser-facing
+// authorization/end-session endpoints move to KEYCLOAK_EXTERNAL_URL.
+//
+// The "ExternalURL set" case also locks in the invariant that makes the
+// in-cluster issuer safe even when Keycloak emits a public `iss` claim
+// (frontendUrl configured): Envoy Gateway performs OIDC discovery against the
+// issuer only when authorizationEndpoint or tokenEndpoint is missing, so with
+// both explicitly set the issuer never leaves the control plane and is never
+// compared against the token's `iss` claim (the Envoy oauth2 filter has no
+// issuer field at all). See #112 for the full investigation.
+func TestBuildSecurityPolicySpec_KeycloakIssuerAndEndpointSplit(t *testing.T) {
+	scheme := runtime.NewScheme()
+	_ = appsv1.AddToScheme(scheme)
+	_ = corev1.AddToScheme(scheme)
+	_ = egv1alpha1.AddToScheme(scheme)
+
+	const internalRealm = "http://keycloak-keycloakx-http.keycloak.svc.cluster.local:8080/realms/nebari"
+
+	tests := []struct {
+		name                  string
+		externalURL           string
+		expectedAuthorization *string
+		expectedEndSession    *string
+	}{
+		{
+			name:                  "ExternalURL set: browser endpoints public, token and issuer in-cluster",
+			externalURL:           "https://keycloak.example.com",
+			expectedAuthorization: ptr.To("https://keycloak.example.com/realms/nebari/protocol/openid-connect/auth"),
+			expectedEndSession:    ptr.To("https://keycloak.example.com/realms/nebari/protocol/openid-connect/logout"),
+		},
+		{
+			// Without an external URL the browser-facing endpoints are left
+			// unset so Envoy Gateway falls back to OIDC discovery against the
+			// in-cluster issuer; the token endpoint stays pinned in-cluster.
+			name:        "ExternalURL unset: only token override is set",
+			externalURL: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			nebariApp := &appsv1.NebariApp{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-app",
+					Namespace: "default",
+				},
+				Spec: appsv1.NebariAppSpec{
+					Hostname: "test.example.com",
+					Auth: &appsv1.AuthConfig{
+						Enabled:  true,
+						Provider: constants.ProviderKeycloak,
+					},
+				},
+			}
+
+			client := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(nebariApp).
+				Build()
+
+			reconciler := &AuthReconciler{
+				Client:   client,
+				Scheme:   scheme,
+				Recorder: record.NewFakeRecorder(10),
+			}
+
+			provider := &providers.KeycloakProvider{
+				Config: config.KeycloakConfig{
+					Realm:                  "nebari",
+					IssuerServiceName:      "keycloak-keycloakx-http",
+					IssuerServiceNamespace: "keycloak",
+					IssuerServicePort:      8080,
+					ExternalURL:            tt.externalURL,
+				},
+			}
+
+			spec, err := reconciler.buildSecurityPolicySpec(context.Background(), nebariApp, provider)
+			if err != nil {
+				t.Fatalf("buildSecurityPolicySpec returned error: %v", err)
+			}
+			if spec.OIDC == nil {
+				t.Fatal("OIDC config is nil")
+			}
+
+			p := spec.OIDC.Provider
+			if p.Issuer != internalRealm {
+				t.Errorf("expected in-cluster issuer %q, got %q", internalRealm, p.Issuer)
+			}
+			expectedToken := ptr.To(internalRealm + "/protocol/openid-connect/token")
+			verifyOptionalEndpoint(t, "token", p.TokenEndpoint, expectedToken)
+			verifyOptionalEndpoint(t, "authorization", p.AuthorizationEndpoint, tt.expectedAuthorization)
+			verifyOptionalEndpoint(t, "endSession", p.EndSessionEndpoint, tt.expectedEndSession)
+		})
+	}
+}
+
 // TestBuildSecurityPolicySpec_ForwardAccessToken covers the forwardAccessToken
 // passthrough in isolation. Kept separate from TestBuildSecurityPolicySpec
 // to keep that table-driven test below the gocyclo complexity threshold.