Merge pull request #5383 from laurazard/convert-registry-to-hostname

login: use normalized hostname when storing

cli/command/registry/login.go

@@ -106,7 +106,7 @@ func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) err
 	// if we failed to authenticate with stored credentials (or didn't have stored credentials),
 	// prompt the user for new credentials
 	if err != nil || authConfig.Username == "" || authConfig.Password == "" {
-		response, err = loginUser(ctx, dockerCli, opts, authConfig.Username, serverAddress)
+		response, err = loginUser(ctx, dockerCli, opts, authConfig.Username, authConfig.ServerAddress)
 		if err != nil {
 			return err
 		}

cli/command/registry/login_test.go

@@ -16,6 +16,7 @@ import (
 	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/api/types/system"
 	"github.com/docker/docker/client"
+	"github.com/docker/docker/registry"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/fs"
@@ -83,88 +84,207 @@ func TestLoginWithCredStoreCreds(t *testing.T) {
 }
 
 func TestRunLogin(t *testing.T) {
-	const (
-		storedServerAddress = "reg1"
-		validUsername       = "u1"
-		validPassword       = "p1"
-		validPassword2      = "p2"
-	)
-
-	validAuthConfig := configtypes.AuthConfig{
-		ServerAddress: storedServerAddress,
-		Username:      validUsername,
-		Password:      validPassword,
-	}
-	expiredAuthConfig := configtypes.AuthConfig{
-		ServerAddress: storedServerAddress,
-		Username:      validUsername,
-		Password:      expiredPassword,
-	}
-	validIdentityToken := configtypes.AuthConfig{
-		ServerAddress: storedServerAddress,
-		Username:      validUsername,
-		IdentityToken: useToken,
-	}
 	testCases := []struct {
 		doc                 string
-		inputLoginOption  loginOptions
+		priorCredentials    map[string]configtypes.AuthConfig
-		inputStoredCred   *configtypes.AuthConfig
+		input               loginOptions
+		expectedCredentials map[string]configtypes.AuthConfig
 		expectedErr         string
-		expectedSavedCred configtypes.AuthConfig
 	}{
 		{
 			doc: "valid auth from store",
-			inputLoginOption: loginOptions{
+			priorCredentials: map[string]configtypes.AuthConfig{
-				serverAddress: storedServerAddress,
+				"reg1": {
+					Username:      "my-username",
+					Password:      "a-password",
+					ServerAddress: "reg1",
+				},
+			},
+			input: loginOptions{
+				serverAddress: "reg1",
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"reg1": {
+					Username:      "my-username",
+					Password:      "a-password",
+					ServerAddress: "reg1",
+				},
 			},
-			inputStoredCred:   &validAuthConfig,
-			expectedSavedCred: validAuthConfig,
 		},
 		{
-			doc: "expired auth",
+			doc: "expired auth from store",
-			inputLoginOption: loginOptions{
+			priorCredentials: map[string]configtypes.AuthConfig{
-				serverAddress: storedServerAddress,
+				"reg1": {
+					Username:      "my-username",
+					Password:      expiredPassword,
+					ServerAddress: "reg1",
+				},
+			},
+			input: loginOptions{
+				serverAddress: "reg1",
 			},
-			inputStoredCred: &expiredAuthConfig,
 			expectedErr: "Error: Cannot perform an interactive login from a non TTY device",
 		},
 		{
-			doc: "valid username and password",
+			doc:              "store valid username and password",
-			inputLoginOption: loginOptions{
+			priorCredentials: map[string]configtypes.AuthConfig{},
-				serverAddress: storedServerAddress,
+			input: loginOptions{
-				user:          validUsername,
+				serverAddress: "reg1",
-				password:      validPassword2,
+				user:          "my-username",
+				password:      "p2",
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"reg1": {
+					Username:      "my-username",
+					Password:      "p2",
+					ServerAddress: "reg1",
 				},
-			inputStoredCred: &validAuthConfig,
-			expectedSavedCred: configtypes.AuthConfig{
-				ServerAddress: storedServerAddress,
-				Username:      validUsername,
-				Password:      validPassword2,
 			},
 		},
 		{
-			doc: "unknown user",
+			doc: "unknown user w/ prior credentials",
-			inputLoginOption: loginOptions{
+			priorCredentials: map[string]configtypes.AuthConfig{
-				serverAddress: storedServerAddress,
+				"reg1": {
+					Username:      "my-username",
+					Password:      "a-password",
+					ServerAddress: "reg1",
+				},
+			},
+			input: loginOptions{
+				serverAddress: "reg1",
 				user:          unknownUser,
-				password:      validPassword,
+				password:      "a-password",
 			},
-			inputStoredCred: &validAuthConfig,
 			expectedErr: errUnknownUser,
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"reg1": {
+					Username:      "a-password",
+					Password:      "a-password",
+					ServerAddress: "reg1",
+				},
+			},
 		},
 		{
-			doc: "valid token",
+			doc:              "unknown user w/o prior credentials",
-			inputLoginOption: loginOptions{
+			priorCredentials: map[string]configtypes.AuthConfig{},
-				serverAddress: storedServerAddress,
+			input: loginOptions{
-				user:          validUsername,
+				serverAddress: "reg1",
+				user:          unknownUser,
+				password:      "a-password",
+			},
+			expectedErr:         errUnknownUser,
+			expectedCredentials: map[string]configtypes.AuthConfig{},
+		},
+		{
+			doc:              "store valid token",
+			priorCredentials: map[string]configtypes.AuthConfig{},
+			input: loginOptions{
+				serverAddress: "reg1",
+				user:          "my-username",
 				password:      useToken,
 			},
-			inputStoredCred:   &validIdentityToken,
+			expectedCredentials: map[string]configtypes.AuthConfig{
-			expectedSavedCred: validIdentityToken,
+				"reg1": {
+					Username:      "my-username",
+					IdentityToken: useToken,
+					ServerAddress: "reg1",
+				},
+			},
+		},
+		{
+			doc: "valid token from store",
+			priorCredentials: map[string]configtypes.AuthConfig{
+				"reg1": {
+					Username:      "my-username",
+					Password:      useToken,
+					ServerAddress: "reg1",
+				},
+			},
+			input: loginOptions{
+				serverAddress: "reg1",
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"reg1": {
+					Username:      "my-username",
+					IdentityToken: useToken,
+					ServerAddress: "reg1",
+				},
+			},
+		},
+		{
+			doc:              "no registry specified defaults to index server",
+			priorCredentials: map[string]configtypes.AuthConfig{},
+			input: loginOptions{
+				user:     "my-username",
+				password: "my-password",
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				registry.IndexServer: {
+					Username:      "my-username",
+					Password:      "my-password",
+					ServerAddress: registry.IndexServer,
+				},
+			},
+		},
+		{
+			doc:              "registry-1.docker.io",
+			priorCredentials: map[string]configtypes.AuthConfig{},
+			input: loginOptions{
+				serverAddress: "registry-1.docker.io",
+				user:          "my-username",
+				password:      "my-password",
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"registry-1.docker.io": {
+					Username:      "my-username",
+					Password:      "my-password",
+					ServerAddress: "registry-1.docker.io",
+				},
+			},
+		},
+		// Regression test for https://github.com/docker/cli/issues/5382
+		{
+			doc:              "sanitizes server address to remove repo",
+			priorCredentials: map[string]configtypes.AuthConfig{},
+			input: loginOptions{
+				serverAddress: "registry-1.docker.io/bork/test",
+				user:          "my-username",
+				password:      "a-password",
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"registry-1.docker.io": {
+					Username:      "my-username",
+					Password:      "a-password",
+					ServerAddress: "registry-1.docker.io",
+				},
+			},
+		},
+		// Regression test for https://github.com/docker/cli/issues/5382
+		{
+			doc: "updates credential if server address includes repo",
+			priorCredentials: map[string]configtypes.AuthConfig{
+				"registry-1.docker.io": {
+					Username:      "my-username",
+					Password:      "a-password",
+					ServerAddress: "registry-1.docker.io",
+				},
+			},
+			input: loginOptions{
+				serverAddress: "registry-1.docker.io/bork/test",
+				user:          "my-username",
+				password:      "new-password",
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"registry-1.docker.io": {
+					Username:      "my-username",
+					Password:      "new-password",
+					ServerAddress: "registry-1.docker.io",
+				},
+			},
 		},
 	}
+
 	for _, tc := range testCases {
-		tc := tc
 		t.Run(tc.doc, func(t *testing.T) {
 			tmpFile := fs.NewFile(t, "test-run-login")
 			defer tmpFile.Remove()
@@ -172,19 +292,23 @@ func TestRunLogin(t *testing.T) {
 			configfile := cli.ConfigFile()
 			configfile.Filename = tmpFile.Path()
 
-			if tc.inputStoredCred != nil {
+			for _, priorCred := range tc.priorCredentials {
-				cred := *tc.inputStoredCred
+				assert.NilError(t, configfile.GetCredentialsStore(priorCred.ServerAddress).Store(priorCred))
-				assert.NilError(t, configfile.GetCredentialsStore(cred.ServerAddress).Store(cred))
 			}
-			loginErr := runLogin(context.Background(), cli, tc.inputLoginOption)
+			storedCreds, err := configfile.GetAllCredentials()
+			assert.NilError(t, err)
+			assert.DeepEqual(t, storedCreds, tc.priorCredentials)
+
+			loginErr := runLogin(context.Background(), cli, tc.input)
 			if tc.expectedErr != "" {
 				assert.Error(t, loginErr, tc.expectedErr)
 				return
 			}
 			assert.NilError(t, loginErr)
-			savedCred, credStoreErr := configfile.GetCredentialsStore(tc.inputStoredCred.ServerAddress).Get(tc.inputStoredCred.ServerAddress)
+
-			assert.Check(t, credStoreErr)
+			outputCreds, err := configfile.GetAllCredentials()
-			assert.DeepEqual(t, tc.expectedSavedCred, savedCred)
+			assert.Check(t, err)
+			assert.DeepEqual(t, outputCreds, tc.expectedCredentials)
 		})
 	}
 }