1. docker
  2. Commit 0eccc3838e4aac5318e98dcbfbe2100e253462de
Browse code

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 <runcom@redhat.com>

Antonio Murdaca authored on 2016/03/02 22:26:29
Showing 3 changed files
api/client/login.go
History View file @ 0eccc38
... ... 
@@ -13,7 +13,6 @@ import (
13 13 
 	"github.com/docker/docker/cliconfig/credentials"
14 14 
 	flag "github.com/docker/docker/pkg/mflag"
15 15 
 	"github.com/docker/docker/pkg/term"
16 
-	"github.com/docker/engine-api/client"
17 16 
 	"github.com/docker/engine-api/types"
18 17 
 )
19 18
... ... 
@@ -55,11 +54,6 @@ func (cli *DockerCli) CmdLogin(args ...string) error {
55 55
56 56 
 	response, err := cli.client.RegistryLogin(authConfig)
57 57 
 	if err != nil {
58 
-		if client.IsErrUnauthorized(err) {
59 
-			if err2 := eraseCredentials(cli.configFile, authConfig.ServerAddress); err2 != nil {
60 
-				fmt.Fprintf(cli.out, "WARNING: could not save credentials: %v\n", err2)
61 
-			}
62 
-		}
63 58 
 		return err
64 59 
 	}
65 60
api/client/logout.go
History View file @ 0eccc38
... ... 
@@ -25,15 +25,16 @@ func (cli *DockerCli) CmdLogout(args ...string) error {
25 25 
 		serverAddress = cli.electAuthServer()
26 26 
 	}
27 27
28 
+	// check if we're logged in based on the records in the config file
29 
+	// which means it couldn't have user/pass cause they may be in the creds store
28 30 
 	if _, ok := cli.configFile.AuthConfigs[serverAddress]; !ok {
29 31 
 		fmt.Fprintf(cli.out, "Not logged in to %s\n", serverAddress)
30 32 
 		return nil
31 33 
 	}
32 34
33 35 
 	fmt.Fprintf(cli.out, "Remove login credentials for %s\n", serverAddress)
34 
-	delete(cli.configFile.AuthConfigs, serverAddress)
35 
-	if err := cli.configFile.Save(); err != nil {
36 
-		return fmt.Errorf("Failed to save docker config: %v", err)
36 
+	if err := eraseCredentials(cli.configFile, serverAddress); err != nil {
37 
+		fmt.Fprintf(cli.out, "WARNING: could not erase credentials: %v\n", err)
37 38 
 	}
38 39
39 40 
 	return nil
integration-cli/docker_cli_logout_test.go
History View file @ 0eccc38
40 41 
new file mode 100644
... ... 
@@ -0,0 +1,56 @@
0 
+package main
1 
+
2 
+import (
3 
+	"fmt"
4 
+	"io/ioutil"
5 
+	"os"
6 
+	"path/filepath"
7 
+
8 
+	"github.com/docker/docker/pkg/integration/checker"
9 
+	"github.com/go-check/check"
10 
+)
11 
+
12 
+func (s *DockerRegistryAuthSuite) TestLogoutWithExternalAuth(c *check.C) {
13 
+	osPath := os.Getenv("PATH")
14 
+	defer os.Setenv("PATH", osPath)
15 
+
16 
+	workingDir, err := os.Getwd()
17 
+	c.Assert(err, checker.IsNil)
18 
+	absolute, err := filepath.Abs(filepath.Join(workingDir, "fixtures", "auth"))
19 
+	c.Assert(err, checker.IsNil)
20 
+	testPath := fmt.Sprintf("%s%c%s", osPath, filepath.ListSeparator, absolute)
21 
+
22 
+	os.Setenv("PATH", testPath)
23 
+
24 
+	repoName := fmt.Sprintf("%v/dockercli/busybox:authtest", privateRegistryURL)
25 
+
26 
+	tmp, err := ioutil.TempDir("", "integration-cli-")
27 
+	c.Assert(err, checker.IsNil)
28 
+
29 
+	externalAuthConfig := `{ "credsStore": "shell-test" }`
30 
+
31 
+	configPath := filepath.Join(tmp, "config.json")
32 
+	err = ioutil.WriteFile(configPath, []byte(externalAuthConfig), 0644)
33 
+	c.Assert(err, checker.IsNil)
34 
+
35 
+	dockerCmd(c, "--config", tmp, "login", "-u", s.reg.username, "-p", s.reg.password, privateRegistryURL)
36 
+
37 
+	b, err := ioutil.ReadFile(configPath)
38 
+	c.Assert(err, checker.IsNil)
39 
+	c.Assert(string(b), checker.Not(checker.Contains), "\"auth\":")
40 
+	c.Assert(string(b), checker.Contains, privateRegistryURL)
41 
+
42 
+	dockerCmd(c, "--config", tmp, "tag", "busybox", repoName)
43 
+	dockerCmd(c, "--config", tmp, "push", repoName)
44 
+
45 
+	dockerCmd(c, "--config", tmp, "logout", privateRegistryURL)
46 
+
47 
+	b, err = ioutil.ReadFile(configPath)
48 
+	c.Assert(err, checker.IsNil)
49 
+	c.Assert(string(b), checker.Not(checker.Contains), privateRegistryURL)
50 
+
51 
+	// check I cannot pull anymore
52 
+	out, _, err := dockerCmdWithError("--config", tmp, "pull", repoName)
53 
+	c.Assert(err, check.NotNil, check.Commentf(out))
54 
+	c.Assert(out, checker.Contains, fmt.Sprintf("Error: image dockercli/busybox not found"))
55 
+}