From 44152144ca766221e97fdaa5200fec3557a64f58 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Tue, 1 Mar 2016 18:04:35 +0100 Subject: [PATCH] cliconfig: credentials: support getting all auths docker build is broken because it sends to the daemon the full cliconfig file which has only Email(s). This patch retrieves all auth configs from the credentials store. Signed-off-by: Antonio Murdaca --- api/client/build.go | 2 +- api/client/login.go | 5 ++ api/client/utils.go | 5 ++ cliconfig/credentials/credentials.go | 2 + cliconfig/credentials/file_store.go | 4 ++ cliconfig/credentials/file_store_test.go | 38 ++++++++++++++ cliconfig/credentials/native_store.go | 18 ++++++- cliconfig/credentials/native_store_test.go | 49 ++++++++++++++++++- integration-cli/docker_cli_build_test.go | 42 ++++++++++++++++ integration-cli/docker_cli_pull_local_test.go | 2 +- integration-cli/docker_utils.go | 1 - 11 files changed, 161 insertions(+), 7 deletions(-) diff --git a/api/client/build.go b/api/client/build.go index 6f8065c933..eef716a99e 100644 --- a/api/client/build.go +++ b/api/client/build.go @@ -229,7 +229,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { ShmSize: shmSize, Ulimits: flUlimits.GetList(), BuildArgs: runconfigopts.ConvertKVStringsToMap(flBuildArg.GetAll()), - AuthConfigs: cli.configFile.AuthConfigs, + AuthConfigs: cli.retrieveAuthConfigs(), } response, err := cli.client.ImageBuild(context.Background(), options) diff --git a/api/client/login.go b/api/client/login.go index 470cb5780c..87fd2b2bc2 100644 --- a/api/client/login.go +++ b/api/client/login.go @@ -147,6 +147,11 @@ func getCredentials(c *cliconfig.ConfigFile, serverAddress string) (types.AuthCo return s.Get(serverAddress) } +func getAllCredentials(c *cliconfig.ConfigFile) (map[string]types.AuthConfig, error) { + s := loadCredentialsStore(c) + return s.GetAll() +} + // storeCredentials saves the user credentials in a credentials store. // The store is determined by the config file settings. func storeCredentials(c *cliconfig.ConfigFile, auth types.AuthConfig) error { diff --git a/api/client/utils.go b/api/client/utils.go index a3500319a4..0ea3cb060c 100644 --- a/api/client/utils.go +++ b/api/client/utils.go @@ -193,3 +193,8 @@ func (cli *DockerCli) resolveAuthConfig(index *registrytypes.IndexInfo) types.Au a, _ := getCredentials(cli.configFile, configKey) return a } + +func (cli *DockerCli) retrieveAuthConfigs() map[string]types.AuthConfig { + acs, _ := getAllCredentials(cli.configFile) + return acs +} diff --git a/cliconfig/credentials/credentials.go b/cliconfig/credentials/credentials.go index a0cfd7d33e..510cf8cf0e 100644 --- a/cliconfig/credentials/credentials.go +++ b/cliconfig/credentials/credentials.go @@ -10,6 +10,8 @@ type Store interface { Erase(serverAddress string) error // Get retrieves credentials from the store for a given server. Get(serverAddress string) (types.AuthConfig, error) + // GetAll retrieves all the credentials from the store. + GetAll() (map[string]types.AuthConfig, error) // Store saves credentials in the store. Store(authConfig types.AuthConfig) error } diff --git a/cliconfig/credentials/file_store.go b/cliconfig/credentials/file_store.go index 99461c1aa5..8e7edd624a 100644 --- a/cliconfig/credentials/file_store.go +++ b/cliconfig/credentials/file_store.go @@ -43,6 +43,10 @@ func (c *fileStore) Get(serverAddress string) (types.AuthConfig, error) { return authConfig, nil } +func (c *fileStore) GetAll() (map[string]types.AuthConfig, error) { + return c.file.AuthConfigs, nil +} + // Store saves the given credentials in the file store. func (c *fileStore) Store(authConfig types.AuthConfig) error { c.file.AuthConfigs[authConfig.ServerAddress] = authConfig diff --git a/cliconfig/credentials/file_store_test.go b/cliconfig/credentials/file_store_test.go index ed00f24df3..668b6f097d 100644 --- a/cliconfig/credentials/file_store_test.go +++ b/cliconfig/credentials/file_store_test.go @@ -70,6 +70,44 @@ func TestFileStoreGet(t *testing.T) { } } +func TestFileStoreGetAll(t *testing.T) { + s1 := "https://example.com" + s2 := "https://example2.com" + f := newConfigFile(map[string]types.AuthConfig{ + s1: { + Auth: "super_secret_token", + Email: "foo@example.com", + ServerAddress: "https://example.com", + }, + s2: { + Auth: "super_secret_token2", + Email: "foo@example2.com", + ServerAddress: "https://example2.com", + }, + }) + + s := NewFileStore(f) + as, err := s.GetAll() + if err != nil { + t.Fatal(err) + } + if len(as) != 2 { + t.Fatalf("wanted 2, got %d", len(as)) + } + if as[s1].Auth != "super_secret_token" { + t.Fatalf("expected auth `super_secret_token`, got %s", as[s1].Auth) + } + if as[s1].Email != "foo@example.com" { + t.Fatalf("expected email `foo@example.com`, got %s", as[s1].Email) + } + if as[s2].Auth != "super_secret_token2" { + t.Fatalf("expected auth `super_secret_token2`, got %s", as[s2].Auth) + } + if as[s2].Email != "foo@example2.com" { + t.Fatalf("expected email `foo@example2.com`, got %s", as[s2].Email) + } +} + func TestFileStoreErase(t *testing.T) { f := newConfigFile(map[string]types.AuthConfig{ "https://example.com": { diff --git a/cliconfig/credentials/native_store.go b/cliconfig/credentials/native_store.go index 37b045ae68..2da041d4b2 100644 --- a/cliconfig/credentials/native_store.go +++ b/cliconfig/credentials/native_store.go @@ -81,6 +81,20 @@ func (c *nativeStore) Get(serverAddress string) (types.AuthConfig, error) { return auth, nil } +// GetAll retrieves all the credentials from the native store. +func (c *nativeStore) GetAll() (map[string]types.AuthConfig, error) { + auths, _ := c.fileStore.GetAll() + + for s, ac := range auths { + creds, _ := c.getCredentialsFromStore(s) + ac.Username = creds.Username + ac.Password = creds.Password + auths[s] = ac + } + + return auths, nil +} + // Store saves the given credentials in the file store. func (c *nativeStore) Store(authConfig types.AuthConfig) error { if err := c.storeCredentialsInStore(authConfig); err != nil { @@ -135,7 +149,7 @@ func (c *nativeStore) getCredentialsFromStore(serverAddress string) (types.AuthC return ret, nil } - logrus.Debugf("error adding credentials - err: %v, out: `%s`", err, t) + logrus.Debugf("error getting credentials - err: %v, out: `%s`", err, t) return ret, fmt.Errorf(t) } @@ -158,7 +172,7 @@ func (c *nativeStore) eraseCredentialsFromStore(serverURL string) error { out, err := cmd.Output() if err != nil { t := strings.TrimSpace(string(out)) - logrus.Debugf("error adding credentials - err: %v, out: `%s`", err, t) + logrus.Debugf("error erasing credentials - err: %v, out: `%s`", err, t) return fmt.Errorf(t) } diff --git a/cliconfig/credentials/native_store_test.go b/cliconfig/credentials/native_store_test.go index cb59bda4a8..454fd0bd91 100644 --- a/cliconfig/credentials/native_store_test.go +++ b/cliconfig/credentials/native_store_test.go @@ -13,6 +13,7 @@ import ( const ( validServerAddress = "https://index.docker.io/v1" + validServerAddress2 = "https://example.com:5002" invalidServerAddress = "https://foobar.example.com" missingCredsAddress = "https://missing.docker.io/v1" ) @@ -46,7 +47,7 @@ func (m *mockCommand) Output() ([]byte, error) { } case "get": switch inS { - case validServerAddress: + case validServerAddress, validServerAddress2: return []byte(`{"Username": "foo", "Password": "bar"}`), nil case missingCredsAddress: return []byte(errCredentialsNotFound.Error()), errCommandExited @@ -67,7 +68,7 @@ func (m *mockCommand) Output() ([]byte, error) { } } - return []byte("unknown argument"), errCommandExited + return []byte(fmt.Sprintf("unknown argument %q with %q", m.arg, inS)), errCommandExited } // Input sets the input to send to a remote credentials helper. @@ -178,6 +179,50 @@ func TestNativeStoreGet(t *testing.T) { } } +func TestNativeStoreGetAll(t *testing.T) { + f := newConfigFile(map[string]types.AuthConfig{ + validServerAddress: { + Email: "foo@example.com", + }, + validServerAddress2: { + Email: "foo@example2.com", + }, + }) + f.CredentialsStore = "mock" + + s := &nativeStore{ + commandFn: mockCommandFn, + fileStore: NewFileStore(f), + } + as, err := s.GetAll() + if err != nil { + t.Fatal(err) + } + + if len(as) != 2 { + t.Fatalf("wanted 2, got %d", len(as)) + } + + if as[validServerAddress].Username != "foo" { + t.Fatalf("expected username `foo` for %s, got %s", validServerAddress, as[validServerAddress].Username) + } + if as[validServerAddress].Password != "bar" { + t.Fatalf("expected password `bar` for %s, got %s", validServerAddress, as[validServerAddress].Password) + } + if as[validServerAddress].Email != "foo@example.com" { + t.Fatalf("expected email `foo@example.com` for %s, got %s", validServerAddress, as[validServerAddress].Email) + } + if as[validServerAddress2].Username != "foo" { + t.Fatalf("expected username `foo` for %s, got %s", validServerAddress2, as[validServerAddress2].Username) + } + if as[validServerAddress2].Password != "bar" { + t.Fatalf("expected password `bar` for %s, got %s", validServerAddress2, as[validServerAddress2].Password) + } + if as[validServerAddress2].Email != "foo@example2.com" { + t.Fatalf("expected email `foo@example2.com` for %s, got %s", validServerAddress2, as[validServerAddress2].Email) + } +} + func TestNativeStoreGetMissingCredentials(t *testing.T) { f := newConfigFile(map[string]types.AuthConfig{ validServerAddress: { diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index f2cc350228..387ee86603 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -6589,3 +6589,45 @@ func (s *DockerRegistryAuthSuite) TestBuildFromAuthenticatedRegistry(c *check.C) c.Assert(err, checker.IsNil) } + +func (s *DockerRegistryAuthSuite) TestBuildWithExternalAuth(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\":") + + dockerCmd(c, "--config", tmp, "tag", "busybox", repoName) + dockerCmd(c, "--config", tmp, "push", repoName) + + // make sure the image is pulled when building + dockerCmd(c, "rmi", repoName) + + buildCmd := exec.Command(dockerBinary, "--config", tmp, "build", "-") + buildCmd.Stdin = strings.NewReader(fmt.Sprintf("FROM %s", repoName)) + + out, _, err := runCommandWithOutput(buildCmd) + c.Assert(err, check.IsNil, check.Commentf(out)) +} diff --git a/integration-cli/docker_cli_pull_local_test.go b/integration-cli/docker_cli_pull_local_test.go index e032abf6bd..54b5b9ef90 100644 --- a/integration-cli/docker_cli_pull_local_test.go +++ b/integration-cli/docker_cli_pull_local_test.go @@ -385,7 +385,7 @@ func (s *DockerRegistryAuthSuite) TestPullWithExternalAuth(c *check.C) { 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, "-e", s.reg.email, privateRegistryURL) + dockerCmd(c, "--config", tmp, "login", "-u", s.reg.username, "-p", s.reg.password, privateRegistryURL) b, err := ioutil.ReadFile(configPath) c.Assert(err, checker.IsNil) diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go index 4bb43d7f80..700610b0d0 100644 --- a/integration-cli/docker_utils.go +++ b/integration-cli/docker_utils.go @@ -38,7 +38,6 @@ import ( func init() { cmd := exec.Command(dockerBinary, "images") cmd.Env = appendBaseEnv(true) - fmt.Println("foobar", cmd.Env) out, err := cmd.CombinedOutput() if err != nil { panic(fmt.Errorf("err=%v\nout=%s\n", err, out))