diff --git a/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go.tmpl b/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go.tmpl index 3211d2e5..5d7277bf 100644 --- a/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go.tmpl +++ b/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"` diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index 5cf79148..c3506bb7 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -149,7 +149,7 @@ func TestGetKubeconfig(t *testing.T) { --static-token string Instead of doing an OIDC-based login, specify a static token --static-token-env string Instead of doing an OIDC-based login, read a static token from the environment --timeout duration Timeout for autodiscovery and validation (default 10m0s) - --upstream-identity-provider-flow string The type of client flow to use with the upstream identity provider during login with a Supervisor (e.g. 'cli_password', 'browser_authcode') + --upstream-identity-provider-flow string The type of client flow to use with the upstream identity provider during login with a Supervisor (e.g. 'cli_password', 'browser_authcode') --upstream-identity-provider-name string The name of the upstream identity provider used during login with a Supervisor --upstream-identity-provider-type string The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap') `) diff --git a/deploy/supervisor/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml b/deploy/supervisor/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml index d9ea45f0..70d3865d 100644 --- a/deploy/supervisor/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml +++ b/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 diff --git a/generated/1.17/README.adoc b/generated/1.17/README.adoc index b709175f..1dc3c516 100644 --- a/generated/1.17/README.adoc +++ b/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. |=== diff --git a/generated/1.17/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/1.17/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index 3211d2e5..5d7277bf 100644 --- a/generated/1.17/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/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"` diff --git a/generated/1.17/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml b/generated/1.17/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml index d9ea45f0..70d3865d 100644 --- a/generated/1.17/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml +++ b/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 diff --git a/generated/1.18/README.adoc b/generated/1.18/README.adoc index 95ed5d61..9c6083ec 100644 --- a/generated/1.18/README.adoc +++ b/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. |=== diff --git a/generated/1.18/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/1.18/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index 3211d2e5..5d7277bf 100644 --- a/generated/1.18/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/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"` diff --git a/generated/1.18/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml b/generated/1.18/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml index d9ea45f0..70d3865d 100644 --- a/generated/1.18/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml +++ b/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 diff --git a/generated/1.19/README.adoc b/generated/1.19/README.adoc index 94587807..5d5825c6 100644 --- a/generated/1.19/README.adoc +++ b/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. |=== diff --git a/generated/1.19/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/1.19/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index 3211d2e5..5d7277bf 100644 --- a/generated/1.19/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/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"` diff --git a/generated/1.19/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml b/generated/1.19/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml index d9ea45f0..70d3865d 100644 --- a/generated/1.19/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml +++ b/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 diff --git a/generated/1.20/README.adoc b/generated/1.20/README.adoc index dba89ffc..5038afa8 100644 --- a/generated/1.20/README.adoc +++ b/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. |=== diff --git a/generated/1.20/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/1.20/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index 3211d2e5..5d7277bf 100644 --- a/generated/1.20/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/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"` diff --git a/generated/1.20/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml b/generated/1.20/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml index d9ea45f0..70d3865d 100644 --- a/generated/1.20/crds/idp.supervisor.pinniped.dev_oidcidentityproviders.yaml +++ b/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 diff --git a/generated/latest/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/latest/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index 3211d2e5..5d7277bf 100644 --- a/generated/latest/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/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"` diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 494b4d22..567ff166 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -158,12 +158,7 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( return nil } - subject, username, err := downstreamsession.GetSubjectAndUsernameFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) - if err != nil { - return err - } - - groups, err := downstreamsession.GetGroupsFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) + subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) if err != nil { return err } diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index e4912da7..73e37b75 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/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) - if err != nil { - return err - } - - groups, err := downstreamsession.GetGroupsFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) + subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) if err != nil { return err } diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index e2b9a31c..7252da03 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/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, diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index e398908a..c09ea8a7 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/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] - if upstreamIssuer == "" { - plog.Warning( - "issuer claim in upstream ID token missing", - "upstreamName", upstreamIDPConfig.GetName(), - "issClaim", upstreamIssuer, - ) - return "", "", httperr.New(http.StatusUnprocessableEntity, "issuer claim in upstream ID token missing") + upstreamIssuer, err := extractStringClaimValue(oidc.IDTokenIssuerClaim, upstreamIDPConfig.GetName(), idTokenClaims) + if err != nil { + return "", "", err } - upstreamIssuerAsString, ok := upstreamIssuer.(string) - if !ok { - plog.Warning( - "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") + upstreamSubject, err := extractStringClaimValue(oidc.IDTokenSubjectClaim, upstreamIDPConfig.GetName(), idTokenClaims) + if err != nil { + return "", "", err } - - 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) + subject := downstreamSubjectFromUpstreamOIDC(upstreamIssuer, upstreamSubject) usernameClaimName := upstreamIDPConfig.GetUsernameClaim() if usernameClaimName == "" { @@ -130,34 +119,52 @@ func GetSubjectAndUsernameFromUpstreamIDToken( } } - usernameAsInterface, ok := idTokenClaims[usernameClaimName] - if !ok { - plog.Warning( - "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") + username, err := extractStringClaimValue(usernameClaimName, upstreamIDPConfig.GetName(), idTokenClaims) + if err != nil { + return "", "", err } 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) { diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index 7591bbca..d7bccd2a 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/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, diff --git a/internal/upstreamoidc/upstreamoidc.go b/internal/upstreamoidc/upstreamoidc.go index 7ca73ee8..c8d3722f 100644 --- a/internal/upstreamoidc/upstreamoidc.go +++ b/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) }