From 276a4b735af69a2b403f007a9c0ef194b878228c Mon Sep 17 00:00:00 2001 From: James Braid Date: Sun, 16 Aug 2026 12:57:47 +0000 Subject: [PATCH] fix(oauth): don't wait for the browser opener to exit (#1093) Fixes `tea login add --oauth` hanging after the user authenticates in the browser. `xdg-open` (at least on Debian) runs the browser in the foreground, so it does not exit until the browser does. `open.Run` waits for it, so tea is blocked and doesn't get the oAuth callback from the browser. This only happens when `xdg-open` has to start the browser. With one already running, the new process hands off and exits immediately. `open.Start` launches the opener and returns. The test mocks `xdg-open` with a script that holds the foreground and fails if `openBrowser` waits on it. Reviewed-on: https://gitea.com/gitea/tea/pulls/1093 Reviewed-by: Lunny Xiao Co-authored-by: James Braid --- modules/auth/oauth.go | 4 +++- modules/auth/oauth_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) 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") + } +}