Some refactors based on PR feedback from @enj

apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go.tmpl

@@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider.
 	// In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes
-	// in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field).
+	// in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field).
 	// By default, only the "openid" scope will be requested.
 	// +optional
 	AdditionalScopes []string `json:"additionalScopes,omitempty"`

deploy/supervisor/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml

@@ -62,7 +62,7 @@ spec:
                       flow with an OIDC identity provider. In the case of a Resource
                       Owner Password Credentials Grant flow, AdditionalScopes are
                       the scopes in addition to "openid" that will be requested as
-                      part of the token request (see also the AllowPasswordGrant field).
+                      part of the token request (see also the allowPasswordGrant field).
                       By default, only the "openid" scope will be requested.
                     items:
                       type: string

generated/1.17/README.adoc

@@ -947,7 +947,7 @@ OIDCAuthorizationConfig provides information about how to form the OAuth2 author
 [cols="25a,75a", options="header"]
 |===
 | Field | Description
-| *`additionalScopes`* __string array__ | AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization request flow with an OIDC identity provider. In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field). By default, only the "openid" scope will be requested.
+| *`additionalScopes`* __string array__ | AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization request flow with an OIDC identity provider. In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field). By default, only the "openid" scope will be requested.
 | *`allowPasswordGrant`* __boolean__ | AllowPasswordGrant, when true, will allow the use of OAuth 2.0's Resource Owner Password Credentials Grant (see https://datatracker.ietf.org/doc/html/rfc6749#section-4.3) to authenticate to the OIDC provider using a username and password without a web browser, in addition to the usual browser-based OIDC Authorization Code Flow. The Resource Owner Password Credentials Grant is not officially part of the OIDC specification, so it may not be supported by your OIDC provider. If your OIDC provider supports returning ID tokens from a Resource Owner Password Credentials Grant token request, then you can choose to set this field to true. This will allow end users to choose to present their username and password to the kubectl CLI (using the Pinniped plugin) to authenticate to the cluster, without using a web browser to log in as is customary in OIDC Authorization Code Flow. This may be convenient for users, especially for identities from your OIDC provider which are not intended to represent a human actor, such as service accounts performing actions in a CI/CD environment. Even if your OIDC provider supports it, you may wish to disable this behavior by setting this field to false when you prefer to only allow users of this OIDCIdentityProvider to log in via the browser-based OIDC Authorization Code Flow. Using the Resource Owner Password Credentials Grant means that the Pinniped CLI and Pinniped Supervisor will directly handle your end users' passwords (similar to LDAPIdentityProvider), and you will not be able to require multi-factor authentication or use the other web-based login features of your OIDC provider during Resource Owner Password Credentials Grant logins. AllowPasswordGrant defaults to false.
 |===
 

generated/1.17/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go

@@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider.
 	// In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes
-	// in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field).
+	// in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field).
 	// By default, only the "openid" scope will be requested.
 	// +optional
 	AdditionalScopes []string `json:"additionalScopes,omitempty"`

generated/1.17/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml

@@ -62,7 +62,7 @@ spec:
                       flow with an OIDC identity provider. In the case of a Resource
                       Owner Password Credentials Grant flow, AdditionalScopes are
                       the scopes in addition to "openid" that will be requested as
-                      part of the token request (see also the AllowPasswordGrant field).
+                      part of the token request (see also the allowPasswordGrant field).
                       By default, only the "openid" scope will be requested.
                     items:
                       type: string

generated/1.18/README.adoc

@@ -947,7 +947,7 @@ OIDCAuthorizationConfig provides information about how to form the OAuth2 author
 [cols="25a,75a", options="header"]
 |===
 | Field | Description
-| *`additionalScopes`* __string array__ | AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization request flow with an OIDC identity provider. In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field). By default, only the "openid" scope will be requested.
+| *`additionalScopes`* __string array__ | AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization request flow with an OIDC identity provider. In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field). By default, only the "openid" scope will be requested.
 | *`allowPasswordGrant`* __boolean__ | AllowPasswordGrant, when true, will allow the use of OAuth 2.0's Resource Owner Password Credentials Grant (see https://datatracker.ietf.org/doc/html/rfc6749#section-4.3) to authenticate to the OIDC provider using a username and password without a web browser, in addition to the usual browser-based OIDC Authorization Code Flow. The Resource Owner Password Credentials Grant is not officially part of the OIDC specification, so it may not be supported by your OIDC provider. If your OIDC provider supports returning ID tokens from a Resource Owner Password Credentials Grant token request, then you can choose to set this field to true. This will allow end users to choose to present their username and password to the kubectl CLI (using the Pinniped plugin) to authenticate to the cluster, without using a web browser to log in as is customary in OIDC Authorization Code Flow. This may be convenient for users, especially for identities from your OIDC provider which are not intended to represent a human actor, such as service accounts performing actions in a CI/CD environment. Even if your OIDC provider supports it, you may wish to disable this behavior by setting this field to false when you prefer to only allow users of this OIDCIdentityProvider to log in via the browser-based OIDC Authorization Code Flow. Using the Resource Owner Password Credentials Grant means that the Pinniped CLI and Pinniped Supervisor will directly handle your end users' passwords (similar to LDAPIdentityProvider), and you will not be able to require multi-factor authentication or use the other web-based login features of your OIDC provider during Resource Owner Password Credentials Grant logins. AllowPasswordGrant defaults to false.
 |===
 

generated/1.18/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go

@@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider.
 	// In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes
-	// in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field).
+	// in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field).
 	// By default, only the "openid" scope will be requested.
 	// +optional
 	AdditionalScopes []string `json:"additionalScopes,omitempty"`

generated/1.18/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml

@@ -62,7 +62,7 @@ spec:
                       flow with an OIDC identity provider. In the case of a Resource
                       Owner Password Credentials Grant flow, AdditionalScopes are
                       the scopes in addition to "openid" that will be requested as
-                      part of the token request (see also the AllowPasswordGrant field).
+                      part of the token request (see also the allowPasswordGrant field).
                       By default, only the "openid" scope will be requested.
                     items:
                       type: string

generated/1.19/README.adoc

@@ -947,7 +947,7 @@ OIDCAuthorizationConfig provides information about how to form the OAuth2 author
 [cols="25a,75a", options="header"]
 |===
 | Field | Description
-| *`additionalScopes`* __string array__ | AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization request flow with an OIDC identity provider. In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field). By default, only the "openid" scope will be requested.
+| *`additionalScopes`* __string array__ | AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization request flow with an OIDC identity provider. In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field). By default, only the "openid" scope will be requested.
 | *`allowPasswordGrant`* __boolean__ | AllowPasswordGrant, when true, will allow the use of OAuth 2.0's Resource Owner Password Credentials Grant (see https://datatracker.ietf.org/doc/html/rfc6749#section-4.3) to authenticate to the OIDC provider using a username and password without a web browser, in addition to the usual browser-based OIDC Authorization Code Flow. The Resource Owner Password Credentials Grant is not officially part of the OIDC specification, so it may not be supported by your OIDC provider. If your OIDC provider supports returning ID tokens from a Resource Owner Password Credentials Grant token request, then you can choose to set this field to true. This will allow end users to choose to present their username and password to the kubectl CLI (using the Pinniped plugin) to authenticate to the cluster, without using a web browser to log in as is customary in OIDC Authorization Code Flow. This may be convenient for users, especially for identities from your OIDC provider which are not intended to represent a human actor, such as service accounts performing actions in a CI/CD environment. Even if your OIDC provider supports it, you may wish to disable this behavior by setting this field to false when you prefer to only allow users of this OIDCIdentityProvider to log in via the browser-based OIDC Authorization Code Flow. Using the Resource Owner Password Credentials Grant means that the Pinniped CLI and Pinniped Supervisor will directly handle your end users' passwords (similar to LDAPIdentityProvider), and you will not be able to require multi-factor authentication or use the other web-based login features of your OIDC provider during Resource Owner Password Credentials Grant logins. AllowPasswordGrant defaults to false.
 |===
 

generated/1.19/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go

@@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider.
 	// In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes
-	// in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field).
+	// in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field).
 	// By default, only the "openid" scope will be requested.
 	// +optional
 	AdditionalScopes []string `json:"additionalScopes,omitempty"`

generated/1.19/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml

@@ -62,7 +62,7 @@ spec:
                       flow with an OIDC identity provider. In the case of a Resource
                       Owner Password Credentials Grant flow, AdditionalScopes are
                       the scopes in addition to "openid" that will be requested as
-                      part of the token request (see also the AllowPasswordGrant field).
+                      part of the token request (see also the allowPasswordGrant field).
                       By default, only the "openid" scope will be requested.
                     items:
                       type: string

generated/1.20/README.adoc

@@ -947,7 +947,7 @@ OIDCAuthorizationConfig provides information about how to form the OAuth2 author
 [cols="25a,75a", options="header"]
 |===
 | Field | Description
-| *`additionalScopes`* __string array__ | AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization request flow with an OIDC identity provider. In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field). By default, only the "openid" scope will be requested.
+| *`additionalScopes`* __string array__ | AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization request flow with an OIDC identity provider. In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field). By default, only the "openid" scope will be requested.
 | *`allowPasswordGrant`* __boolean__ | AllowPasswordGrant, when true, will allow the use of OAuth 2.0's Resource Owner Password Credentials Grant (see https://datatracker.ietf.org/doc/html/rfc6749#section-4.3) to authenticate to the OIDC provider using a username and password without a web browser, in addition to the usual browser-based OIDC Authorization Code Flow. The Resource Owner Password Credentials Grant is not officially part of the OIDC specification, so it may not be supported by your OIDC provider. If your OIDC provider supports returning ID tokens from a Resource Owner Password Credentials Grant token request, then you can choose to set this field to true. This will allow end users to choose to present their username and password to the kubectl CLI (using the Pinniped plugin) to authenticate to the cluster, without using a web browser to log in as is customary in OIDC Authorization Code Flow. This may be convenient for users, especially for identities from your OIDC provider which are not intended to represent a human actor, such as service accounts performing actions in a CI/CD environment. Even if your OIDC provider supports it, you may wish to disable this behavior by setting this field to false when you prefer to only allow users of this OIDCIdentityProvider to log in via the browser-based OIDC Authorization Code Flow. Using the Resource Owner Password Credentials Grant means that the Pinniped CLI and Pinniped Supervisor will directly handle your end users' passwords (similar to LDAPIdentityProvider), and you will not be able to require multi-factor authentication or use the other web-based login features of your OIDC provider during Resource Owner Password Credentials Grant logins. AllowPasswordGrant defaults to false.
 |===
 

generated/1.20/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go

@@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider.
 	// In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes
-	// in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field).
+	// in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field).
 	// By default, only the "openid" scope will be requested.
 	// +optional
 	AdditionalScopes []string `json:"additionalScopes,omitempty"`

generated/1.20/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml

@@ -62,7 +62,7 @@ spec:
                       flow with an OIDC identity provider. In the case of a Resource
                       Owner Password Credentials Grant flow, AdditionalScopes are
                       the scopes in addition to "openid" that will be requested as
-                      part of the token request (see also the AllowPasswordGrant field).
+                      part of the token request (see also the allowPasswordGrant field).
                       By default, only the "openid" scope will be requested.
                     items:
                       type: string

generated/latest/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go

@@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider.
 	// In the case of a Resource Owner Password Credentials Grant flow, AdditionalScopes are the scopes
-	// in addition to "openid" that will be requested as part of the token request (see also the AllowPasswordGrant field).
+	// in addition to "openid" that will be requested as part of the token request (see also the allowPasswordGrant field).
 	// By default, only the "openid" scope will be requested.
 	// +optional
 	AdditionalScopes []string `json:"additionalScopes,omitempty"`

internal/oidc/auth/auth_handler.go

@@ -158,12 +158,7 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant(
 		return nil
 	}
 
-	subject, username, err := downstreamsession.GetSubjectAndUsernameFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims)
+	subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims)
-	if err != nil {
-		return err
-	}
-
-	groups, err := downstreamsession.GetGroupsFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims)
 	if err != nil {
 		return err
 	}

internal/oidc/callback/callback_handler.go

@@ -68,12 +68,7 @@ func NewHandler(
 			return httperr.New(http.StatusBadGateway, "error exchanging and validating upstream tokens")
 		}
 
-		subject, username, err := downstreamsession.GetSubjectAndUsernameFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims)
+		subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims)
-		if err != nil {
-			return err
-		}
-
-		groups, err := downstreamsession.GetGroupsFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims)
 		if err != nil {
 			return err
 		}

internal/oidc/callback/callback_handler_test.go

@@ -634,7 +634,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			path:            newRequestPath().WithState(happyState).String(),
 			csrfCookie:      happyCSRFCookie,
 			wantStatus:      http.StatusUnprocessableEntity,
-			wantBody:        "Unprocessable Entity: no username claim in upstream ID token\n",
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token missing\n",
 			wantContentType: htmlContentType,
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
@@ -675,7 +675,23 @@ func TestCallbackEndpoint(t *testing.T) {
 			csrfCookie:      happyCSRFCookie,
 			wantStatus:      http.StatusUnprocessableEntity,
 			wantContentType: htmlContentType,
-			wantBody:        "Unprocessable Entity: username claim in upstream ID token has invalid format\n",
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token has invalid format\n",
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name: "upstream ID token contains username claim with empty string value",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
+				happyUpstream().WithIDTokenClaim(oidcUpstreamUsernameClaim, "").Build(),
+			),
+			method:          http.MethodGet,
+			path:            newRequestPath().WithState(happyState).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusUnprocessableEntity,
+			wantContentType: htmlContentType,
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token is empty\n",
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
 				args:                    happyExchangeAndValidateTokensArgs,
@@ -683,6 +699,22 @@ func TestCallbackEndpoint(t *testing.T) {
 		},
 		{
 			name: "upstream ID token does not contain iss claim when using default username claim config",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
+				happyUpstream().WithoutIDTokenClaim("iss").WithoutUsernameClaim().Build(),
+			),
+			method:          http.MethodGet,
+			path:            newRequestPath().WithState(happyState).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusUnprocessableEntity,
+			wantContentType: htmlContentType,
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token missing\n",
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name: "upstream ID token does has an empty string value for iss claim when using default username claim config",
 			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
 				happyUpstream().WithIDTokenClaim("iss", "").WithoutUsernameClaim().Build(),
 			),
@@ -691,7 +723,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			csrfCookie:      happyCSRFCookie,
 			wantStatus:      http.StatusUnprocessableEntity,
 			wantContentType: htmlContentType,
-			wantBody:        "Unprocessable Entity: issuer claim in upstream ID token missing\n",
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token is empty\n",
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
 				args:                    happyExchangeAndValidateTokensArgs,
@@ -707,7 +739,55 @@ func TestCallbackEndpoint(t *testing.T) {
 			csrfCookie:      happyCSRFCookie,
 			wantStatus:      http.StatusUnprocessableEntity,
 			wantContentType: htmlContentType,
-			wantBody:        "Unprocessable Entity: issuer claim in upstream ID token has invalid format\n",
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token has invalid format\n",
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name: "upstream ID token does not contain sub claim when using default username claim config",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
+				happyUpstream().WithoutIDTokenClaim("sub").WithoutUsernameClaim().Build(),
+			),
+			method:          http.MethodGet,
+			path:            newRequestPath().WithState(happyState).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusUnprocessableEntity,
+			wantContentType: htmlContentType,
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token missing\n",
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name: "upstream ID token does has an empty string value for sub claim when using default username claim config",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
+				happyUpstream().WithIDTokenClaim("sub", "").WithoutUsernameClaim().Build(),
+			),
+			method:          http.MethodGet,
+			path:            newRequestPath().WithState(happyState).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusUnprocessableEntity,
+			wantContentType: htmlContentType,
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token is empty\n",
+			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
+				performedByUpstreamName: happyUpstreamIDPName,
+				args:                    happyExchangeAndValidateTokensArgs,
+			},
+		},
+		{
+			name: "upstream ID token has an non-string sub claim when using default username claim config",
+			idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
+				happyUpstream().WithIDTokenClaim("sub", 42).WithoutUsernameClaim().Build(),
+			),
+			method:          http.MethodGet,
+			path:            newRequestPath().WithState(happyState).String(),
+			csrfCookie:      happyCSRFCookie,
+			wantStatus:      http.StatusUnprocessableEntity,
+			wantContentType: htmlContentType,
+			wantBody:        "Unprocessable Entity: required claim in upstream ID token has invalid format\n",
 			wantAuthcodeExchangeCall: &expectedAuthcodeExchange{
 				performedByUpstreamName: happyUpstreamIDPName,
 				args:                    happyExchangeAndValidateTokensArgs,

internal/oidc/downstreamsession/downstream_session.go

@@ -56,50 +56,39 @@ func GrantScopesIfRequested(authorizeRequester fosite.AuthorizeRequester) {
 	oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience")
 }
 
-func GetSubjectAndUsernameFromUpstreamIDToken(
+// GetDownstreamIdentityFromUpstreamIDToken returns the mapped subject, username, and group names, in that order.
+func GetDownstreamIdentityFromUpstreamIDToken(
+	upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI,
+	idTokenClaims map[string]interface{},
+) (string, string, []string, error) {
+	subject, username, err := getSubjectAndUsernameFromUpstreamIDToken(upstreamIDPConfig, idTokenClaims)
+	if err != nil {
+		return "", "", nil, err
+	}
+
+	groups, err := getGroupsFromUpstreamIDToken(upstreamIDPConfig, idTokenClaims)
+	if err != nil {
+		return "", "", nil, err
+	}
+
+	return subject, username, groups, err
+}
+
+func getSubjectAndUsernameFromUpstreamIDToken(
 	upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI,
 	idTokenClaims map[string]interface{},
 ) (string, string, error) {
 	// The spec says the "sub" claim is only unique per issuer,
 	// so we will prepend the issuer string to make it globally unique.
-	upstreamIssuer := idTokenClaims[oidc.IDTokenIssuerClaim]
+	upstreamIssuer, err := extractStringClaimValue(oidc.IDTokenIssuerClaim, upstreamIDPConfig.GetName(), idTokenClaims)
-	if upstreamIssuer == "" {
+	if err != nil {
-		plog.Warning(
+		return "", "", err
-			"issuer claim in upstream ID token missing",
-			"upstreamName", upstreamIDPConfig.GetName(),
-			"issClaim", upstreamIssuer,
-		)
-		return "", "", httperr.New(http.StatusUnprocessableEntity, "issuer claim in upstream ID token missing")
 	}
-	upstreamIssuerAsString, ok := upstreamIssuer.(string)
+	upstreamSubject, err := extractStringClaimValue(oidc.IDTokenSubjectClaim, upstreamIDPConfig.GetName(), idTokenClaims)
-	if !ok {
+	if err != nil {
-		plog.Warning(
+		return "", "", err
-			"issuer claim in upstream ID token has invalid format",
-			"upstreamName", upstreamIDPConfig.GetName(),
-			"issClaim", upstreamIssuer,
-		)
-		return "", "", httperr.New(http.StatusUnprocessableEntity, "issuer claim in upstream ID token has invalid format")
 	}
-
+	subject := downstreamSubjectFromUpstreamOIDC(upstreamIssuer, upstreamSubject)
-	subjectAsInterface, ok := idTokenClaims[oidc.IDTokenSubjectClaim]
-	if !ok {
-		plog.Warning(
-			"no subject claim in upstream ID token",
-			"upstreamName", upstreamIDPConfig.GetName(),
-		)
-		return "", "", httperr.New(http.StatusUnprocessableEntity, "no subject claim in upstream ID token")
-	}
-
-	upstreamSubject, ok := subjectAsInterface.(string)
-	if !ok {
-		plog.Warning(
-			"subject claim in upstream ID token has invalid format",
-			"upstreamName", upstreamIDPConfig.GetName(),
-		)
-		return "", "", httperr.New(http.StatusUnprocessableEntity, "subject claim in upstream ID token has invalid format")
-	}
-
-	subject := downstreamSubjectFromUpstreamOIDC(upstreamIssuerAsString, upstreamSubject)
 
 	usernameClaimName := upstreamIDPConfig.GetUsernameClaim()
 	if usernameClaimName == "" {
@@ -130,34 +119,52 @@ func GetSubjectAndUsernameFromUpstreamIDToken(
 		}
 	}
 
-	usernameAsInterface, ok := idTokenClaims[usernameClaimName]
+	username, err := extractStringClaimValue(usernameClaimName, upstreamIDPConfig.GetName(), idTokenClaims)
-	if !ok {
+	if err != nil {
-		plog.Warning(
+		return "", "", err
-			"no username claim in upstream ID token",
-			"upstreamName", upstreamIDPConfig.GetName(),
-			"configuredUsernameClaim", usernameClaimName,
-		)
-		return "", "", httperr.New(http.StatusUnprocessableEntity, "no username claim in upstream ID token")
-	}
-
-	username, ok := usernameAsInterface.(string)
-	if !ok {
-		plog.Warning(
-			"username claim in upstream ID token has invalid format",
-			"upstreamName", upstreamIDPConfig.GetName(),
-			"configuredUsernameClaim", usernameClaimName,
-		)
-		return "", "", httperr.New(http.StatusUnprocessableEntity, "username claim in upstream ID token has invalid format")
 	}
 
 	return subject, username, nil
 }
 
+func extractStringClaimValue(claimName string, upstreamIDPName string, idTokenClaims map[string]interface{}) (string, error) {
+	value, ok := idTokenClaims[claimName]
+	if !ok {
+		plog.Warning(
+			"required claim in upstream ID token missing",
+			"upstreamName", upstreamIDPName,
+			"claimName", claimName,
+		)
+		return "", httperr.New(http.StatusUnprocessableEntity, "required claim in upstream ID token missing")
+	}
+
+	valueAsString, ok := value.(string)
+	if !ok {
+		plog.Warning(
+			"required claim in upstream ID token is not a string value",
+			"upstreamName", upstreamIDPName,
+			"claimName", claimName,
+		)
+		return "", httperr.New(http.StatusUnprocessableEntity, "required claim in upstream ID token has invalid format")
+	}
+
+	if valueAsString == "" {
+		plog.Warning(
+			"required claim in upstream ID token has an empty string value",
+			"upstreamName", upstreamIDPName,
+			"claimName", claimName,
+		)
+		return "", httperr.New(http.StatusUnprocessableEntity, "required claim in upstream ID token is empty")
+	}
+
+	return valueAsString, nil
+}
+
 func downstreamSubjectFromUpstreamOIDC(upstreamIssuerAsString string, upstreamSubject string) string {
 	return fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, oidc.IDTokenSubjectClaim, url.QueryEscape(upstreamSubject))
 }
 
-func GetGroupsFromUpstreamIDToken(
+func getGroupsFromUpstreamIDToken(
 	upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI,
 	idTokenClaims map[string]interface{},
 ) ([]string, error) {

internal/testutil/oidctestutil/oidctestutil.go

@@ -134,9 +134,6 @@ func (u *TestUpstreamOIDCIdentityProvider) AllowsPasswordGrant() bool {
 }
 
 func (u *TestUpstreamOIDCIdentityProvider) PasswordCredentialsGrantAndValidateTokens(ctx context.Context, username, password string) (*oidctypes.Token, error) {
-	if u.passwordCredentialsGrantAndValidateTokensArgs == nil {
-		u.passwordCredentialsGrantAndValidateTokensArgs = make([]*PasswordCredentialsGrantAndValidateTokensArgs, 0)
-	}
 	u.passwordCredentialsGrantAndValidateTokensCallCount++
 	u.passwordCredentialsGrantAndValidateTokensArgs = append(u.passwordCredentialsGrantAndValidateTokensArgs, &PasswordCredentialsGrantAndValidateTokensArgs{
 		Ctx:      ctx,

internal/upstreamoidc/upstreamoidc.go

@@ -88,7 +88,7 @@ func (p *ProviderConfig) PasswordCredentialsGrantAndValidateTokens(ctx context.C
 
 	// There is no nonce to validate for a resource owner password credentials grant because it skips using
 	// the authorize endpoint and goes straight to the token endpoint.
-	skipNonceValidation := nonce.Nonce("")
+	const skipNonceValidation nonce.Nonce = ""
 	return p.ValidateToken(ctx, tok, skipNonceValidation)
 }