Skip to content

Commit 923ece6

Browse files
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>
1 parent a005dee commit 923ece6

2 files changed

Lines changed: 29 additions & 24 deletions

File tree

pkg/operations/operations.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,19 +117,20 @@ func GetCurrentWorkspacePod(client kubernetes.Interface) (*corev1.Pod, error) {
117117
}
118118

119119
func GetCurrentUserUID(token string, clientProvider ClientProvider) (string, error) {
120-
uid, err := getCurrentUserUIDFromSelfSubjectReview(token, clientProvider)
120+
uid, err := getCurrentUserUIDFromOpenShiftUserAPI(token, clientProvider)
121121
if err == nil {
122122
return uid, nil
123123
}
124124

125-
// Fall back to the OpenShift User API on clusters where SelfSubjectReview is unavailable.
126-
uid, fallbackErr := getCurrentUserUIDFromOpenShiftUserAPI(token, clientProvider)
125+
// Fall back to SelfSubjectReview on clusters where the OpenShift User API is unavailable
126+
// (e.g. BYO external authentication without user.openshift.io).
127+
uid, fallbackErr := getCurrentUserUIDFromSelfSubjectReview(token, clientProvider)
127128
if fallbackErr == nil {
128129
return uid, nil
129130
}
130131

131132
return "", fmt.Errorf(
132-
"failed to get current user information: SelfSubjectReview error: %w; OpenShift User API error: %w",
133+
"failed to get current user information: OpenShift User API error: %w; SelfSubjectReview error: %w",
133134
err,
134135
fallbackErr,
135136
)
@@ -149,8 +150,10 @@ func getCurrentUserUIDFromSelfSubjectReview(token string, clientProvider ClientP
149150
return "", err
150151
}
151152

152-
// kube:admin / kubeadmin have no Kubernetes UID; empty string is a valid identifier
153-
// when AUTHENTICATED_USER_ID is also empty (see config.AuthenticatedUserID).
153+
if review.Status.UserInfo.UID == "" {
154+
return "", fmt.Errorf("SelfSubjectReview returned empty UID")
155+
}
156+
154157
return review.Status.UserInfo.UID, nil
155158
}
156159

@@ -164,5 +167,7 @@ func getCurrentUserUIDFromOpenShiftUserAPI(token string, clientProvider ClientPr
164167
return "", err
165168
}
166169

170+
// kube:admin / kubeadmin have no Kubernetes UID; empty string is a valid identifier
171+
// when AUTHENTICATED_USER_ID is also empty (see config.AuthenticatedUserID).
167172
return string(userInfo.GetUID()), nil
168173
}

pkg/operations/operations_test.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,50 +46,50 @@ func TestGetCurrentUserUID(t *testing.T) {
4646
expectedUID string
4747
}{
4848
{
49-
name: "Should return UID from SelfSubjectReview",
49+
name: "Should return UID from OpenShift User API",
5050
provider: testUserIDClientProvider{
51-
userUID: expectedUID,
51+
userAPIUID: expectedUID,
5252
},
5353
expectedUID: expectedUID,
5454
},
5555
{
56-
name: "Should return error when client creation fails",
56+
name: "Should fall back to SelfSubjectReview when OpenShift User API is unavailable",
5757
provider: testUserIDClientProvider{
58-
returnClientError: true,
58+
returnUserAPIError: apierrors.NewNotFound(schema.GroupResource{Group: "user.openshift.io", Resource: "users"}, "~"),
59+
userUID: expectedUID,
5960
},
60-
errRegexp: "failed to create client to check user info",
61+
expectedUID: expectedUID,
6162
},
6263
{
63-
name: "Should fall back to OpenShift User API when SelfSubjectReview is unavailable",
64+
name: "Should return error when client creation fails",
6465
provider: testUserIDClientProvider{
65-
returnReviewError: apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self"),
66-
userAPIUID: expectedUID,
66+
returnClientError: true,
6767
},
68-
expectedUID: expectedUID,
68+
errRegexp: "failed to create client to check user info",
6969
},
7070
{
7171
name: "Should return error when both user lookups fail",
7272
provider: testUserIDClientProvider{
73-
returnReviewError: apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self"),
7473
returnUserAPIError: apierrors.NewNotFound(schema.GroupResource{Group: "user.openshift.io", Resource: "users"}, "~"),
74+
returnReviewError: apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self"),
7575
},
7676
errRegexp: "failed to get current user information",
7777
},
7878
{
79-
name: "Should allow empty UID from SelfSubjectReview for kube:admin",
79+
name: "Should allow empty UID from OpenShift User API for kube:admin",
8080
provider: testUserIDClientProvider{
81-
userUID: "",
81+
userAPIUID: "",
82+
emptyUserAPIUID: true,
8283
},
8384
expectedUID: "",
8485
},
8586
{
86-
name: "Should allow empty UID from OpenShift User API for kube:admin",
87+
name: "Should return error when SelfSubjectReview returns empty UID on fallback",
8788
provider: testUserIDClientProvider{
88-
returnReviewError: apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self"),
89-
userAPIUID: "",
90-
emptyUserAPIUID: true,
89+
returnUserAPIError: apierrors.NewNotFound(schema.GroupResource{Group: "user.openshift.io", Resource: "users"}, "~"),
90+
userUID: "",
9191
},
92-
expectedUID: "",
92+
errRegexp: "SelfSubjectReview returned empty UID",
9393
},
9494
}
9595

@@ -100,8 +100,8 @@ func TestGetCurrentUserUID(t *testing.T) {
100100
assert.Error(t, err)
101101
assert.Regexp(t, tt.errRegexp, err.Error())
102102
if tt.name == "Should return error when both user lookups fail" {
103-
assert.Contains(t, err.Error(), "SelfSubjectReview error:")
104103
assert.Contains(t, err.Error(), "OpenShift User API error:")
104+
assert.Contains(t, err.Error(), "SelfSubjectReview error:")
105105
}
106106
return
107107
}

0 commit comments

Comments
 (0)