diff --git a/cmd/entire/cli/login.go b/cmd/entire/cli/login.go index 40a55b077c..b07c881544 100644 --- a/cmd/entire/cli/login.go +++ b/cmd/entire/cli/login.go @@ -11,6 +11,7 @@ import ( "os/exec" "runtime" "strings" + "sync" "time" "github.com/entireio/auth-go/tokens" @@ -99,6 +100,7 @@ func newLoginCmd() *cobra.Command { useDevice: useDevice, canPrompt: interactive.CanPromptInteractively(), sshSession: isSSHSession(), + wsl: isWSL(), }) }, } @@ -156,7 +158,7 @@ func requireSecureLoginServer(server string, insecureHTTPAuth bool) error { return nil } -func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient, openURL browserOpenFunc, canPrompt bool) error { +func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient, openURL browserOpenFunc, canPrompt, autoOpen bool) error { start, err := client.StartDeviceAuth(ctx) if err != nil { return fmt.Errorf("start login: %w", err) @@ -172,14 +174,20 @@ func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient // code is printed above regardless, so it's still available to confirm // against the page (RFC 8628 §3.3.1) or to enter on the bare-URI fallback. fmt.Fprintf(outW, "Login URL: %s\n\n", approvalURL) - fmt.Fprintf(outW, "Press Enter to open in browser...") - // Read from /dev/tty so we get a real keypress and don't consume piped stdin. - if err := waitForEnter(ctx); err != nil { - return fmt.Errorf("wait for input: %w", err) + // autoOpen skips the Enter prompt and opens straight away: it's set when + // arriving here from the WSL browser-loopback fallback, where the user + // has already pressed Enter to signal the redirect failed. + if !autoOpen { + fmt.Fprintf(outW, "Press Enter to open in browser...") + + // Read from /dev/tty so we get a real keypress and don't consume piped stdin. + if err := waitForEnter(ctx); err != nil { + return fmt.Errorf("wait for input: %w", err) + } + fmt.Fprintln(outW) } - fmt.Fprintln(outW) if err := openURL(ctx, approvalURL); err != nil { fmt.Fprintf(errW, "Warning: failed to open browser: %v\n", err) fmt.Fprintf(outW, "Open this URL in your browser to approve this login: %s\n", approvalURL) @@ -205,6 +213,7 @@ type loginFlowFacts struct { useDevice bool // --device flag canPrompt bool // interactive terminal present sshSession bool // running inside an SSH session + wsl bool // running under WSL (Windows Subsystem for Linux) } // runLoginAuto picks between the browser (loopback authorization-code) and @@ -223,9 +232,24 @@ func runLoginAuto(ctx context.Context, outW, errW io.Writer, deviceClient device // exhausted ports); that shouldn't strand the user — warn and // use the device flow instead. fmt.Fprintf(errW, "Warning: could not start browser sign-in (%v); falling back to the device-code flow.\n", err) - return runLogin(ctx, outW, errW, deviceClient, openURL, facts.canPrompt) + return runLogin(ctx, outW, errW, deviceClient, openURL, facts.canPrompt, false) + } + // Under WSL the Windows browser opened by openURL can't necessarily + // reach this WSL loopback listener (it depends on WSL2 + // localhostForwarding), so arm the Enter-driven fallback that lets the + // user switch to the device-code flow when the redirect never arrives. + var waitFallback func(context.Context) error + if facts.wsl { + waitFallback = waitForEnter } - return runBrowserLogin(ctx, outW, errW, flow, deviceClient.BaseURL(), openURL, browserLoginTimeout) + err = runBrowserLogin(ctx, outW, errW, flow, deviceClient.BaseURL(), openURL, browserLoginTimeout, waitFallback) + if errors.Is(err, errBrowserFallbackRequested) { + fmt.Fprintln(errW, "Browser sign-in didn't complete; switching to code-based sign-in.") + // The user just signalled intent, so open the verification URL + // immediately instead of prompting for another Enter. + return runLogin(ctx, outW, errW, deviceClient, openURL, facts.canPrompt, true) + } + return err } switch { case facts.useDevice: @@ -235,9 +259,14 @@ func runLoginAuto(ctx context.Context, outW, errW io.Writer, deviceClient device case facts.sshSession: fmt.Fprintln(errW, "SSH session detected; using device-code flow (a browser opened here couldn't reach this machine).") } - return runLogin(ctx, outW, errW, deviceClient, openURL, facts.canPrompt) + return runLogin(ctx, outW, errW, deviceClient, openURL, facts.canPrompt, false) } +// errBrowserFallbackRequested signals that the user, during the WSL +// browser-loopback flow, pressed Enter to abandon the (unreachable) redirect +// and switch to the device-code flow. It never escapes runLoginAuto. +var errBrowserFallbackRequested = errors.New("browser login fallback requested") + // shouldUseBrowserLogin reports whether `entire login` should use the // loopback authorization-code (browser) flow. The browser flow is the // default but needs a local browser + reachable 127.0.0.1, so it's only @@ -263,7 +292,7 @@ func isSSHSession() bool { // wait up to waitTimeout for the redirect back to the local listener, then // exchange the code for tokens. Shares the token validation + persistence // tail with runLogin via persistLogin. -func runBrowserLogin(ctx context.Context, outW, errW io.Writer, flow browserAuthFlow, baseURL string, openURL browserOpenFunc, waitTimeout time.Duration) error { +func runBrowserLogin(ctx context.Context, outW, errW io.Writer, flow browserAuthFlow, baseURL string, openURL browserOpenFunc, waitTimeout time.Duration, waitFallback func(context.Context) error) error { // Wait tears the listener down on return, but Close is idempotent and // covers the error paths before Wait runs. defer func() { _ = flow.Close() }() @@ -292,26 +321,85 @@ func runBrowserLogin(ctx context.Context, outW, errW io.Writer, flow browserAuth fmt.Fprintf(outW, "Open this URL in your browser to sign in: %s\n", authURL) } - fmt.Fprint(outW, "Waiting for sign-in... ") - // The clock starts here, after the Enter prompt, so time spent reading // the prompt isn't counted against the sign-in itself. waitCtx, cancel := context.WithTimeout(ctx, waitTimeout) defer cancel() - code, err := flow.Wait(waitCtx) - if err != nil { - if errors.Is(waitCtx.Err(), context.DeadlineExceeded) { - return fmt.Errorf("timed out waiting for sign-in after %v; run `entire login` again, or use `entire login --device`", waitTimeout) + if waitFallback == nil { + fmt.Fprint(outW, "Waiting for sign-in... ") + code, err := flow.Wait(waitCtx) + if err != nil { + return browserWaitError(waitCtx, err, waitTimeout) } - return fmt.Errorf("complete login: %w", err) + return exchangeAndPersist(ctx, outW, flow, baseURL, code) } + // WSL: the Windows browser can't necessarily reach this WSL loopback + // listener (it depends on WSL2 localhostForwarding). Race the OAuth + // callback against an Enter keypress — the user presses Enter when the + // browser shows a "can't reach this page" error — and fall back to the + // device-code flow on Enter. Whichever fires first cancels the shared + // context, unblocking the loser (waitFallback closes /dev/tty on cancel). + fmt.Fprint(outW, "Waiting for sign-in... (press Enter if your browser shows a connection error) ") + + type waitResult struct { + code string + err error + } + callbackCh := make(chan waitResult, 1) + go func() { + code, err := flow.Wait(waitCtx) + callbackCh <- waitResult{code: code, err: err} + }() + fallbackCh := make(chan error, 1) + go func() { fallbackCh <- waitFallback(waitCtx) }() + + select { + case r := <-callbackCh: + cancel() // unblock the fallback waiter (closes /dev/tty) + if r.err != nil { + return browserWaitError(waitCtx, r.err, waitTimeout) + } + return exchangeAndPersist(ctx, outW, flow, baseURL, r.code) + case ferr := <-fallbackCh: + cancel() // unblock flow.Wait + if ferr != nil { + // Not a real keypress — the waiter was cancelled (parent ctx or the + // backstop timeout), so surface that rather than falling back. + return browserWaitError(waitCtx, ferr, waitTimeout) + } + // The redirect may have already succeeded in the instant before Enter + // (a reflexive keypress on a working localhostForwarding setup). Prefer + // that buffered result over discarding a completed login. + select { + case r := <-callbackCh: + if r.err == nil { + return exchangeAndPersist(ctx, outW, flow, baseURL, r.code) + } + default: + } + fmt.Fprintln(outW) + return errBrowserFallbackRequested + } +} + +// browserWaitError maps a Wait/fallback error to the user-facing message, +// distinguishing the backstop timeout from other failures. +func browserWaitError(waitCtx context.Context, err error, waitTimeout time.Duration) error { + if errors.Is(waitCtx.Err(), context.DeadlineExceeded) { + return fmt.Errorf("timed out waiting for sign-in after %v; run `entire login` again, or use `entire login --device`", waitTimeout) + } + return fmt.Errorf("complete login: %w", err) +} + +// exchangeAndPersist exchanges the authorization code for tokens and records +// the login. Shared by the linear and WSL-fallback browser paths. +func exchangeAndPersist(ctx context.Context, outW io.Writer, flow browserAuthFlow, baseURL, code string) error { token, refreshToken, err := flow.Exchange(ctx, code) if err != nil { return fmt.Errorf("complete login: %w", err) } - return persistLogin(outW, baseURL, token, refreshToken) } @@ -507,21 +595,9 @@ func openBrowser(ctx context.Context, browserURL string) error { return errors.New("browser unavailable under test") } - var command string - var args []string - - switch runtime.GOOS { - case "darwin": - command = "open" - args = []string{browserURL} - case "linux": - command = "xdg-open" - args = []string{browserURL} - case "windows": - command = "cmd" - args = []string{"/c", "start", "", browserURL} - default: - return fmt.Errorf("unsupported platform %s", runtime.GOOS) + command, args, err := resolveBrowserLauncher(runtime.GOOS, isWSL(), exec.LookPath, browserURL) + if err != nil { + return err } cmd := exec.CommandContext(ctx, command, args...) @@ -535,3 +611,63 @@ func openBrowser(ctx context.Context, browserURL string) error { return nil } + +// resolveBrowserLauncher picks the command that opens browserURL, given the +// OS, whether we're under WSL, and a PATH probe. It is pure so the platform +// matrix is unit-testable without spawning anything; openBrowser wires in the +// real runtime.GOOS, isWSL(), and exec.LookPath. +// +// Under WSL, xdg-open can't be trusted: with a Linux browser installed via +// WSLg it resolves the https scheme handler to that Linux browser, ignoring +// both $BROWSER and the xdg default, so the user lands in a browser with none +// of their sessions. So on WSL we open the Windows default browser directly +// via wslview (from wslu, preinstalled on Ubuntu-on-WSL), falling back to +// explorer.exe on stripped-down distros without it. explorer.exe takes the +// URL as a plain argument and hands it to the shell URL handler, so unlike +// cmd.exe `start` it never reparses the command line (`&` in OAuth query +// strings would split a cmd.exe line) and doesn't warn about the WSL (UNC) +// working directory. Its exit code is meaningless (1 even on success), which +// is fine here: openBrowser only checks that the process starts. +func resolveBrowserLauncher(goos string, wsl bool, lookPath func(string) (string, error), browserURL string) (command string, args []string, err error) { + switch goos { + case "darwin": + return "open", []string{browserURL}, nil + case "windows": + return "cmd", []string{"/c", "start", "", browserURL}, nil + case "linux": + if wsl { + if path, lerr := lookPath("wslview"); lerr == nil { + return path, []string{browserURL}, nil + } + return "explorer.exe", []string{browserURL}, nil + } + return "xdg-open", []string{browserURL}, nil + default: + return "", nil, fmt.Errorf("unsupported platform %s", goos) + } +} + +// wslFromProcVersion reports whether a /proc/version string indicates WSL. +// Both WSL1 ("...Microsoft...") and WSL2 ("...microsoft-standard-WSL2...") +// carry the marker; the match is case-insensitive. +func wslFromProcVersion(s string) bool { + return strings.Contains(strings.ToLower(s), "microsoft") +} + +var wslDetect = sync.OnceValue(func() bool { + // Non-Linux hosts are never WSL, so /proc/version is only read on Linux. + // Detection keys on /proc/version rather than WSL_* env vars because those + // can be stripped when entire is spawned from a hook, service, or env -i — + // exactly the contexts the CLI must handle. + if runtime.GOOS != "linux" { + return false + } + b, err := os.ReadFile("/proc/version") + if err != nil { + return false + } + return wslFromProcVersion(string(b)) +}) + +// isWSL reports whether we're running under WSL, reading /proc/version once. +func isWSL() bool { return wslDetect() } diff --git a/cmd/entire/cli/login_test.go b/cmd/entire/cli/login_test.go index 92ad238986..a5f690a5c4 100644 --- a/cmd/entire/cli/login_test.go +++ b/cmd/entire/cli/login_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "reflect" "strings" "testing" "time" @@ -507,7 +508,7 @@ func TestRunBrowserLogin_OpensAuthorizationURL(t *testing.T) { // The stubbed Wait returns an error, so runBrowserLogin stops before // persistLogin (which would hit the real keyring); we assert on the // side effects up to that point. - if err := runBrowserLogin(context.Background(), &out, &bytes.Buffer{}, flow, "https://auth.test", openURL, browserLoginTimeout); err == nil { + if err := runBrowserLogin(context.Background(), &out, &bytes.Buffer{}, flow, "https://auth.test", openURL, browserLoginTimeout, nil); err == nil { t.Fatal("expected error from stubbed Wait") } @@ -537,7 +538,7 @@ func TestRunBrowserLogin_OpenBrowserFallback(t *testing.T) { failOpen := func(context.Context, string) error { return errors.New("no browser") } var out, errW bytes.Buffer - if err := runBrowserLogin(context.Background(), &out, &errW, flow, "https://auth.test", failOpen, browserLoginTimeout); err == nil { + if err := runBrowserLogin(context.Background(), &out, &errW, flow, "https://auth.test", failOpen, browserLoginTimeout, nil); err == nil { t.Fatal("expected error from stubbed Wait") } @@ -555,7 +556,7 @@ func TestRunBrowserLogin_WaitError(t *testing.T) { denied := errors.New("access_denied") flow := &fakeBrowserFlow{authURL: "https://auth.test/authorize", waitErr: denied} - err := runBrowserLogin(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, flow, "https://auth.test", noopOpenURL, browserLoginTimeout) + err := runBrowserLogin(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, flow, "https://auth.test", noopOpenURL, browserLoginTimeout, nil) if !errors.Is(err, denied) { t.Fatalf("err = %v, want wrapped %v", err, denied) } @@ -570,7 +571,7 @@ func TestRunBrowserLogin_ExchangeError(t *testing.T) { exchErr: errors.New("invalid_grant"), } - err := runBrowserLogin(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, flow, "https://auth.test", noopOpenURL, browserLoginTimeout) + err := runBrowserLogin(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, flow, "https://auth.test", noopOpenURL, browserLoginTimeout, nil) if err == nil || !strings.Contains(err.Error(), "complete login") { t.Fatalf("err = %v, want complete login error", err) } @@ -586,7 +587,7 @@ func TestRunBrowserLogin_WaitTimeout(t *testing.T) { // come from runBrowserLogin's own timeout, or this test would hang. flow := &fakeBrowserFlow{authURL: "https://auth.test/authorize", waitUntilDone: true} - err := runBrowserLogin(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, flow, "https://auth.test", noopOpenURL, 50*time.Millisecond) + err := runBrowserLogin(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, flow, "https://auth.test", noopOpenURL, 50*time.Millisecond, nil) if err == nil || !strings.Contains(err.Error(), "timed out waiting for sign-in") { t.Fatalf("err = %v, want sign-in timeout", err) } @@ -606,7 +607,7 @@ func TestRunBrowserLogin_ParentCancelNotReportedAsTimeout(t *testing.T) { flow := &fakeBrowserFlow{authURL: "https://auth.test/authorize", waitUntilDone: true} - err := runBrowserLogin(ctx, &bytes.Buffer{}, &bytes.Buffer{}, flow, "https://auth.test", noopOpenURL, time.Minute) + err := runBrowserLogin(ctx, &bytes.Buffer{}, &bytes.Buffer{}, flow, "https://auth.test", noopOpenURL, time.Minute, nil) if !errors.Is(err, context.Canceled) { t.Fatalf("err = %v, want wrapped context.Canceled", err) } @@ -614,3 +615,167 @@ func TestRunBrowserLogin_ParentCancelNotReportedAsTimeout(t *testing.T) { t.Errorf("cancellation must not be reported as a timeout: %v", err) } } + +func TestRunBrowserLogin_WSLFallback_CallbackWins(t *testing.T) { + t.Parallel() + + // Callback returns a code; Exchange errors so we stop before persistLogin + // (which would hit real storage). The fallback blocks until the shared + // context is cancelled, so the callback wins the race. + flow := &fakeBrowserFlow{ + authURL: "https://auth.test/authorize", + waitCode: "cb-code", + exchErr: errors.New("exchange boom"), + } + blockingFallback := func(ctx context.Context) error { <-ctx.Done(); return ctx.Err() } + + err := runBrowserLogin(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, flow, + "https://auth.test", noopOpenURL, browserLoginTimeout, blockingFallback) + + if errors.Is(err, errBrowserFallbackRequested) { + t.Fatalf("callback should win, got fallback: %v", err) + } + if err == nil || !strings.Contains(err.Error(), "complete login") { + t.Fatalf("err = %v, want 'complete login' from exchange error", err) + } + if flow.gotExchangeCode != "cb-code" { + t.Errorf("exchange code = %q, want cb-code (callback path taken)", flow.gotExchangeCode) + } + if !flow.closed { + t.Error("flow not closed") + } +} + +func TestRunBrowserLogin_WSLFallback_EnterWins(t *testing.T) { + t.Parallel() + + // flow.Wait blocks until ctx is done; the fallback returns immediately + // (as if the user pressed Enter), so the fallback wins. + flow := &fakeBrowserFlow{authURL: "https://auth.test/authorize", waitUntilDone: true} + enterNow := func(context.Context) error { return nil } + + err := runBrowserLogin(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, flow, + "https://auth.test", noopOpenURL, browserLoginTimeout, enterNow) + + if !errors.Is(err, errBrowserFallbackRequested) { + t.Fatalf("err = %v, want errBrowserFallbackRequested", err) + } + if flow.gotExchangeCode != "" { + t.Errorf("exchange must not run on fallback, got code %q", flow.gotExchangeCode) + } + if !flow.closed { + t.Error("flow not closed") + } +} + +func TestRunLoginAuto_WSL_FallbackToDevice(t *testing.T) { + t.Parallel() + + // Under test the real waitForEnter (armed only for WSL) returns + // immediately, so the fallback fires and routes to the device flow. The + // browser flow's Wait blocks so the callback never pre-empts the fallback. + flow := &fakeBrowserFlow{authURL: "https://auth.test/authorize", waitUntilDone: true} + var browserCalls int + var errW bytes.Buffer + + err := runLoginAuto(context.Background(), &bytes.Buffer{}, &errW, &mockClient{}, + startBrowserStub(&browserCalls, flow, nil), noopOpenURL, + loginFlowFacts{canPrompt: true, wsl: true}) + + if browserCalls != 1 { + t.Errorf("startBrowser calls = %d, want 1 (WSL still tries the loopback flow first)", browserCalls) + } + if !strings.Contains(errW.String(), "switching to code-based sign-in") { + t.Errorf("stderr missing fallback message:\n%s", errW.String()) + } + // mockClient.StartDeviceAuth errors — proof the device flow was attempted. + if err == nil || !strings.Contains(err.Error(), "not implemented in mock") { + t.Fatalf("err = %v, want device-flow start error from mock", err) + } + if !flow.closed { + t.Error("browser flow not closed before fallback") + } +} + +func TestRunLoginAuto_NonWSL_NoFallbackArmed(t *testing.T) { + t.Parallel() + + // Without wsl, the browser flow must NOT arm the Enter fallback: Wait's + // error surfaces directly as a 'complete login' failure, never a fallback. + flow := &fakeBrowserFlow{authURL: "https://auth.test/authorize", waitErr: errors.New("stop")} + var browserCalls int + + err := runLoginAuto(context.Background(), &bytes.Buffer{}, &bytes.Buffer{}, &mockClient{}, + startBrowserStub(&browserCalls, flow, nil), noopOpenURL, + loginFlowFacts{canPrompt: true}) + + if err == nil || !strings.Contains(err.Error(), "complete login") { + t.Fatalf("err = %v, want browser-flow 'complete login' error (no fallback)", err) + } +} + +func TestWSLFromProcVersion(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + in string + want bool + }{ + {"wsl2", "Linux version 6.18.33.2-microsoft-standard-WSL2 (root@f1bbfb02316b)", true}, + {"wsl1", "Linux version 4.4.0-19041-Microsoft (build)", true}, + {"plain-linux", "Linux version 6.8.0-generic (buildd@lcy02)", false}, + {"empty", "", false}, + } + for _, tc := range cases { + if got := wslFromProcVersion(tc.in); got != tc.want { + t.Errorf("wslFromProcVersion(%s) = %v, want %v", tc.name, got, tc.want) + } + } +} + +func TestResolveBrowserLauncher(t *testing.T) { + t.Parallel() + + const browserURL = "https://auth.test/cli/auth?user_code=ABCD" + found := func(string) (string, error) { return "/usr/bin/wslview", nil } + missing := func(string) (string, error) { return "", errors.New("not found") } + + cases := []struct { + name string + goos string + wsl bool + look func(string) (string, error) + wantCmd string + wantArgs []string + wantErr bool + }{ + {"darwin", "darwin", false, missing, "open", []string{browserURL}, false}, + {"windows", "windows", false, missing, "cmd", []string{"/c", "start", "", browserURL}, false}, + {"linux-non-wsl", "linux", false, found, "xdg-open", []string{browserURL}, false}, + {"wsl-with-wslview", "linux", true, found, "/usr/bin/wslview", []string{browserURL}, false}, + {"wsl-without-wslview", "linux", true, missing, "explorer.exe", []string{browserURL}, false}, + {"unsupported", "plan9", false, missing, "", nil, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cmd, args, err := resolveBrowserLauncher(tc.goos, tc.wsl, tc.look, browserURL) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got cmd=%q", cmd) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cmd != tc.wantCmd { + t.Errorf("cmd = %q, want %q", cmd, tc.wantCmd) + } + if !reflect.DeepEqual(args, tc.wantArgs) { + t.Errorf("args = %v, want %v", args, tc.wantArgs) + } + }) + } +}