Merge pull request #6036 from thaJeztah/improve_username_handling

cli/command/registry: login: improve flag validation
This commit is contained in:
Sebastiaan van Stijn 2025-04-30 19:09:18 +02:00 committed by GitHub
commit 58d318b990
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 86 additions and 5 deletions

View File

@ -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())

View File

@ -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))
}
})
}
}