Skip to content

Commit

Permalink
Merge pull request docker#5401 from laurazard/login-non-tty-standardize
Browse files Browse the repository at this point in the history
login: handle non-tty scenario consistently
  • Loading branch information
thaJeztah authored Sep 3, 2024
2 parents 2dd127a + bbb6e76 commit f75abad
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 11 deletions.
11 changes: 0 additions & 11 deletions cli/command/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
11 changes: 11 additions & 0 deletions cli/command/registry/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
139 changes: 139 additions & 0 deletions cli/command/registry/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit f75abad

Please sign in to comment.