Always prompt for a password when asking for credentials.

There is a weird behavior where we don't ask for a password
when the user you type in the prompt is the same you have configured
in the config file.

This is the source of many frustrations and also a bug.
If the authentication with a registry fails because the password
is incorrect, we won't ask for the password again with the current logic.

With this change, we also stop calling `CmdLogin` directly when
authentication fails. We don't need to parse flags from the cli or
setting up input destriptiors again, like the current behavior is doing.

Signed-off-by: David Calavera <david.calavera@gmail.com>

api/client/login.go

@@ -25,11 +25,9 @@ func (cli *DockerCli) CmdLogin(args ...string) error {
 	cmd := Cli.Subcmd("login", []string{"[SERVER]"}, Cli.DockerCommands["login"].Description+".\nIf no server is specified \""+registry.IndexServer+"\" is the default.", true)
 	cmd.Require(flag.Max, 1)
 
-	var username, password, email string
+	flUser := cmd.String([]string{"u", "-username"}, "", "Username")
-
+	flPassword := cmd.String([]string{"p", "-password"}, "", "Password")
-	cmd.StringVar(&username, []string{"u", "-username"}, "", "Username")
+	flEmail := cmd.String([]string{"e", "-email"}, "", "Email")
-	cmd.StringVar(&password, []string{"p", "-password"}, "", "Password")
-	cmd.StringVar(&email, []string{"e", "-email"}, "", "Email")
 
 	cmd.ParseFlags(args, true)
 
@@ -43,84 +41,12 @@ func (cli *DockerCli) CmdLogin(args ...string) error {
 		serverAddress = cmd.Arg(0)
 	}
 
-	promptDefault := func(prompt string, configDefault string) {
+	authConfig, err := cli.configureAuth(*flUser, *flPassword, *flEmail, serverAddress)
-		if configDefault == "" {
+	if err != nil {
-			fmt.Fprintf(cli.out, "%s: ", prompt)
+		return err
-		} else {
-			fmt.Fprintf(cli.out, "%s (%s): ", prompt, configDefault)
-		}
 	}
 
-	readInput := func(in io.Reader, out io.Writer) string {
+	response, err := cli.client.RegistryLogin(authConfig)
-		reader := bufio.NewReader(in)
-		line, _, err := reader.ReadLine()
-		if err != nil {
-			fmt.Fprintln(out, err.Error())
-			os.Exit(1)
-		}
-		return string(line)
-	}
-
-	authconfig, ok := cli.configFile.AuthConfigs[serverAddress]
-	if !ok {
-		authconfig = types.AuthConfig{}
-	}
-
-	if username == "" {
-		promptDefault("Username", authconfig.Username)
-		username = readInput(cli.in, cli.out)
-		username = strings.TrimSpace(username)
-		if username == "" {
-			username = authconfig.Username
-		}
-	}
-	// Assume that a different username means they may not want to use
-	// the password or email from the config file, so prompt them
-	if username != authconfig.Username {
-		if password == "" {
-			oldState, err := term.SaveState(cli.inFd)
-			if err != nil {
-				return err
-			}
-			fmt.Fprintf(cli.out, "Password: ")
-			term.DisableEcho(cli.inFd, oldState)
-
-			password = readInput(cli.in, cli.out)
-			fmt.Fprint(cli.out, "\n")
-
-			term.RestoreTerminal(cli.inFd, oldState)
-			if password == "" {
-				return fmt.Errorf("Error : Password Required")
-			}
-		}
-
-		if email == "" {
-			promptDefault("Email", authconfig.Email)
-			email = readInput(cli.in, cli.out)
-			if email == "" {
-				email = authconfig.Email
-			}
-		}
-	} else {
-		// However, if they don't override the username use the
-		// password or email from the cmd line if specified. IOW, allow
-		// then to change/override them.  And if not specified, just
-		// use what's in the config file
-		if password == "" {
-			password = authconfig.Password
-		}
-		if email == "" {
-			email = authconfig.Email
-		}
-	}
-	authconfig.Username = username
-	authconfig.Password = password
-	authconfig.Email = email
-	authconfig.ServerAddress = serverAddress
-	cli.configFile.AuthConfigs[serverAddress] = authconfig
-
-	auth := cli.configFile.AuthConfigs[serverAddress]
-	response, err := cli.client.RegistryLogin(auth)
 	if err != nil {
 		if client.IsErrUnauthorized(err) {
 			delete(cli.configFile.AuthConfigs, serverAddress)
@@ -141,3 +67,80 @@ func (cli *DockerCli) CmdLogin(args ...string) error {
 	}
 	return nil
 }
+
+func (cli *DockerCli) promptWithDefault(prompt string, configDefault string) {
+	if configDefault == "" {
+		fmt.Fprintf(cli.out, "%s: ", prompt)
+	} else {
+		fmt.Fprintf(cli.out, "%s (%s): ", prompt, configDefault)
+	}
+}
+
+func (cli *DockerCli) configureAuth(flUser, flPassword, flEmail, serverAddress string) (types.AuthConfig, error) {
+	authconfig, ok := cli.configFile.AuthConfigs[serverAddress]
+	if !ok {
+		authconfig = types.AuthConfig{}
+	}
+
+	if flUser == "" {
+		cli.promptWithDefault("Username", authconfig.Username)
+		flUser = readInput(cli.in, cli.out)
+		flUser = strings.TrimSpace(flUser)
+		if flUser == "" {
+			flUser = authconfig.Username
+		}
+	}
+
+	if flPassword == "" {
+		oldState, err := term.SaveState(cli.inFd)
+		if err != nil {
+			return authconfig, err
+		}
+		fmt.Fprintf(cli.out, "Password: ")
+		term.DisableEcho(cli.inFd, oldState)
+
+		flPassword = readInput(cli.in, cli.out)
+		fmt.Fprint(cli.out, "\n")
+
+		term.RestoreTerminal(cli.inFd, oldState)
+		if flPassword == "" {
+			return authconfig, fmt.Errorf("Error : Password Required")
+		}
+	}
+
+	// Assume that a different username means they may not want to use
+	// the email from the config file, so prompt it
+	if flUser != authconfig.Username {
+		if flEmail == "" {
+			cli.promptWithDefault("Email", authconfig.Email)
+			flEmail = readInput(cli.in, cli.out)
+			if flEmail == "" {
+				flEmail = authconfig.Email
+			}
+		}
+	} else {
+		// However, if they don't override the username use the
+		// email from the cmd line if specified. IOW, allow
+		// then to change/override them.  And if not specified, just
+		// use what's in the config file
+		if flEmail == "" {
+			flEmail = authconfig.Email
+		}
+	}
+	authconfig.Username = flUser
+	authconfig.Password = flPassword
+	authconfig.Email = flEmail
+	authconfig.ServerAddress = serverAddress
+	cli.configFile.AuthConfigs[serverAddress] = authconfig
+	return authconfig, nil
+}
+
+func readInput(in io.Reader, out io.Writer) string {
+	reader := bufio.NewReader(in)
+	line, _, err := reader.ReadLine()
+	if err != nil {
+		fmt.Fprintln(out, err.Error())
+		os.Exit(1)
+	}
+	return string(line)
+}

api/client/utils.go

@@ -35,10 +35,12 @@ func (cli *DockerCli) encodeRegistryAuth(index *registrytypes.IndexInfo) (string
 func (cli *DockerCli) registryAuthenticationPrivilegedFunc(index *registrytypes.IndexInfo, cmdName string) client.RequestPrivilegeFunc {
 	return func() (string, error) {
 		fmt.Fprintf(cli.out, "\nPlease login prior to %s:\n", cmdName)
-		if err := cli.CmdLogin(registry.GetAuthConfigKey(index)); err != nil {
+		indexServer := registry.GetAuthConfigKey(index)
+		authConfig, err := cli.configureAuth("", "", "", indexServer)
+		if err != nil {
 			return "", err
 		}
-		return cli.encodeRegistryAuth(index)
+		return encodeAuthToBase64(authConfig)
 	}
 }