Merge pull request #20860 from runcom/fix-logout-credsStore

api: client: fix login/logout with creds store

api/client/login.go

@@ -13,7 +13,6 @@ import (
 	"github.com/docker/docker/cliconfig/credentials"
 	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/docker/pkg/term"
-	"github.com/docker/engine-api/client"
 	"github.com/docker/engine-api/types"
 )
 
@@ -55,11 +54,6 @@ func (cli *DockerCli) CmdLogin(args ...string) error {
 
 	response, err := cli.client.RegistryLogin(authConfig)
 	if err != nil {
-		if client.IsErrUnauthorized(err) {
-			if err2 := eraseCredentials(cli.configFile, authConfig.ServerAddress); err2 != nil {
-				fmt.Fprintf(cli.out, "WARNING: could not save credentials: %v\n", err2)
-			}
-		}
 		return err
 	}
 

api/client/logout.go

@@ -25,15 +25,16 @@ func (cli *DockerCli) CmdLogout(args ...string) error {
 		serverAddress = cli.electAuthServer()
 	}
 
+	// check if we're logged in based on the records in the config file
+	// which means it couldn't have user/pass cause they may be in the creds store
 	if _, ok := cli.configFile.AuthConfigs[serverAddress]; !ok {
 		fmt.Fprintf(cli.out, "Not logged in to %s\n", serverAddress)
 		return nil
 	}
 
 	fmt.Fprintf(cli.out, "Remove login credentials for %s\n", serverAddress)
-	delete(cli.configFile.AuthConfigs, serverAddress)
-	if err := cli.configFile.Save(); err != nil {
-		return fmt.Errorf("Failed to save docker config: %v", err)
+	if err := eraseCredentials(cli.configFile, serverAddress); err != nil {
+		fmt.Fprintf(cli.out, "WARNING: could not erase credentials: %v\n", err)
 	}
 
 	return nil

integration-cli/docker_cli_logout_test.go

@@ -0,0 +1,56 @@
+package main
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"github.com/docker/docker/pkg/integration/checker"
+	"github.com/go-check/check"
+)
+
+func (s *DockerRegistryAuthSuite) TestLogoutWithExternalAuth(c *check.C) {
+	osPath := os.Getenv("PATH")
+	defer os.Setenv("PATH", osPath)
+
+	workingDir, err := os.Getwd()
+	c.Assert(err, checker.IsNil)
+	absolute, err := filepath.Abs(filepath.Join(workingDir, "fixtures", "auth"))
+	c.Assert(err, checker.IsNil)
+	testPath := fmt.Sprintf("%s%c%s", osPath, filepath.ListSeparator, absolute)
+
+	os.Setenv("PATH", testPath)
+
+	repoName := fmt.Sprintf("%v/dockercli/busybox:authtest", privateRegistryURL)
+
+	tmp, err := ioutil.TempDir("", "integration-cli-")
+	c.Assert(err, checker.IsNil)
+
+	externalAuthConfig := `{ "credsStore": "shell-test" }`
+
+	configPath := filepath.Join(tmp, "config.json")
+	err = ioutil.WriteFile(configPath, []byte(externalAuthConfig), 0644)
+	c.Assert(err, checker.IsNil)
+
+	dockerCmd(c, "--config", tmp, "login", "-u", s.reg.username, "-p", s.reg.password, privateRegistryURL)
+
+	b, err := ioutil.ReadFile(configPath)
+	c.Assert(err, checker.IsNil)
+	c.Assert(string(b), checker.Not(checker.Contains), "\"auth\":")
+	c.Assert(string(b), checker.Contains, privateRegistryURL)
+
+	dockerCmd(c, "--config", tmp, "tag", "busybox", repoName)
+	dockerCmd(c, "--config", tmp, "push", repoName)
+
+	dockerCmd(c, "--config", tmp, "logout", privateRegistryURL)
+
+	b, err = ioutil.ReadFile(configPath)
+	c.Assert(err, checker.IsNil)
+	c.Assert(string(b), checker.Not(checker.Contains), privateRegistryURL)
+
+	// check I cannot pull anymore
+	out, _, err := dockerCmdWithError("--config", tmp, "pull", repoName)
+	c.Assert(err, check.NotNil, check.Commentf(out))
+	c.Assert(out, checker.Contains, fmt.Sprintf("Error: image dockercli/busybox not found"))
+}