Merge pull request #2075 from tchap/login-u-oidc

OCPBUGS-58393: oc login: Preserve ExecProvider on oc login -u

Author: openshift-merge-bot[bot]

pkg/cli/login/loginoptions.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/pkg/browser"
 
+	userv1 "github.com/openshift/api/user/v1"
 	projectv1typedclient "github.com/openshift/client-go/project/clientset/versioned/typed/project/v1"
 	"github.com/openshift/library-go/pkg/oauth/tokenrequest"
 	"github.com/openshift/library-go/pkg/oauth/tokenrequest/challengehandlers"
@@ -94,6 +95,10 @@ type LoginOptions struct {
 	RequestTimeout time.Duration
 
 	genericiooptions.IOStreams
+
+	// whoAmIFunc is used to mock project.WhoAmI. It's only being used in tests.
+	// See LoginOptions.whoAmI for details.
+	whoAmIFunc func(clientConfig *restclient.Config) (*userv1.User, error)
 }
 
 type passwordPrompter func(r io.Reader, w io.Writer, format string, a ...interface{}) string
@@ -234,7 +239,7 @@ func (o *LoginOptions) gatherAuthInfo() error {
 	// if a token were explicitly provided, try to use it
 	if o.tokenProvided() {
 		clientConfig.BearerToken = o.Token
-		me, err := project.WhoAmI(clientConfig)
+		me, err := o.whoAmI(clientConfig)
 		if err != nil {
 			if kerrors.IsUnauthorized(err) {
 				return fmt.Errorf("The token provided is invalid or expired.\n\n")
@@ -259,13 +264,18 @@ func (o *LoginOptions) gatherAuthInfo() error {
 			if matchingClusters.Has(context.Cluster) {
 				clientcmdConfig := kclientcmd.NewDefaultClientConfig(kubeconfig, &kclientcmd.ConfigOverrides{CurrentContext: key})
 				if kubeconfigClientConfig, err := clientcmdConfig.ClientConfig(); err == nil {
-					if me, err := project.WhoAmI(kubeconfigClientConfig); err == nil && (o.Username == me.Name) {
+					if me, err := o.whoAmI(kubeconfigClientConfig); err == nil && (o.Username == me.Name) {
 						clientConfig.BearerToken = kubeconfigClientConfig.BearerToken
 						clientConfig.CertFile = kubeconfigClientConfig.CertFile
 						clientConfig.CertData = kubeconfigClientConfig.CertData
 						clientConfig.KeyFile = kubeconfigClientConfig.KeyFile
 						clientConfig.KeyData = kubeconfigClientConfig.KeyData
 
+						// Preserve ExecProvider configuration.
+						if kubeconfigClientConfig.ExecProvider != nil {
+							clientConfig.ExecProvider = kubeconfigClientConfig.ExecProvider.DeepCopy()
+						}
+
 						o.Config = clientConfig
 
 						fmt.Fprintf(o.Out, "Logged into %q as %q using existing credentials.\n\n", o.Config.Host, o.Username)
@@ -284,7 +294,7 @@ func (o *LoginOptions) gatherAuthInfo() error {
 		}
 
 		clientConfig.ExecProvider = execProvider
-		me, err := project.WhoAmI(clientConfig)
+		me, err := o.whoAmI(clientConfig)
 		if err != nil {
 			return err
 		}
@@ -319,7 +329,7 @@ func (o *LoginOptions) gatherAuthInfo() error {
 
 	clientConfig.BearerToken = token
 
-	me, err := project.WhoAmI(clientConfig)
+	me, err := o.whoAmI(clientConfig)
 	if err != nil {
 		return err
 	}
@@ -565,6 +575,13 @@ func (o *LoginOptions) SaveConfig() (bool, error) {
 	return created, nil
 }
 
+func (o *LoginOptions) whoAmI(clientConfig *restclient.Config) (*userv1.User, error) {
+	if o.whoAmIFunc != nil {
+		return o.whoAmIFunc(clientConfig)
+	}
+	return project.WhoAmI(clientConfig)
+}
+
 func (o *LoginOptions) usernameProvided() bool {
 	return len(o.Username) > 0
 }

pkg/cli/login/loginoptions_test.go

@@ -11,12 +11,15 @@ import (
 	"testing"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/google/go-cmp/cmp"
 
 	kapierrs "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/cli-runtime/pkg/genericiooptions"
 	restclient "k8s.io/client-go/rest"
 	kclientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 
+	userv1 "github.com/openshift/api/user/v1"
 	"github.com/openshift/library-go/pkg/oauth/oauthdiscovery"
 	cliconfig "github.com/openshift/oc/pkg/helpers/kubeconfig"
 )
@@ -374,6 +377,114 @@ func TestDialToHTTPSServer(t *testing.T) {
 	}
 }
 
+func TestPreserveExecProviderOnUsernameLogin(t *testing.T) {
+	// Test that when using -u flag with existing OIDC credentials,
+	// the ExecProvider configuration is preserved
+
+	execProvider := &kclientcmdapi.ExecConfig{
+		APIVersion: "client.authentication.k8s.io/v1",
+		Command:    "oc",
+		Args: []string{
+			"get-token",
+			"--issuer-url=https://oauth.example.com",
+			"--client-id=test-client",
+			"--callback-address=127.0.0.1:8080",
+		},
+		InstallHint:     "Please be sure that oc is defined in $PATH to be executed as credentials exec plugin",
+		InteractiveMode: kclientcmdapi.IfAvailableExecInteractiveMode,
+	}
+
+	testCases := map[string]struct {
+		username             string
+		existingExecProvider *kclientcmdapi.ExecConfig
+		expectedExecProvider *kclientcmdapi.ExecConfig
+	}{
+		"preserve OIDC exec provider": {
+			username:             "oidc-user-test:[email protected]",
+			existingExecProvider: execProvider,
+			expectedExecProvider: execProvider,
+		},
+		"no exec provider preserved when none exists": {
+			username:             "regular-user",
+			existingExecProvider: nil,
+			expectedExecProvider: nil,
+		},
+	}
+
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			// Mock config constants.
+			serverURL := "https://api.test.devcluster.openshift.com:6443"
+			clusterNick := "api-test-devcluster-openshift-com:6443"
+			userNick := test.username + "/" + clusterNick
+			contextNick := "default/" + clusterNick + "/" + test.username
+
+			// Create starting kubeconfig with existing OIDC credentials.
+			startingConfig := &kclientcmdapi.Config{
+				Clusters: map[string]*kclientcmdapi.Cluster{
+					clusterNick: {
+						Server: serverURL,
+					},
+				},
+				AuthInfos: map[string]*kclientcmdapi.AuthInfo{
+					userNick: {
+						Exec: test.existingExecProvider,
+					},
+				},
+				Contexts: map[string]*kclientcmdapi.Context{
+					contextNick: {
+						Cluster:   clusterNick,
+						AuthInfo:  userNick,
+						Namespace: "default",
+					},
+				},
+				CurrentContext: contextNick,
+			}
+
+			// Setup LoginOptions with username and existing config
+			options := &LoginOptions{
+				Username:           test.username,
+				Server:             serverURL,
+				StartingKubeConfig: startingConfig,
+				Config: &restclient.Config{
+					Host:         serverURL,
+					ExecProvider: test.existingExecProvider,
+				},
+				IOStreams: genericiooptions.NewTestIOStreamsDiscard(),
+			}
+
+			// Mock the WhoAmI function to always match the user.
+			whoAmICalled := false
+			options.whoAmIFunc = func(clientConfig *restclient.Config) (*userv1.User, error) {
+				whoAmICalled = true
+				return &userv1.User{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: test.username,
+					},
+				}, nil
+			}
+
+			// Calling gatherAuthInfo should preserve ExecProvider.
+			if err := options.gatherAuthInfo(); err != nil {
+				t.Fatalf("gatherAuthInfo failed: %v", err)
+			}
+
+			// Verify ExecProvider is preserved correctly.
+			if options.Config == nil {
+				t.Fatal("LoginOptions.Config is nil")
+			}
+			if !cmp.Equal(options.Config.ExecProvider, test.expectedExecProvider) {
+				t.Errorf("expected provider mismatch: \n%s\n", cmp.Diff(test.expectedExecProvider, options.Config.ExecProvider))
+			}
+
+			// Just check additionally that whoAmI has been called.
+			if !whoAmICalled {
+				t.Errorf("WhoAmI function has not been called")
+			}
+		})
+	}
+}
+
 func newTLSServer(certString, keyString string) (*httptest.Server, error) {
 	invoked := make(chan struct{}, 1)
 	server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {