From 940b40691dfafba02ee9915a75bca82bb19533c2 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Wed, 28 Jan 2026 14:07:36 +0000 Subject: [PATCH] Bug RHOAIENG-46523: Allow colon in RBAC subject names (system:authenticated) Fix workspace permission grants for groups/users with colons in their names. The UI now correctly allows granting permissions to system:authenticated and other valid RBAC subjects. Root cause: Permission validation incorrectly used isValidKubernetesName() which only accepts DNS-1123 labels (lowercase alphanumeric and hyphens). RBAC subjects are NOT Kubernetes resource names - they're identity references that legitimately contain colons. Changes: - Added isValidRBACSubject() in middleware.go to validate RBAC subjects - Accepts colons, dots, uppercase, email addresses (all valid RBAC subjects) - Max length 253 chars (DNS subdomain limit) - Rejects control characters for security - Updated AddProjectPermission() to use isValidRBACSubject() - Updated tests to validate system:authenticated and similar subjects Valid RBAC subjects now accepted: - system:authenticated (all authenticated users) - system:unauthenticated (anonymous access) - system:serviceaccount:namespace:name (service accounts) - user@domain.com (email addresses) Issue: https://issues.redhat.com/browse/RHOAIENG-46523 Co-Authored-By: Claude (claude-sonnet-4-5) --- components/backend/handlers/middleware.go | 21 ++++++++++++++ components/backend/handlers/permissions.go | 6 ++-- .../backend/handlers/permissions_test.go | 28 +++++++++---------- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/components/backend/handlers/middleware.go b/components/backend/handlers/middleware.go index 625f9f031..b6f311353 100644 --- a/components/backend/handlers/middleware.go +++ b/components/backend/handlers/middleware.go @@ -48,6 +48,27 @@ func isValidKubernetesName(name string) bool { return kubernetesNameRegex.MatchString(name) } +// isValidRBACSubject validates RBAC subject identifiers (user/group names in RoleBindings) +// RBAC subjects can contain colons (e.g., system:authenticated, system:serviceaccount:ns:name) +// Unlike Kubernetes resource names, RBAC subjects follow a different format (RFC 1123 subdomain) +// Returns false if: +// - name is empty (prevents empty string injection) +// - name exceeds 253 characters (max length for DNS subdomain) +// - name contains control characters (prevents injection attacks) +func isValidRBACSubject(name string) bool { + // Max length for RBAC subjects is 253 chars (same as DNS subdomain) + if len(name) == 0 || len(name) > 253 { + return false + } + // Reject control characters and newlines for security + for _, r := range name { + if r < 32 || r == 127 { + return false + } + } + return true +} + // ContentListItem represents a content list item for file browsing type ContentListItem struct { Name string `json:"name"` diff --git a/components/backend/handlers/permissions.go b/components/backend/handlers/permissions.go index 244d64923..4ed757d54 100644 --- a/components/backend/handlers/permissions.go +++ b/components/backend/handlers/permissions.go @@ -157,9 +157,9 @@ func AddProjectPermission(c *gin.Context) { return } - // Validate subject name is a valid Kubernetes resource name - if !isValidKubernetesName(req.SubjectName) { - c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid userName format. Must be a valid Kubernetes resource name."}) + // Validate subject name is a valid RBAC subject identifier + if !isValidRBACSubject(req.SubjectName) { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid subject name format"}) return } diff --git a/components/backend/handlers/permissions_test.go b/components/backend/handlers/permissions_test.go index f45bd400a..dc5dbcaf7 100644 --- a/components/backend/handlers/permissions_test.go +++ b/components/backend/handlers/permissions_test.go @@ -738,17 +738,11 @@ var _ = Describe("Permissions Handler", Ordered, Label(test_constants.LabelUnit, Context("Input Validation", func() { It("Should reject userNames with invalid characters", func() { invalidUserNames := []string{ - "user@domain.com", // @ not allowed in k8s resource names - "user/name", // / not allowed - "user\\name", // \ not allowed - "user:name", // : not allowed - "user name", // space not allowed - "user.name", // . not allowed in k8s resource names - "User-Name", // uppercase not allowed - "user-name-", // can't end with dash - "-user-name", // can't start with dash - "user..name", // consecutive dots not allowed - strings.Repeat("a", 64), // too long (>63 chars) + "user\nname", // newline not allowed (control character) + "user\tname", // tab not allowed (control character) + "user\x00name", // null byte not allowed (control character) + "user\x1fname", // control character not allowed + strings.Repeat("a", 254), // too long (>253 chars) } for _, userName := range invalidUserNames { @@ -770,7 +764,7 @@ var _ = Describe("Permissions Handler", Ordered, Label(test_constants.LabelUnit, httpUtils.AssertHTTPStatus(http.StatusBadRequest) httpUtils.AssertJSONContains(map[string]interface{}{ - "error": "Invalid userName format. Must be a valid Kubernetes resource name.", + "error": "Invalid subject name format", }) logger.Log("Correctly rejected invalid userName: %s", userName) @@ -782,8 +776,14 @@ var _ = Describe("Permissions Handler", Ordered, Label(test_constants.LabelUnit, "user-name", "user123", "123user", - "a", // single character - strings.Repeat("a", 63), // exactly 63 chars (max allowed) + "a", // single character + "system:authenticated", // RBAC group for all authenticated users + "system:unauthenticated", // RBAC group for anonymous access + "system:serviceaccount:test-project:default", // service account reference + "user@domain.com", // email addresses are valid RBAC subjects + "user.name", // dots are valid in RBAC subjects + "User-Name", // uppercase is valid in RBAC subjects + strings.Repeat("a", 253), // exactly 253 chars (max allowed for RBAC subjects) } for _, userName := range validUserNames {