mirrors/pinniped
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/pinniped.git synced 2026-01-05 21:15:26 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #2580 from vmware/adfs_workaround
Some checks failed
CodeQL / Analyze (go) (push) Failing after 2m49s
Details
CodeQL / Analyze (javascript) (push) Failing after 2m5s
Details

Browse Source 
Implement configuration option which allows Supervisor to work with ADFS
This commit is contained in:
Ryan Richard
2025-08-27 16:43:24 -07:00
committed by GitHub
parent 7090fb7038 577797d569
commit cf63d782d4
8 changed files with 469 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
hack/lib/lint-version.txt
View File

@@ -1 +1 @@
2.3.0
2.4.0

60
internal/config/supervisor/config_test.go
View File

@@ -60,6 +60,11 @@ func TestFromPath(t *testing.T) {
audit:
logUsernamesAndGroups: enabled
logInternalPaths: enabled
oidc:
ignoreUserInfoEndpoint:
whenIssuerExactlyMatches:
- https://foo.com
- https://bar.com
`),
wantConfig: &Config{
APIGroupSuffix: ptr.To("some.suffix.com"),
@@ -104,6 +109,14 @@ func TestFromPath(t *testing.T) {
LogUsernamesAndGroups: "enabled",
LogInternalPaths: "enabled",
},
OIDC: OIDCSpec{
IgnoreUserInfoEndpoint: IgnoreUserInfoEndpointSpec{
WhenIssuerExactlyMatches: []string{
"https://foo.com",
"https://bar.com",
},
},
},
},
},
{
@@ -146,8 +159,9 @@ func TestFromPath(t *testing.T) {
LogUsernamesAndGroups: "",
},
AggregatedAPIServerDisableAdmissionPlugins: nil,
TLS: TLSSpec{},
Log: plog.LogSpec{},
TLS: TLSSpec{},
Log: plog.LogSpec{},
OIDC: OIDCSpec{},
},
},
{
@@ -182,6 +196,37 @@ func TestFromPath(t *testing.T) {
},
},
},
{
name: "ignoreUserInfoEndpoint can be explicitly an empty list",
yaml: here.Doc(`
---
names:
defaultTLSCertificateSecret: my-secret-name
oidc:
ignoreUserInfoEndpoint:
whenIssuerExactlyMatches: []
`),
wantConfig: &Config{
APIGroupSuffix: ptr.To("pinniped.dev"),
Labels: map[string]string{},
NamesConfig: NamesConfigSpec{
DefaultTLSCertificateSecret: "my-secret-name",
},
Endpoints: &Endpoints{
HTTPS: &Endpoint{
Network: "tcp",
Address: ":8443",
},
HTTP: &Endpoint{
Network: "disabled",
},
},
AggregatedAPIServerPort: ptr.To[int64](10250),
OIDC: OIDCSpec{
IgnoreUserInfoEndpoint: IgnoreUserInfoEndpointSpec{WhenIssuerExactlyMatches: []string{}},
},
},
},
{
name: "all endpoints disabled",
yaml: here.Doc(`
@@ -370,6 +415,17 @@ func TestFromPath(t *testing.T) {
`),
wantError: "validate aggregatedAPIServerDisableAdmissionPlugins: admission plugin names not recognized: [foobar foobaz] (each must be one of [NamespaceLifecycle MutatingAdmissionPolicy MutatingAdmissionWebhook ValidatingAdmissionPolicy ValidatingAdmissionWebhook])",
},
{
name: "invalid ignoreUserInfoEndpoint",
yaml: here.Doc(`
---
names:
defaultTLSCertificateSecret: my-secret-name
oidc:
ignoreUserInfoEndpoint: "should be a struct, but is a string"
`),
wantError: "decode yaml: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go struct field OIDCSpec.oidc.ignoreUserInfoEndpoint of type supervisor.IgnoreUserInfoEndpointSpec",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {

28
internal/config/supervisor/types.go
View File

@@ -23,6 +23,7 @@ type Config struct {
AggregatedAPIServerDisableAdmissionPlugins []string `json:"aggregatedAPIServerDisableAdmissionPlugins"`
TLS TLSSpec `json:"tls"`
Audit AuditSpec `json:"audit"`
OIDC OIDCSpec `json:"oidc"`
}
type AuditInternalPaths string
@@ -40,6 +41,33 @@ type AuditSpec struct {
LogUsernamesAndGroups AuditUsernamesAndGroups `json:"logUsernamesAndGroups"`
}
type IgnoreUserInfoEndpointSpec struct {
// WhenIssuerExactlyMatches is a list of exact OIDC issuer URLs for which the userinfo endpoint should be avoided.
// This will only take effect for OIDCIdentityProviders who have a spec.issuer which is exactly equal to any one
// of these strings (using exact string equality).
WhenIssuerExactlyMatches []string `json:"whenIssuerExactlyMatches"`
}
type OIDCSpec struct {
// IgnoreUserInfoEndpoint, when configured, will cause all matching OIDCIdentityProviders to ignore the
// potential existence of any userinfo endpoint offered by the external OIDC provider(s) when those OIDC providers
// return refresh tokens.
//
// Please exercise caution when using this setting. Some OIDC providers which return more information from the
// userinfo endpoint than they put into the ID token itself. Pinniped will normally merge the claims from the
// ID token with the response from the userinfo endpoint, but this setting disables that behavior for matching
// OIDC providers.
//
// This was added as a workaround for Microsoft ADFS, which does not correctly implement the userinfo
// endpoint as described in the OIDC specification. There are several circumstances where calls to the
// ADFS userinfo endpoint will result in "403 Forbidden" responses, which cause Pinniped to reject a user's
// login and/or session refresh.
//
// We do not currently have plans to implement ADFS support options directly on the OIDCIdentityProvider CRD
// because Microsoft no longer recommends the use of ADFS.
IgnoreUserInfoEndpoint IgnoreUserInfoEndpointSpec `json:"ignoreUserInfoEndpoint"`
}
type TLSSpec struct {
OneDotTwo TLSProtocolSpec `json:"onedottwo"`
}

22
internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go
View File

@@ -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,22 @@ func (c *ttlProviderCache) putProvider(key oidcDiscoveryCacheKey, value *oidcDis
c.cache.Set(key, value, oidcValidatorCacheTTL)
}
type UserInfoEndpointConfigI interface {
IgnoreUserInfoEndpoint(issuerURL string) bool
}
type IgnoreUserInfoEndpointForExactIssuerMatches struct {
Issuers sets.Set[string] // a set of issuer URLs
}
func (i *IgnoreUserInfoEndpointForExactIssuerMatches) IgnoreUserInfoEndpoint(issuerURL string) bool {
return i.Issuers.Has(issuerURL)
}
type GlobalOIDCConfig struct {
UserInfoEndpointConfig UserInfoEndpointConfigI
}
type oidcWatcherController struct {
cache UpstreamOIDCIdentityProviderICache
log plog.Logger
@@ -137,6 +153,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 +166,7 @@ func New(
log plog.Logger,
withInformer pinnipedcontroller.WithInformerOptionFunc,
validatorCache *cache.Expiring,
globalOIDCConfig GlobalOIDCConfig,
) controllerlib.Controller {
c := oidcWatcherController{
cache: idpCache,
@@ -158,6 +176,7 @@ func New(
secretInformer: secretInformer,
configMapInformer: configMapInformer,
validatorCache: &ttlProviderCache{cache: validatorCache},
globalOIDCConfig: globalOIDCConfig,
}
return controllerlib.New(
controllerlib.Config{Name: oidcControllerName, Syncer: &c},
@@ -235,6 +254,7 @@ func (c *oidcWatcherController) validateUpstream(ctx controllerlib.Context, upst
AdditionalAuthcodeParams: additionalAuthcodeAuthorizeParameters,
AdditionalClaimMappings: upstream.Spec.Claims.AdditionalClaimMappings,
ResourceUID: upstream.UID,
IgnoreUserInfoEndpoint: c.globalOIDCConfig.UserInfoEndpointConfig.IgnoreUserInfoEndpoint(upstream.Spec.Issuer),
}
conditions := []*metav1.Condition{

206
internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
View File

@@ -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
@@ -23,6 +23,7 @@ import (
"k8s.io/apimachinery/pkg/types"
expiringcache "k8s.io/apimachinery/pkg/util/cache"
"k8s.io/apimachinery/pkg/util/net"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
@@ -123,6 +124,7 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) {
logger,
withInformer.WithInformer,
expiringcache.NewExpiring(),
GlobalOIDCConfig{},
)
unrelated := corev1.Secret{}
@@ -182,6 +184,7 @@ func TestOIDCUpstreamWatcherControllerFilterConfigMaps(t *testing.T) {
logger,
withInformer.WithInformer,
expiringcache.NewExpiring(),
GlobalOIDCConfig{},
)
unrelated := corev1.ConfigMap{}
@@ -242,6 +245,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 +955,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 +1019,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
UserInfoURL: true,
Scopes: testDefaultExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@@ -1106,6 +1112,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
UserInfoURL: true,
Scopes: testDefaultExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@@ -1174,6 +1181,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
UserInfoURL: true,
Scopes: testDefaultExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@@ -1240,6 +1248,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
UserInfoURL: true,
Scopes: testDefaultExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@@ -1304,6 +1313,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{
UserInfoEndpointConfig: &IgnoreUserInfoEndpointAlways{},
},
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 +1514,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
UserInfoURL: true,
Scopes: testExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@@ -1446,6 +1590,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 +1779,14 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
}
}
globalOIDCConfig := &GlobalOIDCConfig{
// By default, do not ignore the userinfo endpoint in tests.
UserInfoEndpointConfig: &IgnoreUserInfoEndpointNever{},
}
if tt.inputGlobalOIDCConfig != nil {
globalOIDCConfig = tt.inputGlobalOIDCConfig
}
controller := New(
cache,
fakePinnipedClient,
@@ -1643,6 +1796,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
logger,
controllerlib.WithInformer,
validatorCache,
*globalOIDCConfig,
)
ctx, cancel := context.WithCancel(context.Background())
@@ -1677,6 +1831,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 +1909,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 +1920,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 +1932,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 +2035,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",
})
})
@@ -1876,3 +2047,36 @@ func newTestIssuer(t *testing.T) (string, string) {
return server.URL, string(serverCA)
}
type IgnoreUserInfoEndpointAlways struct{}
func (i *IgnoreUserInfoEndpointAlways) IgnoreUserInfoEndpoint(_issuerURL string) bool {
return true
}
type IgnoreUserInfoEndpointNever struct{}
func (i *IgnoreUserInfoEndpointNever) IgnoreUserInfoEndpoint(_issuerURL string) bool {
return false
}
func TestIgnoreUserInfoEndpointForExactIssuerMatches(t *testing.T) {
t.Parallel()
// With an empty set.
s := &IgnoreUserInfoEndpointForExactIssuerMatches{}
require.False(t, s.IgnoreUserInfoEndpoint("https://foo.com"))
// An alternate way to initialize an empty set.
var empty []string
s = &IgnoreUserInfoEndpointForExactIssuerMatches{Issuers: sets.New(empty...)}
require.False(t, s.IgnoreUserInfoEndpoint("https://foo.com"))
// With a non-empty set.
s = &IgnoreUserInfoEndpointForExactIssuerMatches{
Issuers: sets.New("https://foo.com", "https://bar.com"),
}
require.True(t, s.IgnoreUserInfoEndpoint("https://foo.com"))
require.True(t, s.IgnoreUserInfoEndpoint("https://bar.com"))
require.False(t, s.IgnoreUserInfoEndpoint("https://baz.com"))
}

6
internal/supervisor/server/server.go
View File

@@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/util/cache"
"k8s.io/apimachinery/pkg/util/sets"
apimachineryversion "k8s.io/apimachinery/pkg/version"
genericapifilters "k8s.io/apiserver/pkg/endpoints/filters"
openapinamer "k8s.io/apiserver/pkg/endpoints/openapi"
@@ -311,6 +312,11 @@ func prepareControllers(
plog.New(),
controllerlib.WithInformer,
cache.NewExpiring(),
oidcupstreamwatcher.GlobalOIDCConfig{
UserInfoEndpointConfig: &oidcupstreamwatcher.IgnoreUserInfoEndpointForExactIssuerMatches{
Issuers: sets.New(cfg.OIDC.IgnoreUserInfoEndpoint.WhenIssuerExactlyMatches...),
},
},
),
singletonWorker).
WithController(

13
internal/upstreamoidc/upstreamoidc.go
View File

@@ -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 upstreamoidc implements an abstraction of upstream OIDC provider interactions.
@@ -46,6 +46,7 @@ type ProviderConfig struct {
AdditionalAuthcodeParams map[string]string
AdditionalClaimMappings map[string]string
RevocationURL *url.URL // will commonly be nil: many providers do not offer this
IgnoreUserInfoEndpoint bool
Provider interface {
Verifier(*coreosoidc.Config) *coreosoidc.IDTokenVerifier
Claims(v any) error
@@ -64,6 +65,12 @@ func (p *ProviderConfig) GetRevocationURL() *url.URL {
}
func (p *ProviderConfig) HasUserInfoURL() bool {
if p.IgnoreUserInfoEndpoint {
// When asked via configuration to ignore the userinfo endpoint, then
// return false, regardless of the external provider's discovery response.
return false
}
providerJSON := &struct {
UserInfoURL string `json:"userinfo_endpoint"`
}{}
@@ -398,7 +405,9 @@ func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, t
}
func (p *ProviderConfig) maybeFetchUserInfo(ctx context.Context, tok *oauth2.Token, requireUserInfo bool) (*coreosoidc.UserInfo, error) {
// implementing the user info endpoint is not required by the OIDC spec, but we may require it in certain situations.
// Implementing the user info endpoint is not required by the OIDC spec, but we may require it in certain situations.
// Even when configuration asks us to skip calling the userinfo endpoint, we should not skip it on those situations
// where it is our only source of getting the claims that we need to perform our validations.
if !p.HasUserInfoURL() {
if requireUserInfo {
// TODO should these all be http errors?

165
internal/upstreamoidc/upstreamoidc_test.go
View File

@@ -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 upstreamoidc
@@ -62,6 +62,13 @@ func TestProviderConfig(t *testing.T) {
require.False(t, p.AllowsPasswordGrant())
require.True(t, p.HasUserInfoURL())
// Even when the underlying provider does have a userinfo endpoint,
// this setting should be able to override it.
p.IgnoreUserInfoEndpoint = true
require.False(t, p.HasUserInfoURL())
p.IgnoreUserInfoEndpoint = false
p.Provider = &mockProvider{
rawClaims: []byte(`{"some_other_endpoint": "https://example.com/blah"}`),
}
@@ -100,12 +107,13 @@ func TestProviderConfig(t *testing.T) {
t.Run("PasswordCredentialsGrantAndValidateTokens", func(t *testing.T) {
tests := []struct {
name string
disallowPasswordGrant bool
returnIDTok string
tokenStatusCode int
wantErr string
wantToken oidctypes.Token
name string
disallowPasswordGrant bool
ignoreUserInfoEndpoint bool
returnIDTok string
tokenStatusCode int
wantErr string
wantToken oidctypes.Token
rawClaims []byte
userInfo *coreosoidc.UserInfo
@@ -170,6 +178,37 @@ func TestProviderConfig(t *testing.T) {
userInfo: forceUserInfoWithClaims("test-user", `{"foo":"awesomeness","groups":"fancy-group"}`),
wantUserInfoCalled: true,
},
{
name: "valid with userinfo when configured to ignore the userinfo endpoint",
ignoreUserInfoEndpoint: true,
returnIDTok: validIDToken,
wantToken: oidctypes.Token{
AccessToken: &oidctypes.AccessToken{
Token: "test-access-token",
Expiry: metav1.Time{},
},
RefreshToken: &oidctypes.RefreshToken{
Token: "test-refresh-token",
},
IDToken: &oidctypes.IDToken{
Token: validIDToken,
Expiry: metav1.Time{},
Claims: map[string]any{
"foo": "bar", // does not overwrite existing claim because userinfo is skipped
"bat": "baz",
"aud": "test-client-id",
"iat": 1.606768593e+09,
"jti": "test-jti",
"nbf": 1.606768593e+09,
"sub": "test-user",
// groups claim is not added because userinfo is skipped
},
},
},
// claims is private field so we have to use hacks to set it
userInfo: forceUserInfoWithClaims("test-user", `{"foo":"awesomeness","groups":"fancy-group"}`),
wantUserInfoCalled: false,
},
{
name: "password grant not allowed",
disallowPasswordGrant: true, // password grant is not allowed in this ProviderConfig
@@ -294,8 +333,9 @@ func TestProviderConfig(t *testing.T) {
userInfo: tt.userInfo,
userInfoErr: tt.userInfoErr,
},
AllowPasswordGrant: !tt.disallowPasswordGrant,
Client: http.DefaultClient,
AllowPasswordGrant: !tt.disallowPasswordGrant,
Client: http.DefaultClient,
IgnoreUserInfoEndpoint: tt.ignoreUserInfoEndpoint,
}
tok, err := p.PasswordCredentialsGrantAndValidateTokens(
@@ -741,16 +781,17 @@ func TestProviderConfig(t *testing.T) {
goodIDToken := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJzb21lLXN1YmplY3QiLCJub25jZSI6InNvbWUtbm9uY2UiLCJpc3MiOiJzb21lLWlzc3VlciJ9.eGvzOihLUqzn3M4k6fHsToedgy7Fu89_Xu_u4mwMgRlIyRWZqmEMV76RVLnZd9Ihm9j_VpvrpirIkaj4JM9eRNfLX1n328cmBivBwnTKAzHuTm17dUKO5EvdTmQzmwnN0WZ8nWk4GfR7RzcvE1V8G9tIiWD8FkO3Dr-NR_zTun3N37onAazVLCmF0SDtATDfUH1ETqviHEp8xGx5HD5mv5T3HEjOuer5gxTEnfncef0LurBH3po-C0tXHKu74PD8x88CMJ1DLsRdCalnctwa850slKPkBSTP-ssh0JVg7cdMXoosVpwiXtKYaBkrhu8VS018aFP-cBbW0mYwsHmt3g" //nolint:gosec
tests := []struct {
name string
tok *oauth2.Token
nonce nonce.Nonce
requireIDToken bool
requireUserInfo bool
userInfo *coreosoidc.UserInfo
rawClaims []byte
userInfoErr error
wantErr string
wantMergedTokens *oidctypes.Token
name string
tok *oauth2.Token
nonce nonce.Nonce
ignoreUserInfoEndpoint bool
requireIDToken bool
requireUserInfo bool
userInfo *coreosoidc.UserInfo
rawClaims []byte
userInfoErr error
wantErr string
wantMergedTokens *oidctypes.Token
}{
{
name: "token with id, access and refresh tokens, valid nonce, and no userinfo",
@@ -831,6 +872,34 @@ func TestProviderConfig(t *testing.T) {
},
},
},
{
name: "token with id, access and refresh tokens, valid nonce, and userinfo with a value that doesn't exist in the id token, when configured to ignore userinfo",
tok: testTokenWithoutIDToken.WithExtra(map[string]any{"id_token": goodIDToken}),
nonce: "some-nonce",
requireIDToken: true,
ignoreUserInfoEndpoint: true,
rawClaims: []byte(`{"userinfo_endpoint": "not-empty"}`),
userInfo: forceUserInfoWithClaims("some-subject", `{"name": "Pinny TheSeal", "sub": "some-subject"}`),
wantMergedTokens: &oidctypes.Token{
AccessToken: &oidctypes.AccessToken{
Token: "test-access-token",
Type: "test-token-type",
Expiry: metav1.NewTime(expiryTime),
},
RefreshToken: &oidctypes.RefreshToken{
Token: "test-initial-refresh-token",
},
IDToken: &oidctypes.IDToken{
Token: goodIDToken,
Claims: map[string]any{
"iss": "some-issuer",
"nonce": "some-nonce",
"sub": "some-subject",
// "name" claim returned by userinfo endpoint is not merged, because userinfo was not called due to ignoreUserInfoEndpoint configuration
},
},
},
},
{
name: "userinfo is required, token with id, access and refresh tokens, valid nonce, and userinfo with a value that doesn't exist in the id token",
tok: testTokenWithoutIDToken.WithExtra(map[string]any{"id_token": goodIDToken}),
@@ -1037,6 +1106,15 @@ func TestProviderConfig(t *testing.T) {
rawClaims: []byte(`{}`),
wantErr: "could not fetch user info claims: userinfo endpoint not found, but is required",
},
{
name: "expected to have userinfo, but doesn't, even when configured to otherwise ignore userinfo",
tok: testTokenWithoutIDToken,
nonce: "some-other-nonce",
requireUserInfo: true,
ignoreUserInfoEndpoint: true,
rawClaims: []byte(`{}`),
wantErr: "could not fetch user info claims: userinfo endpoint not found, but is required",
},
{
name: "expected to have id token and userinfo, but doesn't have either",
tok: testTokenWithoutIDToken,
@@ -1104,6 +1182,7 @@ func TestProviderConfig(t *testing.T) {
userInfo: tt.userInfo,
userInfoErr: tt.userInfoErr,
},
IgnoreUserInfoEndpoint: tt.ignoreUserInfoEndpoint,
}
gotTok, err := p.ValidateTokenAndMergeWithUserInfo(context.Background(), tt.tok, tt.nonce, tt.requireIDToken, tt.requireUserInfo)
if tt.wantErr != "" {
@@ -1119,12 +1198,13 @@ func TestProviderConfig(t *testing.T) {
t.Run("ExchangeAuthcodeAndValidateTokens", func(t *testing.T) {
tests := []struct {
name string
authCode string
expectNonce nonce.Nonce
returnIDTok string
wantErr string
wantToken oidctypes.Token
name string
authCode string
ignoreUserInfoEndpoint bool
expectNonce nonce.Nonce
returnIDTok string
wantErr string
wantToken oidctypes.Token
rawClaims []byte
userInfo *coreosoidc.UserInfo
@@ -1301,6 +1381,38 @@ func TestProviderConfig(t *testing.T) {
userInfo: forceUserInfoWithClaims("test-user", `{"foo":"awesomeness","groups":"fancy-group"}`),
wantUserInfoCalled: true,
},
{
name: "valid with user info, when configured to ignore userinfo",
authCode: "valid",
ignoreUserInfoEndpoint: true,
returnIDTok: validIDToken,
wantToken: oidctypes.Token{
AccessToken: &oidctypes.AccessToken{
Token: "test-access-token",
Expiry: metav1.Time{},
},
RefreshToken: &oidctypes.RefreshToken{
Token: "test-refresh-token",
},
IDToken: &oidctypes.IDToken{
Token: validIDToken,
Expiry: metav1.Time{},
Claims: map[string]any{
"foo": "bar", // does not overwrite existing claim because userinfo is skipped
"bat": "baz",
"aud": "test-client-id",
"iat": 1.606768593e+09,
"jti": "test-jti",
"nbf": 1.606768593e+09,
"sub": "test-user",
// groups claim is not added because userinfo is skipped
},
},
},
// claims is private field so we have to use hacks to set it
userInfo: forceUserInfoWithClaims("test-user", `{"foo":"awesomeness","groups":"fancy-group"}`),
wantUserInfoCalled: false,
},
{
name: "invalid sub claim",
authCode: "valid",
@@ -1383,7 +1495,8 @@ func TestProviderConfig(t *testing.T) {
userInfo: tt.userInfo,
userInfoErr: tt.userInfoErr,
},
Client: http.DefaultClient,
Client: http.DefaultClient,
IgnoreUserInfoEndpoint: tt.ignoreUserInfoEndpoint,
}
tok, err := p.ExchangeAuthcodeAndValidateTokens(