add new bool supervisor configmap option to ignore userinfo endpoints

2026-01-07 05:57:02 +00:00 · 2025-08-27 12:12:42 -07:00
parent 44893e6b0d
commit e427a5202e
7 changed files with 403 additions and 30 deletions
--- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go
+++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 // Package oidcupstreamwatcher implements a controller which watches OIDCIdentityProviders.
@@ -129,6 +129,10 @@ func (c *ttlProviderCache) putProvider(key oidcDiscoveryCacheKey, value *oidcDis
 	c.cache.Set(key, value, oidcValidatorCacheTTL)
 }

+type GlobalOIDCConfig struct {
+	IgnoreUserInfoEndpoint bool
+}
+
 type oidcWatcherController struct {
 	cache                        UpstreamOIDCIdentityProviderICache
 	log                          plog.Logger
@@ -137,6 +141,7 @@ type oidcWatcherController struct {
 	secretInformer               corev1informers.SecretInformer
 	configMapInformer            corev1informers.ConfigMapInformer
 	validatorCache               oidcDiscoveryCache
+	globalOIDCConfig             GlobalOIDCConfig
 }

 // New instantiates a new controllerlib.Controller which will populate the provided UpstreamOIDCIdentityProviderICache.
@@ -149,6 +154,7 @@ func New(
 	log plog.Logger,
 	withInformer pinnipedcontroller.WithInformerOptionFunc,
 	validatorCache *cache.Expiring,
+	globalOIDCConfig GlobalOIDCConfig,
 ) controllerlib.Controller {
 	c := oidcWatcherController{
 		cache:                        idpCache,
@@ -158,6 +164,7 @@ func New(
 		secretInformer:               secretInformer,
 		configMapInformer:            configMapInformer,
 		validatorCache:               &ttlProviderCache{cache: validatorCache},
+		globalOIDCConfig:             globalOIDCConfig,
 	}
 	return controllerlib.New(
 		controllerlib.Config{Name: oidcControllerName, Syncer: &c},
@@ -235,6 +242,7 @@ func (c *oidcWatcherController) validateUpstream(ctx controllerlib.Context, upst
 		AdditionalAuthcodeParams: additionalAuthcodeAuthorizeParameters,
 		AdditionalClaimMappings:  upstream.Spec.Claims.AdditionalClaimMappings,
 		ResourceUID:              upstream.UID,
+		IgnoreUserInfoEndpoint:   c.globalOIDCConfig.IgnoreUserInfoEndpoint,
 	}

 	conditions := []*metav1.Condition{
--- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
+++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 package oidcupstreamwatcher
@@ -123,6 +123,7 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) {
 				logger,
 				withInformer.WithInformer,
 				expiringcache.NewExpiring(),
+				GlobalOIDCConfig{},
 			)

 			unrelated := corev1.Secret{}
@@ -182,6 +183,7 @@ func TestOIDCUpstreamWatcherControllerFilterConfigMaps(t *testing.T) {
 				logger,
 				withInformer.WithInformer,
 				expiringcache.NewExpiring(),
+				GlobalOIDCConfig{},
 			)

 			unrelated := corev1.ConfigMap{}
@@ -242,6 +244,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 		inputUpstreams         []runtime.Object
 		inputResources         []runtime.Object
 		inputValidatorCache    func(*testing.T) map[oidcDiscoveryCacheKey]*oidcDiscoveryCacheValue
+		inputGlobalOIDCConfig  *GlobalOIDCConfig
 		wantErr                string
 		wantLogs               []string
 		wantResultingCache     []*oidctestutil.TestUpstreamOIDCIdentityProvider
@@ -951,6 +954,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 					ClientID:                 testClientID,
 					AuthorizationURL:         *testIssuerAuthorizeURL,
 					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              true,
 					Scopes:                   append(testExpectedScopes, "xyz"), // includes openid only once
 					UsernameClaim:            testUsernameClaim,
 					GroupsClaim:              testGroupsClaim,
@@ -1014,6 +1018,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 					ClientID:                 testClientID,
 					AuthorizationURL:         *testIssuerAuthorizeURL,
 					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              true,
 					Scopes:                   testDefaultExpectedScopes,
 					UsernameClaim:            testUsernameClaim,
 					GroupsClaim:              testGroupsClaim,
@@ -1106,6 +1111,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 					ClientID:                 testClientID,
 					AuthorizationURL:         *testIssuerAuthorizeURL,
 					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              true,
 					Scopes:                   testDefaultExpectedScopes,
 					UsernameClaim:            testUsernameClaim,
 					GroupsClaim:              testGroupsClaim,
@@ -1174,6 +1180,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 					ClientID:                 testClientID,
 					AuthorizationURL:         *testIssuerAuthorizeURL,
 					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              true,
 					Scopes:                   testDefaultExpectedScopes,
 					UsernameClaim:            testUsernameClaim,
 					GroupsClaim:              testGroupsClaim,
@@ -1240,6 +1247,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 					ClientID:                 testClientID,
 					AuthorizationURL:         *testIssuerAuthorizeURL,
 					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              true,
 					Scopes:                   testDefaultExpectedScopes,
 					UsernameClaim:            testUsernameClaim,
 					GroupsClaim:              testGroupsClaim,
@@ -1304,6 +1312,140 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 					ClientID:                 testClientID,
 					AuthorizationURL:         *testIssuerAuthorizeURL,
 					RevocationURL:            nil, // no revocation URL is set in the cached provider because none was returned by discovery
+					UserInfoURL:              true,
+					Scopes:                   testDefaultExpectedScopes,
+					UsernameClaim:            testUsernameClaim,
+					GroupsClaim:              testGroupsClaim,
+					AllowPasswordGrant:       false,
+					AdditionalAuthcodeParams: map[string]string{},
+					AdditionalClaimMappings:  nil, // Does not default to empty map
+					ResourceUID:              testUID,
+				},
+			},
+			wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
+				Status: idpv1alpha1.OIDCIdentityProviderStatus{
+					Phase: "Ready",
+					Conditions: []metav1.Condition{
+						{Type: "AdditionalAuthorizeParametersValid", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "additionalAuthorizeParameters parameter names are allowed", ObservedGeneration: 1234},
+						{Type: "ClientCredentialsSecretValid", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "loaded client credentials", ObservedGeneration: 1234},
+						{Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "discovered issuer configuration", ObservedGeneration: 1234},
+						{Type: "TLSConfigurationValid", Status: "True", LastTransitionTime: now, Reason: "Success",
+							Message: "spec.tls is valid: using configured CA bundle", ObservedGeneration: 1234},
+					},
+				},
+			}},
+		},
+		{
+			name: "existing valid upstream with no userinfo endpoint in the discovery document",
+			inputUpstreams: []runtime.Object{&idpv1alpha1.OIDCIdentityProvider{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
+				Spec: idpv1alpha1.OIDCIdentityProviderSpec{
+					Issuer: testIssuerURL + "/valid-without-userinfo",
+					TLS:    &idpv1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
+					Client: idpv1alpha1.OIDCClient{SecretName: testSecretName},
+					Claims: idpv1alpha1.OIDCClaims{Groups: testGroupsClaim, Username: testUsernameClaim},
+				},
+				Status: idpv1alpha1.OIDCIdentityProviderStatus{
+					Phase: "Ready",
+					Conditions: []metav1.Condition{
+						happyAdditionalAuthorizeParametersValidConditionEarlier,
+						{Type: "ClientCredentialsSecretValid", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "loaded client credentials"},
+						{Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "discovered issuer configuration"},
+					},
+				},
+			}},
+			inputResources: []runtime.Object{&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName},
+				Type:       "secrets.pinniped.dev/oidc-client",
+				Data:       testValidSecretData,
+			}},
+			wantLogs: []string{
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","status":"True","reason":"Success","message":"loaded client credentials"}`,
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"True","reason":"Success","message":"discovered issuer configuration"}`,
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`,
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`,
+			},
+			wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{
+				{
+					Name:                     testName,
+					ClientID:                 testClientID,
+					AuthorizationURL:         *testIssuerAuthorizeURL,
+					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              false,
+					Scopes:                   testDefaultExpectedScopes,
+					UsernameClaim:            testUsernameClaim,
+					GroupsClaim:              testGroupsClaim,
+					AllowPasswordGrant:       false,
+					AdditionalAuthcodeParams: map[string]string{},
+					AdditionalClaimMappings:  nil, // Does not default to empty map
+					ResourceUID:              testUID,
+				},
+			},
+			wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
+				Status: idpv1alpha1.OIDCIdentityProviderStatus{
+					Phase: "Ready",
+					Conditions: []metav1.Condition{
+						{Type: "AdditionalAuthorizeParametersValid", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "additionalAuthorizeParameters parameter names are allowed", ObservedGeneration: 1234},
+						{Type: "ClientCredentialsSecretValid", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "loaded client credentials", ObservedGeneration: 1234},
+						{Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "discovered issuer configuration", ObservedGeneration: 1234},
+						{Type: "TLSConfigurationValid", Status: "True", LastTransitionTime: now, Reason: "Success",
+							Message: "spec.tls is valid: using configured CA bundle", ObservedGeneration: 1234},
+					},
+				},
+			}},
+		},
+		{
+			name: "existing valid upstream with userinfo endpoint in the discovery document, but global OIDC config includes setting to ignore provider's userinfo endpoint",
+			inputGlobalOIDCConfig: &GlobalOIDCConfig{
+				IgnoreUserInfoEndpoint: true,
+			},
+			inputUpstreams: []runtime.Object{&idpv1alpha1.OIDCIdentityProvider{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
+				Spec: idpv1alpha1.OIDCIdentityProviderSpec{
+					Issuer: testIssuerURL,
+					TLS:    &idpv1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
+					Client: idpv1alpha1.OIDCClient{SecretName: testSecretName},
+					Claims: idpv1alpha1.OIDCClaims{Groups: testGroupsClaim, Username: testUsernameClaim},
+				},
+				Status: idpv1alpha1.OIDCIdentityProviderStatus{
+					Phase: "Ready",
+					Conditions: []metav1.Condition{
+						happyAdditionalAuthorizeParametersValidConditionEarlier,
+						{Type: "ClientCredentialsSecretValid", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "loaded client credentials"},
+						{Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: earlier, Reason: "Success",
+							Message: "discovered issuer configuration"},
+					},
+				},
+			}},
+			inputResources: []runtime.Object{&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName},
+				Type:       "secrets.pinniped.dev/oidc-client",
+				Data:       testValidSecretData,
+			}},
+			wantLogs: []string{
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","status":"True","reason":"Success","message":"loaded client credentials"}`,
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"True","reason":"Success","message":"discovered issuer configuration"}`,
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`,
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`,
+			},
+			wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{
+				{
+					Name:                     testName,
+					ClientID:                 testClientID,
+					AuthorizationURL:         *testIssuerAuthorizeURL,
+					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              false, // expecting false due to global OIDC configuration override (this provider actually has a userinfo endpoint)
 					Scopes:                   testDefaultExpectedScopes,
 					UsernameClaim:            testUsernameClaim,
 					GroupsClaim:              testGroupsClaim,
@@ -1371,6 +1513,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 					ClientID:                 testClientID,
 					AuthorizationURL:         *testIssuerAuthorizeURL,
 					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              true,
 					Scopes:                   testExpectedScopes,
 					UsernameClaim:            testUsernameClaim,
 					GroupsClaim:              testGroupsClaim,
@@ -1446,6 +1589,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 					ClientID:                 testClientID,
 					AuthorizationURL:         *testIssuerAuthorizeURL,
 					RevocationURL:            testIssuerRevocationURL,
+					UserInfoURL:              true,
 					Scopes:                   testExpectedScopes, // does not include the default scopes
 					UsernameClaim:            testUsernameClaim,
 					GroupsClaim:              testGroupsClaim,
@@ -1634,6 +1778,11 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 				}
 			}

+			globalOIDCConfig := &GlobalOIDCConfig{}
+			if tt.inputGlobalOIDCConfig != nil {
+				globalOIDCConfig = tt.inputGlobalOIDCConfig
+			}
+
 			controller := New(
 				cache,
 				fakePinnipedClient,
@@ -1643,6 +1792,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 				logger,
 				controllerlib.WithInformer,
 				validatorCache,
+				*globalOIDCConfig,
 			)

 			ctx, cancel := context.WithCancel(context.Background())
@@ -1677,6 +1827,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 				require.Equal(t, tt.wantResultingCache[i].GetAdditionalClaimMappings(), actualIDP.GetAdditionalClaimMappings())
 				require.Equal(t, tt.wantResultingCache[i].GetResourceUID(), actualIDP.GetResourceUID())
 				require.Equal(t, tt.wantResultingCache[i].GetRevocationURL(), actualIDP.GetRevocationURL())
+				require.Equal(t, tt.wantResultingCache[i].HasUserInfoURL(), actualIDP.HasUserInfoURL())
 				require.ElementsMatch(t, tt.wantResultingCache[i].GetScopes(), actualIDP.GetScopes())

 				// We always want to use the proxy from env on these clients, so although the following assertions
@@ -1754,6 +1905,7 @@ func newTestIssuer(t *testing.T) (string, string) {
 		TokenURL      string `json:"token_endpoint"`
 		RevocationURL string `json:"revocation_endpoint,omitempty"`
 		JWKSURL       string `json:"jwks_uri"`
+		UserInfoURL   string `json:"userinfo_endpoint"`
 	}

 	// At the root of the server, serve an issuer with a valid discovery response.
@@ -1764,6 +1916,7 @@ func newTestIssuer(t *testing.T) (string, string) {
 			AuthURL:       "https://example.com/authorize",
 			RevocationURL: "https://example.com/revoke",
 			TokenURL:      "https://example.com/token",
+			UserInfoURL:   "https://example.com/userinfo",
 		})
 	})

@@ -1775,6 +1928,19 @@ func newTestIssuer(t *testing.T) (string, string) {
 			AuthURL:       "https://example.com/authorize",
 			RevocationURL: "", // none
 			TokenURL:      "https://example.com/token",
+			UserInfoURL:   "https://example.com/userinfo",
+		})
+	})
+
+	// At "/valid-without-userinfo", serve an issuer with a valid discovery response which does not have a userinfo endpoint.
+	mux.HandleFunc("/valid-without-userinfo/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("content-type", "application/json")
+		_ = json.NewEncoder(w).Encode(&providerJSON{
+			Issuer:        server.URL + "/valid-without-userinfo",
+			AuthURL:       "https://example.com/authorize",
+			RevocationURL: "https://example.com/revoke",
+			TokenURL:      "https://example.com/token",
+			UserInfoURL:   "", // none
 		})
 	})

@@ -1865,6 +2031,7 @@ func newTestIssuer(t *testing.T) (string, string) {
 			AuthURL:       "https://example.com/authorize",
 			RevocationURL: "https://example.com/revoke",
 			TokenURL:      "https://example.com/token",
+			UserInfoURL:   "https://example.com/userinfo",
 		})
 	})