diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index 701978c1cd..c33de68b5f 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -21,6 +21,7 @@ import ( "github.com/docker/docker/registry" "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) type loginOptions struct { @@ -43,6 +44,9 @@ func NewLoginCommand(dockerCLI command.Cli) *cobra.Command { if len(args) > 0 { opts.serverAddress = args[0] } + if err := verifyLoginFlags(cmd.Flags(), opts); err != nil { + return err + } return runLogin(cmd.Context(), dockerCLI, opts) }, Annotations: map[string]string{ @@ -60,17 +64,35 @@ func NewLoginCommand(dockerCLI command.Cli) *cobra.Command { return cmd } -func verifyLoginOptions(dockerCLI command.Cli, opts *loginOptions) error { +// verifyLoginFlags validates flags set on the command. +// +// TODO(thaJeztah); combine with verifyLoginOptions, but this requires rewrites of many tests. +func verifyLoginFlags(flags *pflag.FlagSet, opts loginOptions) error { + if flags.Changed("password-stdin") { + if flags.Changed("password") { + return errors.New("conflicting options: cannot specify both --password and --password-stdin") + } + if !flags.Changed("username") { + return errors.New("the --password-stdin option requires --username to be set") + } + } + if flags.Changed("username") && opts.user == "" { + return errors.New("username is empty") + } + if flags.Changed("password") && opts.password == "" { + return errors.New("password is empty") + } + return nil +} + +func verifyLoginOptions(dockerCLI command.Streams, opts *loginOptions) error { if opts.password != "" { _, _ = fmt.Fprintln(dockerCLI.Err(), "WARNING! Using --password via the CLI is insecure. Use --password-stdin.") - if opts.passwordStdin { - return errors.New("--password and --password-stdin are mutually exclusive") - } } if opts.passwordStdin { if opts.user == "" { - return errors.New("Must provide --username with --password-stdin") + return errors.New("username is empty") } contents, err := io.ReadAll(dockerCLI.In()) diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go index b12b5b6623..58ca73c6a6 100644 --- a/cli/command/registry/login_test.go +++ b/cli/command/registry/login_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "path/filepath" "testing" "time" @@ -539,3 +540,61 @@ func TestIsOauthLoginDisabled(t *testing.T) { assert.Equal(t, disabled, tc.disabled) } } + +func TestLoginValidateFlags(t *testing.T) { + for _, tc := range []struct { + name string + args []string + expectedErr string + }{ + { + name: "--password-stdin without --username", + args: []string{"--password-stdin"}, + expectedErr: `the --password-stdin option requires --username to be set`, + }, + { + name: "--password-stdin with empty --username", + args: []string{"--password-stdin", "--username", ""}, + expectedErr: `username is empty`, + }, + { + name: "empty --username", + args: []string{"--username", ""}, + expectedErr: `username is empty`, + }, + { + name: "--username without value", + args: []string{"--username"}, + expectedErr: `flag needs an argument: --username`, + }, + { + name: "conflicting options --password-stdin and --password", + args: []string{"--password-stdin", "--password", ""}, + expectedErr: `conflicting options: cannot specify both --password and --password-stdin`, + }, + { + name: "empty --password", + args: []string{"--password", ""}, + expectedErr: `password is empty`, + }, + { + name: "--password without value", + args: []string{"--password"}, + expectedErr: `flag needs an argument: --password`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + cmd := NewLoginCommand(test.NewFakeCli(&fakeClient{})) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs(tc.args) + + err := cmd.Execute() + if tc.expectedErr != "" { + assert.Check(t, is.ErrorContains(err, tc.expectedErr)) + } else { + assert.Check(t, is.Nil(err)) + } + }) + } +}