Skip to content

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

Merged
dkwon17 merged 2 commits into
redhat-developer:mainfrom
rohankanojia-forks:fix/kube-admin-empty-uid
Jun 25, 2026
Merged

fix(auth): remove check restricting empty UID to handle kube:admin bootstrap user scenario#135
dkwon17 merged 2 commits into
redhat-developer:mainfrom
rohankanojia-forks:fix/kube-admin-empty-uid

Conversation

@rohanKanojia

@rohanKanojia rohanKanojia commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a regression introduced in #134 where web-terminal-exec rejects users with an empty Kubernetes UID, breaking web terminal auth for bootstrap kube:admin on clusters where oc whoami returns kube:admin.

#134 added SelfSubjectReview (needed for WTO-399 / external auth), but also added:

if review.Status.UserInfo.UID == "" {
    return "", fmt.Errorf("SelfSubjectReview returned empty UID")
}

Before #134, we already allowed empty UIDs, the User API path simply returned string(userInfo.GetUID()) with no blank check, so "" was valid and "" == "" auth succeeded for kube:admin (see openshift/origin#24950).

After #134, the new checks fail auth before we reach that comparison.
Screenshot 2026-06-25 at 14 15 45

This PR restores that behavior with two changes:

  1. Reordered UID lookup — try the OpenShift User API first, then fall back to SelfSubjectReview only
    when the User API is unavailable (e.g. BYO external authentication without user.openshift.io). This
    matches pre-fix(auth): resolve user UID via SelfSubjectReview for external authentication #134 ordering and lets kube:admin resolve via the User API before hitting the stricter
    fallback path.

  2. Partial empty-UID check — allow an empty UID from the OpenShift User API (valid for kube:admin /
    kubeadmin when AUTHENTICATED_USER_ID is also empty), but keep the empty-UID rejection on the
    SelfSubjectReview fallback path so external-auth users without a UID still fail clearly rather than
    silently authenticating.

What issues does this PR fix or reference?

fixes issue found during pre-release-testing, kube:admin users were unable to open terminal

Is it tested? How?

Verified on aws clusterbot CI cluster (oc whoami = kube:admin):

SelfSubjectReview: userInfo.username=kube:admin, no uid field
User API (~): metadata.name=kube:admin, no metadata.uid

Bootstrap kube:admin has no Kubernetes UID.

  1. Deploy WTO with current staging RC
  2. Open the web terminal as kube:admin
  3. You'll see error
  4. Patch Web Terminal Exec template with this image based on this PR
oc patch devworkspacetemplate web-terminal-exec -n "${NS:-openshift-operators}" --type='json'  -p='[{"op":"replace","path":"/spec/components/0/container/image","value":"docker.io/rohankanojia/web-terminal-exec:fix-kube-admin-empty-uid"}]'
  1. Delete DevWorkspace for old terminal and then open again
  2. Terminal should load now as expected

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

Restore pre-redhat-developer#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>
@rohanKanojia rohanKanojia changed the title fix(auth): allow empty UID for kube:admin bootstrap user fix(auth): remove check restricting empty UID to handle kube:admin bootstrap user scenario Jun 25, 2026
@rohanKanojia rohanKanojia marked this pull request as ready for review June 25, 2026 12:35
Comment thread pkg/operations/operations.go Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT if we remove the empty UID string check in getCurrentUserUIDFromOpenShiftUserAPI, but keep it in getCurrentUserUIDFromSelfSubjectReview, but reorder the checks so that we call getCurrentUserUIDFromOpenShiftUserAPI first, and treat getCurrentUserUIDFromSelfSubjectReview as the fallback?

This way, this should fix the regression with kube:admin without completely ignoring the empty UID case?

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>
@dkwon17 dkwon17 merged commit a186a18 into redhat-developer:main Jun 25, 2026
2 checks passed
@rohanKanojia rohanKanojia deleted the fix/kube-admin-empty-uid branch June 25, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants