diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go index 2f54a7e5b2c..db445f433e2 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go @@ -45,32 +45,33 @@ func ValidateAuthenticationConfiguration(c *api.AuthenticationConfiguration, dis root := field.NewPath("jwt") var allErrs field.ErrorList - // This stricter validation is solely based on what the current implementation supports. - // TODO(aramase): when StructuredAuthenticationConfiguration feature gate is added and wired up, - // relax this check to allow 0 authenticators. This will allow us to support the case where - // API server is initially configured with no authenticators and then authenticators are added - // later via dynamic config. - if len(c.JWT) == 0 { - allErrs = append(allErrs, field.Required(root, fmt.Sprintf(atLeastOneRequiredErrFmt, root))) + // We allow 0 authenticators in the authentication configuration. + // This allows us to support scenarios where the API server is initially set up without + // any authenticators and then authenticators are added later via dynamic config. + + if len(c.JWT) > 64 { + allErrs = append(allErrs, field.TooMany(root, len(c.JWT), 64)) return allErrs } - // This stricter validation is because the --oidc-* flag option is singular. - // TODO(aramase): when StructuredAuthenticationConfiguration feature gate is added and wired up, - // remove the 1 authenticator limit check and add set the limit to 64. - if len(c.JWT) > 1 { - allErrs = append(allErrs, field.TooMany(root, len(c.JWT), 1)) - return allErrs - } - - // TODO(aramase): right now we only support a single JWT authenticator as - // this is wired to the --oidc-* flags. When StructuredAuthenticationConfiguration - // feature gate is added and wired up, we will remove the 1 authenticator limit - // check and add validation for duplicate issuers. + seenIssuers := sets.New[string]() + seenDiscoveryURLs := sets.New[string]() for i, a := range c.JWT { fldPath := root.Index(i) _, errs := validateJWTAuthenticator(a, fldPath, sets.New(disallowedIssuers...), utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthenticationConfiguration)) allErrs = append(allErrs, errs...) + + if seenIssuers.Has(a.Issuer.URL) { + allErrs = append(allErrs, field.Duplicate(fldPath.Child("issuer").Child("url"), a.Issuer.URL)) + } + seenIssuers.Insert(a.Issuer.URL) + + if len(a.Issuer.DiscoveryURL) > 0 { + if seenDiscoveryURLs.Has(a.Issuer.DiscoveryURL) { + allErrs = append(allErrs, field.Duplicate(fldPath.Child("issuer").Child("discoveryURL"), a.Issuer.DiscoveryURL)) + } + seenDiscoveryURLs.Insert(a.Issuer.DiscoveryURL) + } } return allErrs diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go index 92d205c5a1d..702a200a426 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go @@ -58,17 +58,97 @@ func TestValidateAuthenticationConfiguration(t *testing.T) { { name: "jwt authenticator is empty", in: &api.AuthenticationConfiguration{}, - want: "jwt: Required value: at least one jwt is required", + want: "", }, { - name: ">1 jwt authenticator", + name: "duplicate issuer across jwt authenticators", in: &api.AuthenticationConfiguration{ JWT: []api.JWTAuthenticator{ - {Issuer: api.Issuer{URL: "https://issuer-url", Audiences: []string{"audience"}}}, - {Issuer: api.Issuer{URL: "https://issuer-url", Audiences: []string{"audience"}}}, + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Claim: "sub", + Prefix: pointer.String("prefix"), + }, + }, + }, + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Claim: "sub", + Prefix: pointer.String("prefix"), + }, + }, + }, }, }, - want: "jwt: Too many: 2: must have at most 1 items", + want: `jwt[1].issuer.url: Duplicate value: "https://issuer-url"`, + }, + { + name: "duplicate discoveryURL across jwt authenticators", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + DiscoveryURL: "https://discovery-url/.well-known/openid-configuration", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Claim: "sub", + Prefix: pointer.String("prefix"), + }, + }, + }, + { + Issuer: api.Issuer{ + URL: "https://different-issuer-url", + DiscoveryURL: "https://discovery-url/.well-known/openid-configuration", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Claim: "sub", + Prefix: pointer.String("prefix"), + }, + }, + }, + }, + }, + want: `jwt[1].issuer.discoveryURL: Duplicate value: "https://discovery-url/.well-known/openid-configuration"`, }, { name: "failed issuer validation", diff --git a/test/integration/apiserver/oidc/oidc_test.go b/test/integration/apiserver/oidc/oidc_test.go index 8d8b87f9b69..0572e433d41 100644 --- a/test/integration/apiserver/oidc/oidc_test.go +++ b/test/integration/apiserver/oidc/oidc_test.go @@ -1057,6 +1057,113 @@ jwt: } } +func TestMultipleJWTAuthenticators(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthenticationConfiguration, true)() + + caCertContent1, _, caFilePath1, caKeyFilePath1 := generateCert(t) + signingPrivateKey1, publicKey1 := rsaGenerateKey(t) + oidcServer1 := utilsoidc.BuildAndRunTestServer(t, caFilePath1, caKeyFilePath1, "") + + caCertContent2, _, caFilePath2, caKeyFilePath2 := generateCert(t) + signingPrivateKey2, publicKey2 := rsaGenerateKey(t) + oidcServer2 := utilsoidc.BuildAndRunTestServer(t, caFilePath2, caKeyFilePath2, "https://example.com") + + authenticationConfig := fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - foo + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claims.sub" + claimValidationRules: + - expression: 'claims.hd == "example.com"' + message: "the hd claim must be set to example.com" +- issuer: + url: "https://example.com" + discoveryURL: %s/.well-known/openid-configuration + audiences: + - bar + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claims.sub" + groups: + expression: '(claims.roles.split(",") + claims.other_roles.split(",")).map(role, "system:" + role)' + uid: + expression: "claims.uid" +`, oidcServer1.URL(), indentCertificateAuthority(string(caCertContent1)), oidcServer2.URL(), indentCertificateAuthority(string(caCertContent2))) + + oidcServer1.JwksHandler().EXPECT().KeySet().AnyTimes().DoAndReturn(utilsoidc.DefaultJwksHandlerBehavior(t, publicKey1)) + oidcServer2.JwksHandler().EXPECT().KeySet().AnyTimes().DoAndReturn(utilsoidc.DefaultJwksHandlerBehavior(t, publicKey2)) + + apiServer := startTestAPIServerForOIDC(t, apiServerOIDCConfig{authenticationConfigYAML: authenticationConfig}, publicKey1) + + idTokenLifetime := time.Second * 1200 + oidcServer1.TokenHandler().EXPECT().Token().Times(1).DoAndReturn(utilsoidc.TokenHandlerBehaviorReturningPredefinedJWT( + t, + signingPrivateKey1, + map[string]interface{}{ + "iss": oidcServer1.URL(), + "sub": defaultOIDCClaimedUsername, + "aud": "foo", + "exp": time.Now().Add(idTokenLifetime).Unix(), + "hd": "example.com", + }, + defaultStubAccessToken, + defaultStubRefreshToken, + )) + + oidcServer2.TokenHandler().EXPECT().Token().Times(1).DoAndReturn(utilsoidc.TokenHandlerBehaviorReturningPredefinedJWT( + t, + signingPrivateKey2, + map[string]interface{}{ + "iss": "https://example.com", + "sub": "not_john_doe", + "aud": "bar", + "roles": "role1,role2", + "other_roles": "role3,role4", + "exp": time.Now().Add(idTokenLifetime).Unix(), + "uid": "1234", + }, + defaultStubAccessToken, + defaultStubRefreshToken, + )) + + tokenURL1, err := oidcServer1.TokenURL() + require.NoError(t, err) + + tokenURL2, err := oidcServer2.TokenURL() + require.NoError(t, err) + + client1 := configureClientFetchingOIDCCredentials(t, apiServer.ClientConfig, caCertContent1, caFilePath1, oidcServer1.URL(), tokenURL1) + client2 := configureClientFetchingOIDCCredentials(t, apiServer.ClientConfig, caCertContent2, caFilePath2, oidcServer2.URL(), tokenURL2) + + ctx := testContext(t) + res, err := client1.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{}) + require.NoError(t, err) + assert.Equal(t, authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, res.Status.UserInfo) + + res, err = client2.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{}) + require.NoError(t, err) + assert.Equal(t, authenticationv1.UserInfo{ + Username: "k8s-not_john_doe", + Groups: []string{"system:role1", "system:role2", "system:role3", "system:role4", "system:authenticated"}, + UID: "1234", + }, res.Status.UserInfo) +} + func rsaGenerateKey(t *testing.T) (*rsa.PrivateKey, *rsa.PublicKey) { t.Helper()