diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go index 66ce59c1a69..fc75c464a29 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go @@ -330,6 +330,10 @@ type ClaimMappings struct { // The claim's value must be a singular string. // Same as the --oidc-username-claim and --oidc-username-prefix flags. // If username.expression is set, the expression must produce a string value. + // If username.expression uses 'claims.email', then 'claims.email_verified' must be used in + // username.expression or extra[*].valueExpression or claimValidationRules[*].expression. + // An example claim validation rule expression that matches the validation automatically + // applied when username.claim is set to 'email' is 'claims.?email_verified.orValue(true)'. // // In the flag based approach, the --oidc-username-claim and --oidc-username-prefix are optional. If --oidc-username-claim is not set, // the default value is "sub". For the authentication config, there is no defaulting for claim or prefix. The claim and prefix must be set explicitly. @@ -339,7 +343,7 @@ type ClaimMappings struct { // set username.prefix="" // (2) --oidc-username-prefix="" and --oidc-username-claim != "email", prefix was "#". For the same // behavior using authentication config, set username.prefix="#" - // (3) --oidc-username-prefix="". For the same behavior using authentication config, set username.prefix="" + // (3) --oidc-username-prefix="". For the same behavior using authentication config, set username.prefix="" // +required Username PrefixedClaimOrExpression `json:"username"` // groups represents an option for the groups attribute. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1beta1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1beta1/types.go index b23dd25d83a..00a55f7a925 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1beta1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1beta1/types.go @@ -301,6 +301,10 @@ type ClaimMappings struct { // The claim's value must be a singular string. // Same as the --oidc-username-claim and --oidc-username-prefix flags. // If username.expression is set, the expression must produce a string value. + // If username.expression uses 'claims.email', then 'claims.email_verified' must be used in + // username.expression or extra[*].valueExpression or claimValidationRules[*].expression. + // An example claim validation rule expression that matches the validation automatically + // applied when username.claim is set to 'email' is 'claims.?email_verified.orValue(true)'. // // In the flag based approach, the --oidc-username-claim and --oidc-username-prefix are optional. If --oidc-username-claim is not set, // the default value is "sub". For the authentication config, there is no defaulting for claim or prefix. The claim and prefix must be set explicitly. @@ -310,7 +314,7 @@ type ClaimMappings struct { // set username.prefix="" // (2) --oidc-username-prefix="" and --oidc-username-claim != "email", prefix was "#". For the same // behavior using authentication config, set username.prefix="#" - // (3) --oidc-username-prefix="". For the same behavior using authentication config, set username.prefix="" + // (3) --oidc-username-prefix="". For the same behavior using authentication config, set username.prefix="" // +required Username PrefixedClaimOrExpression `json:"username"` // groups represents an option for the groups attribute. 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 db445f433e2..35ee8a4503c 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 @@ -25,6 +25,10 @@ import ( "strings" "time" + celgo "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/operators" + exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1" + v1 "k8s.io/api/authorization/v1" "k8s.io/api/authorization/v1beta1" "k8s.io/apimachinery/pkg/util/sets" @@ -88,14 +92,20 @@ func validateJWTAuthenticator(authenticator api.JWTAuthenticator, fldPath *field var allErrs field.ErrorList compiler := authenticationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) - mapper := &authenticationcel.CELMapper{} + state := &validationState{} allErrs = append(allErrs, validateIssuer(authenticator.Issuer, disallowedIssuers, fldPath.Child("issuer"))...) - allErrs = append(allErrs, validateClaimValidationRules(compiler, mapper, authenticator.ClaimValidationRules, fldPath.Child("claimValidationRules"), structuredAuthnFeatureEnabled)...) - allErrs = append(allErrs, validateClaimMappings(compiler, mapper, authenticator.ClaimMappings, fldPath.Child("claimMappings"), structuredAuthnFeatureEnabled)...) - allErrs = append(allErrs, validateUserValidationRules(compiler, mapper, authenticator.UserValidationRules, fldPath.Child("userValidationRules"), structuredAuthnFeatureEnabled)...) + allErrs = append(allErrs, validateClaimValidationRules(compiler, state, authenticator.ClaimValidationRules, fldPath.Child("claimValidationRules"), structuredAuthnFeatureEnabled)...) + allErrs = append(allErrs, validateClaimMappings(compiler, state, authenticator.ClaimMappings, fldPath.Child("claimMappings"), structuredAuthnFeatureEnabled)...) + allErrs = append(allErrs, validateUserValidationRules(compiler, state, authenticator.UserValidationRules, fldPath.Child("userValidationRules"), structuredAuthnFeatureEnabled)...) - return *mapper, allErrs + return state.mapper, allErrs +} + +type validationState struct { + mapper authenticationcel.CELMapper + usesEmailClaim bool + usesEmailVerifiedClaim bool } func validateIssuer(issuer api.Issuer, disallowedIssuers sets.Set[string], fldPath *field.Path) field.ErrorList { @@ -205,7 +215,7 @@ func validateCertificateAuthority(certificateAuthority string, fldPath *field.Pa return allErrs } -func validateClaimValidationRules(compiler authenticationcel.Compiler, celMapper *authenticationcel.CELMapper, rules []api.ClaimValidationRule, fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { +func validateClaimValidationRules(compiler authenticationcel.Compiler, state *validationState, rules []api.ClaimValidationRule, fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { var allErrs field.ErrorList seenClaims := sets.NewString() @@ -258,13 +268,14 @@ func validateClaimValidationRules(compiler authenticationcel.Compiler, celMapper } if structuredAuthnFeatureEnabled && len(compilationResults) > 0 { - celMapper.ClaimValidationRules = authenticationcel.NewClaimsMapper(compilationResults) + state.mapper.ClaimValidationRules = authenticationcel.NewClaimsMapper(compilationResults) + state.usesEmailVerifiedClaim = state.usesEmailVerifiedClaim || anyUsesEmailVerifiedClaim(compilationResults) } return allErrs } -func validateClaimMappings(compiler authenticationcel.Compiler, celMapper *authenticationcel.CELMapper, m api.ClaimMappings, fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { +func validateClaimMappings(compiler authenticationcel.Compiler, state *validationState, m api.ClaimMappings, fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { var allErrs field.ErrorList if !structuredAuthnFeatureEnabled { @@ -282,18 +293,20 @@ func validateClaimMappings(compiler authenticationcel.Compiler, celMapper *authe } } - compilationResult, err := validatePrefixClaimOrExpression(compiler, m.Username, fldPath.Child("username"), true, structuredAuthnFeatureEnabled) + compilationResult, err := validatePrefixClaimOrExpression(compiler, m.Username, fldPath.Child("username"), true) if err != nil { allErrs = append(allErrs, err...) } else if compilationResult != nil && structuredAuthnFeatureEnabled { - celMapper.Username = authenticationcel.NewClaimsMapper([]authenticationcel.CompilationResult{*compilationResult}) + state.usesEmailClaim = state.usesEmailClaim || usesEmailClaim(compilationResult.AST) + state.usesEmailVerifiedClaim = state.usesEmailVerifiedClaim || usesEmailVerifiedClaim(compilationResult.AST) + state.mapper.Username = authenticationcel.NewClaimsMapper([]authenticationcel.CompilationResult{*compilationResult}) } - compilationResult, err = validatePrefixClaimOrExpression(compiler, m.Groups, fldPath.Child("groups"), false, structuredAuthnFeatureEnabled) + compilationResult, err = validatePrefixClaimOrExpression(compiler, m.Groups, fldPath.Child("groups"), false) if err != nil { allErrs = append(allErrs, err...) } else if compilationResult != nil && structuredAuthnFeatureEnabled { - celMapper.Groups = authenticationcel.NewClaimsMapper([]authenticationcel.CompilationResult{*compilationResult}) + state.mapper.Groups = authenticationcel.NewClaimsMapper([]authenticationcel.CompilationResult{*compilationResult}) } switch { @@ -307,7 +320,7 @@ func validateClaimMappings(compiler authenticationcel.Compiler, celMapper *authe if err != nil { allErrs = append(allErrs, err) } else if structuredAuthnFeatureEnabled && compilationResult != nil { - celMapper.UID = authenticationcel.NewClaimsMapper([]authenticationcel.CompilationResult{*compilationResult}) + state.mapper.UID = authenticationcel.NewClaimsMapper([]authenticationcel.CompilationResult{*compilationResult}) } } @@ -351,13 +364,124 @@ func validateClaimMappings(compiler authenticationcel.Compiler, celMapper *authe } if structuredAuthnFeatureEnabled && len(extraCompilationResults) > 0 { - celMapper.Extra = authenticationcel.NewClaimsMapper(extraCompilationResults) + state.mapper.Extra = authenticationcel.NewClaimsMapper(extraCompilationResults) + state.usesEmailVerifiedClaim = state.usesEmailVerifiedClaim || anyUsesEmailVerifiedClaim(extraCompilationResults) + } + + if structuredAuthnFeatureEnabled && state.usesEmailClaim && !state.usesEmailVerifiedClaim { + allErrs = append(allErrs, field.Invalid(fldPath.Child("username", "expression"), m.Username.Expression, + "claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression")) } return allErrs } -func validatePrefixClaimOrExpression(compiler authenticationcel.Compiler, mapping api.PrefixedClaimOrExpression, fldPath *field.Path, claimOrExpressionRequired, structuredAuthnFeatureEnabled bool) (*authenticationcel.CompilationResult, field.ErrorList) { +func usesEmailClaim(ast *celgo.Ast) bool { + return hasSelectExp(ast.Expr(), "claims", "email") +} + +func anyUsesEmailVerifiedClaim(results []authenticationcel.CompilationResult) bool { + for _, result := range results { + if usesEmailVerifiedClaim(result.AST) { + return true + } + } + return false +} + +func usesEmailVerifiedClaim(ast *celgo.Ast) bool { + return hasSelectExp(ast.Expr(), "claims", "email_verified") +} + +func hasSelectExp(exp *exprpb.Expr, operand, field string) bool { + if exp == nil { + return false + } + switch e := exp.ExprKind.(type) { + case *exprpb.Expr_ConstExpr, + *exprpb.Expr_IdentExpr: + return false + case *exprpb.Expr_SelectExpr: + s := e.SelectExpr + if s == nil { + return false + } + if isIdentOperand(s.Operand, operand) && s.Field == field { + return true + } + return hasSelectExp(s.Operand, operand, field) + case *exprpb.Expr_CallExpr: + c := e.CallExpr + if c == nil { + return false + } + if c.Target == nil && c.Function == operators.OptSelect && len(c.Args) == 2 && + isIdentOperand(c.Args[0], operand) && isConstField(c.Args[1], field) { + return true + } + for _, arg := range c.Args { + if hasSelectExp(arg, operand, field) { + return true + } + } + return hasSelectExp(c.Target, operand, field) + case *exprpb.Expr_ListExpr: + l := e.ListExpr + if l == nil { + return false + } + for _, element := range l.Elements { + if hasSelectExp(element, operand, field) { + return true + } + } + return false + case *exprpb.Expr_StructExpr: + s := e.StructExpr + if s == nil { + return false + } + for _, entry := range s.Entries { + if hasSelectExp(entry.GetMapKey(), operand, field) { + return true + } + if hasSelectExp(entry.Value, operand, field) { + return true + } + } + return false + case *exprpb.Expr_ComprehensionExpr: + c := e.ComprehensionExpr + if c == nil { + return false + } + return hasSelectExp(c.IterRange, operand, field) || + hasSelectExp(c.AccuInit, operand, field) || + hasSelectExp(c.LoopCondition, operand, field) || + hasSelectExp(c.LoopStep, operand, field) || + hasSelectExp(c.Result, operand, field) + default: + return false + } +} + +func isIdentOperand(exp *exprpb.Expr, operand string) bool { + if len(operand) == 0 { + return false // sanity check against default values + } + id := exp.GetIdentExpr() // does not panic even if exp is nil + return id != nil && id.Name == operand +} + +func isConstField(exp *exprpb.Expr, field string) bool { + if len(field) == 0 { + return false // sanity check against default values + } + c := exp.GetConstExpr() // does not panic even if exp is nil + return c != nil && c.GetStringValue() == field // does not panic even if c is not a string +} + +func validatePrefixClaimOrExpression(compiler authenticationcel.Compiler, mapping api.PrefixedClaimOrExpression, fldPath *field.Path, claimOrExpressionRequired bool) (*authenticationcel.CompilationResult, field.ErrorList) { var allErrs field.ErrorList var compilationResult *authenticationcel.CompilationResult @@ -389,7 +513,7 @@ func validatePrefixClaimOrExpression(compiler authenticationcel.Compiler, mappin return compilationResult, allErrs } -func validateUserValidationRules(compiler authenticationcel.Compiler, celMapper *authenticationcel.CELMapper, rules []api.UserValidationRule, fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { +func validateUserValidationRules(compiler authenticationcel.Compiler, state *validationState, rules []api.UserValidationRule, fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { var allErrs field.ErrorList var compilationResults []authenticationcel.CompilationResult @@ -428,7 +552,7 @@ func validateUserValidationRules(compiler authenticationcel.Compiler, celMapper } if structuredAuthnFeatureEnabled && len(compilationResults) > 0 { - celMapper.UserValidationRules = authenticationcel.NewUserMapper(compilationResults) + state.mapper.UserValidationRules = authenticationcel.NewUserMapper(compilationResults) } 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 702a200a426..a38d5116b05 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 @@ -282,6 +282,313 @@ func TestValidateAuthenticationConfiguration(t *testing.T) { disallowedIssuers: []string{"a", "b", "https://issuer-url", "c"}, want: `jwt[0].issuer.url: Invalid value: "https://issuer-url": URL must not overlap with disallowed issuers: [a b c https://issuer-url]`, }, + { + name: "valid authentication configuration that uses unverified email", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: "claims.email", + }, + }, + }, + }, + }, + want: `jwt[0].claimMappings.username.expression: Invalid value: "claims.email": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid authentication configuration that almost uses unverified email", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: "claims.email_", + }, + }, + }, + }, + }, + want: "", + }, + { + name: "valid authentication configuration that uses unverified email join", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `['yay', string(claims.email), 'panda'].join(' ')`, + }, + }, + }, + }, + }, + want: `jwt[0].claimMappings.username.expression: Invalid value: "['yay', string(claims.email), 'panda'].join(' ')": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid authentication configuration that uses unverified optional email", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `claims.?email`, + }, + }, + }, + }, + }, + want: `jwt[0].claimMappings.username.expression: Invalid value: "claims.?email": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid authentication configuration that uses unverified optional map email key", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `{claims.?email: "panda"}`, + }, + }, + }, + }, + }, + want: `jwt[0].claimMappings.username.expression: Invalid value: "{claims.?email: \"panda\"}": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid authentication configuration that uses unverified optional map email value", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `{"fancy": claims.?email}`, + }, + }, + }, + }, + }, + want: `jwt[0].claimMappings.username.expression: Invalid value: "{\"fancy\": claims.?email}": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid authentication configuration that uses unverified email value in list iteration", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `["a"].map(i, i + claims.email)`, + }, + }, + }, + }, + }, + want: `jwt[0].claimMappings.username.expression: Invalid value: "[\"a\"].map(i, i + claims.email)": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid authentication configuration that uses verified email join via rule", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Expression: `string(claims.email_verified) == "panda"`, + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `['yay', string(claims.email), 'panda'].join(' ')`, + }, + }, + }, + }, + }, + want: "", + }, + { + name: "valid authentication configuration that uses verified email join via extra", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `['yay', string(claims.email), 'panda'].join(' ')`, + }, + Extra: []api.ExtraMapping{ + {Key: "panda.io/foo", ValueExpression: "claims.email_verified.upperAscii()"}, + }, + }, + }, + }, + }, + want: "", + }, + { + name: "valid authentication configuration that uses verified email join via extra optional", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `['yay', string(claims.email), 'panda'].join(' ')`, + }, + Extra: []api.ExtraMapping{ + {Key: "panda.io/foo", ValueExpression: "claims.?email_verified"}, + }, + }, + }, + }, + }, + want: "", + }, + { + name: "valid authentication configuration that uses email and email_verified || true via username", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + // allow email claim when email_verified is true or absent + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `claims.?email_verified.orValue(true) ? claims.email : claims.sub`, + }, + }, + }, + }, + }, + want: "", + }, + { + name: "valid authentication configuration that uses email and email_verified || false via username", + in: &api.AuthenticationConfiguration{ + JWT: []api.JWTAuthenticator{ + { + Issuer: api.Issuer{ + URL: "https://issuer-url", + Audiences: []string{"audience"}, + }, + ClaimValidationRules: []api.ClaimValidationRule{ + { + Claim: "foo", + RequiredValue: "bar", + }, + }, + // allow email claim only when email_verified is present and true + ClaimMappings: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{ + Expression: `claims.?email_verified.orValue(false) ? claims.email : claims.sub`, + }, + }, + }, + }, + }, + want: "", + }, { name: "valid authentication configuration", in: &api.AuthenticationConfiguration{ @@ -587,6 +894,7 @@ func TestValidateClaimValidationRules(t *testing.T) { structuredAuthnFeatureEnabled bool want string wantCELMapper bool + wantUsesEmailVerifiedClaim bool }{ { name: "claim and expression are empty, structured authn feature enabled", @@ -672,26 +980,52 @@ func TestValidateClaimValidationRules(t *testing.T) { wantCELMapper: true, }, { - name: "valid claim validation rule with multiple rules", + name: "valid claim validation rule with multiple rules and email_verified check", in: []api.ClaimValidationRule{ {Claim: "claim1", RequiredValue: "value1"}, {Claim: "claim2", RequiredValue: "value2"}, + {Expression: "has(claims.email_verified)"}, }, structuredAuthnFeatureEnabled: true, want: "", + wantUsesEmailVerifiedClaim: true, + }, + { + name: "valid claim validation rule with multiple rules and almost email_verified check", + in: []api.ClaimValidationRule{ + {Claim: "claim1", RequiredValue: "value1"}, + {Claim: "claim2", RequiredValue: "value2"}, + {Expression: "has(claims.email_verified_)"}, + }, + structuredAuthnFeatureEnabled: true, + want: "", + wantUsesEmailVerifiedClaim: false, + }, + { + name: "valid claim validation rule with multiple rules", + in: []api.ClaimValidationRule{ + {Claim: "claim1", RequiredValue: "value1"}, + {Claim: "claim2", RequiredValue: "claims.email_verified"}, // not a CEL expression + }, + structuredAuthnFeatureEnabled: true, + want: "", + wantUsesEmailVerifiedClaim: false, }, } for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - celMapper := &authenticationcel.CELMapper{} - got := validateClaimValidationRules(compiler, celMapper, tt.in, fldPath, tt.structuredAuthnFeatureEnabled).ToAggregate() + state := &validationState{} + got := validateClaimValidationRules(compiler, state, tt.in, fldPath, tt.structuredAuthnFeatureEnabled).ToAggregate() if d := cmp.Diff(tt.want, errString(got)); d != "" { t.Fatalf("ClaimValidationRules validation mismatch (-want +got):\n%s", d) } - if tt.wantCELMapper && celMapper.ClaimValidationRules == nil { + if tt.wantCELMapper && state.mapper.ClaimValidationRules == nil { t.Fatalf("ClaimValidationRules validation mismatch: CELMapper.ClaimValidationRules is nil") } + if tt.wantUsesEmailVerifiedClaim != state.usesEmailVerifiedClaim { + t.Fatalf("ClaimValidationRules state.usesEmailVerifiedClaim mismatch: want %v, got %v", tt.wantUsesEmailVerifiedClaim, state.usesEmailVerifiedClaim) + } }) } } @@ -702,6 +1036,7 @@ func TestValidateClaimMappings(t *testing.T) { testCases := []struct { name string in api.ClaimMappings + usesEmailVerifiedClaim bool structuredAuthnFeatureEnabled bool want string wantCELMapper bool @@ -1000,6 +1335,133 @@ func TestValidateClaimMappings(t *testing.T) { structuredAuthnFeatureEnabled: true, want: `issuer.claimMappings.extra[0].key: Invalid value: "example.org/Foo": key must be lowercase`, }, + { + name: "valid claim mappings but uses email without verification", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "claims.email"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: "claims.extra"}, + }, + }, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: `issuer.claimMappings.username.expression: Invalid value: "claims.email": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid claim mappings but uses email in complex CEL expression without verification", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "has(claims.email) ? claims.email : claims.sub"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: "claims.extra"}, + }, + }, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: `issuer.claimMappings.username.expression: Invalid value: "has(claims.email) ? claims.email : claims.sub": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid claim mappings but uses email in CEL expression function without verification", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "claims.email.trim()"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: "claims.extra"}, + }, + }, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: `issuer.claimMappings.username.expression: Invalid value: "claims.email.trim()": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid claim mappings and uses email with verification via extra", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "claims.email"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: "claims.email_verified"}, + }, + }, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: "", + }, + { + name: "valid claim mappings and uses email with verification via extra optional", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "claims.email"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: `has(claims.email_verified) ? string(claims.email_verified) : "false"`}, + }, + }, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: "", + }, + { + name: "valid claim mappings and almost uses email with verification via extra optional", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "claims.email"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: `has(claims.email_verified_) ? string(claims.email_verified_) : "false"`}, + }, + }, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: `issuer.claimMappings.username.expression: Invalid value: "claims.email": claims.email_verified must be used in claimMappings.username.expression or claimMappings.extra[*].valueExpression or claimValidationRules[*].expression when claims.email is used in claimMappings.username.expression`, + }, + { + name: "valid claim mappings and uses email with verification via hasVerifiedEmail", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "claims.email"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: "claims.extra"}, + }, + }, + usesEmailVerifiedClaim: true, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: "", + }, + { + name: "valid claim mappings that almost use claims.email", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "claims.email_"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: "claims.extra"}, + }, + }, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: "", + }, + { + name: "valid claim mappings that almost use claims.email via nesting", + in: api.ClaimMappings{ + Username: api.PrefixedClaimOrExpression{Expression: "claims.other.claims.email"}, + Groups: api.PrefixedClaimOrExpression{Expression: "claims.groups"}, + UID: api.ClaimOrExpression{Expression: "claims.uid"}, + Extra: []api.ExtraMapping{ + {Key: "example.org/foo", ValueExpression: "claims.extra"}, + }, + }, + structuredAuthnFeatureEnabled: true, + wantCELMapper: true, + want: "", + }, { name: "valid claim mappings", in: api.ClaimMappings{ @@ -1018,23 +1480,23 @@ func TestValidateClaimMappings(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - celMapper := &authenticationcel.CELMapper{} - got := validateClaimMappings(compiler, celMapper, tt.in, fldPath, tt.structuredAuthnFeatureEnabled).ToAggregate() + state := &validationState{usesEmailVerifiedClaim: tt.usesEmailVerifiedClaim} + got := validateClaimMappings(compiler, state, tt.in, fldPath, tt.structuredAuthnFeatureEnabled).ToAggregate() if d := cmp.Diff(tt.want, errString(got)); d != "" { fmt.Println(errString(got)) t.Fatalf("ClaimMappings validation mismatch (-want +got):\n%s", d) } if tt.wantCELMapper { - if len(tt.in.Username.Expression) > 0 && celMapper.Username == nil { + if len(tt.in.Username.Expression) > 0 && state.mapper.Username == nil { t.Fatalf("ClaimMappings validation mismatch: CELMapper.Username is nil") } - if len(tt.in.Groups.Expression) > 0 && celMapper.Groups == nil { + if len(tt.in.Groups.Expression) > 0 && state.mapper.Groups == nil { t.Fatalf("ClaimMappings validation mismatch: CELMapper.Groups is nil") } - if len(tt.in.UID.Expression) > 0 && celMapper.UID == nil { + if len(tt.in.UID.Expression) > 0 && state.mapper.UID == nil { t.Fatalf("ClaimMappings validation mismatch: CELMapper.UID is nil") } - if len(tt.in.Extra) > 0 && celMapper.Extra == nil { + if len(tt.in.Extra) > 0 && state.mapper.Extra == nil { t.Fatalf("ClaimMappings validation mismatch: CELMapper.Extra is nil") } } @@ -1107,12 +1569,12 @@ func TestValidateUserValidationRules(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - celMapper := &authenticationcel.CELMapper{} - got := validateUserValidationRules(compiler, celMapper, tt.in, fldPath, tt.structuredAuthnFeatureEnabled).ToAggregate() + state := &validationState{} + got := validateUserValidationRules(compiler, state, tt.in, fldPath, tt.structuredAuthnFeatureEnabled).ToAggregate() if d := cmp.Diff(tt.want, errString(got)); d != "" { t.Fatalf("UserValidationRules validation mismatch (-want +got):\n%s", d) } - if tt.wantCELMapper && celMapper.UserValidationRules == nil { + if tt.wantCELMapper && state.mapper.UserValidationRules == nil { t.Fatalf("UserValidationRules validation mismatch: CELMapper.UserValidationRules is nil") } }) diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/cel/compile.go b/staging/src/k8s.io/apiserver/pkg/authentication/cel/compile.go index 3bcff5e9051..5550955af92 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/cel/compile.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/cel/compile.go @@ -106,6 +106,7 @@ func (c compiler) compile(expressionAccessor ExpressionAccessor, envVarName stri return CompilationResult{ Program: prog, + AST: ast, ExpressionAccessor: expressionAccessor, }, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/cel/interface.go b/staging/src/k8s.io/apiserver/pkg/authentication/cel/interface.go index 7ec0c9af6af..0fc208414e4 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/cel/interface.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/cel/interface.go @@ -35,6 +35,7 @@ type ExpressionAccessor interface { // CompilationResult represents a compiled validations expression. type CompilationResult struct { Program celgo.Program + AST *celgo.Ast ExpressionAccessor ExpressionAccessor }