Refuse logins when no upstream refresh token and no userinfo endpoint

Signed-off-by: Margo Crawford <margaretc@vmware.com>

internal/controller/kubecertagent/mocks/mockdynamiccert.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/controller/kubecertagent/mocks/mockpodcommandexecutor.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/mocks/credentialrequestmocks/credentialrequestmocks.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/mocks/issuermocks/issuermocks.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/mocks/mockkeyset/mockkeyset.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/mocks/mockldapconn/mockldapconn.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/mocks/mocksecrethelper/mocksecrethelper.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/mocks/mocktokenauthenticator/mocktokenauthenticator.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/mocks/mocktokenauthenticatorcloser/mocktokenauthenticatorcloser.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
 

internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go

@@ -14,12 +14,11 @@ import (
 	reflect "reflect"
 
 	gomock "github.com/golang/mock/gomock"
-	oauth2 "golang.org/x/oauth2"
-	types "k8s.io/apimachinery/pkg/types"
-
 	nonce "go.pinniped.dev/pkg/oidcclient/nonce"
 	oidctypes "go.pinniped.dev/pkg/oidcclient/oidctypes"
 	pkce "go.pinniped.dev/pkg/oidcclient/pkce"
+	oauth2 "golang.org/x/oauth2"
+	types "k8s.io/apimachinery/pkg/types"
 )
 
 // MockUpstreamOIDCIdentityProviderI is a mock of UpstreamOIDCIdentityProviderI interface.
@@ -186,6 +185,20 @@ func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) GetUsernameClaim() *gom
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUsernameClaim", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).GetUsernameClaim))
 }
 
+// HasUserInfoURL mocks base method.
+func (m *MockUpstreamOIDCIdentityProviderI) HasUserInfoURL() bool {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "HasUserInfoURL")
+	ret0, _ := ret[0].(bool)
+	return ret0
+}
+
+// HasUserInfoURL indicates an expected call of HasUserInfoURL.
+func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) HasUserInfoURL() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasUserInfoURL", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).HasUserInfoURL))
+}
+
 // PasswordCredentialsGrantAndValidateTokens mocks base method.
 func (m *MockUpstreamOIDCIdentityProviderI) PasswordCredentialsGrantAndValidateTokens(arg0 context.Context, arg1, arg2 string) (*oidctypes.Token, error) {
 	m.ctrl.T.Helper()

internal/oidc/auth/auth_handler_test.go

@@ -161,6 +161,12 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			"state":             happyState,
 		}
 
+		fositeAccessDeniedWithMissingUserInfoEndpointErrorQuery = map[string]string{
+			"error":             "access_denied",
+			"error_description": "The resource owner or authorization server denied the request. Reason: access token was returned by upstream provider but there was no userinfo endpoint.",
+			"state":             happyState,
+		}
+
 		fositeAccessDeniedWithPasswordGrantDisallowedHintErrorQuery = map[string]string{
 			"error":             "access_denied",
 			"error_description": "The resource owner or authorization server denied the request. Resource owner password credentials grant is not allowed for this upstream provider according to its configuration.",
@@ -886,8 +892,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyStringWithLocationInHref:       true,
 		},
 		{
-			name:                              "OIDC password grant happy path when upstream IDP returned empty refresh token but it did return an access token",
+			name:                              "OIDC password grant happy path when upstream IDP returned empty refresh token but it did return an access token and has a userinfo endpoint",
-			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken).Build()),
+			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithUserInfoURL().Build()),
 			method:                            http.MethodGet,
 			path:                              happyGetRequestPath,
 			customUsernameHeader:              pointer.StringPtr(oidcUpstreamUsername),
@@ -908,8 +914,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantDownstreamCustomSessionData:   expectedHappyOIDCPasswordGrantCustomSessionWithAccessToken,
 		},
 		{
-			name:                              "OIDC password grant happy path when upstream IDP did not return a refresh token but it did return an access token",
+			name:                              "OIDC password grant happy path when upstream IDP did not return a refresh token but it did return an access token and has a userinfo endpoint",
-			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken).Build()),
+			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithUserInfoURL().Build()),
 			method:                            http.MethodGet,
 			path:                              happyGetRequestPath,
 			customUsernameHeader:              pointer.StringPtr(oidcUpstreamUsername),
@@ -1071,7 +1077,33 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:       "",
 		},
 		{
-			name:                  "return an error when upstream IDP returns empty refresh token and empty access token",
+			name:                  "password grant returns an error when upstream IDP returns no refresh token with an access token but has no userinfo endpoint",
+			idps:                  oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithoutUserInfoURL().Build()),
+			method:                http.MethodGet,
+			path:                  happyGetRequestPath,
+			customUsernameHeader:  pointer.StringPtr(oidcUpstreamUsername),
+			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
+			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
+			wantStatus:            http.StatusFound,
+			wantContentType:       "application/json; charset=utf-8",
+			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUserInfoEndpointErrorQuery),
+			wantBodyString:        "",
+		},
+		{
+			name:                  "password grant returns an error when upstream IDP returns empty refresh token with an access token but has no userinfo endpoint",
+			idps:                  oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithoutUserInfoURL().Build()),
+			method:                http.MethodGet,
+			path:                  happyGetRequestPath,
+			customUsernameHeader:  pointer.StringPtr(oidcUpstreamUsername),
+			customPasswordHeader:  pointer.StringPtr(oidcUpstreamPassword),
+			wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation,
+			wantStatus:            http.StatusFound,
+			wantContentType:       "application/json; charset=utf-8",
+			wantLocationHeader:    urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUserInfoEndpointErrorQuery),
+			wantBodyString:        "",
+		},
+		{
+			name:                  "password grant returns an error when upstream IDP returns empty refresh token and empty access token",
 			idps:                  oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithEmptyAccessToken().Build()),
 			method:                http.MethodGet,
 			path:                  happyGetRequestPath,
@@ -1084,7 +1116,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:        "",
 		},
 		{
-			name:                  "return an error when upstream IDP returns no refresh and no access token",
+			name:                  "password grant returns an error when upstream IDP returns no refresh and no access token",
 			idps:                  oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithoutAccessToken().Build()),
 			method:                http.MethodGet,
 			path:                  happyGetRequestPath,
@@ -1097,7 +1129,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:        "",
 		},
 		{
-			name:                  "return an error when upstream IDP returns no refresh token and empty access token",
+			name:                  "password grant returns an error when upstream IDP returns no refresh token and empty access token",
 			idps:                  oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithEmptyAccessToken().Build()),
 			method:                http.MethodGet,
 			path:                  happyGetRequestPath,
@@ -1110,7 +1142,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
 			wantBodyString:        "",
 		},
 		{
-			name:                  "return an error when upstream IDP returns empty refresh token and no access token",
+			name:                  "password grant returns an error when upstream IDP returns empty refresh token and no access token",
 			idps:                  oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithoutAccessToken().Build()),
 			method:                http.MethodGet,
 			path:                  happyGetRequestPath,

internal/oidc/callback/callback_handler_test.go

@@ -212,8 +212,8 @@ func TestCallbackEndpoint(t *testing.T) {
 			},
 		},
 		{
-			name:                              "GET with authcode exchange that returns an access token but no refresh token returns 303 to downstream client callback with its state and code",
+			name:                              "GET with authcode exchange that returns an access token but no refresh token when there is a userinfo endpoint returns 303 to downstream client callback with its state and code",
-			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken).Build()),
+			idps:                              oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithUserInfoURL().Build()),
 			method:                            http.MethodGet,
 			path:                              newRequestPath().WithState(happyState).String(),
 			csrfCookie:                        happyCSRFCookie,
@@ -356,6 +356,20 @@ func TestCallbackEndpoint(t *testing.T) {
 				args:                    happyExchangeAndValidateTokensArgs,
 			},
 		},
+		{
+			name:            "return an error when upstream IDP returned no refresh token with an access token when there is no userinfo endpoint",
+			idps:            oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithoutUserInfoURL().Build()),
+			method:          http.MethodGet,
+			path:            newRequestPath().WithState(happyState).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusUnprocessableEntity,
+			wantContentType: htmlContentType,
+			wantBody:        "Unprocessable Entity: access token was returned by upstream provider but there was no userinfo endpoint\n",
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
 		{
 			name:            "return an error when upstream IDP returned no refresh token and no access token",
 			idps:            oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithoutRefreshToken().WithoutAccessToken().Build()),

internal/oidc/downstreamsession/downstream_session.go

@@ -80,27 +80,32 @@ func MakeDownstreamOIDCCustomSessionData(oidcUpstream provider.UpstreamOIDCIdent
 		},
 	}
 
+	const pleaseCheck = "please check configuration of OIDCIdentityProvider and the client in the " +
+		"upstream provider's API/UI and try to get a refresh token if possible"
+	logKV := []interface{}{
+		"upstreamName", oidcUpstream.GetName(),
+		"scopes", oidcUpstream.GetScopes(),
+		"additionalParams", oidcUpstream.GetAdditionalAuthcodeParams(),
+	}
+
 	hasRefreshToken := token.RefreshToken != nil && token.RefreshToken.Token != ""
 	hasAccessToken := token.AccessToken != nil && token.AccessToken.Token != ""
 	switch {
 	case hasRefreshToken: // we prefer refresh tokens, so check for this first
 		customSessionData.OIDC.UpstreamRefreshToken = token.RefreshToken.Token
-	case hasAccessToken:
+	case hasAccessToken: // as a fallback, we can use the access token as long as there is a userinfo endpoint
-		plog.Info("refresh token not returned by upstream provider during password grant, using access token instead. "+
+		if !oidcUpstream.HasUserInfoURL() {
-			"please check configuration of OIDCIdentityProvider and the client in the upstream provider's API/UI "+
+			plog.Warning("access token was returned by upstream provider during login without a refresh token "+
-			"and try to get a refresh token if possible",
+				"and there was no userinfo endpoint available on the provider. "+pleaseCheck, logKV...)
-			"upstreamName", oidcUpstream.GetName(),
+			return nil, errors.New("access token was returned by upstream provider but there was no userinfo endpoint")
-			"scopes", oidcUpstream.GetScopes(),
+		}
-			"additionalParams", oidcUpstream.GetAdditionalAuthcodeParams())
+		plog.Info("refresh token not returned by upstream provider during login, using access token instead. "+pleaseCheck, logKV...)
 		customSessionData.OIDC.UpstreamAccessToken = token.AccessToken.Token
 	default:
-		plog.Warning("refresh token and access token not returned by upstream provider during password grant, "+
+		plog.Warning("refresh token and access token not returned by upstream provider during login. "+pleaseCheck, logKV...)
-			"please check configuration of OIDCIdentityProvider and the client in the upstream provider's API/UI",
-			"upstreamName", oidcUpstream.GetName(),
-			"scopes", oidcUpstream.GetScopes(),
-			"additionalParams", oidcUpstream.GetAdditionalAuthcodeParams())
 		return nil, errors.New("neither access token nor refresh token returned by upstream provider")
 	}
+
 	return customSessionData, nil
 }
 

internal/oidc/provider/dynamic_upstream_idp_provider.go

@@ -31,6 +31,9 @@ type UpstreamOIDCIdentityProviderI interface {
 	// GetAuthorizationURL returns the Authorization Endpoint fetched from discovery.
 	GetAuthorizationURL() *url.URL
 
+	// HasUserInfoURL returns whether there is a non-empty value for userinfo_endpoint fetched from discovery.
+	HasUserInfoURL() bool
+
 	// GetScopes returns the scopes to request in authorization (authcode or password grant) flow.
 	GetScopes() []string
 

internal/testutil/oidctestutil/oidctestutil.go

@@ -150,6 +150,7 @@ type TestUpstreamOIDCIdentityProvider struct {
 	ClientID                 string
 	ResourceUID              types.UID
 	AuthorizationURL         url.URL
+	UserInfoURL              bool
 	RevocationURL            *url.URL
 	UsernameClaim            string
 	GroupsClaim              string
@@ -210,6 +211,10 @@ func (u *TestUpstreamOIDCIdentityProvider) GetAuthorizationURL() *url.URL {
 	return &u.AuthorizationURL
 }
 
+func (u *TestUpstreamOIDCIdentityProvider) HasUserInfoURL() bool {
+	return u.UserInfoURL
+}
+
 func (u *TestUpstreamOIDCIdentityProvider) GetRevocationURL() *url.URL {
 	return u.RevocationURL
 }
@@ -612,6 +617,7 @@ type TestUpstreamOIDCIdentityProviderBuilder struct {
 	refreshedTokens          *oauth2.Token
 	validatedTokens          *oidctypes.Token
 	authorizationURL         url.URL
+	hasUserInfoURL           bool
 	additionalAuthcodeParams map[string]string
 	allowPasswordGrant       bool
 	authcodeExchangeErr      error
@@ -641,6 +647,16 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAuthorizationURL(value url
 	return u
 }
 
+func (u *TestUpstreamOIDCIdentityProviderBuilder) WithUserInfoURL() *TestUpstreamOIDCIdentityProviderBuilder {
+	u.hasUserInfoURL = true
+	return u
+}
+
+func (u *TestUpstreamOIDCIdentityProviderBuilder) WithoutUserInfoURL() *TestUpstreamOIDCIdentityProviderBuilder {
+	u.hasUserInfoURL = false
+	return u
+}
+
 func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAllowPasswordGrant(value bool) *TestUpstreamOIDCIdentityProviderBuilder {
 	u.allowPasswordGrant = value
 	return u
@@ -763,6 +779,7 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdent
 		Scopes:                   u.scopes,
 		AllowPasswordGrant:       u.allowPasswordGrant,
 		AuthorizationURL:         u.authorizationURL,
+		UserInfoURL:              u.hasUserInfoURL,
 		AdditionalAuthcodeParams: u.additionalAuthcodeParams,
 		ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) {
 			if u.authcodeExchangeErr != nil {

internal/upstreamoidc/upstreamoidc.go

@@ -61,6 +61,19 @@ func (p *ProviderConfig) GetRevocationURL() *url.URL {
 	return p.RevocationURL
 }
 
+func (p *ProviderConfig) HasUserInfoURL() bool {
+	providerJSON := &struct {
+		UserInfoURL string `json:"userinfo_endpoint"`
+	}{}
+	if err := p.Provider.Claims(providerJSON); err != nil {
+		// This should never happen in practice because we should have already successfully
+		// parsed these claims when p.Provider was created.
+		return false
+	}
+
+	return len(providerJSON.UserInfoURL) > 0
+}
+
 func (p *ProviderConfig) GetAdditionalAuthcodeParams() map[string]string {
 	return p.AdditionalAuthcodeParams
 }
@@ -356,16 +369,8 @@ func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, t
 }
 
 func (p *ProviderConfig) maybeFetchUserInfo(ctx context.Context, tok *oauth2.Token) (*coreosoidc.UserInfo, error) {
-	providerJSON := &struct {
-		UserInfoURL string `json:"userinfo_endpoint"`
-	}{}
-	if err := p.Provider.Claims(providerJSON); err != nil {
-		// this should never happen because we should have already parsed these claims at an earlier stage
-		return nil, httperr.Wrap(http.StatusInternalServerError, "could not unmarshal discovery JSON", err)
-	}
-
 	// implementing the user info endpoint is not required, skip this logic when it is absent
-	if len(providerJSON.UserInfoURL) == 0 {
+	if !p.HasUserInfoURL() {
 		return nil, nil
 	}
 

internal/upstreamoidc/upstreamoidc_test.go

@@ -40,6 +40,9 @@ func TestProviderConfig(t *testing.T) {
 				Endpoint: oauth2.Endpoint{AuthURL: "https://example.com"},
 				Scopes:   []string{"scope1", "scope2"},
 			},
+			Provider: &mockProvider{
+				rawClaims: []byte(`{"userinfo_endpoint": "https://example.com/userinfo"}`),
+			},
 		}
 		require.Equal(t, "test-name", p.GetName())
 		require.Equal(t, "test-client-id", p.GetClientID())
@@ -54,6 +57,16 @@ func TestProviderConfig(t *testing.T) {
 		require.True(t, p.AllowsPasswordGrant())
 		p.AllowPasswordGrant = false
 		require.False(t, p.AllowsPasswordGrant())
+
+		require.True(t, p.HasUserInfoURL())
+		p.Provider = &mockProvider{
+			rawClaims: []byte(`{"some_other_endpoint": "https://example.com/blah"}`),
+		}
+		require.False(t, p.HasUserInfoURL())
+		p.Provider = &mockProvider{
+			rawClaims: []byte(`{`),
+		}
+		require.False(t, p.HasUserInfoURL())
 	})
 
 	const (
@@ -748,6 +761,31 @@ func TestProviderConfig(t *testing.T) {
 					},
 				},
 			},
+			{
+				name:           "token with id, access and refresh tokens and valid nonce, but no userinfo endpoint from discovery",
+				tok:            testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": goodIDToken}),
+				nonce:          "some-nonce",
+				requireIDToken: true,
+				rawClaims:      []byte(`{"not_the_userinfo_endpoint": "some-other-endpoint"}`),
+				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]interface{}{
+							"iss":   "some-issuer",
+							"nonce": "some-nonce",
+							"sub":   "some-subject",
+						},
+					},
+				},
+			},
 			{
 				name:           "token with no id token but valid userinfo",
 				tok:            testTokenWithoutIDToken,
@@ -982,6 +1020,36 @@ func TestProviderConfig(t *testing.T) {
 				rawClaims:          []byte(`{}`), // user info not supported
 				wantUserInfoCalled: false,
 			},
+			{
+				name:        "valid but userinfo endpoint could not be found due to parse error",
+				authCode:    "valid",
+				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]interface{}{
+							"foo": "bar",
+							"bat": "baz",
+							"aud": "test-client-id",
+							"iat": 1.606768593e+09,
+							"jti": "test-jti",
+							"nbf": 1.606768593e+09,
+							"sub": "test-user",
+						},
+					},
+				},
+				// cannot be parsed as json, but note that in this case constructing a real provider would have failed
+				rawClaims:          []byte(`{`),
+				wantUserInfoCalled: false,
+			},
 			{
 				name:        "valid",
 				authCode:    "valid",
@@ -1011,13 +1079,6 @@ func TestProviderConfig(t *testing.T) {
 				rawClaims:          []byte(`{}`), // user info not supported
 				wantUserInfoCalled: false,
 			},
-			{
-				name:        "user info discovery parse error",
-				authCode:    "valid",
-				returnIDTok: validIDToken,
-				rawClaims:   []byte(`junk`), // user info discovery fails
-				wantErr:     "could not fetch user info claims: could not unmarshal discovery JSON: invalid character 'j' looking for beginning of value",
-			},
 			{
 				name:        "user info fetch error",
 				authCode:    "valid",