Merge pull request #12962 from juanvallejo/jvallejo/delete-sa-token-kubeconfig-logout

Merged by openshift-bot

Author: OpenShift Bot

pkg/cmd/cli/cmd/login/logout.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 
 	"k8s.io/kubernetes/pkg/client/restclient"
@@ -124,24 +125,29 @@ func (o LogoutOptions) RunLogout() error {
 	}
 
 	if err := client.OAuthAccessTokens().Delete(token); err != nil {
-		return err
+		glog.V(1).Infof("%v", err)
 	}
 
-	newConfig := *o.StartingKubeConfig
+	configErr := deleteTokenFromConfig(*o.StartingKubeConfig, o.PathOptions, token)
+	if configErr == nil {
+		glog.V(1).Infof("Removed token from your local configuration.")
+
+		// only return error instead of successful message if removing token from client
+		// config fails. Any error that occurs deleting token using api is logged above.
+		fmt.Fprintf(o.Out, "Logged %q out on %q\n", userInfo.Name, o.Config.Host)
+	}
+
+	return configErr
+}
 
-	for key, value := range newConfig.AuthInfos {
-		if value.Token == token {
+func deleteTokenFromConfig(config kclientcmdapi.Config, pathOptions *kclientcmd.PathOptions, bearerToken string) error {
+	for key, value := range config.AuthInfos {
+		if value.Token == bearerToken {
 			value.Token = ""
-			newConfig.AuthInfos[key] = value
+			config.AuthInfos[key] = value
 			// don't break, its possible that more than one user stanza has the same token.
 		}
 	}
 
-	if err := kclientcmd.ModifyConfig(o.PathOptions, newConfig, true); err != nil {
-		return err
-	}
-
-	fmt.Fprintf(o.Out, "Logged %q out on %q\n", userInfo.Name, o.Config.Host)
-
-	return nil
+	return kclientcmd.ModifyConfig(pathOptions, config, true)
 }

test/cmd/login.sh

@@ -87,6 +87,15 @@ os::cmd::expect_failure_and_text 'oc get pods' '"system:anonymous" cannot list p
 # os::cmd::expect_failure_and_text "oc login '${KUBERNETES_MASTER}' -u test -p test '--config=${templocation}/file' --insecure-skip-tls-verify" 'KUBECONFIG is set to a file that cannot be created or modified'
 echo "login warnings: ok"
 
+# login and create serviceaccount and test login and logout with a service account token
+os::cmd::expect_success "oc login ${KUBERNETES_MASTER} --certificate-authority='${MASTER_CONFIG_DIR}/ca.crt' -u test-user -p anything --api-version=v1"
+os::cmd::expect_success_and_text "oc create sa testserviceaccount" "serviceaccount \"testserviceaccount\" created"
+os::cmd::expect_success_and_text "oc login --token=$(oc sa get-token testserviceaccount)" "system:serviceaccount:project-foo:testserviceaccount"
+# attempt to logout successfully
+os::cmd::expect_success_and_text "oc logout" "Logged \"system:serviceaccount:project-foo:testserviceaccount\" out"
+# verify that the token is no longer present in our local config
+os::cmd::expect_failure_and_text "oc whoami" "User \"system:anonymous\" cannot get users"
+
 # log in and set project to use from now on
 os::cmd::expect_success "oc login --server=${KUBERNETES_MASTER} --certificate-authority='${MASTER_CONFIG_DIR}/ca.crt' -u test-user -p anything"
 os::cmd::expect_success 'oc get projects'