fix(auth): remove check restricting empty UID to handle kube:admin bootstrap user scenario (#135)

* fix(auth): allow empty UID for kube:admin bootstrap user

SelfSubjectReview and the OpenShift User API return no UID for kube:admin,
which is expected for the bootstrap admin. PR #134 rejected empty UIDs and
broke web terminal auth on clusters where oc whoami is kube:admin.

Restore pre-#134 semantics for empty UID while keeping SelfSubjectReview as
the primary lookup for external authentication (WTO-399).

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Rohan Kumar <rohaan@redhat.com>

* fix(auth): prefer OpenShift User API for UID lookup with SSR fallback

Resolve kube:admin via the OpenShift User API first, allowing empty UID
for bootstrap users, and fall back to SelfSubjectReview only when the User
API is unavailable while rejecting empty UIDs on that path.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

Author: rohanKanojia

pkg/operations/operations.go

@@ -117,19 +117,20 @@ func GetCurrentWorkspacePod(client kubernetes.Interface) (*corev1.Pod, error) {
 }
 
 func GetCurrentUserUID(token string, clientProvider ClientProvider) (string, error) {
-	uid, err := getCurrentUserUIDFromSelfSubjectReview(token, clientProvider)
+	uid, err := getCurrentUserUIDFromOpenShiftUserAPI(token, clientProvider)
 	if err == nil {
 		return uid, nil
 	}
 
-	// Fall back to the OpenShift User API on clusters where SelfSubjectReview is unavailable.
-	uid, fallbackErr := getCurrentUserUIDFromOpenShiftUserAPI(token, clientProvider)
+	// Fall back to SelfSubjectReview on clusters where the OpenShift User API is unavailable
+	// (e.g. BYO external authentication without user.openshift.io).
+	uid, fallbackErr := getCurrentUserUIDFromSelfSubjectReview(token, clientProvider)
 	if fallbackErr == nil {
 		return uid, nil
 	}
 
 	return "", fmt.Errorf(
-		"failed to get current user information: SelfSubjectReview error: %w; OpenShift User API error: %w",
+		"failed to get current user information: OpenShift User API error: %w; SelfSubjectReview error: %w",
 		err,
 		fallbackErr,
 	)
@@ -166,10 +167,7 @@ func getCurrentUserUIDFromOpenShiftUserAPI(token string, clientProvider ClientPr
 		return "", err
 	}
 
-	uid := string(userInfo.GetUID())
-	if uid == "" {
-		return "", fmt.Errorf("OpenShift User API returned empty UID")
-	}
-
-	return uid, nil
+	// kube:admin / kubeadmin have no Kubernetes UID; empty string is a valid identifier
+	// when AUTHENTICATED_USER_ID is also empty (see config.AuthenticatedUserID).
+	return string(userInfo.GetUID()), nil
 }

pkg/operations/operations_test.go

@@ -46,50 +46,50 @@ func TestGetCurrentUserUID(t *testing.T) {
 		expectedUID string
 	}{
 		{
-			name: "Should return UID from SelfSubjectReview",
+			name: "Should return UID from OpenShift User API",
 			provider: testUserIDClientProvider{
-				userUID: expectedUID,
+				userAPIUID: expectedUID,
 			},
 			expectedUID: expectedUID,
 		},
 		{
-			name: "Should return error when client creation fails",
+			name: "Should fall back to SelfSubjectReview when OpenShift User API is unavailable",
 			provider: testUserIDClientProvider{
-				returnClientError: true,
+				returnUserAPIError: apierrors.NewNotFound(schema.GroupResource{Group: "user.openshift.io", Resource: "users"}, "~"),
+				userUID:            expectedUID,
 			},
-			errRegexp: "failed to create client to check user info",
+			expectedUID: expectedUID,
 		},
 		{
-			name: "Should fall back to OpenShift User API when SelfSubjectReview is unavailable",
+			name: "Should return error when client creation fails",
 			provider: testUserIDClientProvider{
-				returnReviewError: apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self"),
-				userAPIUID:        expectedUID,
+				returnClientError: true,
 			},
-			expectedUID: expectedUID,
+			errRegexp: "failed to create client to check user info",
 		},
 		{
 			name: "Should return error when both user lookups fail",
 			provider: testUserIDClientProvider{
-				returnReviewError:  apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self"),
 				returnUserAPIError: apierrors.NewNotFound(schema.GroupResource{Group: "user.openshift.io", Resource: "users"}, "~"),
+				returnReviewError:  apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self"),
 			},
 			errRegexp: "failed to get current user information",
 		},
 		{
-			name: "Should return error when SelfSubjectReview returns empty UID",
+			name: "Should allow empty UID from OpenShift User API for kube:admin",
 			provider: testUserIDClientProvider{
-				userUID: "",
+				userAPIUID:      "",
+				emptyUserAPIUID: true,
 			},
-			errRegexp: "SelfSubjectReview returned empty UID",
+			expectedUID: "",
 		},
 		{
-			name: "Should return error when OpenShift User API returns empty UID",
+			name: "Should return error when SelfSubjectReview returns empty UID on fallback",
 			provider: testUserIDClientProvider{
-				returnReviewError: apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self"),
-				userAPIUID:        "",
-				emptyUserAPIUID:   true,
+				returnUserAPIError: apierrors.NewNotFound(schema.GroupResource{Group: "user.openshift.io", Resource: "users"}, "~"),
+				userUID:            "",
 			},
-			errRegexp: "OpenShift User API returned empty UID",
+			errRegexp: "SelfSubjectReview returned empty UID",
 		},
 	}
 
@@ -100,8 +100,8 @@ func TestGetCurrentUserUID(t *testing.T) {
 				assert.Error(t, err)
 				assert.Regexp(t, tt.errRegexp, err.Error())
 				if tt.name == "Should return error when both user lookups fail" {
-					assert.Contains(t, err.Error(), "SelfSubjectReview error:")
 					assert.Contains(t, err.Error(), "OpenShift User API error:")
+					assert.Contains(t, err.Error(), "SelfSubjectReview error:")
 				}
 				return
 			}