Merge pull request #23100 from runcom/fix-stores

support legacy registries in exernal stores

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

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

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

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]
-}

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))
+}

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)

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
 		}
 	}