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 b745e06169..58ca73c6a6 100644 --- a/cli/command/registry/login_test.go +++ b/cli/command/registry/login_test.go @@ -550,12 +550,17 @@ func TestLoginValidateFlags(t *testing.T) { { name: "--password-stdin without --username", args: []string{"--password-stdin"}, - expectedErr: `Must provide --username with --password-stdin`, + expectedErr: `the --password-stdin option requires --username to be set`, }, { name: "--password-stdin with empty --username", args: []string{"--password-stdin", "--username", ""}, - expectedErr: `Must provide --username with --password-stdin`, + expectedErr: `username is empty`, + }, + { + name: "empty --username", + args: []string{"--username", ""}, + expectedErr: `username is empty`, }, { name: "--username without value", @@ -565,7 +570,12 @@ func TestLoginValidateFlags(t *testing.T) { { name: "conflicting options --password-stdin and --password", args: []string{"--password-stdin", "--password", ""}, - expectedErr: `Must provide --username with --password-stdin`, + expectedErr: `conflicting options: cannot specify both --password and --password-stdin`, + }, + { + name: "empty --password", + args: []string{"--password", ""}, + expectedErr: `password is empty`, }, { name: "--password without value",