fix(auth): resolve user UID via SelfSubjectReview for external authentication (#134)

* fix(auth): resolve user UID via SelfSubjectReview for BYO external auth

Use authentication.k8s.io SelfSubjectReview as the primary method to
obtain the authenticated user's Kubernetes UID when verifying requests
to /exec/init and /activity/tick. Fall back to the OpenShift User API
when SelfSubjectReview is unavailable, preserving compatibility with
older clusters.

Fixes Web Terminal failures with BYO External Authentication where
user.openshift.io/v1 is not available.

* fix : update error messages to be more generic

Signed-off-by: Rohan Kumar <rohaan@redhat.com>

* fix : handle empty UID case in token claims

Signed-off-by: Rohan Kumar <rohaan@redhat.com>

---------

Signed-off-by: Rohan Kumar <rohaan@redhat.com>

Author: rohanKanojia

README.md

@@ -43,7 +43,7 @@ kubectl exec -it <POD_NAME> <CONTAINER_NAME> -- <COMMAND>...
 ### Authentication
 Endpoints that require authentication expect a user's OpenShift token to be passed in a `X-Access-Token` or `X-Forwarded-Access-Token` header on the request. This token is used to
 
-1. Verify that the user making the request is the authorized user for the current terminal
+1. Verify that the user making the request is the authorized user for the current terminal, by resolving the user's Kubernetes UID via `SelfSubjectReview` (with fallback to the OpenShift User API when needed)
 2. Execute the pods/exec API call that interacts with the container into which kubeconfig is being injected (if applicable)
 
 If a token is not provided or does not match what is expected, the server returns `HTTP 401`

go.mod

@@ -4,49 +4,50 @@ go 1.25.7
 
 require (
 	github.com/sirupsen/logrus v1.9.1
-	github.com/stretchr/testify v1.8.0
-	k8s.io/api v0.25.4
-	k8s.io/apimachinery v0.25.4
-	k8s.io/client-go v0.25.4
-	sigs.k8s.io/yaml v1.2.0
+	github.com/stretchr/testify v1.8.4
+	k8s.io/api v0.29.14
+	k8s.io/apimachinery v0.29.14
+	k8s.io/client-go v0.29.14
+	sigs.k8s.io/yaml v1.3.0
 )
 
 require (
-	github.com/PuerkitoBio/purell v1.1.1 // indirect
-	github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/emicklei/go-restful/v3 v3.8.0 // indirect
+	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
 	github.com/evanphx/json-patch v4.12.0+incompatible // indirect
-	github.com/go-logr/logr v1.2.3 // indirect
-	github.com/go-openapi/jsonpointer v0.19.5 // indirect
-	github.com/go-openapi/jsonreference v0.19.5 // indirect
-	github.com/go-openapi/swag v0.19.14 // indirect
+	github.com/go-logr/logr v1.3.0 // indirect
+	github.com/go-openapi/jsonpointer v0.19.6 // indirect
+	github.com/go-openapi/jsonreference v0.20.2 // indirect
+	github.com/go-openapi/swag v0.22.3 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
-	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/google/gnostic v0.5.7-v3refs // indirect
-	github.com/google/gofuzz v1.1.0 // indirect
+	github.com/golang/protobuf v1.5.4 // indirect
+	github.com/google/gnostic-models v0.6.8 // indirect
+	github.com/google/gofuzz v1.2.0 // indirect
+	github.com/google/uuid v1.3.0 // indirect
+	github.com/gorilla/websocket v1.5.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
-	github.com/mailru/easyjson v0.7.6 // indirect
+	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/moby/spdystream v0.2.0 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	golang.org/x/net v0.38.0 // indirect
 	golang.org/x/oauth2 v0.27.0 // indirect
 	golang.org/x/sys v0.31.0 // indirect
 	golang.org/x/term v0.30.0 // indirect
 	golang.org/x/text v0.23.0 // indirect
-	golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 // indirect
+	golang.org/x/time v0.3.0 // indirect
 	google.golang.org/protobuf v1.33.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/klog/v2 v2.70.1 // indirect
-	k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1 // indirect
-	k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed // indirect
-	sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
-	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
+	k8s.io/klog/v2 v2.110.1 // indirect
+	k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect
+	k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
+	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
+	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 )

go.sum

@@ -26,7 +26,8 @@ func Authenticate(r *http.Request, clientProvider operations.ClientProvider) err
 	}
 	uid, err := operations.GetCurrentUserUID(token, clientProvider)
 	if err != nil {
-		return fmt.Errorf("unable to verify user: %s", err)
+		logrus.Errorf("Unable to verify user: %v", err)
+		return fmt.Errorf("unable to verify user")
 	}
 	if uid != config.AuthenticatedUserID {
 		logrus.Debugf("User failed to authenticate: authorized user = '%s', requested user = '%s'", config.AuthenticatedUserID, uid)

pkg/auth/user.go

@@ -17,13 +17,23 @@ import (
 	"github.com/redhat-developer/web-terminal-exec/pkg/config"
 	"github.com/redhat-developer/web-terminal-exec/pkg/operations/test"
 	"github.com/stretchr/testify/assert"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/dynamic"
+	fakedynamic "k8s.io/client-go/dynamic/fake"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/rest"
+	k8stesting "k8s.io/client-go/testing"
 )
 
 func TestAuthenticate(t *testing.T) {
 	tests := []struct {
 		name      string
 		headers   http.Header
 		errRegexp string
+		provider  operationsClientProvider
 	}{
 		{
 			name:      "No auth token in request",
@@ -35,6 +45,12 @@ func TestAuthenticate(t *testing.T) {
 			headers:   http.Header{"X-Access-Token": []string{"incorrect"}},
 			errRegexp: "the current user is not authorized to access this web terminal",
 		},
+		{
+			name:      "SelfSubjectReview failure",
+			headers:   http.Header{"X-Access-Token": []string{testToken}},
+			errRegexp: "unable to verify user",
+			provider:  selfSubjectReviewErrorClientProvider{},
+		},
 		{
 			name:    "Correct token",
 			headers: http.Header{"X-Access-Token": []string{testToken}},
@@ -45,17 +61,48 @@ func TestAuthenticate(t *testing.T) {
 			config.AuthenticatedUserID = testToken
 			defer config.ResetConfigForTest()
 
-			clientProvider := test.FakeClientProvider{
-				UserToken: testToken,
+			clientProvider := tt.provider
+			if clientProvider == nil {
+				clientProvider = test.FakeClientProvider{
+					UserToken: testToken,
+				}
 			}
 			req := &http.Request{Header: tt.headers}
 			err := Authenticate(req, clientProvider)
 			if tt.errRegexp != "" {
 				assert.Error(t, err)
 				assert.Regexp(t, tt.errRegexp, err.Error())
+				if tt.name == "SelfSubjectReview failure" {
+					assert.Equal(t, "unable to verify user", err.Error())
+					assert.NotContains(t, err.Error(), "not found")
+				}
 			} else {
 				assert.NoError(t, err)
 			}
 		})
 	}
 }
+
+type operationsClientProvider interface {
+	NewDevWorkspaceClient() (dynamic.Interface, *rest.Config, error)
+	NewClientWithToken(token string) (kubernetes.Interface, *rest.Config, error)
+	NewOpenShiftUserClient(token string) (dynamic.Interface, *rest.Config, error)
+}
+
+type selfSubjectReviewErrorClientProvider struct{}
+
+func (selfSubjectReviewErrorClientProvider) NewDevWorkspaceClient() (dynamic.Interface, *rest.Config, error) {
+	return nil, nil, nil
+}
+
+func (selfSubjectReviewErrorClientProvider) NewClientWithToken(string) (kubernetes.Interface, *rest.Config, error) {
+	client := fake.NewSimpleClientset()
+	client.PrependReactor("create", "selfsubjectreviews", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
+		return true, nil, apierrors.NewNotFound(schema.GroupResource{Group: "authentication.k8s.io", Resource: "selfsubjectreviews"}, "self")
+	})
+	return client, &rest.Config{}, nil
+}
+
+func (selfSubjectReviewErrorClientProvider) NewOpenShiftUserClient(string) (dynamic.Interface, *rest.Config, error) {
+	return fakedynamic.NewSimpleDynamicClient(&runtime.Scheme{}), &rest.Config{}, nil
+}

pkg/auth/user_test.go

@@ -111,7 +111,7 @@ func (defaultClientProvider) NewOpenShiftUserClient(token string) (dynamic.Inter
 	}
 
 	config.APIPath = "/apis"
-	config.GroupVersion = &devworkspaceGroupVersion
+	config.GroupVersion = &userGroupVersion
 	config.BearerToken = token
 	config.BearerTokenFile = ""
 

pkg/operations/clients.go

@@ -18,6 +18,7 @@ import (
 	"strings"
 
 	"github.com/redhat-developer/web-terminal-exec/pkg/config"
+	authenticationv1 "k8s.io/api/authentication/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -116,14 +117,59 @@ func GetCurrentWorkspacePod(client kubernetes.Interface) (*corev1.Pod, error) {
 }
 
 func GetCurrentUserUID(token string, clientProvider ClientProvider) (string, error) {
+	uid, err := getCurrentUserUIDFromSelfSubjectReview(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)
+	if fallbackErr == nil {
+		return uid, nil
+	}
+
+	return "", fmt.Errorf(
+		"failed to get current user information: SelfSubjectReview error: %w; OpenShift User API error: %w",
+		err,
+		fallbackErr,
+	)
+}
+
+func getCurrentUserUIDFromSelfSubjectReview(token string, clientProvider ClientProvider) (string, error) {
+	client, _, err := clientProvider.NewClientWithToken(token)
+	if err != nil {
+		return "", fmt.Errorf("failed to create client to check user info: %w", err)
+	}
+	review, err := client.AuthenticationV1().SelfSubjectReviews().Create(
+		context.Background(),
+		&authenticationv1.SelfSubjectReview{},
+		metav1.CreateOptions{},
+	)
+	if err != nil {
+		return "", err
+	}
+
+	if review.Status.UserInfo.UID == "" {
+		return "", fmt.Errorf("SelfSubjectReview returned empty UID")
+	}
+
+	return review.Status.UserInfo.UID, nil
+}
+
+func getCurrentUserUIDFromOpenShiftUserAPI(token string, clientProvider ClientProvider) (string, error) {
 	userClient, _, err := clientProvider.NewOpenShiftUserClient(token)
 	if err != nil {
-		return "", fmt.Errorf("failed to create client to check user info")
+		return "", err
 	}
-	userInfo, err := userClient.Resource(userGVR).Namespace("").Get(context.TODO(), "~", metav1.GetOptions{})
+	userInfo, err := userClient.Resource(userGVR).Namespace("").Get(context.Background(), "~", metav1.GetOptions{})
 	if err != nil {
-		return "", fmt.Errorf("failed to get current user information: %s", err)
+		return "", err
+	}
+
+	uid := string(userInfo.GetUID())
+	if uid == "" {
+		return "", fmt.Errorf("OpenShift User API returned empty UID")
 	}
 
-	return string(userInfo.GetUID()), nil
+	return uid, nil
 }