internal/oauth: assorted fixes and cleanups

cli/command/registry/login.go

@@ -19,6 +19,7 @@ import (
 	"github.com/docker/cli/internal/tui"
 	registrytypes "github.com/moby/moby/api/types/registry"
 	"github.com/moby/moby/client"
+	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -226,7 +227,7 @@ func loginWithStoredCredentials(ctx context.Context, dockerCLI command.Cli, auth
 	return resp.Auth.Status, err
 }
 
-func loginUser(ctx context.Context, dockerCLI command.Cli, opts loginOptions, defaultUsername, serverAddress string) (msg string, _ error) {
+func loginUser(ctx context.Context, dockerCLI command.Cli, opts loginOptions, defaultUsername, serverAddress string) (message string, _ error) {
 	// Some links documenting this:
 	// - https://code.google.com/archive/p/mintty/issues/56
 	// - https://github.com/docker/docker/issues/15272
@@ -240,13 +241,12 @@ func loginUser(ctx context.Context, dockerCLI command.Cli, opts loginOptions, de
 
 	// 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 == "" {
-		var err error
-		msg, 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) {
+		msg, err := loginWithDeviceCodeFlow(ctx, dockerCLI)
+		if err == nil || !errors.Is(err, manager.ErrDeviceLoginStartFail) {
 			return msg, err
 		}
+
+		logrus.WithError(err).Debug("failed to start web-based login")
 		_, _ = fmt.Fprint(dockerCLI.Err(), "Failed to start web-based login - falling back to command line login...\n\n")
 	}
 

docs/reference/commandline/login.md

@@ -180,13 +180,12 @@ code, and authenticate.
 ```console
 $ docker login
 
-USING WEB-BASED LOGIN
-To sign in with credentials on the command line, use 'docker login -u <username>'
+Using web-based login. To sign in with credentials on the command line, use 'docker login -u <username>'
 
 Your one-time device confirmation code is: LNFR-PGCJ
-Press ENTER to open your browser or submit your device code here: https://login.docker.com/activate
+Open this URL to authenticate: https://login.docker.com/activate?user_code=LNFR-PGCJ
 
-Waiting for authentication in the browser…
+Press ENTER to open the URL in your browser, or Ctrl+C to cancel.
 ```
 
 After entering the code in your browser, you are authenticated to Docker Hub

e2e/registry/login_test.go

@@ -30,5 +30,5 @@ func TestOauthLogin(t *testing.T) {
 	assert.NilError(t, err)
 
 	output, _ := io.ReadAll(p)
-	assert.Check(t, strings.Contains(string(output), "USING WEB-BASED LOGIN"), string(output))
+	assert.Check(t, strings.Contains(string(output), "Using web-based login"), string(output))
 }

internal/oauth/manager/manager.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"os/exec"
 	"strings"
 
 	"github.com/docker/cli/cli/config/credentials"
@@ -17,8 +18,6 @@ import (
 	"github.com/docker/cli/internal/registry"
 	"github.com/docker/cli/internal/tui"
 	"github.com/morikuni/aec"
-	"github.com/sirupsen/logrus"
-
 	"github.com/pkg/browser"
 )
 
@@ -83,29 +82,29 @@ var ErrDeviceLoginStartFail = errors.New("failed to start device code flow login
 // The retrieved tokens are stored in the credentials store (under a separate
 // key), and the refresh token is concatenated with the client ID.
 func (m *OAuthManager) LoginDevice(ctx context.Context, w io.Writer) (*types.AuthConfig, error) {
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
 	state, err := m.api.GetDeviceCode(ctx, m.audience)
 	if err != nil {
-		logrus.Debugf("failed to start device code login: %v", err)
-		return nil, ErrDeviceLoginStartFail
+		return nil, fmt.Errorf("%w: %w", ErrDeviceLoginStartFail, err)
 	}
 
 	if state.UserCode == "" {
-		logrus.Debugf("failed to start device code login: missing user code")
-		return nil, ErrDeviceLoginStartFail
+		return nil, fmt.Errorf("%w: missing user code", ErrDeviceLoginStartFail)
 	}
 
-	_, _ = fmt.Fprintln(w, aec.Bold.Apply("\nUSING WEB-BASED LOGIN"))
-
 	var out tui.Output
 	switch stream := w.(type) {
 	case *streams.Out:
 		out = tui.NewOutput(stream)
 	default:
 		out = tui.NewOutput(streams.NewOut(w))
 	}
-	out.PrintNote("To sign in with credentials on the command line, use 'docker login -u <username>'\n")
-	_, _ = fmt.Fprintf(w, "\nYour one-time device confirmation code is: "+aec.Bold.Apply("%s\n"), state.UserCode)
-	_, _ = fmt.Fprintf(w, aec.Bold.Apply("Press ENTER")+" to open your browser or submit your device code here: "+aec.Underline.Apply("%s\n"), strings.Split(state.VerificationURI, "?")[0])
+	out.PrintNote("Using web-based login. To sign in with credentials on the command line, use 'docker login -u <username>'\n")
+	_, _ = fmt.Fprintln(w, "Your one-time device confirmation code is:", aec.Bold.Apply(state.UserCode))
+	_, _ = fmt.Fprintln(w, "Open this URL to authenticate:", aec.Underline.Apply(state.VerificationURI))
+	_, _ = fmt.Fprintln(w, "\n"+aec.Bold.Apply("Press ENTER")+" to open the URL in your browser, or "+aec.Bold.Apply("Ctrl+C")+" to cancel.")
 
 	tokenResChan := make(chan api.TokenResponse)
 	waitForTokenErrChan := make(chan error)
@@ -120,11 +119,25 @@ func (m *OAuthManager) LoginDevice(ctx context.Context, w io.Writer) (*types.Aut
 
 	go func() {
 		reader := bufio.NewReader(os.Stdin)
-		_, _ = reader.ReadString('\n')
-		_ = m.openBrowser(state.VerificationURI)
+		if _, err := reader.ReadString('\n'); err != nil || ctx.Err() != nil {
+			return
+		}
+		err := m.openBrowser(state.VerificationURI)
+		if ctx.Err() != nil {
+			return
+		}
+		if err != nil {
+			if errors.Is(err, exec.ErrNotFound) {
+				_, _ = fmt.Fprintln(w, "No browser found. Open the URL above manually to continue.")
+			} else {
+				_, _ = fmt.Fprintln(w, "Failed to open browser:", err)
+				_, _ = fmt.Fprintln(w, "Open the URL above manually to continue.")
+			}
+			return
+		}
+		_, _ = fmt.Fprintln(w, "\nOpening browser…")
 	}()
 
-	_, _ = fmt.Fprint(w, "\nWaiting for authentication in the browser…\n")
 	var tokenRes api.TokenResponse
 	select {
 	case <-ctx.Done():