Skip to content

Commit d0c1a80

Browse files
laurazardlaurazard
committed
login: handle non-tty scenario consistently
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 <[email protected]> (cherry picked from commit bbb6e76) Signed-off-by: Laura Brehm <[email protected]>
1 parent 383c428 commit d0c1a80

File tree

3 files changed

+150
-11
lines changed

3 files changed

+150
-11
lines changed

‎cli/command/registry.go

-11
Original file line numberDiff line numberDiff line change
@@ -124,17 +124,6 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword
124124
cli.SetIn(streams.NewIn(os.Stdin))
125125
}
126126

127-
// Some links documenting this:
128-
// - https://code.google.com/archive/p/mintty/issues/56
129-
// - https://github.com/docker/docker/issues/15272
130-
// - https://mintty.github.io/ (compatibility)
131-
// Linux will hit this if you attempt `cat | docker login`, and Windows
132-
// will hit this if you attempt docker login from mintty where stdin
133-
// is a pipe, not a character based console.
134-
if argPassword == "" && !cli.In().IsTerminal() {
135-
return authConfig, errors.Errorf("Error: Cannot perform an interactive login from a non TTY device")
136-
}
137-
138127
isDefaultRegistry := serverAddress == registry.IndexServer
139128
defaultUsername = strings.TrimSpace(defaultUsername)
140129

‎cli/command/registry/login.go

+11
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,17 @@ func isOauthLoginDisabled() bool {
176176
}
177177

178178
func loginUser(ctx context.Context, dockerCli command.Cli, opts loginOptions, defaultUsername, serverAddress string) (*registrytypes.AuthenticateOKBody, error) {
179+
// Some links documenting this:
180+
// - https://code.google.com/archive/p/mintty/issues/56
181+
// - https://github.com/docker/docker/issues/15272
182+
// - https://mintty.github.io/ (compatibility)
183+
// Linux will hit this if you attempt `cat | docker login`, and Windows
184+
// will hit this if you attempt docker login from mintty where stdin
185+
// is a pipe, not a character based console.
186+
if (opts.user == "" || opts.password == "") && !dockerCli.In().IsTerminal() {
187+
return nil, errors.Errorf("Error: Cannot perform an interactive login from a non TTY device")
188+
}
189+
179190
// If we're logging into the index server and the user didn't provide a username or password, use the device flow
180191
if serverAddress == registry.IndexServer && opts.user == "" && opts.password == "" && !isOauthLoginDisabled() {
181192
response, err := loginWithDeviceCodeFlow(ctx, dockerCli)

‎cli/command/registry/login_test.go

+139
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,145 @@ func TestRunLogin(t *testing.T) {
313313
}
314314
}
315315

316+
func TestLoginNonInteractive(t *testing.T) {
317+
t.Run("no prior credentials", func(t *testing.T) {
318+
testCases := []struct {
319+
doc string
320+
username bool
321+
password bool
322+
expectedErr string
323+
}{
324+
{
325+
doc: "success - w/ user w/ password",
326+
username: true,
327+
password: true,
328+
},
329+
{
330+
doc: "error - w/o user w/o pass ",
331+
username: false,
332+
password: false,
333+
expectedErr: "Error: Cannot perform an interactive login from a non TTY device",
334+
},
335+
{
336+
doc: "error - w/ user w/o pass",
337+
username: true,
338+
password: false,
339+
expectedErr: "Error: Cannot perform an interactive login from a non TTY device",
340+
},
341+
{
342+
doc: "error - w/o user w/ pass",
343+
username: false,
344+
password: true,
345+
expectedErr: "Error: Cannot perform an interactive login from a non TTY device",
346+
},
347+
}
348+
349+
// "" meaning default registry
350+
registries := []string{"", "my-registry.com"}
351+
352+
for _, registry := range registries {
353+
for _, tc := range testCases {
354+
t.Run(tc.doc, func(t *testing.T) {
355+
tmpFile := fs.NewFile(t, "test-run-login")
356+
defer tmpFile.Remove()
357+
cli := test.NewFakeCli(&fakeClient{})
358+
configfile := cli.ConfigFile()
359+
configfile.Filename = tmpFile.Path()
360+
options := loginOptions{
361+
serverAddress: registry,
362+
}
363+
if tc.username {
364+
options.user = "my-username"
365+
}
366+
if tc.password {
367+
options.password = "my-password"
368+
}
369+
370+
loginErr := runLogin(context.Background(), cli, options)
371+
if tc.expectedErr != "" {
372+
assert.Error(t, loginErr, tc.expectedErr)
373+
return
374+
}
375+
assert.NilError(t, loginErr)
376+
})
377+
}
378+
}
379+
})
380+
381+
t.Run("w/ prior credentials", func(t *testing.T) {
382+
testCases := []struct {
383+
doc string
384+
username bool
385+
password bool
386+
expectedErr string
387+
}{
388+
{
389+
doc: "success - w/ user w/ password",
390+
username: true,
391+
password: true,
392+
},
393+
{
394+
doc: "success - w/o user w/o pass ",
395+
username: false,
396+
password: false,
397+
},
398+
{
399+
doc: "error - w/ user w/o pass",
400+
username: true,
401+
password: false,
402+
expectedErr: "Error: Cannot perform an interactive login from a non TTY device",
403+
},
404+
{
405+
doc: "error - w/o user w/ pass",
406+
username: false,
407+
password: true,
408+
expectedErr: "Error: Cannot perform an interactive login from a non TTY device",
409+
},
410+
}
411+
412+
// "" meaning default registry
413+
registries := []string{"", "my-registry.com"}
414+
415+
for _, registry := range registries {
416+
for _, tc := range testCases {
417+
t.Run(tc.doc, func(t *testing.T) {
418+
tmpFile := fs.NewFile(t, "test-run-login")
419+
defer tmpFile.Remove()
420+
cli := test.NewFakeCli(&fakeClient{})
421+
configfile := cli.ConfigFile()
422+
configfile.Filename = tmpFile.Path()
423+
serverAddress := registry
424+
if serverAddress == "" {
425+
serverAddress = "https://index.docker.io/v1/"
426+
}
427+
assert.NilError(t, configfile.GetCredentialsStore(serverAddress).Store(configtypes.AuthConfig{
428+
Username: "my-username",
429+
Password: "my-password",
430+
ServerAddress: serverAddress,
431+
}))
432+
433+
options := loginOptions{
434+
serverAddress: registry,
435+
}
436+
if tc.username {
437+
options.user = "my-username"
438+
}
439+
if tc.password {
440+
options.password = "my-password"
441+
}
442+
443+
loginErr := runLogin(context.Background(), cli, options)
444+
if tc.expectedErr != "" {
445+
assert.Error(t, loginErr, tc.expectedErr)
446+
return
447+
}
448+
assert.NilError(t, loginErr)
449+
})
450+
}
451+
}
452+
})
453+
}
454+
316455
func TestLoginTermination(t *testing.T) {
317456
p, tty, err := pty.Open()
318457
assert.NilError(t, err)

0 commit comments

Comments
 (0)