diff --git a/apis/concierge/authentication/v1alpha1/types_jwt.go.tmpl b/apis/concierge/authentication/v1alpha1/types_jwt.go.tmpl index 5d0604bd..3e159148 100644 --- a/apis/concierge/authentication/v1alpha1/types_jwt.go.tmpl +++ b/apis/concierge/authentication/v1alpha1/types_jwt.go.tmpl @@ -27,11 +27,30 @@ type JWTAuthenticatorSpec struct { // +kubebuilder:validation:MinLength=1 Audience string `json:"audience"` + // Claims allows customization of the claims that will be mapped to user identity + // for Kubernetes access. + // +optional + Claims JWTTokenClaims `json:"claims"` + // TLS configuration for communicating with the OIDC provider. // +optional TLS *TLSSpec `json:"tls,omitempty"` } +// JWTTokenClaims allows customization of the claims that will be mapped to user identity +// for Kubernetes access. +type JWTTokenClaims struct { + // Groups is the name of the claim which should be read to extract the user's + // group membership from the JWT token. When not specified, it will default to "groups". + // +optional + Groups string `json:"groups"` + + // Username is the name of the claim which should be read to extract the + // username from the JWT token. When not specified, it will default to "username". + // +optional + Username string `json:"username"` +} + // JWTAuthenticator describes the configuration of a JWT authenticator. // // Upon receiving a signed JWT, a JWTAuthenticator will performs some validation on it (e.g., valid diff --git a/deploy/concierge/authentication.concierge.pinniped.dev_jwtauthenticators.yaml b/deploy/concierge/authentication.concierge.pinniped.dev_jwtauthenticators.yaml index ed16bf0c..e800411e 100644 --- a/deploy/concierge/authentication.concierge.pinniped.dev_jwtauthenticators.yaml +++ b/deploy/concierge/authentication.concierge.pinniped.dev_jwtauthenticators.yaml @@ -51,6 +51,21 @@ spec: description: Audience is the required value of the "aud" JWT claim. minLength: 1 type: string + claims: + description: Claims allows customization of the claims that will be + mapped to user identity for Kubernetes access. + properties: + groups: + description: Groups is the name of the claim which should be read + to extract the user's group membership from the JWT token. When + not specified, it will default to "groups". + type: string + username: + description: Username is the name of the claim which should be + read to extract the username from the JWT token. When not specified, + it will default to "username". + type: string + type: object issuer: description: Issuer is the OIDC issuer URL that will be used to discover public signing keys. Issuer is also used to validate the "iss" JWT diff --git a/generated/1.17/README.adoc b/generated/1.17/README.adoc index 75d13c3e..6db75766 100644 --- a/generated/1.17/README.adoc +++ b/generated/1.17/README.adoc @@ -92,6 +92,7 @@ Spec for configuring a JWT authenticator. | Field | Description | *`issuer`* __string__ | Issuer is the OIDC issuer URL that will be used to discover public signing keys. Issuer is also used to validate the "iss" JWT claim. | *`audience`* __string__ | Audience is the required value of the "aud" JWT claim. +| *`claims`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-concierge-authentication-v1alpha1-jwttokenclaims[$$JWTTokenClaims$$]__ | Claims allows customization of the claims that will be mapped to user identity for Kubernetes access. | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-concierge-authentication-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for communicating with the OIDC provider. |=== @@ -113,6 +114,24 @@ Status of a JWT authenticator. |=== +[id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-concierge-authentication-v1alpha1-jwttokenclaims"] +==== JWTTokenClaims + +JWTTokenClaims allows customization of the claims that will be mapped to user identity for Kubernetes access. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-concierge-authentication-v1alpha1-jwtauthenticatorspec[$$JWTAuthenticatorSpec$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`groups`* __string__ | Groups is the name of the claim which should be read to extract the user's group membership from the JWT token. When not specified, it will default to "groups". +| *`username`* __string__ | Username is the name of the claim which should be read to extract the username from the JWT token. When not specified, it will default to "username". +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-concierge-authentication-v1alpha1-tlsspec"] ==== TLSSpec diff --git a/generated/1.17/apis/concierge/authentication/v1alpha1/types_jwt.go b/generated/1.17/apis/concierge/authentication/v1alpha1/types_jwt.go index 5d0604bd..3e159148 100644 --- a/generated/1.17/apis/concierge/authentication/v1alpha1/types_jwt.go +++ b/generated/1.17/apis/concierge/authentication/v1alpha1/types_jwt.go @@ -27,11 +27,30 @@ type JWTAuthenticatorSpec struct { // +kubebuilder:validation:MinLength=1 Audience string `json:"audience"` + // Claims allows customization of the claims that will be mapped to user identity + // for Kubernetes access. + // +optional + Claims JWTTokenClaims `json:"claims"` + // TLS configuration for communicating with the OIDC provider. // +optional TLS *TLSSpec `json:"tls,omitempty"` } +// JWTTokenClaims allows customization of the claims that will be mapped to user identity +// for Kubernetes access. +type JWTTokenClaims struct { + // Groups is the name of the claim which should be read to extract the user's + // group membership from the JWT token. When not specified, it will default to "groups". + // +optional + Groups string `json:"groups"` + + // Username is the name of the claim which should be read to extract the + // username from the JWT token. When not specified, it will default to "username". + // +optional + Username string `json:"username"` +} + // JWTAuthenticator describes the configuration of a JWT authenticator. // // Upon receiving a signed JWT, a JWTAuthenticator will performs some validation on it (e.g., valid diff --git a/generated/1.17/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go b/generated/1.17/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go index 93d4e837..c7c6968a 100644 --- a/generated/1.17/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.17/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go @@ -92,6 +92,7 @@ func (in *JWTAuthenticatorList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JWTAuthenticatorSpec) DeepCopyInto(out *JWTAuthenticatorSpec) { *out = *in + out.Claims = in.Claims if in.TLS != nil { in, out := &in.TLS, &out.TLS *out = new(TLSSpec) @@ -133,6 +134,22 @@ func (in *JWTAuthenticatorStatus) DeepCopy() *JWTAuthenticatorStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *JWTTokenClaims) DeepCopyInto(out *JWTTokenClaims) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new JWTTokenClaims. +func (in *JWTTokenClaims) DeepCopy() *JWTTokenClaims { + if in == nil { + return nil + } + out := new(JWTTokenClaims) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TLSSpec) DeepCopyInto(out *TLSSpec) { *out = *in diff --git a/generated/1.17/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml b/generated/1.17/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml index ed16bf0c..e800411e 100644 --- a/generated/1.17/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml +++ b/generated/1.17/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml @@ -51,6 +51,21 @@ spec: description: Audience is the required value of the "aud" JWT claim. minLength: 1 type: string + claims: + description: Claims allows customization of the claims that will be + mapped to user identity for Kubernetes access. + properties: + groups: + description: Groups is the name of the claim which should be read + to extract the user's group membership from the JWT token. When + not specified, it will default to "groups". + type: string + username: + description: Username is the name of the claim which should be + read to extract the username from the JWT token. When not specified, + it will default to "username". + type: string + type: object issuer: description: Issuer is the OIDC issuer URL that will be used to discover public signing keys. Issuer is also used to validate the "iss" JWT diff --git a/generated/1.18/README.adoc b/generated/1.18/README.adoc index 01a0c718..a8c51180 100644 --- a/generated/1.18/README.adoc +++ b/generated/1.18/README.adoc @@ -92,6 +92,7 @@ Spec for configuring a JWT authenticator. | Field | Description | *`issuer`* __string__ | Issuer is the OIDC issuer URL that will be used to discover public signing keys. Issuer is also used to validate the "iss" JWT claim. | *`audience`* __string__ | Audience is the required value of the "aud" JWT claim. +| *`claims`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-concierge-authentication-v1alpha1-jwttokenclaims[$$JWTTokenClaims$$]__ | Claims allows customization of the claims that will be mapped to user identity for Kubernetes access. | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-concierge-authentication-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for communicating with the OIDC provider. |=== @@ -113,6 +114,24 @@ Status of a JWT authenticator. |=== +[id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-concierge-authentication-v1alpha1-jwttokenclaims"] +==== JWTTokenClaims + +JWTTokenClaims allows customization of the claims that will be mapped to user identity for Kubernetes access. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-concierge-authentication-v1alpha1-jwtauthenticatorspec[$$JWTAuthenticatorSpec$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`groups`* __string__ | Groups is the name of the claim which should be read to extract the user's group membership from the JWT token. When not specified, it will default to "groups". +| *`username`* __string__ | Username is the name of the claim which should be read to extract the username from the JWT token. When not specified, it will default to "username". +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-concierge-authentication-v1alpha1-tlsspec"] ==== TLSSpec diff --git a/generated/1.18/apis/concierge/authentication/v1alpha1/types_jwt.go b/generated/1.18/apis/concierge/authentication/v1alpha1/types_jwt.go index 5d0604bd..3e159148 100644 --- a/generated/1.18/apis/concierge/authentication/v1alpha1/types_jwt.go +++ b/generated/1.18/apis/concierge/authentication/v1alpha1/types_jwt.go @@ -27,11 +27,30 @@ type JWTAuthenticatorSpec struct { // +kubebuilder:validation:MinLength=1 Audience string `json:"audience"` + // Claims allows customization of the claims that will be mapped to user identity + // for Kubernetes access. + // +optional + Claims JWTTokenClaims `json:"claims"` + // TLS configuration for communicating with the OIDC provider. // +optional TLS *TLSSpec `json:"tls,omitempty"` } +// JWTTokenClaims allows customization of the claims that will be mapped to user identity +// for Kubernetes access. +type JWTTokenClaims struct { + // Groups is the name of the claim which should be read to extract the user's + // group membership from the JWT token. When not specified, it will default to "groups". + // +optional + Groups string `json:"groups"` + + // Username is the name of the claim which should be read to extract the + // username from the JWT token. When not specified, it will default to "username". + // +optional + Username string `json:"username"` +} + // JWTAuthenticator describes the configuration of a JWT authenticator. // // Upon receiving a signed JWT, a JWTAuthenticator will performs some validation on it (e.g., valid diff --git a/generated/1.18/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go b/generated/1.18/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go index 93d4e837..c7c6968a 100644 --- a/generated/1.18/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.18/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go @@ -92,6 +92,7 @@ func (in *JWTAuthenticatorList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JWTAuthenticatorSpec) DeepCopyInto(out *JWTAuthenticatorSpec) { *out = *in + out.Claims = in.Claims if in.TLS != nil { in, out := &in.TLS, &out.TLS *out = new(TLSSpec) @@ -133,6 +134,22 @@ func (in *JWTAuthenticatorStatus) DeepCopy() *JWTAuthenticatorStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *JWTTokenClaims) DeepCopyInto(out *JWTTokenClaims) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new JWTTokenClaims. +func (in *JWTTokenClaims) DeepCopy() *JWTTokenClaims { + if in == nil { + return nil + } + out := new(JWTTokenClaims) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TLSSpec) DeepCopyInto(out *TLSSpec) { *out = *in diff --git a/generated/1.18/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml b/generated/1.18/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml index ed16bf0c..e800411e 100644 --- a/generated/1.18/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml +++ b/generated/1.18/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml @@ -51,6 +51,21 @@ spec: description: Audience is the required value of the "aud" JWT claim. minLength: 1 type: string + claims: + description: Claims allows customization of the claims that will be + mapped to user identity for Kubernetes access. + properties: + groups: + description: Groups is the name of the claim which should be read + to extract the user's group membership from the JWT token. When + not specified, it will default to "groups". + type: string + username: + description: Username is the name of the claim which should be + read to extract the username from the JWT token. When not specified, + it will default to "username". + type: string + type: object issuer: description: Issuer is the OIDC issuer URL that will be used to discover public signing keys. Issuer is also used to validate the "iss" JWT diff --git a/generated/1.19/README.adoc b/generated/1.19/README.adoc index a61e1073..c8a5461a 100644 --- a/generated/1.19/README.adoc +++ b/generated/1.19/README.adoc @@ -92,6 +92,7 @@ Spec for configuring a JWT authenticator. | Field | Description | *`issuer`* __string__ | Issuer is the OIDC issuer URL that will be used to discover public signing keys. Issuer is also used to validate the "iss" JWT claim. | *`audience`* __string__ | Audience is the required value of the "aud" JWT claim. +| *`claims`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-concierge-authentication-v1alpha1-jwttokenclaims[$$JWTTokenClaims$$]__ | Claims allows customization of the claims that will be mapped to user identity for Kubernetes access. | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-concierge-authentication-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for communicating with the OIDC provider. |=== @@ -113,6 +114,24 @@ Status of a JWT authenticator. |=== +[id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-concierge-authentication-v1alpha1-jwttokenclaims"] +==== JWTTokenClaims + +JWTTokenClaims allows customization of the claims that will be mapped to user identity for Kubernetes access. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-concierge-authentication-v1alpha1-jwtauthenticatorspec[$$JWTAuthenticatorSpec$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`groups`* __string__ | Groups is the name of the claim which should be read to extract the user's group membership from the JWT token. When not specified, it will default to "groups". +| *`username`* __string__ | Username is the name of the claim which should be read to extract the username from the JWT token. When not specified, it will default to "username". +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-concierge-authentication-v1alpha1-tlsspec"] ==== TLSSpec diff --git a/generated/1.19/apis/concierge/authentication/v1alpha1/types_jwt.go b/generated/1.19/apis/concierge/authentication/v1alpha1/types_jwt.go index 5d0604bd..3e159148 100644 --- a/generated/1.19/apis/concierge/authentication/v1alpha1/types_jwt.go +++ b/generated/1.19/apis/concierge/authentication/v1alpha1/types_jwt.go @@ -27,11 +27,30 @@ type JWTAuthenticatorSpec struct { // +kubebuilder:validation:MinLength=1 Audience string `json:"audience"` + // Claims allows customization of the claims that will be mapped to user identity + // for Kubernetes access. + // +optional + Claims JWTTokenClaims `json:"claims"` + // TLS configuration for communicating with the OIDC provider. // +optional TLS *TLSSpec `json:"tls,omitempty"` } +// JWTTokenClaims allows customization of the claims that will be mapped to user identity +// for Kubernetes access. +type JWTTokenClaims struct { + // Groups is the name of the claim which should be read to extract the user's + // group membership from the JWT token. When not specified, it will default to "groups". + // +optional + Groups string `json:"groups"` + + // Username is the name of the claim which should be read to extract the + // username from the JWT token. When not specified, it will default to "username". + // +optional + Username string `json:"username"` +} + // JWTAuthenticator describes the configuration of a JWT authenticator. // // Upon receiving a signed JWT, a JWTAuthenticator will performs some validation on it (e.g., valid diff --git a/generated/1.19/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go b/generated/1.19/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go index 93d4e837..c7c6968a 100644 --- a/generated/1.19/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.19/apis/concierge/authentication/v1alpha1/zz_generated.deepcopy.go @@ -92,6 +92,7 @@ func (in *JWTAuthenticatorList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JWTAuthenticatorSpec) DeepCopyInto(out *JWTAuthenticatorSpec) { *out = *in + out.Claims = in.Claims if in.TLS != nil { in, out := &in.TLS, &out.TLS *out = new(TLSSpec) @@ -133,6 +134,22 @@ func (in *JWTAuthenticatorStatus) DeepCopy() *JWTAuthenticatorStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *JWTTokenClaims) DeepCopyInto(out *JWTTokenClaims) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new JWTTokenClaims. +func (in *JWTTokenClaims) DeepCopy() *JWTTokenClaims { + if in == nil { + return nil + } + out := new(JWTTokenClaims) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TLSSpec) DeepCopyInto(out *TLSSpec) { *out = *in diff --git a/generated/1.19/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml b/generated/1.19/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml index ed16bf0c..e800411e 100644 --- a/generated/1.19/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml +++ b/generated/1.19/crds/authentication.concierge.pinniped.dev_jwtauthenticators.yaml @@ -51,6 +51,21 @@ spec: description: Audience is the required value of the "aud" JWT claim. minLength: 1 type: string + claims: + description: Claims allows customization of the claims that will be + mapped to user identity for Kubernetes access. + properties: + groups: + description: Groups is the name of the claim which should be read + to extract the user's group membership from the JWT token. When + not specified, it will default to "groups". + type: string + username: + description: Username is the name of the claim which should be + read to extract the username from the JWT token. When not specified, + it will default to "username". + type: string + type: object issuer: description: Issuer is the OIDC issuer URL that will be used to discover public signing keys. Issuer is also used to validate the "iss" JWT diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index de31961a..03fea206 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -29,7 +29,7 @@ import ( // These default values come from the way that the Supervisor issues and signs tokens. We make these // the defaults for a JWTAuthenticator so that they can easily integrate with the Supervisor. const ( - defaultUsernameClaim = "sub" + defaultUsernameClaim = "username" defaultGroupsClaim = "groups" ) @@ -169,12 +169,20 @@ func newJWTAuthenticator(spec *auth1alpha1.JWTAuthenticatorSpec) (*jwtAuthentica caFile = temp.Name() } + usernameClaim := spec.Claims.Username + if usernameClaim == "" { + usernameClaim = defaultUsernameClaim + } + groupsClaim := spec.Claims.Groups + if groupsClaim == "" { + groupsClaim = defaultGroupsClaim + } authenticator, err := oidc.New(oidc.Options{ IssuerURL: spec.Issuer, ClientID: spec.Audience, - UsernameClaim: defaultUsernameClaim, - GroupsClaim: defaultGroupsClaim, + UsernameClaim: usernameClaim, + GroupsClaim: groupsClaim, SupportedSigningAlgs: defaultSupportedSigningAlgos(), CAFile: caFile, }) diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index eaafa798..9cec76e6 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -19,11 +19,10 @@ import ( "testing" "time" - "gopkg.in/square/go-jose.v2/jwt" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" "gopkg.in/square/go-jose.v2" + "gopkg.in/square/go-jose.v2/jwt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authentication/authenticator" @@ -41,225 +40,10 @@ import ( func TestController(t *testing.T) { t.Parallel() - someJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ - Issuer: "https://some-issuer.com", - Audience: "some-audience", - TLS: &auth1alpha1.TLSSpec{CertificateAuthorityData: "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVVENDQWptZ0F3SUJBZ0lWQUpzNStTbVRtaTJXeUI0bGJJRXBXaUs5a1RkUE1BMEdDU3FHU0liM0RRRUIKQ3dVQU1COHhDekFKQmdOVkJBWVRBbFZUTVJBd0RnWURWUVFLREFkUWFYWnZkR0ZzTUI0WERUSXdNRFV3TkRFMgpNamMxT0ZvWERUSTBNRFV3TlRFMk1qYzFPRm93SHpFTE1Ba0dBMVVFQmhNQ1ZWTXhFREFPQmdOVkJBb01CMUJwCmRtOTBZV3d3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRRERZWmZvWGR4Z2NXTEMKZEJtbHB5a0tBaG9JMlBuUWtsVFNXMno1cGcwaXJjOGFRL1E3MXZzMTRZYStmdWtFTGlvOTRZYWw4R01DdVFrbApMZ3AvUEE5N1VYelhQNDBpK25iNXcwRGpwWWd2dU9KQXJXMno2MFRnWE5NSFh3VHk4ME1SZEhpUFVWZ0VZd0JpCmtkNThzdEFVS1Y1MnBQTU1reTJjNy9BcFhJNmRXR2xjalUvaFBsNmtpRzZ5dEw2REtGYjJQRWV3MmdJM3pHZ2IKOFVVbnA1V05DZDd2WjNVY0ZHNXlsZEd3aGc3cnZ4U1ZLWi9WOEhCMGJmbjlxamlrSVcxWFM4dzdpUUNlQmdQMApYZWhKZmVITlZJaTJtZlczNlVQbWpMdnVKaGpqNDIrdFBQWndvdDkzdWtlcEgvbWpHcFJEVm9wamJyWGlpTUYrCkYxdnlPNGMxQWdNQkFBR2pnWU13Z1lBd0hRWURWUjBPQkJZRUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1IKTUI4R0ExVWRJd1FZTUJhQUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1JNQjBHQTFVZEpRUVdNQlFHQ0NzRwpBUVVGQndNQ0JnZ3JCZ0VGQlFjREFUQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BNEdBMVVkRHdFQi93UUVBd0lCCkJqQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFYbEh4M2tIMDZwY2NDTDlEVE5qTnBCYnlVSytGd2R6T2IwWFYKcmpNaGtxdHVmdEpUUnR5T3hKZ0ZKNXhUR3pCdEtKamcrVU1pczBOV0t0VDBNWThVMU45U2c5SDl0RFpHRHBjVQpxMlVRU0Y4dXRQMVR3dnJIUzIrdzB2MUoxdHgrTEFiU0lmWmJCV0xXQ21EODUzRlVoWlFZekkvYXpFM28vd0p1CmlPUklMdUpNUk5vNlBXY3VLZmRFVkhaS1RTWnk3a25FcHNidGtsN3EwRE91eUFWdG9HVnlkb3VUR0FOdFhXK2YKczNUSTJjKzErZXg3L2RZOEJGQTFzNWFUOG5vZnU3T1RTTzdiS1kzSkRBUHZOeFQzKzVZUXJwNGR1Nmh0YUFMbAppOHNaRkhidmxpd2EzdlhxL3p1Y2JEaHEzQzBhZnAzV2ZwRGxwSlpvLy9QUUFKaTZLQT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"}, - } - otherJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ - Issuer: "https://some-other-issuer.com", - Audience: "some-audience", - TLS: &auth1alpha1.TLSSpec{CertificateAuthorityData: "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVVENDQWptZ0F3SUJBZ0lWQUpzNStTbVRtaTJXeUI0bGJJRXBXaUs5a1RkUE1BMEdDU3FHU0liM0RRRUIKQ3dVQU1COHhDekFKQmdOVkJBWVRBbFZUTVJBd0RnWURWUVFLREFkUWFYWnZkR0ZzTUI0WERUSXdNRFV3TkRFMgpNamMxT0ZvWERUSTBNRFV3TlRFMk1qYzFPRm93SHpFTE1Ba0dBMVVFQmhNQ1ZWTXhFREFPQmdOVkJBb01CMUJwCmRtOTBZV3d3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRRERZWmZvWGR4Z2NXTEMKZEJtbHB5a0tBaG9JMlBuUWtsVFNXMno1cGcwaXJjOGFRL1E3MXZzMTRZYStmdWtFTGlvOTRZYWw4R01DdVFrbApMZ3AvUEE5N1VYelhQNDBpK25iNXcwRGpwWWd2dU9KQXJXMno2MFRnWE5NSFh3VHk4ME1SZEhpUFVWZ0VZd0JpCmtkNThzdEFVS1Y1MnBQTU1reTJjNy9BcFhJNmRXR2xjalUvaFBsNmtpRzZ5dEw2REtGYjJQRWV3MmdJM3pHZ2IKOFVVbnA1V05DZDd2WjNVY0ZHNXlsZEd3aGc3cnZ4U1ZLWi9WOEhCMGJmbjlxamlrSVcxWFM4dzdpUUNlQmdQMApYZWhKZmVITlZJaTJtZlczNlVQbWpMdnVKaGpqNDIrdFBQWndvdDkzdWtlcEgvbWpHcFJEVm9wamJyWGlpTUYrCkYxdnlPNGMxQWdNQkFBR2pnWU13Z1lBd0hRWURWUjBPQkJZRUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1IKTUI4R0ExVWRJd1FZTUJhQUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1JNQjBHQTFVZEpRUVdNQlFHQ0NzRwpBUVVGQndNQ0JnZ3JCZ0VGQlFjREFUQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BNEdBMVVkRHdFQi93UUVBd0lCCkJqQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFYbEh4M2tIMDZwY2NDTDlEVE5qTnBCYnlVSytGd2R6T2IwWFYKcmpNaGtxdHVmdEpUUnR5T3hKZ0ZKNXhUR3pCdEtKamcrVU1pczBOV0t0VDBNWThVMU45U2c5SDl0RFpHRHBjVQpxMlVRU0Y4dXRQMVR3dnJIUzIrdzB2MUoxdHgrTEFiU0lmWmJCV0xXQ21EODUzRlVoWlFZekkvYXpFM28vd0p1CmlPUklMdUpNUk5vNlBXY3VLZmRFVkhaS1RTWnk3a25FcHNidGtsN3EwRE91eUFWdG9HVnlkb3VUR0FOdFhXK2YKczNUSTJjKzErZXg3L2RZOEJGQTFzNWFUOG5vZnU3T1RTTzdiS1kzSkRBUHZOeFQzKzVZUXJwNGR1Nmh0YUFMbAppOHNaRkhidmxpd2EzdlhxL3p1Y2JEaHEzQzBhZnAzV2ZwRGxwSlpvLy9QUUFKaTZLQT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"}, - } - missingTLSJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ - Issuer: "https://some-issuer.com", - Audience: "some-audience", - } - invalidTLSJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ - Issuer: "https://some-other-issuer.com", - Audience: "some-audience", - TLS: &auth1alpha1.TLSSpec{CertificateAuthorityData: "invalid base64-encoded data"}, - } - - tests := []struct { - name string - cache func(*testing.T, *authncache.Cache, bool) - wantClose bool - syncKey controllerlib.Key - jwtAuthenticators []runtime.Object - wantErr string - wantLogs []string - wantCacheEntries int - }{ - { - name: "not found", - syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, - wantLogs: []string{ - `jwtcachefiller-controller "level"=0 "msg"="Sync() found that the JWTAuthenticator does not exist yet or was deleted"`, - }, - }, - { - name: "valid jwt authenticator with CA", - syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, - jwtAuthenticators: []runtime.Object{ - &auth1alpha1.JWTAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-namespace", - Name: "test-name", - }, - Spec: *someJWTAuthenticatorSpec, - }, - }, - wantLogs: []string{ - `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="https://some-issuer.com" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, - }, - wantCacheEntries: 1, - }, - { - name: "updating jwt authenticator with new fields closes previous instance", - cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { - cache.Store( - authncache.Key{ - Name: "test-name", - Namespace: "test-namespace", - Kind: "JWTAuthenticator", - APIGroup: auth1alpha1.SchemeGroupVersion.Group, - }, - newCacheValue(t, *otherJWTAuthenticatorSpec, wantClose), - ) - }, - wantClose: true, - syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, - jwtAuthenticators: []runtime.Object{ - &auth1alpha1.JWTAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-namespace", - Name: "test-name", - }, - Spec: *someJWTAuthenticatorSpec, - }, - }, - wantLogs: []string{ - `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="https://some-issuer.com" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, - }, - wantCacheEntries: 1, - }, - { - name: "updating jwt authenticator with the same value does nothing", - cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { - cache.Store( - authncache.Key{ - Name: "test-name", - Namespace: "test-namespace", - Kind: "JWTAuthenticator", - APIGroup: auth1alpha1.SchemeGroupVersion.Group, - }, - newCacheValue(t, *someJWTAuthenticatorSpec, wantClose), - ) - }, - wantClose: false, - syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, - jwtAuthenticators: []runtime.Object{ - &auth1alpha1.JWTAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-namespace", - Name: "test-name", - }, - Spec: *someJWTAuthenticatorSpec, - }, - }, - wantLogs: []string{ - `jwtcachefiller-controller "level"=0 "msg"="actual jwt authenticator and desired jwt authenticator are the same" "issuer"="https://some-issuer.com" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, - }, - wantCacheEntries: 1, - }, - { - name: "updating jwt authenticator when cache value is wrong type", - cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { - cache.Store( - authncache.Key{ - Name: "test-name", - Namespace: "test-namespace", - Kind: "JWTAuthenticator", - APIGroup: auth1alpha1.SchemeGroupVersion.Group, - }, - struct{ authenticator.Token }{}, - ) - }, - syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, - jwtAuthenticators: []runtime.Object{ - &auth1alpha1.JWTAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-namespace", - Name: "test-name", - }, - Spec: *someJWTAuthenticatorSpec, - }, - }, - wantLogs: []string{ - `jwtcachefiller-controller "level"=0 "msg"="wrong JWT authenticator type in cache" "actualType"="struct { authenticator.Token }"`, - `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="https://some-issuer.com" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, - }, - wantCacheEntries: 1, - }, - { - name: "valid jwt authenticator without CA", - syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, - jwtAuthenticators: []runtime.Object{ - &auth1alpha1.JWTAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-namespace", - Name: "test-name", - }, - Spec: *missingTLSJWTAuthenticatorSpec, - }, - }, - wantLogs: []string{ - `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="https://some-issuer.com" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, - }, - wantCacheEntries: 1, - }, - { - name: "invalid jwt authenticator CA", - syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, - jwtAuthenticators: []runtime.Object{ - &auth1alpha1.JWTAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-namespace", - Name: "test-name", - }, - Spec: *invalidTLSJWTAuthenticatorSpec, - }, - }, - wantErr: "failed to build jwt authenticator: invalid TLS configuration: illegal base64 data at input byte 7", - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - fakeClient := pinnipedfake.NewSimpleClientset(tt.jwtAuthenticators...) - informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0) - cache := authncache.New() - testLog := testlogger.New(t) - - if tt.cache != nil { - tt.cache(t, cache, tt.wantClose) - } - - controller := New(cache, informers.Authentication().V1alpha1().JWTAuthenticators(), testLog) - - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - - informers.Start(ctx.Done()) - controllerlib.TestRunSynchronously(t, controller) - - syncCtx := controllerlib.Context{Context: ctx, Key: tt.syncKey} - - if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { - require.EqualError(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - require.Equal(t, tt.wantLogs, testLog.Lines()) - require.Equal(t, tt.wantCacheEntries, len(cache.Keys())) - }) - } -} - -func TestNewJWTAuthenticator(t *testing.T) { - t.Parallel() - const ( - goodSubject = "some-subject" - goodAudience = "some-audience" - group0 = "some-group-0" - group1 = "some-group-1" - goodECSigningKeyID = "some-ec-key-id" goodRSASigningKeyID = "some-rsa-key-id" + goodAudience = "some-audience" ) goodECSigningKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) @@ -299,29 +83,399 @@ func TestNewJWTAuthenticator(t *testing.T) { })) goodIssuer := server.URL - a, err := newJWTAuthenticator(&auth1alpha1.JWTAuthenticatorSpec{ + + someJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: goodIssuer, Audience: goodAudience, TLS: tlsSpecFromTLSConfig(server.TLS), - }) - require.NoError(t, err) - t.Cleanup(a.Close) - - // The implementation of AuthenticateToken() that we use waits 10 seconds after creation to - // perform OIDC discovery. Therefore, the JWTAuthenticator is not functional for the first 10 - // seconds. We sleep for 13 seconds in this unit test to give a little bit of cushion to that 10 - // second delay. - // - // We should get rid of this 10 second delay. See - // https://github.com/vmware-tanzu/pinniped/issues/260. - if testing.Short() { - t.Skip("skipping this test when '-short' flag is passed to avoid necessary 13 second sleep") } - time.Sleep(time.Second * 13) + someJWTAuthenticatorSpecWithUsernameClaim := &auth1alpha1.JWTAuthenticatorSpec{ + Issuer: goodIssuer, + Audience: goodAudience, + TLS: tlsSpecFromTLSConfig(server.TLS), + Claims: auth1alpha1.JWTTokenClaims{ + Username: "my-custom-username-claim", + }, + } + someJWTAuthenticatorSpecWithGroupsClaim := &auth1alpha1.JWTAuthenticatorSpec{ + Issuer: goodIssuer, + Audience: goodAudience, + TLS: tlsSpecFromTLSConfig(server.TLS), + Claims: auth1alpha1.JWTTokenClaims{ + Groups: "my-custom-groups-claim", + }, + } + otherJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ + Issuer: "https://some-other-issuer.com", + Audience: goodAudience, + TLS: &auth1alpha1.TLSSpec{CertificateAuthorityData: "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVVENDQWptZ0F3SUJBZ0lWQUpzNStTbVRtaTJXeUI0bGJJRXBXaUs5a1RkUE1BMEdDU3FHU0liM0RRRUIKQ3dVQU1COHhDekFKQmdOVkJBWVRBbFZUTVJBd0RnWURWUVFLREFkUWFYWnZkR0ZzTUI0WERUSXdNRFV3TkRFMgpNamMxT0ZvWERUSTBNRFV3TlRFMk1qYzFPRm93SHpFTE1Ba0dBMVVFQmhNQ1ZWTXhFREFPQmdOVkJBb01CMUJwCmRtOTBZV3d3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRRERZWmZvWGR4Z2NXTEMKZEJtbHB5a0tBaG9JMlBuUWtsVFNXMno1cGcwaXJjOGFRL1E3MXZzMTRZYStmdWtFTGlvOTRZYWw4R01DdVFrbApMZ3AvUEE5N1VYelhQNDBpK25iNXcwRGpwWWd2dU9KQXJXMno2MFRnWE5NSFh3VHk4ME1SZEhpUFVWZ0VZd0JpCmtkNThzdEFVS1Y1MnBQTU1reTJjNy9BcFhJNmRXR2xjalUvaFBsNmtpRzZ5dEw2REtGYjJQRWV3MmdJM3pHZ2IKOFVVbnA1V05DZDd2WjNVY0ZHNXlsZEd3aGc3cnZ4U1ZLWi9WOEhCMGJmbjlxamlrSVcxWFM4dzdpUUNlQmdQMApYZWhKZmVITlZJaTJtZlczNlVQbWpMdnVKaGpqNDIrdFBQWndvdDkzdWtlcEgvbWpHcFJEVm9wamJyWGlpTUYrCkYxdnlPNGMxQWdNQkFBR2pnWU13Z1lBd0hRWURWUjBPQkJZRUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1IKTUI4R0ExVWRJd1FZTUJhQUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1JNQjBHQTFVZEpRUVdNQlFHQ0NzRwpBUVVGQndNQ0JnZ3JCZ0VGQlFjREFUQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BNEdBMVVkRHdFQi93UUVBd0lCCkJqQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFYbEh4M2tIMDZwY2NDTDlEVE5qTnBCYnlVSytGd2R6T2IwWFYKcmpNaGtxdHVmdEpUUnR5T3hKZ0ZKNXhUR3pCdEtKamcrVU1pczBOV0t0VDBNWThVMU45U2c5SDl0RFpHRHBjVQpxMlVRU0Y4dXRQMVR3dnJIUzIrdzB2MUoxdHgrTEFiU0lmWmJCV0xXQ21EODUzRlVoWlFZekkvYXpFM28vd0p1CmlPUklMdUpNUk5vNlBXY3VLZmRFVkhaS1RTWnk3a25FcHNidGtsN3EwRE91eUFWdG9HVnlkb3VUR0FOdFhXK2YKczNUSTJjKzErZXg3L2RZOEJGQTFzNWFUOG5vZnU3T1RTTzdiS1kzSkRBUHZOeFQzKzVZUXJwNGR1Nmh0YUFMbAppOHNaRkhidmxpd2EzdlhxL3p1Y2JEaHEzQzBhZnAzV2ZwRGxwSlpvLy9QUUFKaTZLQT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"}, + } + missingTLSJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ + Issuer: goodIssuer, + Audience: goodAudience, + } + invalidTLSJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ + Issuer: "https://some-other-issuer.com", + Audience: goodAudience, + TLS: &auth1alpha1.TLSSpec{CertificateAuthorityData: "invalid base64-encoded data"}, + } - var tests = []struct { + tests := []struct { + name string + cache func(*testing.T, *authncache.Cache, bool) + syncKey controllerlib.Key + jwtAuthenticators []runtime.Object + wantClose bool + wantErr string + wantLogs []string + wantCacheEntries int + wantUsernameClaim string + wantGroupsClaim string + runTestsOnResultingAuthenticator bool + }{ + { + name: "not found", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="Sync() found that the JWTAuthenticator does not exist yet or was deleted"`, + }, + }, + { + name: "valid jwt authenticator with CA", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="` + goodIssuer + `" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + runTestsOnResultingAuthenticator: true, + }, + { + name: "valid jwt authenticator with custom username claim", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpecWithUsernameClaim, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="` + goodIssuer + `" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + wantUsernameClaim: someJWTAuthenticatorSpecWithUsernameClaim.Claims.Username, + runTestsOnResultingAuthenticator: true, + }, + { + name: "valid jwt authenticator with custom groups claim", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpecWithGroupsClaim, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="` + goodIssuer + `" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + wantGroupsClaim: someJWTAuthenticatorSpecWithGroupsClaim.Claims.Groups, + runTestsOnResultingAuthenticator: true, + }, + { + name: "updating jwt authenticator with new fields closes previous instance", + cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { + cache.Store( + authncache.Key{ + Name: "test-name", + Namespace: "test-namespace", + Kind: "JWTAuthenticator", + APIGroup: auth1alpha1.SchemeGroupVersion.Group, + }, + newCacheValue(t, *otherJWTAuthenticatorSpec, wantClose), + ) + }, + wantClose: true, + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="` + goodIssuer + `" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + runTestsOnResultingAuthenticator: true, + }, + { + name: "updating jwt authenticator with the same value does nothing", + cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { + cache.Store( + authncache.Key{ + Name: "test-name", + Namespace: "test-namespace", + Kind: "JWTAuthenticator", + APIGroup: auth1alpha1.SchemeGroupVersion.Group, + }, + newCacheValue(t, *someJWTAuthenticatorSpec, wantClose), + ) + }, + wantClose: false, + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="actual jwt authenticator and desired jwt authenticator are the same" "issuer"="` + goodIssuer + `" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + runTestsOnResultingAuthenticator: false, // skip the tests because the authenticator left in the cache is the mock version that was added above + }, + { + name: "updating jwt authenticator when cache value is wrong type", + cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { + cache.Store( + authncache.Key{ + Name: "test-name", + Namespace: "test-namespace", + Kind: "JWTAuthenticator", + APIGroup: auth1alpha1.SchemeGroupVersion.Group, + }, + struct{ authenticator.Token }{}, + ) + }, + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="wrong JWT authenticator type in cache" "actualType"="struct { authenticator.Token }"`, + `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="` + goodIssuer + `" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + runTestsOnResultingAuthenticator: true, + }, + { + name: "valid jwt authenticator without CA", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: *missingTLSJWTAuthenticatorSpec, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="` + goodIssuer + `" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + runTestsOnResultingAuthenticator: false, // skip the tests because the authenticator left in the cache doesn't have the CA for our test discovery server + }, + { + name: "invalid jwt authenticator CA", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: *invalidTLSJWTAuthenticatorSpec, + }, + }, + wantErr: "failed to build jwt authenticator: invalid TLS configuration: illegal base64 data at input byte 7", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fakeClient := pinnipedfake.NewSimpleClientset(tt.jwtAuthenticators...) + informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0) + cache := authncache.New() + testLog := testlogger.New(t) + + if tt.cache != nil { + tt.cache(t, cache, tt.wantClose) + } + + controller := New(cache, informers.Authentication().V1alpha1().JWTAuthenticators(), testLog) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + informers.Start(ctx.Done()) + controllerlib.TestRunSynchronously(t, controller) + + syncCtx := controllerlib.Context{Context: ctx, Key: tt.syncKey} + + if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantLogs, testLog.Lines()) + require.Equal(t, tt.wantCacheEntries, len(cache.Keys())) + + if !tt.runTestsOnResultingAuthenticator { + return // end of test unless we wanted to run tests on the resulting authenticator from the cache + } + + // The implementation of AuthenticateToken() that we use waits 10 seconds after creation to + // perform OIDC discovery. Therefore, the JWTAuthenticator is not functional for the first 10 + // seconds. We sleep for 13 seconds in this unit test to give a little bit of cushion to that 10 + // second delay. + // + // We should get rid of this 10 second delay. See + // https://github.com/vmware-tanzu/pinniped/issues/260. + time.Sleep(time.Second * 13) + + // We expected the cache to have an entry, so pull that entry from the cache and test it. + expectedCacheKey := authncache.Key{ + APIGroup: auth1alpha1.GroupName, + Kind: "JWTAuthenticator", + Namespace: syncCtx.Key.Namespace, + Name: syncCtx.Key.Name, + } + cachedAuthenticator := cache.Get(expectedCacheKey) + require.NotNil(t, cachedAuthenticator) + + // Schedule it to be closed at the end of the test. + t.Cleanup(cachedAuthenticator.(*jwtAuthenticator).Close) + + const ( + goodSubject = "some-subject" + group0 = "some-group-0" + group1 = "some-group-1" + goodUsername = "pinny123" + ) + + if tt.wantUsernameClaim == "" { + tt.wantUsernameClaim = "username" + } + + if tt.wantGroupsClaim == "" { + tt.wantGroupsClaim = "groups" + } + + for _, test := range testTableForAuthenticateTokenTests( + t, + goodRSASigningKey, + goodRSASigningAlgo, + goodRSASigningKeyID, + group0, + group1, + goodUsername, + tt.wantUsernameClaim, + tt.wantGroupsClaim, + ) { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + wellKnownClaims := jwt.Claims{ + Issuer: goodIssuer, + Subject: goodSubject, + Audience: []string{goodAudience}, + Expiry: jwt.NewNumericDate(time.Now().Add(time.Hour)), + NotBefore: jwt.NewNumericDate(time.Now().Add(-time.Hour)), + IssuedAt: jwt.NewNumericDate(time.Now().Add(-time.Hour)), + } + var groups interface{} + username := goodUsername + if test.jwtClaims != nil { + test.jwtClaims(&wellKnownClaims, &groups, &username) + } + + var signingKey interface{} = goodECSigningKey + signingAlgo := goodECSigningAlgo + signingKID := goodECSigningKeyID + if test.jwtSignature != nil { + test.jwtSignature(&signingKey, &signingAlgo, &signingKID) + } + + jwt := createJWT( + t, + signingKey, + signingAlgo, + signingKID, + &wellKnownClaims, + tt.wantGroupsClaim, + groups, + tt.wantUsernameClaim, + username, + ) + rsp, authenticated, err := cachedAuthenticator.AuthenticateToken(context.Background(), jwt) + if test.wantErrorRegexp != "" { + require.Error(t, err) + require.Regexp(t, test.wantErrorRegexp, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, test.wantResponse, rsp) + require.Equal(t, test.wantAuthenticated, authenticated) + } + }) + } + }) + } +} + +func testTableForAuthenticateTokenTests( + t *testing.T, + goodRSASigningKey *rsa.PrivateKey, + goodRSASigningAlgo jose.SignatureAlgorithm, + goodRSASigningKeyID string, + group0 string, + group1 string, + goodUsername string, + expectedUsernameClaim string, + expectedGroupsClaim string, +) []struct { + name string + jwtClaims func(wellKnownClaims *jwt.Claims, groups *interface{}, username *string) + jwtSignature func(key *interface{}, algo *jose.SignatureAlgorithm, kid *string) + wantResponse *authenticator.Response + wantAuthenticated bool + wantErrorRegexp string +} { + tests := []struct { name string - jwtClaims func(wellKnownClaims *jwt.Claims, groups *interface{}) + jwtClaims func(wellKnownClaims *jwt.Claims, groups *interface{}, username *string) jwtSignature func(key *interface{}, algo *jose.SignatureAlgorithm, kid *string) wantResponse *authenticator.Response wantAuthenticated bool @@ -331,7 +485,7 @@ func TestNewJWTAuthenticator(t *testing.T) { name: "good token without groups and with EC signature", wantResponse: &authenticator.Response{ User: &user.DefaultInfo{ - Name: goodSubject, + Name: goodUsername, }, }, wantAuthenticated: true, @@ -345,19 +499,19 @@ func TestNewJWTAuthenticator(t *testing.T) { }, wantResponse: &authenticator.Response{ User: &user.DefaultInfo{ - Name: goodSubject, + Name: goodUsername, }, }, wantAuthenticated: true, }, { name: "good token with groups as array", - jwtClaims: func(_ *jwt.Claims, groups *interface{}) { + jwtClaims: func(_ *jwt.Claims, groups *interface{}, username *string) { *groups = []string{group0, group1} }, wantResponse: &authenticator.Response{ User: &user.DefaultInfo{ - Name: goodSubject, + Name: goodUsername, Groups: []string{group0, group1}, }, }, @@ -365,12 +519,12 @@ func TestNewJWTAuthenticator(t *testing.T) { }, { name: "good token with groups as string", - jwtClaims: func(_ *jwt.Claims, groups *interface{}) { + jwtClaims: func(_ *jwt.Claims, groups *interface{}, username *string) { *groups = group0 }, wantResponse: &authenticator.Response{ User: &user.DefaultInfo{ - Name: goodSubject, + Name: goodUsername, Groups: []string{group0}, }, }, @@ -378,26 +532,26 @@ func TestNewJWTAuthenticator(t *testing.T) { }, { name: "good token with nbf unset", - jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + jwtClaims: func(claims *jwt.Claims, _ *interface{}, username *string) { claims.NotBefore = nil }, wantResponse: &authenticator.Response{ User: &user.DefaultInfo{ - Name: goodSubject, + Name: goodUsername, }, }, wantAuthenticated: true, }, { name: "bad token with groups as map", - jwtClaims: func(_ *jwt.Claims, groups *interface{}) { + jwtClaims: func(_ *jwt.Claims, groups *interface{}, username *string) { *groups = map[string]string{"not an array": "or a string"} }, - wantErrorRegexp: "oidc: parse groups claim \"groups\": json: cannot unmarshal object into Go value of type string", + wantErrorRegexp: "oidc: parse groups claim \"" + expectedGroupsClaim + "\": json: cannot unmarshal object into Go value of type string", }, { name: "bad token with wrong issuer", - jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + jwtClaims: func(claims *jwt.Claims, _ *interface{}, username *string) { claims.Issuer = "wrong-issuer" }, wantResponse: nil, @@ -405,38 +559,45 @@ func TestNewJWTAuthenticator(t *testing.T) { }, { name: "bad token with no audience", - jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + jwtClaims: func(claims *jwt.Claims, _ *interface{}, username *string) { claims.Audience = nil }, wantErrorRegexp: `oidc: verify token: oidc: expected audience "some-audience" got \[\]`, }, { name: "bad token with wrong audience", - jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + jwtClaims: func(claims *jwt.Claims, _ *interface{}, username *string) { claims.Audience = []string{"wrong-audience"} }, wantErrorRegexp: `oidc: verify token: oidc: expected audience "some-audience" got \["wrong-audience"\]`, }, { name: "bad token with nbf in the future", - jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + jwtClaims: func(claims *jwt.Claims, _ *interface{}, username *string) { claims.NotBefore = jwt.NewNumericDate(time.Date(3020, 2, 3, 4, 5, 6, 7, time.UTC)) }, wantErrorRegexp: `oidc: verify token: oidc: current time .* before the nbf \(not before\) time: 3020-.*`, }, { name: "bad token with exp in past", - jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + jwtClaims: func(claims *jwt.Claims, _ *interface{}, username *string) { claims.Expiry = jwt.NewNumericDate(time.Date(1, 2, 3, 4, 5, 6, 7, time.UTC)) }, - wantErrorRegexp: `oidc: verify token: oidc: token is expired \(Token Expiry: 0001-02-02 23:09:04 -0456 LMT\)`, + wantErrorRegexp: `oidc: verify token: oidc: token is expired \(Token Expiry: .+`, }, { name: "bad token without exp", - jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + jwtClaims: func(claims *jwt.Claims, _ *interface{}, username *string) { claims.Expiry = nil }, - wantErrorRegexp: `oidc: verify token: oidc: token is expired \(Token Expiry: 0001-01-01 00:00:00 \+0000 UTC\)`, + wantErrorRegexp: `oidc: verify token: oidc: token is expired \(Token Expiry: .+`, + }, + { + name: "token does not have username claim", + jwtClaims: func(claims *jwt.Claims, _ *interface{}, username *string) { + *username = "" + }, + wantErrorRegexp: `oidc: parse username claims "` + expectedUsernameClaim + `": claim not present`, }, { name: "signing key is wrong", @@ -459,43 +620,8 @@ func TestNewJWTAuthenticator(t *testing.T) { wantErrorRegexp: `oidc: verify token: oidc: id token signed with unsupported algorithm, expected \["RS256" "ES256"\] got "ES384"`, }, } - for _, test := range tests { - test := test - t.Run(test.name, func(t *testing.T) { - t.Parallel() - wellKnownClaims := jwt.Claims{ - Issuer: goodIssuer, - Subject: goodSubject, - Audience: []string{goodAudience}, - Expiry: jwt.NewNumericDate(time.Now().Add(time.Hour)), - NotBefore: jwt.NewNumericDate(time.Now().Add(-time.Hour)), - IssuedAt: jwt.NewNumericDate(time.Now().Add(-time.Hour)), - } - var groups interface{} - if test.jwtClaims != nil { - test.jwtClaims(&wellKnownClaims, &groups) - } - - var signingKey interface{} = goodECSigningKey - signingAlgo := goodECSigningAlgo - signingKID := goodECSigningKeyID - if test.jwtSignature != nil { - test.jwtSignature(&signingKey, &signingAlgo, &signingKID) - } - - jwt := createJWT(t, signingKey, signingAlgo, signingKID, &wellKnownClaims, groups) - rsp, authenticated, err := a.AuthenticateToken(context.Background(), jwt) - if test.wantErrorRegexp != "" { - require.Error(t, err) - require.Regexp(t, test.wantErrorRegexp, err.Error()) - } else { - require.NoError(t, err) - require.Equal(t, test.wantResponse, rsp) - require.Equal(t, test.wantAuthenticated, authenticated) - } - }) - } + return tests } func tlsSpecFromTLSConfig(tls *tls.Config) *auth1alpha1.TLSSpec { @@ -519,7 +645,10 @@ func createJWT( signingAlgo jose.SignatureAlgorithm, kid string, claims *jwt.Claims, - groups interface{}, + groupsClaim string, + groupsValue interface{}, + usernameClaim string, + usernameValue string, ) string { t.Helper() @@ -530,8 +659,11 @@ func createJWT( require.NoError(t, err) builder := jwt.Signed(sig).Claims(claims) - if groups != nil { - builder = builder.Claims(map[string]interface{}{"groups": groups}) + if groupsValue != nil { + builder = builder.Claims(map[string]interface{}{groupsClaim: groupsValue}) + } + if usernameValue != "" { + builder = builder.Claims(map[string]interface{}{usernameClaim: usernameValue}) } jwt, err := builder.CompactSerialize() require.NoError(t, err) diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index a8ad9b66..66e7f3b1 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -24,22 +24,6 @@ import ( "go.pinniped.dev/internal/plog" ) -const ( - // The name of the issuer claim specified in the OIDC spec. - idTokenIssuerClaim = "iss" - - // The name of the subject claim specified in the OIDC spec. - idTokenSubjectClaim = "sub" - - // defaultUpstreamUsernameClaim is what we will use to extract the username from an upstream OIDC - // ID token if the upstream OIDC IDP did not tell us to use another claim. - defaultUpstreamUsernameClaim = idTokenSubjectClaim - - // downstreamGroupsClaim is what we will use to encode the groups in the downstream OIDC ID token - // information. - downstreamGroupsClaim = "groups" -) - func NewHandler( idpListGetter oidc.IDPListGetter, oauthHelper fosite.OAuth2Provider, @@ -89,7 +73,7 @@ func NewHandler( return httperr.New(http.StatusBadGateway, "error exchanging and validating upstream tokens") } - username, err := getUsernameFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) + subject, username, err := getSubjectAndUsernameFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) if err != nil { return err } @@ -99,7 +83,7 @@ func NewHandler( return err } - openIDSession := makeDownstreamSession(username, groups) + openIDSession := makeDownstreamSession(subject, username, groups) authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) if err != nil { plog.WarningErr("error while generating and saving authcode", err, "upstreamName", upstreamIDPConfig.GetName()) @@ -193,36 +177,54 @@ func readState(r *http.Request, stateDecoder oidc.Decoder) (*oidc.UpstreamStateP return &state, nil } -func getUsernameFromUpstreamIDToken( +func getSubjectAndUsernameFromUpstreamIDToken( upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, -) (string, error) { - usernameClaim := upstreamIDPConfig.GetUsernameClaim() +) (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") + } + 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") + } - user := "" + 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 := fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, oidc.IDTokenSubjectClaim, upstreamSubject) + + usernameClaim := upstreamIDPConfig.GetUsernameClaim() if usernameClaim == "" { - // The spec says the "sub" claim is only unique per issuer, so by default when there is - // no specific username claim configured we will prepend the issuer string to make it globally unique. - upstreamIssuer := idTokenClaims[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") - } - 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") - } - user = fmt.Sprintf("%s?%s=", upstreamIssuerAsString, idTokenSubjectClaim) - usernameClaim = defaultUpstreamUsernameClaim + return subject, subject, nil } usernameAsInterface, ok := idTokenClaims[usernameClaim] @@ -233,7 +235,7 @@ func getUsernameFromUpstreamIDToken( "configuredUsernameClaim", upstreamIDPConfig.GetUsernameClaim(), "usernameClaim", usernameClaim, ) - return "", httperr.New(http.StatusUnprocessableEntity, "no username claim in upstream ID token") + return "", "", httperr.New(http.StatusUnprocessableEntity, "no username claim in upstream ID token") } username, ok := usernameAsInterface.(string) @@ -244,16 +246,16 @@ func getUsernameFromUpstreamIDToken( "configuredUsernameClaim", upstreamIDPConfig.GetUsernameClaim(), "usernameClaim", usernameClaim, ) - return "", httperr.New(http.StatusUnprocessableEntity, "username claim in upstream ID token has invalid format") + return "", "", httperr.New(http.StatusUnprocessableEntity, "username claim in upstream ID token has invalid format") } - return fmt.Sprintf("%s%s", user, username), nil + return subject, username, nil } func getGroupsFromUpstreamIDToken( upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, -) ([]string, error) { +) (interface{}, error) { groupsClaim := upstreamIDPConfig.GetGroupsClaim() if groupsClaim == "" { return nil, nil @@ -270,8 +272,9 @@ func getGroupsFromUpstreamIDToken( return nil, httperr.New(http.StatusUnprocessableEntity, "no groups claim in upstream ID token") } - groups, ok := groupsAsInterface.([]string) - if !ok { + groupsAsArray, okAsArray := groupsAsInterface.([]string) + groupsAsString, okAsString := groupsAsInterface.(string) + if !okAsArray && !okAsString { plog.Warning( "groups claim in upstream ID token has invalid format", "upstreamName", upstreamIDPConfig.GetName(), @@ -281,22 +284,26 @@ func getGroupsFromUpstreamIDToken( return nil, httperr.New(http.StatusUnprocessableEntity, "groups claim in upstream ID token has invalid format") } - return groups, nil + if okAsArray { + return groupsAsArray, nil + } + return groupsAsString, nil } -func makeDownstreamSession(username string, groups []string) *openid.DefaultSession { +func makeDownstreamSession(subject string, username string, groups interface{}) *openid.DefaultSession { now := time.Now().UTC() openIDSession := &openid.DefaultSession{ Claims: &jwt.IDTokenClaims{ - Subject: username, + Subject: subject, RequestedAt: now, AuthTime: now, }, } + openIDSession.Claims.Extra = map[string]interface{}{ + oidc.DownstreamUsernameClaim: username, + } if groups != nil { - openIDSession.Claims.Extra = map[string]interface{}{ - downstreamGroupsClaim: groups, - } + openIDSession.Claims.Extra[oidc.DownstreamGroupsClaim] = groups } return openIDSession } diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 8641212d..cce60f17 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -133,7 +133,8 @@ func TestCallbackEndpoint(t *testing.T) { wantRedirectLocationRegexp string wantDownstreamGrantedScopes []string wantDownstreamIDTokenSubject string - wantDownstreamIDTokenGroups []string + wantDownstreamIDTokenUsername string + wantDownstreamIDTokenGroups interface{} wantDownstreamRequestedScopes []string wantDownstreamNonce string wantDownstreamPKCEChallenge string @@ -150,7 +151,8 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamUsername, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: upstreamUsername, wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, wantDownstreamGrantedScopes: happyDownstreamScopesGranted, @@ -169,6 +171,7 @@ func TestCallbackEndpoint(t *testing.T) { wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: upstreamIssuer + "?sub=" + upstreamSubject, wantDownstreamIDTokenGroups: nil, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, wantDownstreamGrantedScopes: happyDownstreamScopesGranted, @@ -186,7 +189,8 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: upstreamSubject, wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, wantDownstreamGrantedScopes: happyDownstreamScopesGranted, @@ -195,6 +199,25 @@ func TestCallbackEndpoint(t *testing.T) { wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, + { + name: "upstream IDP's configured groups claim in the ID token has a non-array value", + idp: happyUpstream().WithIDTokenClaim(upstreamGroupsClaim, "notAnArrayGroup1 notAnArrayGroup2").Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusFound, + wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, + wantBody: "", + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: upstreamUsername, + wantDownstreamIDTokenGroups: "notAnArrayGroup1 notAnArrayGroup2", + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, + }, // Pre-upstream-exchange verification { @@ -312,7 +335,8 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie: happyCSRFCookie, wantStatus: http.StatusFound, wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyDownstreamState, - wantDownstreamIDTokenSubject: upstreamUsername, + wantDownstreamIDTokenUsername: upstreamUsername, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, wantDownstreamRequestedScopes: []string{"profile", "email"}, wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamNonce: downstreamNonce, @@ -333,7 +357,8 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie: happyCSRFCookie, wantStatus: http.StatusFound, wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=openid%20offline_access&state=` + happyDownstreamState, - wantDownstreamIDTokenSubject: upstreamUsername, + wantDownstreamIDTokenUsername: upstreamUsername, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, wantDownstreamRequestedScopes: []string{"openid", "offline_access"}, wantDownstreamGrantedScopes: []string{"openid", "offline_access"}, wantDownstreamIDTokenGroups: upstreamGroupMembership, @@ -524,6 +549,7 @@ func TestCallbackEndpoint(t *testing.T) { authcodeDataAndSignature[1], // Authcode store key is authcode signature test.wantDownstreamGrantedScopes, test.wantDownstreamIDTokenSubject, + test.wantDownstreamIDTokenUsername, test.wantDownstreamIDTokenGroups, test.wantDownstreamRequestedScopes, ) @@ -677,8 +703,8 @@ func happyUpstream() *upstreamOIDCIdentityProviderBuilder { } } -func (u *upstreamOIDCIdentityProviderBuilder) WithUsernameClaim(claim string) *upstreamOIDCIdentityProviderBuilder { - u.usernameClaim = claim +func (u *upstreamOIDCIdentityProviderBuilder) WithUsernameClaim(value string) *upstreamOIDCIdentityProviderBuilder { + u.usernameClaim = value return u } @@ -744,7 +770,8 @@ func validateAuthcodeStorage( storeKey string, wantDownstreamGrantedScopes []string, wantDownstreamIDTokenSubject string, - wantDownstreamIDTokenGroups []string, + wantDownstreamIDTokenUsername string, + wantDownstreamIDTokenGroups interface{}, wantDownstreamRequestedScopes []string, ) (*fosite.Request, *openid.DefaultSession) { t.Helper() @@ -780,13 +807,24 @@ func validateAuthcodeStorage( // Now confirm the ID token claims. actualClaims := storedSessionFromAuthcode.Claims - // Check the user's identity, which are put into the downstream ID token's subject and groups claims. + // Check the user's identity, which are put into the downstream ID token's subject, username and groups claims. require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject) - if wantDownstreamIDTokenGroups != nil { - require.Len(t, actualClaims.Extra, 1) - require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualClaims.Extra["groups"]) + require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"]) + if wantDownstreamIDTokenGroups != nil { //nolint:nestif // there are some nested if's here but its probably fine for a test + require.Len(t, actualClaims.Extra, 2) + wantArray, ok := wantDownstreamIDTokenGroups.([]string) + if ok { + require.ElementsMatch(t, wantArray, actualClaims.Extra["groups"]) + } else { + wantString, ok := wantDownstreamIDTokenGroups.(string) + if ok { + require.Equal(t, wantString, actualClaims.Extra["groups"]) + } else { + require.Fail(t, "wantDownstreamIDTokenGroups should be of type: either []string or string") + } + } } else { - require.Empty(t, actualClaims.Extra) + require.Len(t, actualClaims.Extra, 1) require.NotContains(t, actualClaims.Extra, "groups") } diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 94a697b0..d07a47ac 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -45,6 +45,21 @@ const ( // cookie contents. CSRFCookieEncodingName = "csrf" + // The name of the issuer claim specified in the OIDC spec. + IDTokenIssuerClaim = "iss" + + // The name of the subject claim specified in the OIDC spec. + IDTokenSubjectClaim = "sub" + + // DownstreamUsernameClaim is a custom claim in the downstream ID token + // whose value is mapped from a claim in the upstream token. + // By default the value is the same as the downstream subject claim's. + DownstreamUsernameClaim = "username" + + // DownstreamGroupsClaim is what we will use to encode the groups in the downstream OIDC ID token + // information. + DownstreamGroupsClaim = "groups" + // CSRFCookieLifespan is the length of time that the CSRF cookie is valid. After this time, the // Supervisor's authorization endpoint should give the browser a new CSRF cookie. We set it to // a week so that it is unlikely to expire during a login. diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 002bcc9a..8d0cdbc9 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -53,8 +53,9 @@ const ( goodRedirectURI = "http://127.0.0.1/callback" goodPKCECodeVerifier = "some-pkce-verifier-that-must-be-at-least-43-characters-to-meet-entropy-requirements" goodNonce = "some-nonce-value-with-enough-bytes-to-exceed-min-allowed" - goodSubject = "some-subject" + goodSubject = "https://issuer?sub=some-subject" goodUsername = "some-username" + goodGroups = "group1,groups2" hmacSecret = "this needs to be at least 32 characters to meet entropy requirements" @@ -785,11 +786,13 @@ func TestTokenExchange(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - subject, rsp, _, _, secrets, storage := exchangeAuthcodeForTokens(t, test.authcodeExchange) - var parsedResponseBody map[string]interface{} - require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedResponseBody)) + t.Parallel() - request := happyTokenExchangeRequest(test.requestedAudience, parsedResponseBody["access_token"].(string)) + subject, rsp, _, _, secrets, storage := exchangeAuthcodeForTokens(t, test.authcodeExchange) + var parsedAuthcodeExchangeResponseBody map[string]interface{} + require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &parsedAuthcodeExchangeResponseBody)) + + request := happyTokenExchangeRequest(test.requestedAudience, parsedAuthcodeExchangeResponseBody["access_token"].(string)) if test.modifyStorage != nil { test.modifyStorage(t, storage, request) } @@ -805,6 +808,10 @@ func TestTokenExchange(t *testing.T) { existingSecrets, err := secrets.List(context.Background(), metav1.ListOptions{}) require.NoError(t, err) + // Wait one second before performing the token exchange so we can see that the new ID token has new issued + // at and expires at dates which are newer than the old tokens. + time.Sleep(1 * time.Second) + subject.ServeHTTP(rsp, req) t.Logf("response: %#v", rsp) t.Logf("response body: %q", rsp.Body.String()) @@ -820,6 +827,12 @@ func TestTokenExchange(t *testing.T) { return } + claimsOfFirstIDToken := map[string]interface{}{} + originalIDToken := parsedAuthcodeExchangeResponseBody["id_token"].(string) + firstIDTokenDecoded, _ := josejwt.ParseSigned(originalIDToken) + err = firstIDTokenDecoded.UnsafeClaimsWithoutVerification(&claimsOfFirstIDToken) + require.NoError(t, err) + var responseBody map[string]interface{} require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &responseBody)) @@ -827,18 +840,43 @@ func TestTokenExchange(t *testing.T) { require.Equal(t, "N_A", responseBody["token_type"]) require.Equal(t, "urn:ietf:params:oauth:token-type:jwt", responseBody["issued_token_type"]) - // Assert that the returned token has expected claims. + // Parse the returned token. parsedJWT, err := jose.ParseSigned(responseBody["access_token"].(string)) require.NoError(t, err) var tokenClaims map[string]interface{} require.NoError(t, json.Unmarshal(parsedJWT.UnsafePayloadWithoutVerification(), &tokenClaims)) - require.Contains(t, tokenClaims, "iat") - require.Contains(t, tokenClaims, "rat") - require.Contains(t, tokenClaims, "jti") + + // Make sure that these are the only fields in the token. + idTokenFields := []string{"sub", "aud", "iss", "jti", "nonce", "auth_time", "exp", "iat", "rat", "groups", "username"} + require.ElementsMatch(t, idTokenFields, getMapKeys(tokenClaims)) + + // Assert that the returned token has expected claims values. + require.NotEmpty(t, tokenClaims["jti"]) + require.NotEmpty(t, tokenClaims["auth_time"]) + require.NotEmpty(t, tokenClaims["exp"]) + require.NotEmpty(t, tokenClaims["iat"]) + require.NotEmpty(t, tokenClaims["rat"]) + require.Empty(t, tokenClaims["nonce"]) // ID tokens only contain nonce during an authcode exchange require.Len(t, tokenClaims["aud"], 1) require.Contains(t, tokenClaims["aud"], test.requestedAudience) require.Equal(t, goodSubject, tokenClaims["sub"]) require.Equal(t, goodIssuer, tokenClaims["iss"]) + require.Equal(t, goodUsername, tokenClaims["username"]) + require.Equal(t, goodGroups, tokenClaims["groups"]) + + // Also assert that some are the same as the original downstream ID token. + requireClaimsAreEqual(t, "iss", claimsOfFirstIDToken, tokenClaims) // issuer + requireClaimsAreEqual(t, "sub", claimsOfFirstIDToken, tokenClaims) // subject + requireClaimsAreEqual(t, "rat", claimsOfFirstIDToken, tokenClaims) // requested at + requireClaimsAreEqual(t, "auth_time", claimsOfFirstIDToken, tokenClaims) // auth time + + // Also assert which are the different from the original downstream ID token. + requireClaimsAreNotEqual(t, "jti", claimsOfFirstIDToken, tokenClaims) // JWT ID + requireClaimsAreNotEqual(t, "aud", claimsOfFirstIDToken, tokenClaims) // audience + requireClaimsAreNotEqual(t, "exp", claimsOfFirstIDToken, tokenClaims) // expires at + require.Greater(t, tokenClaims["exp"], claimsOfFirstIDToken["exp"]) + requireClaimsAreNotEqual(t, "iat", claimsOfFirstIDToken, tokenClaims) // issued at + require.Greater(t, tokenClaims["iat"], claimsOfFirstIDToken["iat"]) // Assert that nothing in storage has been modified. newSecrets, err := secrets.List(context.Background(), metav1.ListOptions{}) @@ -1390,11 +1428,15 @@ func simulateAuthEndpointHavingAlreadyRun(t *testing.T, authRequest *http.Reques session := &openid.DefaultSession{ Claims: &jwt.IDTokenClaims{ Subject: goodSubject, - AuthTime: goodAuthTime, RequestedAt: goodRequestedAtTime, + AuthTime: goodAuthTime, + Extra: map[string]interface{}{ + oidc.DownstreamUsernameClaim: goodUsername, + oidc.DownstreamGroupsClaim: goodGroups, + }, }, - Subject: goodSubject, - Username: goodUsername, + Subject: "", // not used, note that callback_handler.go does not set this + Username: "", // not used, note that callback_handler.go does not set this } authRequester, err := oauthHelper.NewAuthorizeRequest(ctx, authRequest) require.NoError(t, err) @@ -1613,6 +1655,12 @@ func requireValidStoredRequest( require.Empty(t, claims.JTI) // When claims.JTI is empty, Fosite will generate a UUID for this field. require.Equal(t, goodSubject, claims.Subject) + // Our custom claims from the authorize endpoint should still be set. + require.Equal(t, map[string]interface{}{ + "username": goodUsername, + "groups": goodGroups, + }, claims.Extra) + // We are in charge of setting these fields. For the purpose of testing, we ensure that the // sentinel test value is set correctly. require.Equal(t, goodRequestedAtTime, claims.RequestedAt) @@ -1637,7 +1685,6 @@ func requireValidStoredRequest( require.Empty(t, claims.AuthenticationContextClassReference) require.Empty(t, claims.AuthenticationMethodsReference) require.Empty(t, claims.CodeHash) - require.Empty(t, claims.Extra) } // Assert that the session headers are what we think they should be. @@ -1668,9 +1715,9 @@ func requireValidStoredRequest( require.False(t, ok, "expected session to not hold expiration time for access token, but it did") } - // Assert that the session's username and subject are correct. - require.Equal(t, goodUsername, session.Username) - require.Equal(t, goodSubject, session.Subject) + // We don't use these, so they should be empty. + require.Empty(t, session.Username) + require.Empty(t, session.Subject) } func requireValidIDToken( @@ -1702,12 +1749,14 @@ func requireValidIDToken( IssuedAt int64 `json:"iat"` RequestedAt int64 `json:"rat"` AuthTime int64 `json:"auth_time"` + Groups string `json:"groups"` + Username string `json:"username"` } // Note that there is a bug in fosite which prevents the `at_hash` claim from appearing in this ID token // during the initial authcode exchange, but does not prevent `at_hash` from appearing in the refreshed ID token. // We can add a workaround for this later. - idTokenFields := []string{"sub", "aud", "iss", "jti", "nonce", "auth_time", "exp", "iat", "rat"} + idTokenFields := []string{"sub", "aud", "iss", "jti", "nonce", "auth_time", "exp", "iat", "rat", "groups", "username"} if wantAtHashClaimInIDToken { idTokenFields = append(idTokenFields, "at_hash") } @@ -1721,6 +1770,8 @@ func requireValidIDToken( err := token.Claims(&claims) require.NoError(t, err) require.Equal(t, goodSubject, claims.Subject) + require.Equal(t, goodUsername, claims.Username) + require.Equal(t, goodGroups, claims.Groups) require.Len(t, claims.Audience, 1) require.Equal(t, goodClient, claims.Audience[0]) require.Equal(t, goodIssuer, claims.Issuer) diff --git a/test/integration/concierge_credentialrequest_test.go b/test/integration/concierge_credentialrequest_test.go index 5346e6c4..18b3c174 100644 --- a/test/integration/concierge_credentialrequest_test.go +++ b/test/integration/concierge_credentialrequest_test.go @@ -65,11 +65,13 @@ func TestSuccessfulCredentialRequest(t *testing.T) { credOutput, _ := runPinnipedLoginOIDC(ctx, t, pinnipedExe) token := credOutput.Status.Token - // By default, the JWTAuthenticator expects the username to be in the "sub" claim and the + // By default, the JWTAuthenticator expects the username to be in the "username" claim and the // groups to be in the "groups" claim. + // However, we are configuring Pinniped in the `CreateTestJWTAuthenticatorForCLIUpstream` method above + // to read the username from the "sub" claim of the token instead. username, groups := getJWTSubAndGroupsClaims(t, token) - return credOutput.Status.Token, username, groups + return token, username, groups }, }, } diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 3d1914f1..00b2317d 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -183,7 +183,7 @@ func TestSupervisorLogin(t *testing.T) { tokenResponse, err := downstreamOAuth2Config.Exchange(oidcHTTPClientContext, authcode, pkceParam.Verifier()) require.NoError(t, err) - expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat"} + expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat", "username"} verifyTokenResponse(t, tokenResponse, discovery, downstreamOAuth2Config, env.SupervisorTestUpstream.Issuer, nonceParam, expectedIDTokenClaims) // token exchange on the original token @@ -240,6 +240,10 @@ func verifyTokenResponse( idTokenClaimNames = append(idTokenClaimNames, k) } require.ElementsMatch(t, expectedIDTokenClaims, idTokenClaimNames) + expectedUsernamePrefix := upstreamIssuerName + "?sub=" + require.True(t, strings.HasPrefix(idTokenClaims["username"].(string), expectedUsernamePrefix)) + require.Greater(t, len(idTokenClaims["username"].(string)), len(expectedUsernamePrefix), + "the ID token Username should include the upstream user ID after the upstream issuer name") // Some light verification of the other tokens that were returned. require.NotEmpty(t, tokenResponse.AccessToken) diff --git a/test/library/client.go b/test/library/client.go index a6e5fa57..9f6baa57 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -180,6 +180,9 @@ func CreateTestJWTAuthenticatorForCLIUpstream(ctx context.Context, t *testing.T) spec := auth1alpha1.JWTAuthenticatorSpec{ Issuer: testEnv.CLITestUpstream.Issuer, Audience: testEnv.CLITestUpstream.ClientID, + // The default UsernameClaim is "username" but the upstreams that we use for + // integration tests won't necessarily have that claim, so use "sub" here. + Claims: auth1alpha1.JWTTokenClaims{Username: "sub"}, } // If the test upstream does not have a CA bundle specified, then don't configure one in the // JWTAuthenticator. Leaving TLSSpec set to nil will result in OIDC discovery using the OS's root