From ab019d3f1814a5fc8228d508c01a3e5df9772811 Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:42:46 +0800 Subject: [PATCH 1/2] feat(auth): add no-browser option for OAuth login --- cmd/picoclaw/internal/auth/helpers.go | 14 +-- cmd/picoclaw/internal/auth/login.go | 8 +- cmd/picoclaw/internal/auth/login_test.go | 1 + pkg/auth/oauth.go | 19 +++- pkg/auth/oauth_test.go | 115 +++++++++++++++++++++++ 5 files changed, 146 insertions(+), 11 deletions(-) diff --git a/cmd/picoclaw/internal/auth/helpers.go b/cmd/picoclaw/internal/auth/helpers.go index 531cb76aa..41255a04e 100644 --- a/cmd/picoclaw/internal/auth/helpers.go +++ b/cmd/picoclaw/internal/auth/helpers.go @@ -21,20 +21,20 @@ const ( defaultAnthropicModel = "claude-sonnet-4.6" ) -func authLoginCmd(provider string, useDeviceCode bool, useOauth bool) error { +func authLoginCmd(provider string, useDeviceCode bool, useOauth bool, noBrowser bool) error { switch provider { case "openai": - return authLoginOpenAI(useDeviceCode) + return authLoginOpenAI(useDeviceCode, noBrowser) case "anthropic": return authLoginAnthropic(useOauth) case "google-antigravity", "antigravity": - return authLoginGoogleAntigravity() + return authLoginGoogleAntigravity(noBrowser) default: return fmt.Errorf("unsupported provider: %s (%s)", provider, supportedProvidersMsg) } } -func authLoginOpenAI(useDeviceCode bool) error { +func authLoginOpenAI(useDeviceCode bool, noBrowser bool) error { cfg := auth.OpenAIOAuthConfig() var cred *auth.AuthCredential @@ -43,7 +43,7 @@ func authLoginOpenAI(useDeviceCode bool) error { if useDeviceCode { cred, err = auth.LoginDeviceCode(cfg) } else { - cred, err = auth.LoginBrowser(cfg) + cred, err = auth.LoginBrowserWithOptions(cfg, auth.LoginBrowserOptions{NoBrowser: noBrowser}) } if err != nil { @@ -92,10 +92,10 @@ func authLoginOpenAI(useDeviceCode bool) error { return nil } -func authLoginGoogleAntigravity() error { +func authLoginGoogleAntigravity(noBrowser bool) error { cfg := auth.GoogleAntigravityOAuthConfig() - cred, err := auth.LoginBrowser(cfg) + cred, err := auth.LoginBrowserWithOptions(cfg, auth.LoginBrowserOptions{NoBrowser: noBrowser}) if err != nil { return fmt.Errorf("login failed: %w", err) } diff --git a/cmd/picoclaw/internal/auth/login.go b/cmd/picoclaw/internal/auth/login.go index afbe098aa..406144917 100644 --- a/cmd/picoclaw/internal/auth/login.go +++ b/cmd/picoclaw/internal/auth/login.go @@ -7,6 +7,7 @@ func newLoginCommand() *cobra.Command { provider string useDeviceCode bool useOauth bool + noBrowser bool ) cmd := &cobra.Command{ @@ -14,12 +15,15 @@ func newLoginCommand() *cobra.Command { Short: "Login via OAuth or paste token", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - return authLoginCmd(provider, useDeviceCode, useOauth) + return authLoginCmd(provider, useDeviceCode, useOauth, noBrowser) }, } - cmd.Flags().StringVarP(&provider, "provider", "p", "", "Provider to login with (openai, anthropic)") + cmd.Flags().StringVarP( + &provider, "provider", "p", "", "Provider to login with (openai, anthropic, google-antigravity)", + ) cmd.Flags().BoolVar(&useDeviceCode, "device-code", false, "Use device code flow (for headless environments)") + cmd.Flags().BoolVar(&noBrowser, "no-browser", false, "Do not auto-open a browser during OAuth login") cmd.Flags().BoolVar( &useOauth, "setup-token", false, "Use setup-token flow for Anthropic (from `claude setup-token`)", diff --git a/cmd/picoclaw/internal/auth/login_test.go b/cmd/picoclaw/internal/auth/login_test.go index d6a03c25b..5129d9aaf 100644 --- a/cmd/picoclaw/internal/auth/login_test.go +++ b/cmd/picoclaw/internal/auth/login_test.go @@ -18,6 +18,7 @@ func TestNewLoginSubCommand(t *testing.T) { assert.True(t, cmd.HasFlags()) assert.NotNil(t, cmd.Flags().Lookup("device-code")) + assert.NotNil(t, cmd.Flags().Lookup("no-browser")) providerFlag := cmd.Flags().Lookup("provider") require.NotNil(t, providerFlag) diff --git a/pkg/auth/oauth.go b/pkg/auth/oauth.go index 2bf719dd4..0db693475 100644 --- a/pkg/auth/oauth.go +++ b/pkg/auth/oauth.go @@ -30,6 +30,15 @@ type OAuthProviderConfig struct { Port int } +type LoginBrowserOptions struct { + NoBrowser bool +} + +var ( + openBrowserFunc = OpenBrowser + browserLoginInput io.Reader = os.Stdin +) + func OpenAIOAuthConfig() OAuthProviderConfig { return OAuthProviderConfig{ Issuer: "https://auth.openai.com", @@ -76,6 +85,10 @@ func GenerateState() (string, error) { } func LoginBrowser(cfg OAuthProviderConfig) (*AuthCredential, error) { + return LoginBrowserWithOptions(cfg, LoginBrowserOptions{}) +} + +func LoginBrowserWithOptions(cfg OAuthProviderConfig, opts LoginBrowserOptions) (*AuthCredential, error) { pkce, err := GeneratePKCE() if err != nil { return nil, fmt.Errorf("generating PKCE: %w", err) @@ -128,7 +141,9 @@ func LoginBrowser(cfg OAuthProviderConfig) (*AuthCredential, error) { fmt.Printf("Open this URL to authenticate:\n\n%s\n\n", authURL) - if err := OpenBrowser(authURL); err != nil { + if opts.NoBrowser { + fmt.Println("Browser auto-open disabled. Open the URL manually to continue.") + } else if err := openBrowserFunc(authURL); err != nil { fmt.Printf("Could not open browser automatically.\nPlease open this URL manually:\n\n%s\n\n", authURL) } @@ -144,7 +159,7 @@ func LoginBrowser(cfg OAuthProviderConfig) (*AuthCredential, error) { // Start manual input in a goroutine manualCh := make(chan string) go func() { - reader := bufio.NewReader(os.Stdin) + reader := bufio.NewReader(browserLoginInput) input, _ := reader.ReadString('\n') manualCh <- strings.TrimSpace(input) }() diff --git a/pkg/auth/oauth_test.go b/pkg/auth/oauth_test.go index 230ac7c2a..0bf0558e7 100644 --- a/pkg/auth/oauth_test.go +++ b/pkg/auth/oauth_test.go @@ -3,6 +3,7 @@ package auth import ( "encoding/base64" "encoding/json" + "net" "net/http" "net/http/httptest" "net/url" @@ -373,3 +374,117 @@ func TestParseDeviceCodeResponseInvalidInterval(t *testing.T) { t.Fatal("expected error for invalid interval") } } + +func TestLoginBrowserWithOptionsSkipsAutoOpenWhenDisabled(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/oauth/token" { + http.Error(w, "not found", http.StatusNotFound) + return + } + + resp := map[string]any{ + "access_token": "mock-access-token", + "refresh_token": "mock-refresh-token", + "expires_in": 3600, + } + _ = json.NewEncoder(w).Encode(resp) + })) + defer server.Close() + + origOpenBrowserFunc := openBrowserFunc + origBrowserLoginInput := browserLoginInput + t.Cleanup(func() { + openBrowserFunc = origOpenBrowserFunc + browserLoginInput = origBrowserLoginInput + }) + + var openCalls int + openBrowserFunc = func(string) error { + openCalls++ + return nil + } + browserLoginInput = strings.NewReader("manual-code\n") + + cfg := OAuthProviderConfig{ + Issuer: server.URL, + ClientID: "test-client", + Scopes: "openid", + Port: freeLocalPort(t), + } + + cred, err := LoginBrowserWithOptions(cfg, LoginBrowserOptions{NoBrowser: true}) + if err != nil { + t.Fatalf("LoginBrowserWithOptions() error: %v", err) + } + + if openCalls != 0 { + t.Fatalf("openBrowserFunc call count = %d, want 0", openCalls) + } + if cred.AccessToken != "mock-access-token" { + t.Fatalf("AccessToken = %q, want %q", cred.AccessToken, "mock-access-token") + } +} + +func TestLoginBrowserWithOptionsAutoOpensByDefault(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/oauth/token" { + http.Error(w, "not found", http.StatusNotFound) + return + } + + resp := map[string]any{ + "access_token": "mock-access-token", + "refresh_token": "mock-refresh-token", + "expires_in": 3600, + } + _ = json.NewEncoder(w).Encode(resp) + })) + defer server.Close() + + origOpenBrowserFunc := openBrowserFunc + origBrowserLoginInput := browserLoginInput + t.Cleanup(func() { + openBrowserFunc = origOpenBrowserFunc + browserLoginInput = origBrowserLoginInput + }) + + var openCalls int + openBrowserFunc = func(string) error { + openCalls++ + return nil + } + browserLoginInput = strings.NewReader("manual-code\n") + + cfg := OAuthProviderConfig{ + Issuer: server.URL, + ClientID: "test-client", + Scopes: "openid", + Port: freeLocalPort(t), + } + + _, err := LoginBrowserWithOptions(cfg, LoginBrowserOptions{}) + if err != nil { + t.Fatalf("LoginBrowserWithOptions() error: %v", err) + } + + if openCalls != 1 { + t.Fatalf("openBrowserFunc call count = %d, want 1", openCalls) + } +} + +func freeLocalPort(t *testing.T) int { + t.Helper() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.Listen() error: %v", err) + } + defer listener.Close() + + addr, ok := listener.Addr().(*net.TCPAddr) + if !ok { + t.Fatalf("listener addr type = %T, want *net.TCPAddr", listener.Addr()) + } + + return addr.Port +} From ffd30d7db740a5200936e2bc3998b0f5558b8ec3 Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Thu, 16 Apr 2026 23:01:28 +0800 Subject: [PATCH 2/2] fix(auth): improve no-browser OAuth login --- cmd/picoclaw/internal/auth/helpers.go | 2 +- cmd/picoclaw/internal/auth/login.go | 2 +- pkg/auth/oauth.go | 108 ++++++++++++++------- pkg/auth/oauth_test.go | 129 +++++++++++++------------- 4 files changed, 139 insertions(+), 102 deletions(-) diff --git a/cmd/picoclaw/internal/auth/helpers.go b/cmd/picoclaw/internal/auth/helpers.go index 41255a04e..523f6a16a 100644 --- a/cmd/picoclaw/internal/auth/helpers.go +++ b/cmd/picoclaw/internal/auth/helpers.go @@ -17,7 +17,7 @@ import ( ) const ( - supportedProvidersMsg = "supported providers: openai, anthropic, google-antigravity" + supportedProvidersMsg = "supported providers: openai, anthropic, google-antigravity, antigravity" defaultAnthropicModel = "claude-sonnet-4.6" ) diff --git a/cmd/picoclaw/internal/auth/login.go b/cmd/picoclaw/internal/auth/login.go index 406144917..b9b44db34 100644 --- a/cmd/picoclaw/internal/auth/login.go +++ b/cmd/picoclaw/internal/auth/login.go @@ -20,7 +20,7 @@ func newLoginCommand() *cobra.Command { } cmd.Flags().StringVarP( - &provider, "provider", "p", "", "Provider to login with (openai, anthropic, google-antigravity)", + &provider, "provider", "p", "", "Provider to login with (openai, anthropic, google-antigravity, antigravity)", ) cmd.Flags().BoolVar(&useDeviceCode, "device-code", false, "Use device code flow (for headless environments)") cmd.Flags().BoolVar(&noBrowser, "no-browser", false, "Do not auto-open a browser during OAuth login") diff --git a/pkg/auth/oauth.go b/pkg/auth/oauth.go index 0db693475..c03c30d10 100644 --- a/pkg/auth/oauth.go +++ b/pkg/auth/oauth.go @@ -99,45 +99,33 @@ func LoginBrowserWithOptions(cfg OAuthProviderConfig, opts LoginBrowserOptions) return nil, fmt.Errorf("generating state: %w", err) } - redirectURI := fmt.Sprintf("http://localhost:%d/auth/callback", cfg.Port) + redirectURI := oauthCallbackRedirectURI(cfg.Port) + callbackPort := cfg.Port + var resultCh <-chan callbackResult - authURL := buildAuthorizeURL(cfg, pkce, state, redirectURI) - - resultCh := make(chan callbackResult, 1) - - mux := http.NewServeMux() - mux.HandleFunc("/auth/callback", func(w http.ResponseWriter, r *http.Request) { - if r.URL.Query().Get("state") != state { - resultCh <- callbackResult{err: fmt.Errorf("state mismatch")} - http.Error(w, "State mismatch", http.StatusBadRequest) - return + if !opts.NoBrowser { + callbackResultCh := make(chan callbackResult, 1) + listener, actualPort, err := listenOAuthCallback(cfg.Port) + if err != nil { + return nil, fmt.Errorf("starting callback server on port %d: %w", cfg.Port, err) } - code := r.URL.Query().Get("code") - if code == "" { - errMsg := r.URL.Query().Get("error") - resultCh <- callbackResult{err: fmt.Errorf("no code received: %s", errMsg)} - http.Error(w, "No authorization code received", http.StatusBadRequest) - return - } + redirectURI = oauthCallbackRedirectURI(actualPort) + callbackPort = actualPort + resultCh = callbackResultCh - w.Header().Set("Content-Type", "text/html") - fmt.Fprint(w, "

Authentication successful!

You can close this window.

") - resultCh <- callbackResult{code: code} - }) - - listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", cfg.Port)) - if err != nil { - return nil, fmt.Errorf("starting callback server on port %d: %w", cfg.Port, err) + server := &http.Server{Handler: oauthCallbackHandler(state, callbackResultCh)} + go func() { + _ = server.Serve(listener) + }() + defer func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _ = server.Shutdown(ctx) + }() } - server := &http.Server{Handler: mux} - go server.Serve(listener) - defer func() { - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - server.Shutdown(ctx) - }() + authURL := buildAuthorizeURL(cfg, pkce, state, redirectURI) fmt.Printf("Open this URL to authenticate:\n\n%s\n\n", authURL) @@ -149,7 +137,7 @@ func LoginBrowserWithOptions(cfg OAuthProviderConfig, opts LoginBrowserOptions) fmt.Printf( "Wait! If you are in a headless environment (like Coolify/VPS) and cannot reach localhost:%d,\n", - cfg.Port, + callbackPort, ) fmt.Println( "please complete the login in your local browser and then PASTE the final redirect URL (or just the code) here.", @@ -157,11 +145,16 @@ func LoginBrowserWithOptions(cfg OAuthProviderConfig, opts LoginBrowserOptions) fmt.Println("Waiting for authentication (browser or manual paste)...") // Start manual input in a goroutine - manualCh := make(chan string) + manualCh := make(chan string, 1) + manualDone := make(chan struct{}) + defer close(manualDone) go func() { reader := bufio.NewReader(browserLoginInput) input, _ := reader.ReadString('\n') - manualCh <- strings.TrimSpace(input) + select { + case manualCh <- strings.TrimSpace(input): + case <-manualDone: + } }() select { @@ -191,6 +184,49 @@ func LoginBrowserWithOptions(cfg OAuthProviderConfig, opts LoginBrowserOptions) } } +func oauthCallbackRedirectURI(port int) string { + return fmt.Sprintf("http://localhost:%d/auth/callback", port) +} + +func oauthCallbackHandler(state string, resultCh chan<- callbackResult) http.Handler { + mux := http.NewServeMux() + mux.HandleFunc("/auth/callback", func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("state") != state { + resultCh <- callbackResult{err: fmt.Errorf("state mismatch")} + http.Error(w, "State mismatch", http.StatusBadRequest) + return + } + + code := r.URL.Query().Get("code") + if code == "" { + errMsg := r.URL.Query().Get("error") + resultCh <- callbackResult{err: fmt.Errorf("no code received: %s", errMsg)} + http.Error(w, "No authorization code received", http.StatusBadRequest) + return + } + + w.Header().Set("Content-Type", "text/html") + fmt.Fprint(w, "

Authentication successful!

You can close this window.

") + resultCh <- callbackResult{code: code} + }) + return mux +} + +func listenOAuthCallback(port int) (net.Listener, int, error) { + listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if err != nil { + return nil, 0, err + } + + tcpAddr, ok := listener.Addr().(*net.TCPAddr) + if !ok { + _ = listener.Close() + return nil, 0, fmt.Errorf("unexpected listener address type %T", listener.Addr()) + } + + return listener, tcpAddr.Port, nil +} + type callbackResult struct { code string err error diff --git a/pkg/auth/oauth_test.go b/pkg/auth/oauth_test.go index 0bf0558e7..b318934f9 100644 --- a/pkg/auth/oauth_test.go +++ b/pkg/auth/oauth_test.go @@ -375,22 +375,16 @@ func TestParseDeviceCodeResponseInvalidInterval(t *testing.T) { } } -func TestLoginBrowserWithOptionsSkipsAutoOpenWhenDisabled(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/oauth/token" { - http.Error(w, "not found", http.StatusNotFound) - return - } - - resp := map[string]any{ - "access_token": "mock-access-token", - "refresh_token": "mock-refresh-token", - "expires_in": 3600, - } - _ = json.NewEncoder(w).Encode(resp) - })) +func TestLoginBrowserWithOptionsNoBrowserDoesNotRequireCallbackPort(t *testing.T) { + server := newMockOAuthTokenServer() defer server.Close() + reservedListener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.Listen() error: %v", err) + } + defer reservedListener.Close() + reservedPort := reservedListener.Addr().(*net.TCPAddr).Port origOpenBrowserFunc := openBrowserFunc origBrowserLoginInput := browserLoginInput t.Cleanup(func() { @@ -409,7 +403,7 @@ func TestLoginBrowserWithOptionsSkipsAutoOpenWhenDisabled(t *testing.T) { Issuer: server.URL, ClientID: "test-client", Scopes: "openid", - Port: freeLocalPort(t), + Port: reservedPort, } cred, err := LoginBrowserWithOptions(cfg, LoginBrowserOptions{NoBrowser: true}) @@ -426,7 +420,62 @@ func TestLoginBrowserWithOptionsSkipsAutoOpenWhenDisabled(t *testing.T) { } func TestLoginBrowserWithOptionsAutoOpensByDefault(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := newMockOAuthTokenServer() + defer server.Close() + + origOpenBrowserFunc := openBrowserFunc + origBrowserLoginInput := browserLoginInput + t.Cleanup(func() { + openBrowserFunc = origOpenBrowserFunc + browserLoginInput = origBrowserLoginInput + }) + + var ( + openCalls int + browserURL string + ) + openBrowserFunc = func(url string) error { + openCalls++ + browserURL = url + return nil + } + browserLoginInput = strings.NewReader("manual-code\n") + + cfg := OAuthProviderConfig{ + Issuer: server.URL, + ClientID: "test-client", + Scopes: "openid", + Port: 0, + } + + _, err := LoginBrowserWithOptions(cfg, LoginBrowserOptions{}) + if err != nil { + t.Fatalf("LoginBrowserWithOptions() error: %v", err) + } + + if openCalls != 1 { + t.Fatalf("openBrowserFunc call count = %d, want 1", openCalls) + } + + parsedBrowserURL, err := url.Parse(browserURL) + if err != nil { + t.Fatalf("url.Parse(browserURL) error: %v", err) + } + + redirectURI, err := url.Parse(parsedBrowserURL.Query().Get("redirect_uri")) + if err != nil { + t.Fatalf("url.Parse(redirectURI) error: %v", err) + } + if redirectURI.Port() == "" { + t.Fatal("redirectURI port is empty") + } + if redirectURI.Port() == "0" { + t.Fatalf("redirectURI port = %q, want dynamically assigned port", redirectURI.Port()) + } +} + +func newMockOAuthTokenServer() *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/oauth/token" { http.Error(w, "not found", http.StatusNotFound) return @@ -439,52 +488,4 @@ func TestLoginBrowserWithOptionsAutoOpensByDefault(t *testing.T) { } _ = json.NewEncoder(w).Encode(resp) })) - defer server.Close() - - origOpenBrowserFunc := openBrowserFunc - origBrowserLoginInput := browserLoginInput - t.Cleanup(func() { - openBrowserFunc = origOpenBrowserFunc - browserLoginInput = origBrowserLoginInput - }) - - var openCalls int - openBrowserFunc = func(string) error { - openCalls++ - return nil - } - browserLoginInput = strings.NewReader("manual-code\n") - - cfg := OAuthProviderConfig{ - Issuer: server.URL, - ClientID: "test-client", - Scopes: "openid", - Port: freeLocalPort(t), - } - - _, err := LoginBrowserWithOptions(cfg, LoginBrowserOptions{}) - if err != nil { - t.Fatalf("LoginBrowserWithOptions() error: %v", err) - } - - if openCalls != 1 { - t.Fatalf("openBrowserFunc call count = %d, want 1", openCalls) - } -} - -func freeLocalPort(t *testing.T) int { - t.Helper() - - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatalf("net.Listen() error: %v", err) - } - defer listener.Close() - - addr, ok := listener.Addr().(*net.TCPAddr) - if !ok { - t.Fatalf("listener addr type = %T, want *net.TCPAddr", listener.Addr()) - } - - return addr.Port }