From 2321e60ec1a2c446f7b5edfc3b86142a7ced2526 Mon Sep 17 00:00:00 2001
From: Anders Eknert <anders.eknert@bisnode.com>
Date: Thu, 4 Jun 2020 00:12:05 +0200
Subject: [PATCH] Presence of bearer token should cancel exec action

If a bearer token is present in a request, the exec credential plugin should accept that as the chosen method of authentication. Judging by an [earlier comment in exec.go](https://github.com/kubernetes/kubernetes/blob/c18bc7e9f7a27d7d197053d98c52896e1dc3bb3e/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go#L217), this was already intended. This would however not work since UpdateTransportConfig would set the GetCert callback which would then get called by the transport, triggering the exec plugin action even with a token present in the request. See linked issue for further details.

See #87369 for further details.

Signed-off-by: Anders Eknert <anders.eknert@bisnode.com>

Kubernetes-commit: b423216a3b781009fb4ec4d5974eeb3f882f9d2d
---
 plugin/pkg/client/auth/exec/exec.go      |  9 +++++++++
 plugin/pkg/client/auth/exec/exec_test.go | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/plugin/pkg/client/auth/exec/exec.go b/plugin/pkg/client/auth/exec/exec.go
index a56abd29..627bb2de 100644
--- a/plugin/pkg/client/auth/exec/exec.go
+++ b/plugin/pkg/client/auth/exec/exec.go
@@ -237,6 +237,15 @@ type credentials struct {
 // UpdateTransportConfig updates the transport.Config to use credentials
 // returned by the plugin.
 func (a *Authenticator) UpdateTransportConfig(c *transport.Config) error {
+	// If a bearer token is present in the request - avoid the GetCert callback when
+	// 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() {
+		return nil
+	}
+
 	c.Wrap(func(rt http.RoundTripper) http.RoundTripper {
 		return &roundTripper{a, rt}
 	})
diff --git a/plugin/pkg/client/auth/exec/exec_test.go b/plugin/pkg/client/auth/exec/exec_test.go
index 8f5f6e60..43b9c087 100644
--- a/plugin/pkg/client/auth/exec/exec_test.go
+++ b/plugin/pkg/client/auth/exec/exec_test.go
@@ -651,6 +651,27 @@ func TestRoundTripper(t *testing.T) {
 	get(t, http.StatusOK)
 }
 
+func TestTokenPresentCancelsExecAction(t *testing.T) {
+	a, err := newAuthenticator(newCache(), &api.ExecConfig{
+		Command:    "./testdata/test-plugin.sh",
+		APIVersion: "client.authentication.k8s.io/v1alpha1",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// UpdateTransportConfig returns error on existing TLS certificate callback, unless a bearer token is present in the
+	// transport config, in which case it takes precedence
+	cert := func() (*tls.Certificate, error) {
+		return nil, nil
+	}
+	tc := &transport.Config{BearerToken: "token1", TLS: transport.TLSConfig{Insecure: true, GetCert: cert}}
+
+	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) {
 	now := time.Now()