diff --git a/api/client/registry.go b/api/client/registry.go index e67ea7d9c7..104d68278d 100644 --- a/api/client/registry.go +++ b/api/client/registry.go @@ -49,7 +49,8 @@ func (cli *DockerCli) RegistryAuthenticationPrivilegedFunc(index *registrytypes. return func() (string, error) { fmt.Fprintf(cli.out, "\nPlease login prior to %s:\n", cmdName) indexServer := registry.GetAuthConfigKey(index) - authConfig, err := cli.ConfigureAuth("", "", indexServer, false) + isDefaultRegistry := indexServer == cli.ElectAuthServer(context.Background()) + authConfig, err := cli.ConfigureAuth("", "", indexServer, isDefaultRegistry) if err != nil { return "", err } @@ -91,6 +92,10 @@ func (cli *DockerCli) ConfigureAuth(flUser, flPassword, serverAddress string, is cli.in = os.Stdin } + if !isDefaultRegistry { + serverAddress = registry.ConvertToHostname(serverAddress) + } + authconfig, err := GetCredentials(cli.configFile, serverAddress) if err != nil { return authconfig, err diff --git a/api/client/registry/login.go b/api/client/registry/login.go index 452ac71513..ffcdb96bea 100644 --- a/api/client/registry/login.go +++ b/api/client/registry/login.go @@ -50,14 +50,18 @@ func runLogin(dockerCli *client.DockerCli, opts loginOptions) error { ctx := context.Background() clnt := dockerCli.Client() - var serverAddress string - var isDefaultRegistry bool + var ( + serverAddress string + authServer = dockerCli.ElectAuthServer(ctx) + ) if opts.serverAddress != "" { serverAddress = opts.serverAddress } else { - serverAddress = dockerCli.ElectAuthServer(ctx) - isDefaultRegistry = true + serverAddress = authServer } + + isDefaultRegistry := serverAddress == authServer + authConfig, err := dockerCli.ConfigureAuth(opts.user, opts.password, serverAddress, isDefaultRegistry) if err != nil { return err diff --git a/api/client/registry/logout.go b/api/client/registry/logout.go index 847dd6ddc8..55e2ecd079 100644 --- a/api/client/registry/logout.go +++ b/api/client/registry/logout.go @@ -7,6 +7,7 @@ import ( "github.com/docker/docker/api/client" "github.com/docker/docker/cli" + "github.com/docker/docker/registry" "github.com/spf13/cobra" ) @@ -31,21 +32,45 @@ func NewLogoutCommand(dockerCli *client.DockerCli) *cobra.Command { func runLogout(dockerCli *client.DockerCli, serverAddress string) error { ctx := context.Background() + var isDefaultRegistry bool if serverAddress == "" { serverAddress = dockerCli.ElectAuthServer(ctx) + isDefaultRegistry = true + } + + var ( + loggedIn bool + regsToLogout []string + hostnameAddress = serverAddress + regsToTry = []string{serverAddress} + ) + if !isDefaultRegistry { + hostnameAddress = registry.ConvertToHostname(serverAddress) + // the tries below are kept for backward compatibily where a user could have + // saved the registry in one of the following format. + regsToTry = append(regsToTry, hostnameAddress, "http://"+hostnameAddress, "https://"+hostnameAddress) } // 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 := dockerCli.ConfigFile().AuthConfigs[serverAddress]; !ok { - fmt.Fprintf(dockerCli.Out(), "Not logged in to %s\n", serverAddress) + for _, s := range regsToTry { + if _, ok := dockerCli.ConfigFile().AuthConfigs[s]; ok { + loggedIn = true + regsToLogout = append(regsToLogout, s) + } + } + + if !loggedIn { + fmt.Fprintf(dockerCli.Out(), "Not logged in to %s\n", hostnameAddress) return nil } - fmt.Fprintf(dockerCli.Out(), "Removing login credentials for %s\n", serverAddress) - if err := client.EraseCredentials(dockerCli.ConfigFile(), serverAddress); err != nil { - fmt.Fprintf(dockerCli.Err(), "WARNING: could not erase credentials: %v\n", err) + fmt.Fprintf(dockerCli.Out(), "Removing login credentials for %s\n", hostnameAddress) + for _, r := range regsToLogout { + if err := client.EraseCredentials(dockerCli.ConfigFile(), r); err != nil { + fmt.Fprintf(dockerCli.Err(), "WARNING: could not erase credentials: %v\n", err) + } } return nil diff --git a/cliconfig/credentials/file_store.go b/cliconfig/credentials/file_store.go index cf1c89fcc5..55fa4f8dac 100644 --- a/cliconfig/credentials/file_store.go +++ b/cliconfig/credentials/file_store.go @@ -1,9 +1,8 @@ package credentials import ( - "strings" - "github.com/docker/docker/cliconfig/configfile" + "github.com/docker/docker/registry" "github.com/docker/engine-api/types" ) @@ -32,8 +31,8 @@ func (c *fileStore) Get(serverAddress string) (types.AuthConfig, error) { if !ok { // Maybe they have a legacy config file, we will iterate the keys converting // them to the new format and testing - for registry, ac := range c.file.AuthConfigs { - if serverAddress == convertToHostname(registry) { + for r, ac := range c.file.AuthConfigs { + if serverAddress == registry.ConvertToHostname(r) { return ac, nil } } @@ -52,16 +51,3 @@ func (c *fileStore) Store(authConfig types.AuthConfig) error { c.file.AuthConfigs[authConfig.ServerAddress] = authConfig return c.file.Save() } - -func convertToHostname(url string) string { - stripped := url - if strings.HasPrefix(url, "http://") { - stripped = strings.Replace(url, "http://", "", 1) - } else if strings.HasPrefix(url, "https://") { - stripped = strings.Replace(url, "https://", "", 1) - } - - nameParts := strings.SplitN(stripped, "/", 2) - - return nameParts[0] -} diff --git a/integration-cli/docker_cli_logout_test.go b/integration-cli/docker_cli_logout_test.go index 52d4fff303..a5f4b108cf 100644 --- a/integration-cli/docker_cli_logout_test.go +++ b/integration-cli/docker_cli_logout_test.go @@ -1,9 +1,11 @@ package main import ( + "bytes" "fmt" "io/ioutil" "os" + "os/exec" "path/filepath" "github.com/docker/docker/pkg/integration/checker" @@ -54,3 +56,45 @@ func (s *DockerRegistryAuthHtpasswdSuite) TestLogoutWithExternalAuth(c *check.C) c.Assert(err, check.NotNil, check.Commentf(out)) c.Assert(out, checker.Contains, "Error: image dockercli/busybox:authtest not found") } + +// #23100 +func (s *DockerRegistryAuthHtpasswdSuite) TestLogoutWithWrongHostnamesStored(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) + + cmd := exec.Command("docker-credential-shell-test", "store") + stdin := bytes.NewReader([]byte(fmt.Sprintf(`{"ServerURL": "https://%s", "Username": "%s", "Secret": "%s"}`, privateRegistryURL, s.reg.username, s.reg.password))) + cmd.Stdin = stdin + c.Assert(cmd.Run(), checker.IsNil) + + tmp, err := ioutil.TempDir("", "integration-cli-") + c.Assert(err, checker.IsNil) + + externalAuthConfig := fmt.Sprintf(`{ "auths": {"https://%s": {}}, "credsStore": "shell-test" }`, privateRegistryURL) + + 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.Contains, fmt.Sprintf("\"https://%s\": {}", privateRegistryURL)) + c.Assert(string(b), checker.Contains, fmt.Sprintf("\"%s\": {}", privateRegistryURL)) + + dockerCmd(c, "--config", tmp, "logout", privateRegistryURL) + + b, err = ioutil.ReadFile(configPath) + c.Assert(err, checker.IsNil) + c.Assert(string(b), checker.Not(checker.Contains), fmt.Sprintf("\"https://%s\": {}", privateRegistryURL)) + c.Assert(string(b), checker.Not(checker.Contains), fmt.Sprintf("\"%s\": {}", privateRegistryURL)) +} diff --git a/integration-cli/docker_cli_pull_local_test.go b/integration-cli/docker_cli_pull_local_test.go index bd4fb4bd2a..cb14c2c702 100644 --- a/integration-cli/docker_cli_pull_local_test.go +++ b/integration-cli/docker_cli_pull_local_test.go @@ -387,6 +387,52 @@ func (s *DockerRegistrySuite) TestPullManifestList(c *check.C) { dockerCmd(c, "rmi", repoName) } +// #23100 +func (s *DockerRegistryAuthHtpasswdSuite) TestPullWithExternalAuthLoginWithScheme(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) + + dockerCmd(c, "--config", tmp, "logout", privateRegistryURL) + dockerCmd(c, "--config", tmp, "login", "-u", s.reg.username, "-p", s.reg.password, "https://"+privateRegistryURL) + dockerCmd(c, "--config", tmp, "pull", repoName) + + // likewise push should work + repoName2 := fmt.Sprintf("%v/dockercli/busybox:nocreds", privateRegistryURL) + dockerCmd(c, "tag", repoName, repoName2) + dockerCmd(c, "--config", tmp, "push", repoName2) + + // logout should work w scheme also because it will be stripped + dockerCmd(c, "--config", tmp, "logout", "https://"+privateRegistryURL) +} + func (s *DockerRegistryAuthHtpasswdSuite) TestPullWithExternalAuth(c *check.C) { osPath := os.Getenv("PATH") defer os.Setenv("PATH", osPath) diff --git a/registry/auth.go b/registry/auth.go index 0b5257182e..045c8deac1 100644 --- a/registry/auth.go +++ b/registry/auth.go @@ -206,6 +206,21 @@ func v2AuthHTTPClient(endpoint *url.URL, authTransport http.RoundTripper, modifi } +// ConvertToHostname converts a registry url which has http|https prepended +// to just an hostname. +func ConvertToHostname(url string) string { + stripped := url + if strings.HasPrefix(url, "http://") { + stripped = strings.TrimPrefix(url, "http://") + } else if strings.HasPrefix(url, "https://") { + stripped = strings.TrimPrefix(url, "https://") + } + + nameParts := strings.SplitN(stripped, "/", 2) + + return nameParts[0] +} + // ResolveAuthConfig matches an auth configuration to a server address or a URL func ResolveAuthConfig(authConfigs map[string]types.AuthConfig, index *registrytypes.IndexInfo) types.AuthConfig { configKey := GetAuthConfigKey(index) @@ -214,23 +229,10 @@ func ResolveAuthConfig(authConfigs map[string]types.AuthConfig, index *registryt return c } - convertToHostname := func(url string) string { - stripped := url - if strings.HasPrefix(url, "http://") { - stripped = strings.Replace(url, "http://", "", 1) - } else if strings.HasPrefix(url, "https://") { - stripped = strings.Replace(url, "https://", "", 1) - } - - nameParts := strings.SplitN(stripped, "/", 2) - - return nameParts[0] - } - // Maybe they have a legacy config file, we will iterate the keys converting // them to the new format and testing for registry, ac := range authConfigs { - if configKey == convertToHostname(registry) { + if configKey == ConvertToHostname(registry) { return ac } }