Merge pull request kubernetes#121709 from aramase/aramase/f/authn_user_info_fix

[StructuredAuthn] Ensure empty fields of user object are accessible by CEL

Author: k8s-ci-robot

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go

@@ -615,23 +615,12 @@ func (a *Authenticator) AuthenticateToken(ctx context.Context, token string) (*a
 	}
 
 	if a.celMapper.UserValidationRules != nil {
-		userInfo := &authenticationv1.UserInfo{
-			Extra:    make(map[string]authenticationv1.ExtraValue),
-			Groups:   info.GetGroups(),
-			UID:      info.GetUID(),
-			Username: info.GetName(),
-		}
-		// Convert the extra information in the user object
-		for key, val := range info.GetExtra() {
-			userInfo.Extra[key] = authenticationv1.ExtraValue(val)
-		}
-
 		// Convert the user info to unstructured so that we can evaluate the CEL expressions
 		// against the user info. This is done once here so that we don't have to convert
 		// the user info to unstructured multiple times in the CEL mapper for each mapping.
-		userInfoUnstructured, err := convertObjectToUnstructured(userInfo)
+		userInfoUnstructured, err := convertUserInfoToUnstructured(info)
 		if err != nil {
-			return nil, false, fmt.Errorf("oidc: could not convert user info to unstructured: %v", err)
+			return nil, false, fmt.Errorf("oidc: could not convert user info to unstructured: %w", err)
 		}
 
 		evalResult, err := a.celMapper.UserValidationRules.EvalUser(ctx, userInfoUnstructured)
@@ -944,3 +933,40 @@ func convertObjectToUnstructured(obj interface{}) (*unstructured.Unstructured, e
 	}
 	return &unstructured.Unstructured{Object: ret}, nil
 }
+
+func convertUserInfoToUnstructured(info user.Info) (*unstructured.Unstructured, error) {
+	userInfo := &authenticationv1.UserInfo{
+		Extra:    make(map[string]authenticationv1.ExtraValue),
+		Groups:   info.GetGroups(),
+		UID:      info.GetUID(),
+		Username: info.GetName(),
+	}
+	// Convert the extra information in the user object
+	for key, val := range info.GetExtra() {
+		userInfo.Extra[key] = authenticationv1.ExtraValue(val)
+	}
+
+	// Convert the user info to unstructured so that we can evaluate the CEL expressions
+	// against the user info. This is done once here so that we don't have to convert
+	// the user info to unstructured multiple times in the CEL mapper for each mapping.
+	userInfoUnstructured, err := convertObjectToUnstructured(userInfo)
+	if err != nil {
+		return nil, err
+	}
+
+	// check if the user info contains the required fields. If not, set them to empty values.
+	// This is done because the CEL expressions expect these fields to be present.
+	if userInfoUnstructured.Object["username"] == nil {
+		userInfoUnstructured.Object["username"] = ""
+	}
+	if userInfoUnstructured.Object["uid"] == nil {
+		userInfoUnstructured.Object["uid"] = ""
+	}
+	if userInfoUnstructured.Object["groups"] == nil {
+		userInfoUnstructured.Object["groups"] = []string{}
+	}
+	if userInfoUnstructured.Object["extra"] == nil {
+		userInfoUnstructured.Object["extra"] = map[string]authenticationv1.ExtraValue{}
+	}
+	return userInfoUnstructured, nil
+}

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go

@@ -2727,6 +2727,59 @@ func TestToken(t *testing.T) {
 				Name: "jane",
 			},
 		},
+		// test to ensure omitempty fields not included in user info
+		// are set and accessible for CEL evaluation.
+		{
+			name: "test user validation rule doesn't fail when user info is empty",
+			options: Options{
+				JWTAuthenticator: apiserver.JWTAuthenticator{
+					Issuer: apiserver.Issuer{
+						URL:       "https://auth.example.com",
+						Audiences: []string{"my-client"},
+					},
+					ClaimMappings: apiserver.ClaimMappings{
+						Username: apiserver.PrefixedClaimOrExpression{
+							Expression: "claims.username",
+						},
+						Groups: apiserver.PrefixedClaimOrExpression{
+							Expression: "claims.groups",
+						},
+					},
+					UserValidationRules: []apiserver.UserValidationRule{
+						{
+							Expression: `user.username == ""`,
+							Message:    "username must be empty string",
+						},
+						{
+							Expression: `user.uid == ""`,
+							Message:    "uid must be empty string",
+						},
+						{
+							Expression: `!('bar' in user.groups)`,
+							Message:    "groups must not contain bar",
+						},
+						{
+							Expression: `!('bar' in user.extra)`,
+							Message:    "extra must not contain bar",
+						},
+					},
+				},
+				now: func() time.Time { return now },
+			},
+			signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
+			pubKeys: []*jose.JSONWebKey{
+				loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
+			},
+			claims: fmt.Sprintf(`{
+				"iss": "https://auth.example.com",
+				"aud": "my-client",
+				"username": "",
+				"groups": null,
+				"exp": %d,
+				"baz": "qux"
+			}`, valid.Unix()),
+			want: &user.DefaultInfo{},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, test.run)