diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index ec509a1aea..3facb75ccc 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -11,6 +11,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" + "github.com/docker/cli/cli/config/configfile" configtypes "github.com/docker/cli/cli/config/types" "github.com/docker/cli/cli/internal/oauth/manager" registrytypes "github.com/docker/docker/api/types/registry" @@ -88,7 +89,7 @@ func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) err } var ( serverAddress string - response *registrytypes.AuthenticateOKBody + msg string ) if opts.serverAddress != "" && opts.serverAddress != registry.DefaultNamespace { serverAddress = opts.serverAddress @@ -100,25 +101,25 @@ func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) err // attempt login with current (stored) credentials authConfig, err := command.GetDefaultAuthConfig(dockerCli.ConfigFile(), opts.user == "" && opts.password == "", serverAddress, isDefaultRegistry) if err == nil && authConfig.Username != "" && authConfig.Password != "" { - response, err = loginWithStoredCredentials(ctx, dockerCli, authConfig) + msg, err = loginWithStoredCredentials(ctx, dockerCli, authConfig) } // 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, authConfig.ServerAddress) + msg, err = loginUser(ctx, dockerCli, opts, authConfig.Username, authConfig.ServerAddress) if err != nil { return err } } - if response != nil && response.Status != "" { - _, _ = fmt.Fprintln(dockerCli.Out(), response.Status) + if msg != "" { + _, _ = fmt.Fprintln(dockerCli.Out(), msg) } return nil } -func loginWithStoredCredentials(ctx context.Context, dockerCli command.Cli, authConfig registrytypes.AuthConfig) (*registrytypes.AuthenticateOKBody, error) { +func loginWithStoredCredentials(ctx context.Context, dockerCli command.Cli, authConfig registrytypes.AuthConfig) (msg string, _ error) { _, _ = fmt.Fprintf(dockerCli.Out(), "Authenticating with existing credentials...\n") response, err := dockerCli.Client().RegistryLogin(ctx, authConfig) if err != nil { @@ -134,11 +135,11 @@ func loginWithStoredCredentials(ctx context.Context, dockerCli command.Cli, auth authConfig.IdentityToken = response.IdentityToken } - if err := storeCredentials(dockerCli, authConfig); err != nil { - return nil, err + if err := storeCredentials(dockerCli.ConfigFile(), authConfig); err != nil { + return "", err } - return &response, err + return response.Status, err } const OauthLoginEscapeHatchEnvVar = "DOCKER_CLI_DISABLE_OAUTH_LOGIN" @@ -154,7 +155,7 @@ func isOauthLoginDisabled() bool { return false } -func loginUser(ctx context.Context, dockerCli command.Cli, opts loginOptions, defaultUsername, serverAddress string) (*registrytypes.AuthenticateOKBody, error) { +func loginUser(ctx context.Context, dockerCli command.Cli, opts loginOptions, defaultUsername, serverAddress string) (msg string, _ error) { // Some links documenting this: // - https://code.google.com/archive/p/mintty/issues/56 // - https://github.com/docker/docker/issues/15272 @@ -163,16 +164,17 @@ func loginUser(ctx context.Context, dockerCli command.Cli, opts loginOptions, de // will hit this if you attempt docker login from mintty where stdin // is a pipe, not a character based console. if (opts.user == "" || opts.password == "") && !dockerCli.In().IsTerminal() { - return nil, errors.Errorf("Error: Cannot perform an interactive login from a non TTY device") + return "", errors.Errorf("Error: Cannot perform an interactive login from a non TTY device") } // If we're logging into the index server and the user didn't provide a username or password, use the device flow if serverAddress == registry.IndexServer && opts.user == "" && opts.password == "" && !isOauthLoginDisabled() { - response, err := loginWithDeviceCodeFlow(ctx, dockerCli) + var err error + msg, err = loginWithDeviceCodeFlow(ctx, dockerCli) // if the error represents a failure to initiate the device-code flow, // then we fallback to regular cli credentials login if !errors.Is(err, manager.ErrDeviceLoginStartFail) { - return response, err + return msg, err } _, _ = fmt.Fprint(dockerCli.Err(), "Failed to start web-based login - falling back to command line login...\n\n") } @@ -180,50 +182,50 @@ func loginUser(ctx context.Context, dockerCli command.Cli, opts loginOptions, de return loginWithUsernameAndPassword(ctx, dockerCli, opts, defaultUsername, serverAddress) } -func loginWithUsernameAndPassword(ctx context.Context, dockerCli command.Cli, opts loginOptions, defaultUsername, serverAddress string) (*registrytypes.AuthenticateOKBody, error) { +func loginWithUsernameAndPassword(ctx context.Context, dockerCli command.Cli, opts loginOptions, defaultUsername, serverAddress string) (msg string, _ error) { // Prompt user for credentials authConfig, err := command.PromptUserForCredentials(ctx, dockerCli, opts.user, opts.password, defaultUsername, serverAddress) if err != nil { - return nil, err + return "", err } - response, err := loginWithRegistry(ctx, dockerCli, authConfig) + response, err := loginWithRegistry(ctx, dockerCli.Client(), authConfig) if err != nil { - return nil, err + return "", err } if response.IdentityToken != "" { authConfig.Password = "" authConfig.IdentityToken = response.IdentityToken } - if err = storeCredentials(dockerCli, authConfig); err != nil { - return nil, err + if err = storeCredentials(dockerCli.ConfigFile(), authConfig); err != nil { + return "", err } - return &response, nil + return response.Status, nil } -func loginWithDeviceCodeFlow(ctx context.Context, dockerCli command.Cli) (*registrytypes.AuthenticateOKBody, error) { +func loginWithDeviceCodeFlow(ctx context.Context, dockerCli command.Cli) (msg string, _ error) { store := dockerCli.ConfigFile().GetCredentialsStore(registry.IndexServer) authConfig, err := manager.NewManager(store).LoginDevice(ctx, dockerCli.Err()) if err != nil { - return nil, err + return "", err } - response, err := loginWithRegistry(ctx, dockerCli, registrytypes.AuthConfig(*authConfig)) + response, err := loginWithRegistry(ctx, dockerCli.Client(), registrytypes.AuthConfig(*authConfig)) if err != nil { - return nil, err + return "", err } - if err = storeCredentials(dockerCli, registrytypes.AuthConfig(*authConfig)); err != nil { - return nil, err + if err = storeCredentials(dockerCli.ConfigFile(), registrytypes.AuthConfig(*authConfig)); err != nil { + return "", err } - return &response, nil + return response.Status, nil } -func storeCredentials(dockerCli command.Cli, authConfig registrytypes.AuthConfig) error { - creds := dockerCli.ConfigFile().GetCredentialsStore(authConfig.ServerAddress) +func storeCredentials(cfg *configfile.ConfigFile, authConfig registrytypes.AuthConfig) error { + creds := cfg.GetCredentialsStore(authConfig.ServerAddress) if err := creds.Store(configtypes.AuthConfig(authConfig)); err != nil { return errors.Errorf("Error saving credentials: %v", err) } @@ -231,30 +233,32 @@ func storeCredentials(dockerCli command.Cli, authConfig registrytypes.AuthConfig return nil } -func loginWithRegistry(ctx context.Context, dockerCli command.Cli, authConfig registrytypes.AuthConfig) (registrytypes.AuthenticateOKBody, error) { - response, err := dockerCli.Client().RegistryLogin(ctx, authConfig) - if err != nil && client.IsErrConnectionFailed(err) { - // If the server isn't responding (yet) attempt to login purely client side - response, err = loginClientSide(ctx, authConfig) - } - // If we (still) have an error, give up +func loginWithRegistry(ctx context.Context, apiClient client.SystemAPIClient, authConfig registrytypes.AuthConfig) (*registrytypes.AuthenticateOKBody, error) { + response, err := apiClient.RegistryLogin(ctx, authConfig) if err != nil { - return registrytypes.AuthenticateOKBody{}, err + if client.IsErrConnectionFailed(err) { + // daemon isn't responding; attempt to login client side. + return loginClientSide(ctx, authConfig) + } + return nil, err } - return response, nil + return &response, nil } -func loginClientSide(ctx context.Context, auth registrytypes.AuthConfig) (registrytypes.AuthenticateOKBody, error) { +func loginClientSide(ctx context.Context, auth registrytypes.AuthConfig) (*registrytypes.AuthenticateOKBody, error) { svc, err := registry.NewService(registry.ServiceOptions{}) if err != nil { - return registrytypes.AuthenticateOKBody{}, err + return nil, err } status, token, err := svc.Auth(ctx, &auth, command.UserAgent()) + if err != nil { + return nil, err + } - return registrytypes.AuthenticateOKBody{ + return ®istrytypes.AuthenticateOKBody{ Status: status, IdentityToken: token, - }, err + }, nil } diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go index 0dbfaa6570..a8af6acf81 100644 --- a/cli/command/registry/login_test.go +++ b/cli/command/registry/login_test.go @@ -1,10 +1,10 @@ package registry import ( - "bytes" "context" "errors" "fmt" + "path/filepath" "testing" "time" @@ -55,8 +55,9 @@ func (c *fakeClient) RegistryLogin(_ context.Context, auth registrytypes.AuthCon func TestLoginWithCredStoreCreds(t *testing.T) { testCases := []struct { inputAuthConfig registrytypes.AuthConfig - expectedMsg string expectedErr string + expectedMsg string + expectedErrMsg string }{ { inputAuthConfig: registrytypes.AuthConfig{}, @@ -66,20 +67,25 @@ func TestLoginWithCredStoreCreds(t *testing.T) { inputAuthConfig: registrytypes.AuthConfig{ Username: unknownUser, }, - expectedMsg: "Authenticating with existing credentials...\n", - expectedErr: fmt.Sprintf("Login did not succeed, error: %s\n", errUnknownUser), + expectedErr: errUnknownUser, + expectedMsg: "Authenticating with existing credentials...\n", + expectedErrMsg: fmt.Sprintf("Login did not succeed, error: %s\n", errUnknownUser), }, } ctx := context.Background() + cli := test.NewFakeCli(&fakeClient{}) + cli.ConfigFile().Filename = filepath.Join(t.TempDir(), "config.json") for _, tc := range testCases { - cli := test.NewFakeCli(&fakeClient{}) - errBuf := new(bytes.Buffer) - cli.SetErr(streams.NewOut(errBuf)) - loginWithStoredCredentials(ctx, cli, tc.inputAuthConfig) - outputString := cli.OutBuffer().String() - assert.Check(t, is.Equal(tc.expectedMsg, outputString)) - errorString := errBuf.String() - assert.Check(t, is.Equal(tc.expectedErr, errorString)) + _, err := loginWithStoredCredentials(ctx, cli, tc.inputAuthConfig) + if tc.expectedErrMsg != "" { + assert.Check(t, is.Error(err, tc.expectedErr)) + } else { + assert.NilError(t, err) + } + assert.Check(t, is.Equal(tc.expectedMsg, cli.OutBuffer().String())) + assert.Check(t, is.Equal(tc.expectedErrMsg, cli.ErrBuffer().String())) + cli.ErrBuffer().Reset() + cli.OutBuffer().Reset() } } @@ -349,16 +355,16 @@ func TestLoginNonInteractive(t *testing.T) { // "" meaning default registry registries := []string{"", "my-registry.com"} - for _, registry := range registries { + for _, registryAddr := range registries { for _, tc := range testCases { t.Run(tc.doc, func(t *testing.T) { tmpFile := fs.NewFile(t, "test-run-login") defer tmpFile.Remove() cli := test.NewFakeCli(&fakeClient{}) - configfile := cli.ConfigFile() - configfile.Filename = tmpFile.Path() + cfg := cli.ConfigFile() + cfg.Filename = tmpFile.Path() options := loginOptions{ - serverAddress: registry, + serverAddress: registryAddr, } if tc.username { options.user = "my-username" @@ -412,26 +418,26 @@ func TestLoginNonInteractive(t *testing.T) { // "" meaning default registry registries := []string{"", "my-registry.com"} - for _, registry := range registries { + for _, registryAddr := range registries { for _, tc := range testCases { t.Run(tc.doc, func(t *testing.T) { tmpFile := fs.NewFile(t, "test-run-login") defer tmpFile.Remove() cli := test.NewFakeCli(&fakeClient{}) - configfile := cli.ConfigFile() - configfile.Filename = tmpFile.Path() - serverAddress := registry + cfg := cli.ConfigFile() + cfg.Filename = tmpFile.Path() + serverAddress := registryAddr if serverAddress == "" { serverAddress = "https://index.docker.io/v1/" } - assert.NilError(t, configfile.GetCredentialsStore(serverAddress).Store(configtypes.AuthConfig{ + assert.NilError(t, cfg.GetCredentialsStore(serverAddress).Store(configtypes.AuthConfig{ Username: "my-username", Password: "my-password", ServerAddress: serverAddress, })) options := loginOptions{ - serverAddress: registry, + serverAddress: registryAddr, } if tc.username { options.user = "my-username"