From bbb6e7643d2e2061ff7c848e7270bab12b943317 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Tue, 3 Sep 2024 14:10:06 +0100 Subject: [PATCH] login: handle non-tty scenario consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running `docker login` in a non-interactive environment sometimes errors out if no username/pwd is provided. This handling is somewhat inconsistent – this commit addresses that. Before: | `--username` | `--password` | Result | |:------------:|:------------:| ------------------------------------------------------------------ | | ✅ | ✅ | ✅ | | ❌ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` | | ✅ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` | | ❌ | ✅ | hangs | After: | `--username` | `--password` | Result | |:------------:|:------------:| ------------------------------------------------------------------ | | ✅ | ✅ | ✅ | | ❌ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` | | ✅ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` | | ❌ | ✅ | `Error: Cannot perform an interactive login from a non TTY device` | It's worth calling out a separate scenario – if there are previous, valid credentials, then running `docker login` with no username or password provided will use the previously stored credentials, and not error out. ```console cat ~/.docker/config.json { "auths": { "https://index.docker.io/v1/": { "auth": "xxxxxxxxxxx" } } } ⭑ docker login 0>/dev/null Authenticating with existing credentials... Login Succeeded ``` This commit also applies the same non-interactive handling logic to the new web-based login flow, which means that now, if there are no prior credentials stored and a user runs `docker login`, instead of initiating the new web-based login flow, an error is returned. Signed-off-by: Laura Brehm --- cli/command/registry.go | 11 --- cli/command/registry/login.go | 11 +++ cli/command/registry/login_test.go | 139 +++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 11 deletions(-) diff --git a/cli/command/registry.go b/cli/command/registry.go index dab062bbe4ba..1e018aa012c5 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -124,17 +124,6 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword cli.SetIn(streams.NewIn(os.Stdin)) } - // Some links documenting this: - // - https://code.google.com/archive/p/mintty/issues/56 - // - https://github.com/docker/docker/issues/15272 - // - https://mintty.github.io/ (compatibility) - // Linux will hit this if you attempt `cat | docker login`, and Windows - // will hit this if you attempt docker login from mintty where stdin - // is a pipe, not a character based console. - if argPassword == "" && !cli.In().IsTerminal() { - return authConfig, errors.Errorf("Error: Cannot perform an interactive login from a non TTY device") - } - isDefaultRegistry := serverAddress == registry.IndexServer defaultUsername = strings.TrimSpace(defaultUsername) diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index 8bad626a9956..de34e8b5560a 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -155,6 +155,17 @@ func isOauthLoginDisabled() bool { } func loginUser(ctx context.Context, dockerCli command.Cli, opts loginOptions, defaultUsername, serverAddress string) (*registrytypes.AuthenticateOKBody, error) { + // Some links documenting this: + // - https://code.google.com/archive/p/mintty/issues/56 + // - https://github.com/docker/docker/issues/15272 + // - https://mintty.github.io/ (compatibility) + // Linux will hit this if you attempt `cat | docker login`, and Windows + // 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") + } + // 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) diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go index df2caa0ca954..0dbfaa65706d 100644 --- a/cli/command/registry/login_test.go +++ b/cli/command/registry/login_test.go @@ -313,6 +313,145 @@ func TestRunLogin(t *testing.T) { } } +func TestLoginNonInteractive(t *testing.T) { + t.Run("no prior credentials", func(t *testing.T) { + testCases := []struct { + doc string + username bool + password bool + expectedErr string + }{ + { + doc: "success - w/ user w/ password", + username: true, + password: true, + }, + { + doc: "error - w/o user w/o pass ", + username: false, + password: false, + expectedErr: "Error: Cannot perform an interactive login from a non TTY device", + }, + { + doc: "error - w/ user w/o pass", + username: true, + password: false, + expectedErr: "Error: Cannot perform an interactive login from a non TTY device", + }, + { + doc: "error - w/o user w/ pass", + username: false, + password: true, + expectedErr: "Error: Cannot perform an interactive login from a non TTY device", + }, + } + + // "" meaning default registry + registries := []string{"", "my-registry.com"} + + for _, registry := 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() + options := loginOptions{ + serverAddress: registry, + } + if tc.username { + options.user = "my-username" + } + if tc.password { + options.password = "my-password" + } + + loginErr := runLogin(context.Background(), cli, options) + if tc.expectedErr != "" { + assert.Error(t, loginErr, tc.expectedErr) + return + } + assert.NilError(t, loginErr) + }) + } + } + }) + + t.Run("w/ prior credentials", func(t *testing.T) { + testCases := []struct { + doc string + username bool + password bool + expectedErr string + }{ + { + doc: "success - w/ user w/ password", + username: true, + password: true, + }, + { + doc: "success - w/o user w/o pass ", + username: false, + password: false, + }, + { + doc: "error - w/ user w/o pass", + username: true, + password: false, + expectedErr: "Error: Cannot perform an interactive login from a non TTY device", + }, + { + doc: "error - w/o user w/ pass", + username: false, + password: true, + expectedErr: "Error: Cannot perform an interactive login from a non TTY device", + }, + } + + // "" meaning default registry + registries := []string{"", "my-registry.com"} + + for _, registry := 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 + if serverAddress == "" { + serverAddress = "https://index.docker.io/v1/" + } + assert.NilError(t, configfile.GetCredentialsStore(serverAddress).Store(configtypes.AuthConfig{ + Username: "my-username", + Password: "my-password", + ServerAddress: serverAddress, + })) + + options := loginOptions{ + serverAddress: registry, + } + if tc.username { + options.user = "my-username" + } + if tc.password { + options.password = "my-password" + } + + loginErr := runLogin(context.Background(), cli, options) + if tc.expectedErr != "" { + assert.Error(t, loginErr, tc.expectedErr) + return + } + assert.NilError(t, loginErr) + }) + } + } + }) +} + func TestLoginTermination(t *testing.T) { p, tty, err := pty.Open() assert.NilError(t, err)