UPSTREAM: <carry>: admission: authentication: validate CEL expressions compilation

Signed-off-by: Bryce Palmer <[email protected]>

Author: everettraven

openshift-kube-apiserver/admission/customresourcevalidation/authentication/validate_authentication.go

@@ -12,6 +12,7 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 
 	configv1 "github.com/openshift/api/config/v1"
+	authenticationcel "k8s.io/apiserver/pkg/authentication/cel"
 	crvalidation "k8s.io/kubernetes/openshift-kube-apiserver/admission/customresourcevalidation"
 )
 
@@ -121,14 +122,83 @@ func validateAuthenticationSpec(spec configv1.AuthenticationSpec) field.ErrorLis
 				spec.WebhookTokenAuthenticator, fmt.Sprintf("this field cannot be set with the %q .spec.type", spec.Type),
 			))
 		}
-
 	}
 
 	errs = append(errs, crvalidation.ValidateConfigMapReference(specField.Child("oauthMetadata"), spec.OAuthMetadata, false)...)
 
+	// Perform External OIDC Provider related validations
+	// ----------------
+
+	// There is currently no guarantee that these fields are not set when the spec.Type is != OIDC.
+	// To ensure we are enforcing approriate admission validations at all times, just always iterate through the list
+	// of OIDC Providers and perform the validations.
+	// If/when the openshift/api admission validations are updated to enforce that this field is not configured
+	// when Type != OIDC, this loop should be a no-op due to an empty list.
+	for i, provider := range spec.OIDCProviders {
+		errs = append(errs, validateOIDCProvider(specField.Child("oidcProviders").Index(i), provider)...)
+	}
+	// ----------------
+
 	return errs
 }
 
 func validateAuthenticationStatus(status configv1.AuthenticationStatus) field.ErrorList {
 	return crvalidation.ValidateConfigMapReference(field.NewPath("status", "integratedOAuthMetadata"), status.IntegratedOAuthMetadata, false)
 }
+
+func validateOIDCProvider(path *field.Path, provider configv1.OIDCProvider) field.ErrorList {
+	errs := field.ErrorList{}
+	errs = append(errs, validateClaimMappings(path, provider.ClaimMappings)...)
+	return errs
+}
+
+func validateClaimMappings(path *field.Path, claimMappings configv1.TokenClaimMappings) field.ErrorList {
+	path = path.Child("claimMappings")
+	errs := field.ErrorList{}
+	compiler := authenticationcel.NewDefaultCompiler()
+	errs = append(errs, validateUIDClaimMapping(path, compiler, claimMappings.UID)...)
+	errs = append(errs, validateExtraClaimMapping(path, compiler, claimMappings.Extra...)...)
+	return errs
+}
+
+func validateUIDClaimMapping(path *field.Path, compiler authenticationcel.Compiler, uid *configv1.TokenClaimOrExpressionMapping) field.ErrorList {
+	if uid == nil {
+		return nil
+	}
+
+	if uid.Expression != "" {
+		err := validateCELExpression(compiler, &authenticationcel.ClaimMappingExpression{
+			Expression: uid.Expression,
+		})
+		if err != nil {
+			return field.ErrorList{field.Invalid(path.Child("uid", "expression"), uid.Expression, err.Error())}
+		}
+	}
+
+	return nil
+}
+
+func validateExtraClaimMapping(path *field.Path, compiler authenticationcel.Compiler, extras ...configv1.ExtraMapping) field.ErrorList {
+	errs := field.ErrorList{}
+	for i, extra := range extras {
+		errs = append(errs, validateExtra(path.Child("extra").Index(i), compiler, extra)...)
+	}
+	return errs
+}
+
+func validateExtra(path *field.Path, compiler authenticationcel.Compiler, extra configv1.ExtraMapping) field.ErrorList {
+	err := validateCELExpression(compiler, &authenticationcel.ExtraMappingExpression{
+		Key:        extra.Key,
+		Expression: extra.ValueExpression,
+	})
+	if err != nil {
+		return field.ErrorList{field.Invalid(path.Child("valueExpression"), extra.ValueExpression, err.Error())}
+	}
+
+	return nil
+}
+
+func validateCELExpression(compiler authenticationcel.Compiler, accessor authenticationcel.ExpressionAccessor) error {
+	_, err := compiler.CompileClaimsExpression(accessor)
+	return err
+}

openshift-kube-apiserver/admission/customresourcevalidation/authentication/validate_authentication_test.go

@@ -49,6 +49,41 @@ func TestFailValidateAuthenticationSpec(t *testing.T) {
 			errorType:  field.ErrorTypeInvalid,
 			errorField: "spec.webhookTokenAuthenticator",
 		},
+		"invalid UID CEL expression": {
+			spec: configv1.AuthenticationSpec{
+				Type: "OIDC",
+				OIDCProviders: []configv1.OIDCProvider{
+					{
+						ClaimMappings: configv1.TokenClaimMappings{
+							UID: &configv1.TokenClaimOrExpressionMapping{
+								Expression: "!@^#&(!^@(*#&(",
+							},
+						},
+					},
+				},
+			},
+			errorType:  field.ErrorTypeInvalid,
+			errorField: "spec.oidcProviders[0].claimMappings.uid.expression",
+		},
+		"invalid Extra CEL expression": {
+			spec: configv1.AuthenticationSpec{
+				Type: "OIDC",
+				OIDCProviders: []configv1.OIDCProvider{
+					{
+						ClaimMappings: configv1.TokenClaimMappings{
+							Extra: []configv1.ExtraMapping{
+								{
+									Key:             "foo/bar",
+									ValueExpression: "!@*(&#^(!@*)&^&",
+								},
+							},
+						},
+					},
+				},
+			},
+			errorType:  field.ErrorTypeInvalid,
+			errorField: "spec.oidcProviders[0].claimMappings.extra[0].valueExpression",
+		},
 	}
 
 	for tcName, tc := range errorCases {
@@ -109,6 +144,33 @@ func TestSucceedValidateAuthenticationSpec(t *testing.T) {
 				{KubeConfig: configv1.SecretNameReference{Name: "thisisawebhook33"}},
 			},
 		},
+		"valid uid CEL expression": {
+			Type: "OIDC",
+			OIDCProviders: []configv1.OIDCProvider{
+				{
+					ClaimMappings: configv1.TokenClaimMappings{
+						UID: &configv1.TokenClaimOrExpressionMapping{
+							Expression: "claims.uid",
+						},
+					},
+				},
+			},
+		},
+		"valid Extra CEL expression": {
+			Type: "OIDC",
+			OIDCProviders: []configv1.OIDCProvider{
+				{
+					ClaimMappings: configv1.TokenClaimMappings{
+						Extra: []configv1.ExtraMapping{
+							{
+								Key:             "foo/bar",
+								ValueExpression: "claims.roles",
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for tcName, s := range successCases {
@@ -175,5 +237,4 @@ func TestSucceedValidateAuthenticationStatus(t *testing.T) {
 			t.Errorf("'%s': expected success, but failed: %v", tcName, errs.ToAggregate().Error())
 		}
 	}
-
 }