From 8883cd636a91f7c38f31005c09d14cfd6fe25a92 Mon Sep 17 00:00:00 2001 From: "shhsu@microsoft.com" Date: Tue, 30 May 2017 14:36:15 -0700 Subject: [PATCH] Silent login: if user did not provide -u and -p flag for login command but both username and password are retrieved in cred store, docker will automatically use the credentials found in the cred store to log in Signed-off-by: shhsu@microsoft.com Signed-off-by: Peter Hsu Signed-off-by: shhsu Signed-off-by: Peter Hsu --- cli/command/registry.go | 49 ++++++---- cli/command/registry/login.go | 55 ++++++++--- cli/command/registry/login_test.go | 151 +++++++++++++++++++++++++++++ cli/command/registry_test.go | 71 ++++++++++++++ internal/test/store.go | 30 +++--- 5 files changed, 313 insertions(+), 43 deletions(-) create mode 100644 cli/command/registry/login_test.go diff --git a/cli/command/registry.go b/cli/command/registry.go index f6e5ac4659b7..cb9a3947da8c 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -52,11 +52,15 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf fmt.Fprintf(cli.Out(), "\nPlease login prior to %s:\n", cmdName) indexServer := registry.GetAuthConfigKey(index) isDefaultRegistry := indexServer == ElectAuthServer(context.Background(), cli) - authConfig, err := ConfigureAuth(cli, "", "", indexServer, isDefaultRegistry) + authConfig, err := GetDefaultAuthConfig(cli, true, indexServer, isDefaultRegistry) + if err != nil { + fmt.Fprintf(cli.Err(), "Unable to retrieve stored credentials for %s, error: %s.\n", indexServer, err) + } + err = ConfigureAuth(cli, "", "", authConfig, isDefaultRegistry) if err != nil { return "", err } - return EncodeAuthToBase64(authConfig) + return EncodeAuthToBase64(*authConfig) } } @@ -73,20 +77,29 @@ func ResolveAuthConfig(ctx context.Context, cli Cli, index *registrytypes.IndexI return a } -// ConfigureAuth returns an AuthConfig from the specified user, password and server. -func ConfigureAuth(cli Cli, flUser, flPassword, serverAddress string, isDefaultRegistry bool) (types.AuthConfig, error) { - // On Windows, force the use of the regular OS stdin stream. Fixes #14336/#14210 - if runtime.GOOS == "windows" { - cli.SetIn(NewInStream(os.Stdin)) - } - +// GetDefaultAuthConfig gets the default auth config given a serverAddress +// If credentials for given serverAddress exists in the credential store, the configuration will be populated with values in it +func GetDefaultAuthConfig(cli Cli, checkCredStore bool, serverAddress string, isDefaultRegistry bool) (*types.AuthConfig, error) { if !isDefaultRegistry { serverAddress = registry.ConvertToHostname(serverAddress) } + var authconfig types.AuthConfig + var err error + if checkCredStore { + authconfig, err = cli.ConfigFile().GetAuthConfig(serverAddress) + } else { + authconfig = types.AuthConfig{} + } + authconfig.ServerAddress = serverAddress + authconfig.IdentityToken = "" + return &authconfig, err +} - authconfig, err := cli.ConfigFile().GetAuthConfig(serverAddress) - if err != nil { - return authconfig, err +// ConfigureAuth handles prompting of user's username and password if needed +func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *types.AuthConfig, isDefaultRegistry bool) error { + // On Windows, force the use of the regular OS stdin stream. Fixes #14336/#14210 + if runtime.GOOS == "windows" { + cli.SetIn(NewInStream(os.Stdin)) } // Some links documenting this: @@ -97,7 +110,7 @@ func ConfigureAuth(cli Cli, flUser, flPassword, serverAddress string, isDefaultR // will hit this if you attempt docker login from mintty where stdin // is a pipe, not a character based console. if flPassword == "" && !cli.In().IsTerminal() { - return authconfig, errors.Errorf("Error: Cannot perform an interactive login from a non TTY device") + return errors.Errorf("Error: Cannot perform an interactive login from a non TTY device") } authconfig.Username = strings.TrimSpace(authconfig.Username) @@ -115,12 +128,12 @@ func ConfigureAuth(cli Cli, flUser, flPassword, serverAddress string, isDefaultR } } if flUser == "" { - return authconfig, errors.Errorf("Error: Non-null Username Required") + return errors.Errorf("Error: Non-null Username Required") } if flPassword == "" { oldState, err := term.SaveState(cli.In().FD()) if err != nil { - return authconfig, err + return err } fmt.Fprintf(cli.Out(), "Password: ") term.DisableEcho(cli.In().FD(), oldState) @@ -130,16 +143,14 @@ func ConfigureAuth(cli Cli, flUser, flPassword, serverAddress string, isDefaultR term.RestoreTerminal(cli.In().FD(), oldState) if flPassword == "" { - return authconfig, errors.Errorf("Error: Password Required") + return errors.Errorf("Error: Password Required") } } authconfig.Username = flUser authconfig.Password = flPassword - authconfig.ServerAddress = serverAddress - authconfig.IdentityToken = "" - return authconfig, nil + return nil } func readInput(in io.Reader, out io.Writer) string { diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index 938fef8ab7d0..b538fb008418 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -9,6 +9,9 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" + "github.com/docker/docker/api/types" + registrytypes "github.com/docker/docker/api/types/registry" + "github.com/docker/docker/client" "github.com/docker/docker/registry" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -47,10 +50,7 @@ func NewLoginCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runLogin(dockerCli command.Cli, opts loginOptions) error { - ctx := context.Background() - clnt := dockerCli.Client() - +func verifyloginOptions(dockerCli command.Cli, opts *loginOptions) error { if opts.password != "" { fmt.Fprintln(dockerCli.Err(), "WARNING! Using --password via the CLI is insecure. Use --password-stdin.") if opts.passwordStdin { @@ -71,7 +71,15 @@ func runLogin(dockerCli command.Cli, opts loginOptions) error { opts.password = strings.TrimSuffix(string(contents), "\n") opts.password = strings.TrimSuffix(opts.password, "\r") } + return nil +} +func runLogin(dockerCli command.Cli, opts loginOptions) error { + ctx := context.Background() + clnt := dockerCli.Client() + if err := verifyloginOptions(dockerCli, &opts); err != nil { + return err + } var ( serverAddress string authServer = command.ElectAuthServer(ctx, dockerCli) @@ -82,21 +90,30 @@ func runLogin(dockerCli command.Cli, opts loginOptions) error { serverAddress = authServer } + var err error + var authConfig *types.AuthConfig + var response registrytypes.AuthenticateOKBody isDefaultRegistry := serverAddress == authServer - - authConfig, err := command.ConfigureAuth(dockerCli, opts.user, opts.password, serverAddress, isDefaultRegistry) - if err != nil { - return err + authConfig, err = command.GetDefaultAuthConfig(dockerCli, opts.user == "" && opts.password == "", serverAddress, isDefaultRegistry) + if err == nil && authConfig.Username != "" && authConfig.Password != "" { + response, err = loginWithCredStoreCreds(ctx, dockerCli, authConfig) } - response, err := clnt.RegistryLogin(ctx, authConfig) - if err != nil { - return err + if err != nil || authConfig.Username == "" || authConfig.Password == "" { + err = command.ConfigureAuth(dockerCli, opts.user, opts.password, authConfig, isDefaultRegistry) + if err != nil { + return err + } + + response, err = clnt.RegistryLogin(ctx, *authConfig) + if err != nil { + return err + } } if response.IdentityToken != "" { authConfig.Password = "" authConfig.IdentityToken = response.IdentityToken } - if err := dockerCli.ConfigFile().GetCredentialsStore(serverAddress).Store(authConfig); err != nil { + if err := dockerCli.ConfigFile().GetCredentialsStore(serverAddress).Store(*authConfig); err != nil { return errors.Errorf("Error saving credentials: %v", err) } @@ -105,3 +122,17 @@ func runLogin(dockerCli command.Cli, opts loginOptions) error { } return nil } + +func loginWithCredStoreCreds(ctx context.Context, dockerCli command.Cli, authConfig *types.AuthConfig) (registrytypes.AuthenticateOKBody, error) { + fmt.Fprintf(dockerCli.Out(), "Authenticating with existing credentials...\n") + cliClient := dockerCli.Client() + response, err := cliClient.RegistryLogin(ctx, *authConfig) + if err != nil { + if client.IsErrUnauthorized(err) { + fmt.Fprintf(dockerCli.Err(), "Stored credentials invalid or expired\n") + } else { + fmt.Fprintf(dockerCli.Err(), "Login did not succeed, error: %s\n", err) + } + } + return response, err +} diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go new file mode 100644 index 000000000000..498e0267f27a --- /dev/null +++ b/cli/command/registry/login_test.go @@ -0,0 +1,151 @@ +package registry + +import ( + "bytes" + "fmt" + "testing" + + "golang.org/x/net/context" + + "github.com/docker/cli/internal/test" + "github.com/docker/docker/api/types" + registrytypes "github.com/docker/docker/api/types/registry" + "github.com/docker/docker/client" + "github.com/stretchr/testify/assert" +) + +const userErr = "userunknownError" +const testAuthErrMsg = "UNKNOWN_ERR" + +var testAuthErrors = map[string]error{ + userErr: fmt.Errorf(testAuthErrMsg), +} + +var expiredPassword = "I_M_EXPIRED" + +type fakeClient struct { + client.Client +} + +// nolint: unparam +func (c fakeClient) RegistryLogin(ctx context.Context, auth types.AuthConfig) (registrytypes.AuthenticateOKBody, error) { + if auth.Password == expiredPassword { + return registrytypes.AuthenticateOKBody{}, fmt.Errorf("Invalid Username or Password") + } + err := testAuthErrors[auth.Username] + return registrytypes.AuthenticateOKBody{}, err +} + +func TestLoginWithCredStoreCreds(t *testing.T) { + testCases := []struct { + inputAuthConfig types.AuthConfig + expectedMsg string + expectedErr string + }{ + { + inputAuthConfig: types.AuthConfig{}, + expectedMsg: "Authenticating with existing credentials...\n", + }, + { + inputAuthConfig: types.AuthConfig{ + Username: userErr, + }, + expectedMsg: "Authenticating with existing credentials...\n", + expectedErr: fmt.Sprintf("Login did not succeed, error: %s\n", testAuthErrMsg), + }, + // can't easily test the 401 case because client.IsErrUnauthorized(err) involving + // creating an error of a private type + } + ctx := context.Background() + for _, tc := range testCases { + cli := (*test.FakeCli)(test.NewFakeCli(&fakeClient{})) + errBuf := new(bytes.Buffer) + cli.SetErr(errBuf) + loginWithCredStoreCreds(ctx, cli, &tc.inputAuthConfig) + outputString := cli.OutBuffer().String() + assert.Equal(t, tc.expectedMsg, outputString) + errorString := errBuf.String() + assert.Equal(t, tc.expectedErr, errorString) + } +} + +func TestRunLogin(t *testing.T) { + const storedServerAddress = "reg1" + const validUsername = "u1" + const validPassword = "p1" + const validPassword2 = "p2" + + validAuthConfig := types.AuthConfig{ + ServerAddress: storedServerAddress, + Username: validUsername, + Password: validPassword, + } + expiredAuthConfig := types.AuthConfig{ + ServerAddress: storedServerAddress, + Username: validUsername, + Password: expiredPassword, + } + testCases := []struct { + inputLoginOption loginOptions + inputStoredCred *types.AuthConfig + expectedErr string + expectedSavedCred types.AuthConfig + }{ + { + inputLoginOption: loginOptions{ + serverAddress: storedServerAddress, + }, + inputStoredCred: &validAuthConfig, + expectedErr: "", + expectedSavedCred: validAuthConfig, + }, + { + inputLoginOption: loginOptions{ + serverAddress: storedServerAddress, + }, + inputStoredCred: &expiredAuthConfig, + expectedErr: "Error: Cannot perform an interactive login from a non TTY device", + }, + { + inputLoginOption: loginOptions{ + serverAddress: storedServerAddress, + user: validUsername, + password: validPassword2, + }, + inputStoredCred: &validAuthConfig, + expectedErr: "", + expectedSavedCred: types.AuthConfig{ + ServerAddress: storedServerAddress, + Username: validUsername, + Password: validPassword2, + }, + }, + { + inputLoginOption: loginOptions{ + serverAddress: storedServerAddress, + user: userErr, + password: validPassword, + }, + inputStoredCred: &validAuthConfig, + expectedErr: testAuthErrMsg, + }, + } + for _, tc := range testCases { + cli := test.NewFakeCli(&fakeClient{}) + errBuf := new(bytes.Buffer) + cli.SetErr(errBuf) + if tc.inputStoredCred != nil { + cred := *tc.inputStoredCred + cli.ConfigFile().GetCredentialsStore(cred.ServerAddress).Store(cred) + } + loginErr := runLogin(cli, tc.inputLoginOption) + if tc.expectedErr != "" { + assert.Equal(t, tc.expectedErr, loginErr.Error()) + } else { + assert.Nil(t, loginErr) + savedCred, credStoreErr := cli.ConfigFile().GetCredentialsStore(tc.inputStoredCred.ServerAddress).Get(tc.inputStoredCred.ServerAddress) + assert.Nil(t, credStoreErr) + assert.Equal(t, tc.expectedSavedCred, savedCred) + } + } +} diff --git a/cli/command/registry_test.go b/cli/command/registry_test.go index 3f3d5f579ffc..3672ae8d456d 100644 --- a/cli/command/registry_test.go +++ b/cli/command/registry_test.go @@ -1,6 +1,8 @@ package command_test import ( + "bytes" + "fmt" "testing" "github.com/pkg/errors" @@ -8,6 +10,7 @@ import ( "golang.org/x/net/context" // Prevents a circular import with "github.com/docker/cli/internal/test" + . "github.com/docker/cli/cli/command" "github.com/docker/cli/internal/test" "github.com/docker/docker/api/types" @@ -19,6 +22,19 @@ type fakeClient struct { infoFunc func() (types.Info, error) } +var testAuthConfigs = []types.AuthConfig{ + { + ServerAddress: "https://index.docker.io/v1/", + Username: "u0", + Password: "p0", + }, + { + ServerAddress: "server1.io", + Username: "u1", + Password: "p1", + }, +} + func (cli *fakeClient) Info(_ context.Context) (types.Info, error) { if cli.infoFunc != nil { return cli.infoFunc() @@ -73,3 +89,58 @@ func TestElectAuthServer(t *testing.T) { } } } + +func TestGetDefaultAuthConfig(t *testing.T) { + testCases := []struct { + checkCredStore bool + inputServerAddress string + expectedErr string + expectedAuthConfig types.AuthConfig + }{ + { + checkCredStore: false, + inputServerAddress: "", + expectedErr: "", + expectedAuthConfig: types.AuthConfig{ + ServerAddress: "", + Username: "", + Password: "", + }, + }, + { + checkCredStore: true, + inputServerAddress: testAuthConfigs[0].ServerAddress, + expectedErr: "", + expectedAuthConfig: testAuthConfigs[0], + }, + { + checkCredStore: true, + inputServerAddress: testAuthConfigs[1].ServerAddress, + expectedErr: "", + expectedAuthConfig: testAuthConfigs[1], + }, + { + checkCredStore: true, + inputServerAddress: fmt.Sprintf("https://%s", testAuthConfigs[1].ServerAddress), + expectedErr: "", + expectedAuthConfig: testAuthConfigs[1], + }, + } + cli := test.NewFakeCli(&fakeClient{}) + errBuf := new(bytes.Buffer) + cli.SetErr(errBuf) + for _, authconfig := range testAuthConfigs { + cli.ConfigFile().GetCredentialsStore(authconfig.ServerAddress).Store(authconfig) + } + for _, tc := range testCases { + serverAddress := tc.inputServerAddress + authconfig, err := GetDefaultAuthConfig(cli, tc.checkCredStore, serverAddress, serverAddress == "https://index.docker.io/v1/") + if tc.expectedErr != "" { + assert.NotNil(t, err) + assert.Equal(t, tc.expectedErr, err.Error()) + } else { + assert.Nil(t, err) + assert.Equal(t, tc.expectedAuthConfig, *authconfig) + } + } +} diff --git a/internal/test/store.go b/internal/test/store.go index e5b3de7abb77..35565dc68ebd 100644 --- a/internal/test/store.go +++ b/internal/test/store.go @@ -5,8 +5,8 @@ import ( "github.com/docker/docker/api/types" ) -// fake store implements a credentials.Store that only acts as an in memory map -type fakeStore struct { +// FakeStore implements a credentials.Store that only acts as an in memory map +type FakeStore struct { store map[string]types.AuthConfig eraseFunc func(serverAddress string) error getFunc func(serverAddress string) (types.AuthConfig, error) @@ -16,31 +16,36 @@ type fakeStore struct { // NewFakeStore creates a new file credentials store. func NewFakeStore() credentials.Store { - return &fakeStore{store: map[string]types.AuthConfig{}} + return &FakeStore{store: map[string]types.AuthConfig{}} } -func (c *fakeStore) SetStore(store map[string]types.AuthConfig) { +// SetStore is used to overrides Set function +func (c *FakeStore) SetStore(store map[string]types.AuthConfig) { c.store = store } -func (c *fakeStore) SetEraseFunc(eraseFunc func(string) error) { +// SetEraseFunc is used to overrides Erase function +func (c *FakeStore) SetEraseFunc(eraseFunc func(string) error) { c.eraseFunc = eraseFunc } -func (c *fakeStore) SetGetFunc(getFunc func(string) (types.AuthConfig, error)) { +// SetGetFunc is used to overrides Get function +func (c *FakeStore) SetGetFunc(getFunc func(string) (types.AuthConfig, error)) { c.getFunc = getFunc } -func (c *fakeStore) SetGetAllFunc(getAllFunc func() (map[string]types.AuthConfig, error)) { +// SetGetAllFunc is used to overrides GetAll function +func (c *FakeStore) SetGetAllFunc(getAllFunc func() (map[string]types.AuthConfig, error)) { c.getAllFunc = getAllFunc } -func (c *fakeStore) SetStoreFunc(storeFunc func(types.AuthConfig) error) { +// SetStoreFunc is used to override Store function +func (c *FakeStore) SetStoreFunc(storeFunc func(types.AuthConfig) error) { c.storeFunc = storeFunc } // Erase removes the given credentials from the map store -func (c *fakeStore) Erase(serverAddress string) error { +func (c *FakeStore) Erase(serverAddress string) error { if c.eraseFunc != nil { return c.eraseFunc(serverAddress) } @@ -49,14 +54,15 @@ func (c *fakeStore) Erase(serverAddress string) error { } // Get retrieves credentials for a specific server from the map store. -func (c *fakeStore) Get(serverAddress string) (types.AuthConfig, error) { +func (c *FakeStore) Get(serverAddress string) (types.AuthConfig, error) { if c.getFunc != nil { return c.getFunc(serverAddress) } return c.store[serverAddress], nil } -func (c *fakeStore) GetAll() (map[string]types.AuthConfig, error) { +// GetAll returns the key value pairs of ServerAddress => Username +func (c *FakeStore) GetAll() (map[string]types.AuthConfig, error) { if c.getAllFunc != nil { return c.getAllFunc() } @@ -64,7 +70,7 @@ func (c *fakeStore) GetAll() (map[string]types.AuthConfig, error) { } // Store saves the given credentials in the map store. -func (c *fakeStore) Store(authConfig types.AuthConfig) error { +func (c *FakeStore) Store(authConfig types.AuthConfig) error { if c.storeFunc != nil { return c.storeFunc(authConfig) }