diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go index 52655c0bcb3..ce24be17539 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go +++ b/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 } diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go index 6aee2e25216..d295391ef31 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go @@ -922,24 +922,47 @@ 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", - }, nil) - if err != nil { - t.Fatal(err) +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", + }, nil) + 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}} + // 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{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") + if err := a.UpdateTransportConfig(tc); err != nil { + t.Error("Expected presence of bearer token in config to cancel exec action") + } + }) } } diff --git a/test/cmd/authentication.sh b/test/cmd/authentication.sh index c5592a857d5..be1f36268f4 100644 --- a/test/cmd/authentication.sh +++ b/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 diff --git a/test/integration/client/exec_test.go b/test/integration/client/exec_test.go index 3d97d6b248b..785866f38a4 100644 --- a/test/integration/client/exec_test.go +++ b/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",