exec credential provider: don't run exec plugin with basic auth

If a user specifies basic auth, then apply the same short circuit logic
that we do for bearer tokens (see comment).

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go

@@ -263,8 +263,9 @@ func (a *Authenticator) UpdateTransportConfig(c *transport.Config) error {
 	// setting up the transport, as that triggers the exec action if the server is
 	// also configured to allow client certificates for authentication. For requests
 	// like "kubectl get --token (token) pods" we should assume the intention is to
-	// use the provided token for authentication.
-	if c.HasTokenAuth() {
+	// use the provided token for authentication. The same can be said for when the
+	// user specifies basic auth.
+	if c.HasTokenAuth() || c.HasBasicAuth() {
 		return nil
 	}
 

staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go

@@ -922,7 +922,27 @@ func TestRoundTripper(t *testing.T) {
 	get(t, http.StatusOK)
 }
 
-func TestTokenPresentCancelsExecAction(t *testing.T) {
+func TestAuthorizationHeaderPresentCancelsExecAction(t *testing.T) {
+	tests := []struct {
+		name               string
+		setTransportConfig func(*transport.Config)
+	}{
+		{
+			name: "bearer token",
+			setTransportConfig: func(config *transport.Config) {
+				config.BearerToken = "token1f"
+			},
+		},
+		{
+			name: "basic auth",
+			setTransportConfig: func(config *transport.Config) {
+				config.Username = "marshmallow"
+				config.Password = "zelda"
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
 			a, err := newAuthenticator(newCache(), &api.ExecConfig{
 				Command:    "./testdata/test-plugin.sh",
 				APIVersion: "client.authentication.k8s.io/v1alpha1",
@@ -936,11 +956,14 @@ func TestTokenPresentCancelsExecAction(t *testing.T) {
 			cert := func() (*tls.Certificate, error) {
 				return nil, nil
 			}
-	tc := &transport.Config{BearerToken: "token1", TLS: transport.TLSConfig{Insecure: true, GetCert: cert}}
+			tc := &transport.Config{TLS: transport.TLSConfig{Insecure: true, GetCert: cert}}
+			test.setTransportConfig(tc)
 
 			if err := a.UpdateTransportConfig(tc); err != nil {
 				t.Error("Expected presence of bearer token in config to cancel exec action")
 			}
+		})
+	}
 }
 
 func TestTLSCredentials(t *testing.T) {

test/cmd/authentication.sh

@@ -74,7 +74,60 @@ EOF
   fi
   # Post-condition: None
 
+  cat > "${TMPDIR:-/tmp}"/valid_exec_plugin.yaml << EOF
+apiVersion: v1
+clusters:
+- cluster:
+  name: test
+contexts:
+- context:
+    cluster: test
+    user: valid_token_user
+  name: test
+current-context: test
+kind: Config
+preferences: {}
+users:
+- name: valid_token_user
+  user:
+    exec:
+      apiVersion: client.authentication.k8s.io/v1beta1
+      # Any invalid exec credential plugin will do to demonstrate
+      command: echo
+      args:
+        - '{"apiVersion":"client.authentication.k8s.io/v1beta1","status":{"token":"admin-token"}}'
+EOF
+
+  ### Valid exec plugin should authenticate user properly
+  # Pre-condition: Client certificate authentication enabled on the API server - already checked by positive test above
+
+  # Command
+  output3=$(kubectl "${kube_flags_without_token[@]:?}" --kubeconfig="${TMPDIR:-/tmp}"/valid_exec_plugin.yaml get namespace kube-system -o name 2>&1 || true)
+
+  if [[ "${output3}" == "namespace/kube-system" ]]; then
+    kube::log::status "exec credential plugin triggered and provided valid credentials"
+  else
+    kube::log::status "Unexpected output when using valid exec credential plugin for authentication. Output: ${output3}"
+    exit 1
+  fi
+  # Post-condition: None
+
+  ### Provided --username/--password should take precedence, thus not triggering the (valid) exec credential plugin
+  # Pre-condition: Client certificate authentication enabled on the API server - already checked by positive test above
+
+  # Command
+  output4=$(kubectl "${kube_flags_without_token[@]:?}" --username bad --password wrong --kubeconfig="${TMPDIR:-/tmp}"/valid_exec_plugin.yaml get namespace kube-system -o name 2>&1 || true)
+
+  if [[ "${output4}" =~ "Unauthorized" ]]; then
+    kube::log::status "exec credential plugin not triggered since kubectl was called with provided --username/--password"
+  else
+    kube::log::status "Unexpected output when providing --username/--password for authentication - exec credential plugin likely triggered. Output: ${output4}"
+    exit 1
+  fi
+  # Post-condition: None
+
   rm "${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml
+  rm "${TMPDIR:-/tmp}"/valid_exec_plugin.yaml
 
   set +o nounset
   set +o errexit

test/integration/client/exec_test.go

@@ -329,12 +329,8 @@ func TestExecPluginViaClient(t *testing.T) {
 				c.Password = "unauthorized"
 			},
 			wantAuthorizationHeaderValues: [][]string{{"Basic " + basicAuthHeaderValue("unauthorized", "unauthorized")}},
-			wantCertificate:               &tls.Certificate{},
 			wantClientErrorPrefix:         "Unauthorized",
-			// I don't think we should be calling the exec plugin here. We don't call the exec
-			// plugin in the case where bearer tokens are already present, and this case is
-			// similar. See https://github.com/kubernetes/kubernetes/pull/102175.
-			wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}},
+			wantMetrics:                   &execPluginMetrics{},
 		},
 		{
 			name: "good token with static auth bearer token favors static auth bearer token",