Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 50 additions & 26 deletions cli/command/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
default:
}

err = ConfigureAuth(ctx, cli, "", "", &authConfig, isDefaultRegistry)
authConfig, err = PromptUserForCredentials(ctx, cli, "", "", authConfig.Username, indexServer)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -86,8 +86,32 @@ func GetDefaultAuthConfig(cfg *configfile.ConfigFile, checkCredStore bool, serve
return registrytypes.AuthConfig(authconfig), nil
}

// ConfigureAuth handles prompting of user's username and password if needed
func ConfigureAuth(ctx context.Context, cli Cli, flUser, flPassword string, authconfig *registrytypes.AuthConfig, isDefaultRegistry bool) error {
// ConfigureAuth handles prompting of user's username and password if needed.
// Deprecated: use PromptUserForCredentials instead.
func ConfigureAuth(ctx context.Context, cli Cli, flUser, flPassword string, authConfig *registrytypes.AuthConfig, _ bool) error {
defaultUsername := authConfig.Username
serverAddress := authConfig.ServerAddress

newAuthConfig, err := PromptUserForCredentials(ctx, cli, flUser, flPassword, defaultUsername, serverAddress)
if err != nil {
return err
}

authConfig.Username = newAuthConfig.Username
authConfig.Password = newAuthConfig.Password
return nil
}

// PromptUserForCredentials handles the CLI prompt for the user to input
// credentials.
// If argUser is not empty, then the user is only prompted for their password.
// If argPassword is not empty, then the user is only prompted for their username
// If neither argUser nor argPassword are empty, then the user is not prompted and
// an AuthConfig is returned with those values.
// If defaultUsername is not empty, the username prompt includes that username
// and the user can hit enter without inputting a username to use that default
// username.
func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword, defaultUsername, serverAddress string) (authConfig registrytypes.AuthConfig, err error) {
// On Windows, force the use of the regular OS stdin stream.
//
// See:
Expand All @@ -107,13 +131,14 @@ func ConfigureAuth(ctx context.Context, cli Cli, flUser, flPassword string, auth
// 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 flPassword == "" && !cli.In().IsTerminal() {
return errors.Errorf("Error: Cannot perform an interactive login from a non TTY device")
if argPassword == "" && !cli.In().IsTerminal() {
return authConfig, errors.Errorf("Error: Cannot perform an interactive login from a non TTY device")
}

authconfig.Username = strings.TrimSpace(authconfig.Username)
isDefaultRegistry := serverAddress == registry.IndexServer
defaultUsername = strings.TrimSpace(defaultUsername)

if flUser = strings.TrimSpace(flUser); flUser == "" {
if argUser = strings.TrimSpace(argUser); argUser == "" {
if isDefaultRegistry {
// if this is a default registry (docker hub), then display the following message.
fmt.Fprintln(cli.Out(), "Log in with your Docker ID or email address to push and pull images from Docker Hub. If you don't have a Docker ID, head over to https://hub.docker.com/ to create one.")
Expand All @@ -124,44 +149,43 @@ func ConfigureAuth(ctx context.Context, cli Cli, flUser, flPassword string, auth
}

var prompt string
if authconfig.Username == "" {
if defaultUsername == "" {
prompt = "Username: "
} else {
prompt = fmt.Sprintf("Username (%s): ", authconfig.Username)
prompt = fmt.Sprintf("Username (%s): ", defaultUsername)
}
var err error
flUser, err = PromptForInput(ctx, cli.In(), cli.Out(), prompt)
argUser, err = PromptForInput(ctx, cli.In(), cli.Out(), prompt)
if err != nil {
return err
return authConfig, err
}
if flUser == "" {
flUser = authconfig.Username
if argUser == "" {
argUser = defaultUsername
}
}
if flUser == "" {
return errors.Errorf("Error: Non-null Username Required")
if argUser == "" {
return authConfig, errors.Errorf("Error: Non-null Username Required")
}
if flPassword == "" {
if argPassword == "" {
restoreInput, err := DisableInputEcho(cli.In())
if err != nil {
return err
return authConfig, err
}
defer restoreInput()

flPassword, err = PromptForInput(ctx, cli.In(), cli.Out(), "Password: ")
argPassword, err = PromptForInput(ctx, cli.In(), cli.Out(), "Password: ")
if err != nil {
return err
return authConfig, err
}
fmt.Fprint(cli.Out(), "\n")
if flPassword == "" {
return errors.Errorf("Error: Password Required")
if argPassword == "" {
return authConfig, errors.Errorf("Error: Password Required")
}
}

authconfig.Username = flUser
authconfig.Password = flPassword

return nil
authConfig.Username = argUser
authConfig.Password = argPassword
authConfig.ServerAddress = serverAddress
return authConfig, nil
}

// RetrieveAuthTokenFromImage retrieves an encoded auth token given a complete
Expand Down
132 changes: 104 additions & 28 deletions cli/command/registry/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/completion"
configtypes "github.com/docker/cli/cli/config/types"
"github.com/docker/cli/cli/internal/oauth/manager"
registrytypes "github.com/docker/docker/api/types/registry"
"github.com/docker/docker/client"
"github.com/docker/docker/errdefs"
Expand Down Expand Up @@ -79,70 +80,145 @@ func verifyloginOptions(dockerCli command.Cli, opts *loginOptions) error {
return nil
}

func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) error { //nolint:gocyclo
clnt := dockerCli.Client()
func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) error {
if err := verifyloginOptions(dockerCli, &opts); err != nil {
return err
}
var (
serverAddress string
response registrytypes.AuthenticateOKBody
response *registrytypes.AuthenticateOKBody
)
if opts.serverAddress != "" && opts.serverAddress != registry.DefaultNamespace {
if opts.serverAddress != "" &&
opts.serverAddress != registry.DefaultNamespace &&
opts.serverAddress != registry.DefaultRegistryHost {
serverAddress = opts.serverAddress
} else {
serverAddress = registry.IndexServer
}

isDefaultRegistry := serverAddress == registry.IndexServer

// attempt login with current (stored) credentials
authConfig, err := command.GetDefaultAuthConfig(dockerCli.ConfigFile(), opts.user == "" && opts.password == "", serverAddress, isDefaultRegistry)
if err == nil && authConfig.Username != "" && authConfig.Password != "" {
response, err = loginWithCredStoreCreds(ctx, dockerCli, &authConfig)
response, err = loginWithStoredCredentials(ctx, dockerCli, authConfig)
}

// if we failed to authenticate with stored credentials (or didn't have stored credentials),
// prompt the user for new credentials
if err != nil || authConfig.Username == "" || authConfig.Password == "" {
err = command.ConfigureAuth(ctx, dockerCli, opts.user, opts.password, &authConfig, isDefaultRegistry)
response, err = loginUser(ctx, dockerCli, opts, authConfig.Username, serverAddress)
if err != nil {
return err
}
}

if response != nil && response.Status != "" {
_, _ = fmt.Fprintln(dockerCli.Out(), response.Status)
}
return nil
}

response, err = clnt.RegistryLogin(ctx, authConfig)
if err != nil && client.IsErrConnectionFailed(err) {
// If the server isn't responding (yet) attempt to login purely client side
response, err = loginClientSide(ctx, authConfig)
func loginWithStoredCredentials(ctx context.Context, dockerCli command.Cli, authConfig registrytypes.AuthConfig) (*registrytypes.AuthenticateOKBody, error) {
_, _ = fmt.Fprintf(dockerCli.Out(), "Authenticating with existing credentials...\n")
response, err := dockerCli.Client().RegistryLogin(ctx, authConfig)
if err != nil {
if errdefs.IsUnauthorized(err) {
_, _ = fmt.Fprintf(dockerCli.Err(), "Stored credentials invalid or expired\n")
} else {
_, _ = fmt.Fprintf(dockerCli.Err(), "Login did not succeed, error: %s\n", err)
}
// If we (still) have an error, give up
if err != nil {
return err
}

if response.IdentityToken != "" {
authConfig.Password = ""
authConfig.IdentityToken = response.IdentityToken
}

if err := storeCredentials(dockerCli, authConfig); err != nil {
return nil, err
}

return &response, err
}

func loginUser(ctx context.Context, dockerCli command.Cli, opts loginOptions, defaultUsername, serverAddress string) (*registrytypes.AuthenticateOKBody, error) {
// 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 == "" {
response, err := loginWithDeviceCodeFlow(ctx, dockerCli)
// if the error represents a failure to initiate the device-code flow,
// then we fallback to regular cli credentials login
if !errors.Is(err, manager.ErrDeviceLoginStartFail) {
return response, err
}
fmt.Fprint(dockerCli.Err(), "Failed to start web-based login - falling back to command line login...\n\n")
}

return loginWithUsernameAndPassword(ctx, dockerCli, opts, defaultUsername, serverAddress)
}

func loginWithUsernameAndPassword(ctx context.Context, dockerCli command.Cli, opts loginOptions, defaultUsername, serverAddress string) (*registrytypes.AuthenticateOKBody, error) {
// Prompt user for credentials
authConfig, err := command.PromptUserForCredentials(ctx, dockerCli, opts.user, opts.password, defaultUsername, serverAddress)
if err != nil {
return nil, err
}

response, err := loginWithRegistry(ctx, dockerCli, authConfig)
if err != nil {
return nil, err
}

if response.IdentityToken != "" {
authConfig.Password = ""
authConfig.IdentityToken = response.IdentityToken
}
if err = storeCredentials(dockerCli, authConfig); err != nil {
return nil, err
}

return &response, nil
}

func loginWithDeviceCodeFlow(ctx context.Context, dockerCli command.Cli) (*registrytypes.AuthenticateOKBody, error) {
store := dockerCli.ConfigFile().GetCredentialsStore(registry.IndexServer)
authConfig, err := manager.NewManager(store).LoginDevice(ctx, dockerCli.Err())
if err != nil {
return nil, err
}

response, err := loginWithRegistry(ctx, dockerCli, registrytypes.AuthConfig(*authConfig))
if err != nil {
return nil, err
}

if err = storeCredentials(dockerCli, registrytypes.AuthConfig(*authConfig)); err != nil {
return nil, err
}

creds := dockerCli.ConfigFile().GetCredentialsStore(serverAddress)
return &response, nil
}

func storeCredentials(dockerCli command.Cli, authConfig registrytypes.AuthConfig) error {
creds := dockerCli.ConfigFile().GetCredentialsStore(authConfig.ServerAddress)
if err := creds.Store(configtypes.AuthConfig(authConfig)); err != nil {
return errors.Errorf("Error saving credentials: %v", err)
}

if response.Status != "" {
fmt.Fprintln(dockerCli.Out(), response.Status)
}
return nil
}

func loginWithCredStoreCreds(ctx context.Context, dockerCli command.Cli, authConfig *registrytypes.AuthConfig) (registrytypes.AuthenticateOKBody, error) {
fmt.Fprintf(dockerCli.Out(), "Authenticating with existing credentials...\n")
cliClient := dockerCli.Client()
response, err := cliClient.RegistryLogin(ctx, *authConfig)
func loginWithRegistry(ctx context.Context, dockerCli command.Cli, authConfig registrytypes.AuthConfig) (registrytypes.AuthenticateOKBody, error) {
response, err := dockerCli.Client().RegistryLogin(ctx, authConfig)
if err != nil && client.IsErrConnectionFailed(err) {
// If the server isn't responding (yet) attempt to login purely client side
response, err = loginClientSide(ctx, authConfig)
}
// If we (still) have an error, give up
if err != nil {
if errdefs.IsUnauthorized(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 registrytypes.AuthenticateOKBody{}, err
}
return response, err

return response, nil
}

func loginClientSide(ctx context.Context, auth registrytypes.AuthConfig) (registrytypes.AuthenticateOKBody, error) {
Expand Down
6 changes: 4 additions & 2 deletions cli/command/registry/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestLoginWithCredStoreCreds(t *testing.T) {
cli := test.NewFakeCli(&fakeClient{})
errBuf := new(bytes.Buffer)
cli.SetErr(streams.NewOut(errBuf))
loginWithCredStoreCreds(ctx, cli, &tc.inputAuthConfig)
loginWithStoredCredentials(ctx, cli, tc.inputAuthConfig)
outputString := cli.OutBuffer().String()
assert.Check(t, is.Equal(tc.expectedMsg, outputString))
errorString := errBuf.String()
Expand Down Expand Up @@ -213,7 +213,9 @@ func TestLoginTermination(t *testing.T) {

runErr := make(chan error)
go func() {
runErr <- runLogin(ctx, cli, loginOptions{})
runErr <- runLogin(ctx, cli, loginOptions{
user: "test-user",
})
}()

// Let the prompt get canceled by the context
Expand Down
10 changes: 9 additions & 1 deletion cli/command/registry/logout.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/config/credentials"
"github.com/docker/cli/cli/internal/oauth/manager"
"github.com/docker/docker/registry"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -34,7 +35,7 @@ func NewLogoutCommand(dockerCli command.Cli) *cobra.Command {
return cmd
}

func runLogout(_ context.Context, dockerCli command.Cli, serverAddress string) error {
func runLogout(ctx context.Context, dockerCli command.Cli, serverAddress string) error {
var isDefaultRegistry bool

if serverAddress == "" {
Expand All @@ -53,6 +54,13 @@ func runLogout(_ context.Context, dockerCli command.Cli, serverAddress string) e
regsToLogout = append(regsToLogout, hostnameAddress, "http://"+hostnameAddress, "https://"+hostnameAddress)
}

if isDefaultRegistry {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think this is new, but we're now using it twice; wondering if we're missing a new check for isDefaultRegistry after the above branch does a credentials.ConvertToHostname(serverAddress)

(unrelated: I think the intent of ConvertToHostname is effectively "normalise storage key" - we need to look at some of that at some point - naming is confusing, and not always properly describes intent)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so? But we could dig into that after this. Much like command/registry/login.go, this could use a bit of a cleanup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On another look, I guess a user could try to logout by doing docker logout index.docker.io and we should make that work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On another look, I guess a user could try to logout by doing docker logout index.docker.io and we should make that work.

Yes, that's roughly the scenario I had in mind. Technically it could go even further (log out for [http(s)://]registry-1.docker.io etc), but it looks like the current code only assumes "no registry" passed to mean docker hub. For the old code that made no real difference, as in that case it would at most do too many logouts (hostnameAddress, "http://"+hostnameAddress, "https://"+hostnameAddress), but for the new flow, it may be relevant that we would miss the manager.NewManager(store).Logout() call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I started looking into this, and the current situation is – the web-based flow only triggers when the user runs either:

  • docker login, or
  • docker login https://index.docker.io/v1/
    the following do not work:
  • docker login index.docker.io
  • `docker login index.docker.io/v1/
  • docker login https://index.docker.io

For logout, the following trigger an oauth logout:

  • docker logout, or
  • docker logout https://index.docker.io/v1/

This seems consistent to me, but WDYT @thaJeztah?

store := dockerCli.ConfigFile().GetCredentialsStore(registry.IndexServer)
if err := manager.NewManager(store).Logout(ctx); err != nil {
fmt.Fprintf(dockerCli.Err(), "WARNING: %v\n", err)
}
}

fmt.Fprintf(dockerCli.Out(), "Removing login credentials for %s\n", hostnameAddress)
errs := make(map[string]error)
for _, r := range regsToLogout {
Expand Down
8 changes: 7 additions & 1 deletion cli/config/credentials/file_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/url"
"os"
"strings"
"sync/atomic"

"github.com/docker/cli/cli/config/types"
)
Expand Down Expand Up @@ -65,6 +66,10 @@ Configure a credential helper to remove this warning. See
https://docs.docker.com/go/credential-store/
`

// alreadyPrinted ensures that we only print the unencryptedWarning once per
// CLI invocation (no need to warn the user multiple times per command).
var alreadyPrinted atomic.Bool

// Store saves the given credentials in the file store.
func (c *fileStore) Store(authConfig types.AuthConfig) error {
authConfigs := c.file.GetAuthConfigs()
Expand All @@ -73,11 +78,12 @@ func (c *fileStore) Store(authConfig types.AuthConfig) error {
return err
}

if authConfig.Password != "" {
if !alreadyPrinted.Load() && authConfig.Password != "" {
// Display a warning if we're storing the users password (not a token).
//
// FIXME(thaJeztah): make output configurable instead of hardcoding to os.Stderr
_, _ = fmt.Fprintln(os.Stderr, fmt.Sprintf(unencryptedWarning, c.file.GetFilename()))
alreadyPrinted.Store(true)
}

return nil
Expand Down
Loading