fix(auth): prevent duplicate OpenAI OAuth transactions
- commit
- 852dd3a91c3150d35d19bc990436c42354f1dcc9
- author
- Zeno Jiricek · · 2026-09-26 05:29 UTC
- parent
- c2267b3
- signature
- unsigned
5 files · +29 -13
M packages/agent/modes/interactive.go +2 -10
| @@ -5352,20 +5352,12 @@ func (i *Interactive) startOAuthFlow(provider string) { | |||
| 5352 | 5352 | i.dialog.ShowWaiting(loginURL) | |
| 5353 | 5353 | return | |
| 5354 | 5354 | } | |
| 5355 | - | // Always run the manual/copy-code flow in parallel with the local | |
| 5356 | - | // callback server so headless environments (docker, SSH) can paste | |
| 5357 | - | // the authorization code directly without first pressing 'p'. | |
| 5358 | - | _, err := i.cfg.AuthManager.StartOAuth(provider) | |
| 5355 | + | loginURL, err := i.cfg.AuthManager.StartOAuth(provider) | |
| 5359 | 5356 | if err != nil { | |
| 5360 | 5357 | i.dialog.ShowResult(false, err.Error()) | |
| 5361 | 5358 | return | |
| 5362 | 5359 | } | |
| 5363 | - | manualURL, mErr := i.cfg.AuthManager.StartManualOAuth(provider) | |
| 5364 | - | if mErr == nil { | |
| 5365 | - | i.dialog.ShowWaiting(manualURL) | |
| 5366 | - | } else { | |
| 5367 | - | i.dialog.ShowResult(false, mErr.Error()) | |
| 5368 | - | } | |
| 5360 | + | i.dialog.ShowWaiting(loginURL) | |
| 5369 | 5361 | } | |
| 5370 | 5362 | ||
| 5371 | 5363 | func (i *Interactive) startManualOAuthFlow(provider string) { | |
| @@ -5352,20 +5352,12 @@ func (i *Interactive) startOAuthFlow(provider string) { | |||||
| 5352 | i.dialog.ShowWaiting(loginURL) | 5352 | i.dialog.ShowWaiting(loginURL) | ||
| 5353 | return | 5353 | return | ||
| 5354 | } | 5354 | } | ||
| 5355 | - | // Always run the manual/copy-code flow in parallel with the local | 5355 | + | loginURL, err := i.cfg.AuthManager.StartOAuth(provider) |
| 5356 | - | // callback server so headless environments (docker, SSH) can paste | |||
| 5357 | - | // the authorization code directly without first pressing 'p'. | |||
| 5358 | - | _, err := i.cfg.AuthManager.StartOAuth(provider) | |||
| 5359 | if err != nil { | 5356 | if err != nil { | ||
| 5360 | i.dialog.ShowResult(false, err.Error()) | 5357 | i.dialog.ShowResult(false, err.Error()) | ||
| 5361 | return | 5358 | return | ||
| 5362 | } | 5359 | } | ||
| 5363 | - | manualURL, mErr := i.cfg.AuthManager.StartManualOAuth(provider) | 5360 | + | i.dialog.ShowWaiting(loginURL) |
| 5364 | - | if mErr == nil { | |||
| 5365 | - | i.dialog.ShowWaiting(manualURL) | |||
| 5366 | - | } else { | |||
| 5367 | - | i.dialog.ShowResult(false, mErr.Error()) | |||
| 5368 | - | } | |||
| 5369 | } | 5361 | } | ||
| 5370 | 5362 | ||||
| 5371 | func (i *Interactive) startManualOAuthFlow(provider string) { | 5363 | func (i *Interactive) startManualOAuthFlow(provider string) { | ||
M packages/agent/modes/login_dialog.go +1 -1
| @@ -216,7 +216,7 @@ func (d *loginDialog) Render(th tui.Theme, width int) []string { | |||
| 216 | 216 | lines = append(lines, th.FG256(th.Accent, seg)) | |
| 217 | 217 | } | |
| 218 | 218 | lines = append(lines, "") | |
| 219 | - | if d.provider == "kimi" || d.provider == "xai" || d.provider == "github-copilot" { | |
| 219 | + | if d.provider == "openai-codex" || d.provider == "kimi" || d.provider == "xai" || d.provider == "github-copilot" { | |
| 220 | 220 | lines = append(lines, th.FG256(th.Muted, "complete sign-in in the browser - esc cancels")) | |
| 221 | 221 | lines = append(lines, frameRule(th, width)) | |
| 222 | 222 | break | |
| @@ -216,7 +216,7 @@ func (d *loginDialog) Render(th tui.Theme, width int) []string { | |||||
| 216 | lines = append(lines, th.FG256(th.Accent, seg)) | 216 | lines = append(lines, th.FG256(th.Accent, seg)) | ||
| 217 | } | 217 | } | ||
| 218 | lines = append(lines, "") | 218 | lines = append(lines, "") | ||
| 219 | - | if d.provider == "kimi" || d.provider == "xai" || d.provider == "github-copilot" { | 219 | + | if d.provider == "openai-codex" || d.provider == "kimi" || d.provider == "xai" || d.provider == "github-copilot" { |
| 220 | lines = append(lines, th.FG256(th.Muted, "complete sign-in in the browser - esc cancels")) | 220 | lines = append(lines, th.FG256(th.Muted, "complete sign-in in the browser - esc cancels")) | ||
| 221 | lines = append(lines, frameRule(th, width)) | 221 | lines = append(lines, frameRule(th, width)) | ||
| 222 | break | 222 | break | ||
M packages/agent/modes/login_dialog_test.go +16 -0
| @@ -69,6 +69,22 @@ func TestLoginDialogLlamaCPPValidatesURLAndAcceptsOptionalKey(t *testing.T) { | |||
| 69 | 69 | } | |
| 70 | 70 | } | |
| 71 | 71 | ||
| 72 | + | func TestLoginDialogOpenAIOAuthDoesNotOfferCopyCode(t *testing.T) { | |
| 73 | + | d := newLoginDialog() | |
| 74 | + | d.Open(t.TempDir()) | |
| 75 | + | d.method = "oauth" | |
| 76 | + | d.provider = "openai-codex" | |
| 77 | + | d.ShowWaiting("http://localhost:1455/auth/callback") | |
| 78 | + | ||
| 79 | + | text := stripANSIBytes(strings.Join(d.Render(tui.Theme{}, 80), "\n")) | |
| 80 | + | if strings.Contains(text, "paste the authorization code") { | |
| 81 | + | t.Fatal("OpenAI OAuth dialog offers manual code entry") | |
| 82 | + | } | |
| 83 | + | if !strings.Contains(text, "complete sign-in in the browser") { | |
| 84 | + | t.Fatal("OpenAI OAuth dialog does not require browser sign-in") | |
| 85 | + | } | |
| 86 | + | } | |
| 87 | + | ||
| 72 | 88 | func TestLoginDialogCursorPosMatchesPaddedInputRow(t *testing.T) { | |
| 73 | 89 | d := newLoginDialog() | |
| 74 | 90 | d.Open(t.TempDir()) | |
| @@ -69,6 +69,22 @@ func TestLoginDialogLlamaCPPValidatesURLAndAcceptsOptionalKey(t *testing.T) { | |||||
| 69 | } | 69 | } | ||
| 70 | } | 70 | } | ||
| 71 | 71 | ||||
| 72 | + | func TestLoginDialogOpenAIOAuthDoesNotOfferCopyCode(t *testing.T) { | |||
| 73 | + | d := newLoginDialog() | |||
| 74 | + | d.Open(t.TempDir()) | |||
| 75 | + | d.method = "oauth" | |||
| 76 | + | d.provider = "openai-codex" | |||
| 77 | + | d.ShowWaiting("http://localhost:1455/auth/callback") | |||
| 78 | + | ||||
| 79 | + | text := stripANSIBytes(strings.Join(d.Render(tui.Theme{}, 80), "\n")) | |||
| 80 | + | if strings.Contains(text, "paste the authorization code") { | |||
| 81 | + | t.Fatal("OpenAI OAuth dialog offers manual code entry") | |||
| 82 | + | } | |||
| 83 | + | if !strings.Contains(text, "complete sign-in in the browser") { | |||
| 84 | + | t.Fatal("OpenAI OAuth dialog does not require browser sign-in") | |||
| 85 | + | } | |||
| 86 | + | } | |||
| 87 | + | ||||
| 72 | func TestLoginDialogCursorPosMatchesPaddedInputRow(t *testing.T) { | 88 | func TestLoginDialogCursorPosMatchesPaddedInputRow(t *testing.T) { | ||
| 73 | d := newLoginDialog() | 89 | d := newLoginDialog() | ||
| 74 | d.Open(t.TempDir()) | 90 | d.Open(t.TempDir()) | ||
M packages/provider/auth/auth_test.go +9 -0
| @@ -101,6 +101,15 @@ func TestPKCE(t *testing.T) { | |||
| 101 | 101 | } | |
| 102 | 102 | } | |
| 103 | 103 | ||
| 104 | + | func TestStartManualOAuthRejectsOpenAI(t *testing.T) { | |
| 105 | + | m := NewManager(NewStore(filepath.Join(t.TempDir(), "auth.json"))) | |
| 106 | + | if _, err := m.StartManualOAuth("openai-codex"); err == nil { | |
| 107 | + | t.Fatal("expected manual OpenAI OAuth to be rejected") | |
| 108 | + | } else if !strings.Contains(err.Error(), "manual code login is not supported") { | |
| 109 | + | t.Fatalf("unexpected error: %v", err) | |
| 110 | + | } | |
| 111 | + | } | |
| 112 | + | ||
| 104 | 113 | func TestAuthorizeURL(t *testing.T) { | |
| 105 | 114 | p, _ := NewPKCE() | |
| 106 | 115 | u, state, err := AnthropicOAuth.AuthorizeURL(p) | |
| @@ -101,6 +101,15 @@ func TestPKCE(t *testing.T) { | |||||
| 101 | } | 101 | } | ||
| 102 | } | 102 | } | ||
| 103 | 103 | ||||
| 104 | + | func TestStartManualOAuthRejectsOpenAI(t *testing.T) { | |||
| 105 | + | m := NewManager(NewStore(filepath.Join(t.TempDir(), "auth.json"))) | |||
| 106 | + | if _, err := m.StartManualOAuth("openai-codex"); err == nil { | |||
| 107 | + | t.Fatal("expected manual OpenAI OAuth to be rejected") | |||
| 108 | + | } else if !strings.Contains(err.Error(), "manual code login is not supported") { | |||
| 109 | + | t.Fatalf("unexpected error: %v", err) | |||
| 110 | + | } | |||
| 111 | + | } | |||
| 112 | + | ||||
| 104 | func TestAuthorizeURL(t *testing.T) { | 113 | func TestAuthorizeURL(t *testing.T) { | ||
| 105 | p, _ := NewPKCE() | 114 | p, _ := NewPKCE() | ||
| 106 | u, state, err := AnthropicOAuth.AuthorizeURL(p) | 115 | u, state, err := AnthropicOAuth.AuthorizeURL(p) | ||
M packages/provider/auth/manager.go +1 -2
| @@ -354,8 +354,7 @@ func (m *Manager) StartManualOAuth(provider string) (string, error) { | |||
| 354 | 354 | case "anthropic": | |
| 355 | 355 | op = AnthropicManualOAuth | |
| 356 | 356 | case "openai", "openai-codex": | |
| 357 | - | op = OpenAIOAuth | |
| 358 | - | storeProvider = "openai" | |
| 357 | + | return "", fmt.Errorf("openai login requires the browser callback flow; manual code login is not supported") | |
| 359 | 358 | case "google": | |
| 360 | 359 | return "", fmt.Errorf("google login is api-key only; use api key login for gemini") | |
| 361 | 360 | case "deepseek": | |
| @@ -354,8 +354,7 @@ func (m *Manager) StartManualOAuth(provider string) (string, error) { | |||||
| 354 | case "anthropic": | 354 | case "anthropic": | ||
| 355 | op = AnthropicManualOAuth | 355 | op = AnthropicManualOAuth | ||
| 356 | case "openai", "openai-codex": | 356 | case "openai", "openai-codex": | ||
| 357 | - | op = OpenAIOAuth | 357 | + | return "", fmt.Errorf("openai login requires the browser callback flow; manual code login is not supported") |
| 358 | - | storeProvider = "openai" | |||
| 359 | case "google": | 358 | case "google": | ||
| 360 | return "", fmt.Errorf("google login is api-key only; use api key login for gemini") | 359 | return "", fmt.Errorf("google login is api-key only; use api key login for gemini") | ||
| 361 | case "deepseek": | 360 | case "deepseek": | ||