From 0eccc3838e4aac5318e98dcbfbe2100e253462de Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Wed, 2 Mar 2016 14:26:29 +0100 Subject: [PATCH] api: client: fix login/logout with creds store Make sure credentials are removed from the store at logout (not only in the config file). Remove not needed error check and auth erasing at login (auths aren't stored anywhere at that point). Add regression test. Signed-off-by: Antonio Murdaca --- api/client/login.go | 6 --- api/client/logout.go | 7 +-- integration-cli/docker_cli_logout_test.go | 56 +++++++++++++++++++++++ 3 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 integration-cli/docker_cli_logout_test.go diff --git a/api/client/login.go b/api/client/login.go index 470cb5780c..f1915da1c1 100644 --- a/api/client/login.go +++ b/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 } diff --git a/api/client/logout.go b/api/client/logout.go index f81eb8dd12..b5ff59ddd2 100644 --- a/api/client/logout.go +++ b/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 diff --git a/integration-cli/docker_cli_logout_test.go b/integration-cli/docker_cli_logout_test.go new file mode 100644 index 0000000000..8b756146f1 --- /dev/null +++ b/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")) +}