diff --git a/modules/auth/oauth.go b/modules/auth/oauth.go index c98a22fa..ccda4ab7 100644 --- a/modules/auth/oauth.go +++ b/modules/auth/oauth.go @@ -366,7 +366,9 @@ func startLocalServerAndOpenBrowser(authURL, expectedState string, opts *OAuthOp var openBrowser = func(url string) error { fmt.Printf("Please authorize the application by visiting this URL in your browser:\n%s\n", url) - return open.Run(url) + // Don't wait for the opener to exit, so a browser that holds the + // foreground can't block the wait for the callback. + return open.Start(url) } // createLoginFromToken creates a login entry using the obtained access token diff --git a/modules/auth/oauth_test.go b/modules/auth/oauth_test.go index 6ce82086..693ea830 100644 --- a/modules/auth/oauth_test.go +++ b/modules/auth/oauth_test.go @@ -6,11 +6,16 @@ package auth import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "net/url" + "os" + "path/filepath" + "runtime" "sync" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -89,3 +94,36 @@ func TestPerformBrowserOAuthFlow_RedirectURIMatchesAcrossAuthorizeAndExchange(t assert.Equal(t, authorizeRedirectURI, exchangeRedirectURI, "redirect_uri must match between authorize and token exchange (RFC 6749 ยง4.1.3)") } + +// Regression test for the browser opener hang: xdg-open does not exit until +// the browser it launched does, and the callback is only consumed after +// openBrowser returns. Waiting on the opener hangs the CLI even though the +// user authenticated successfully. +func TestOpenBrowser_DoesNotWaitForOpener(t *testing.T) { + if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { + t.Skip("xdg-open is not the opener on this platform") + } + + const ( + fakeOpenerSleepTime = 10 * time.Second + openBrowserTimeout = 2 * time.Second + ) + + // A stand-in xdg-open that holds the foreground the way a browser it had + // to launch would. + dir := t.TempDir() + opener := filepath.Join(dir, "xdg-open") + script := fmt.Sprintf("#!/bin/sh\nexec sleep %d\n", int(fakeOpenerSleepTime.Seconds())) + require.NoError(t, os.WriteFile(opener, []byte(script), 0o755)) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + + done := make(chan error, 1) + go func() { done <- openBrowser("http://127.0.0.1:1/") }() + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(openBrowserTimeout): + t.Fatal("openBrowser blocked on the opener; the callback would never be consumed") + } +}