From 4eaefb0ceee8833094a0538f9ff197d303295e86 Mon Sep 17 00:00:00 2001
From: Monis Khan <i@monis.app>
Date: Wed, 28 Feb 2024 12:53:08 -0500
Subject: [PATCH] jwt: fail on empty username via CEL expression

Signed-off-by: Monis Khan <mok@microsoft.com>

Kubernetes-commit: 8345ad0bac4fee6d25f033f0445e2e10eae6afbe
---
 plugin/pkg/authenticator/token/oidc/oidc.go   |  8 ++-
 .../pkg/authenticator/token/oidc/oidc_test.go | 54 ++++++++++++++++++-
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/plugin/pkg/authenticator/token/oidc/oidc.go b/plugin/pkg/authenticator/token/oidc/oidc.go
index 77a72aaeb..0ce213d9f 100644
--- a/plugin/pkg/authenticator/token/oidc/oidc.go
+++ b/plugin/pkg/authenticator/token/oidc/oidc.go
@@ -769,7 +769,13 @@ func (a *Authenticator) getUsername(ctx context.Context, c claims, claimsUnstruc
 			return "", fmt.Errorf("oidc: error evaluating username claim expression: %w", fmt.Errorf("username claim expression must return a string"))
 		}
 
-		return evalResult.EvalResult.Value().(string), nil
+		username := evalResult.EvalResult.Value().(string)
+
+		if len(username) == 0 {
+			return "", fmt.Errorf("oidc: empty username via CEL expression is not allowed")
+		}
+
+		return username, nil
 	}
 
 	var username string
diff --git a/plugin/pkg/authenticator/token/oidc/oidc_test.go b/plugin/pkg/authenticator/token/oidc/oidc_test.go
index 05276d932..3b246df4d 100644
--- a/plugin/pkg/authenticator/token/oidc/oidc_test.go
+++ b/plugin/pkg/authenticator/token/oidc/oidc_test.go
@@ -2982,7 +2982,7 @@ func TestToken(t *testing.T) {
 		// 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",
+			name: "test user validation rule doesn't fail when user info is empty except username",
 			options: Options{
 				JWTAuthenticator: apiserver.JWTAuthenticator{
 					Issuer: apiserver.Issuer{
@@ -2997,6 +2997,58 @@ func TestToken(t *testing.T) {
 							Expression: "claims.groups",
 						},
 					},
+					UserValidationRules: []apiserver.UserValidationRule{
+						{
+							Expression: `user.username == " "`,
+							Message:    "username must be single space",
+						},
+						{
+							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{Name: " "},
+		},
+		{
+			name: "empty username is allowed via claim",
+			options: Options{
+				JWTAuthenticator: apiserver.JWTAuthenticator{
+					Issuer: apiserver.Issuer{
+						URL:       "https://auth.example.com",
+						Audiences: []string{"my-client"},
+					},
+					ClaimMappings: apiserver.ClaimMappings{
+						Username: apiserver.PrefixedClaimOrExpression{
+							Claim:  "username",
+							Prefix: pointer.String(""),
+						},
+						Groups: apiserver.PrefixedClaimOrExpression{
+							Expression: "claims.groups",
+						},
+					},
 					UserValidationRules: []apiserver.UserValidationRule{
 						{
 							Expression: `user.username == ""`,