fix(management): support external base url for webui oauth callbacks#2605
fix(management): support external base url for webui oauth callbacks#2605cikichen wants to merge 1 commit intorouter-for-me:devfrom
Conversation
|
This pull request targeted The base branch has been automatically changed to |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds an "external-base-url" configuration to allow remote management clients to use public callback URLs instead of local forwarders for OAuth authentication across multiple providers. While the logic is implemented for several services, the PR is currently incomplete as it references a missing "ExchangeCodeForTokensWithRedirect" method for the Codex provider, which will prevent the code from compiling. Additionally, the feedback highlights an architectural inconsistency in how the Codex redirect URI is managed compared to other providers, recommending a more standardized approach within the authentication packages to eliminate manual URL manipulation.
| log.Debug("Authorization code received, exchanging for tokens...") | ||
| // Exchange code for tokens using internal auth service | ||
| bundle, errExchange := openaiAuth.ExchangeCodeForTokens(ctx, code, pkceCodes) | ||
| bundle, errExchange := openaiAuth.ExchangeCodeForTokensWithRedirect(ctx, code, redirectURI, pkceCodes) |
There was a problem hiding this comment.
The method openaiAuth.ExchangeCodeForTokensWithRedirect is called here, but its definition is missing from the pull request. It appears that the necessary changes to internal/auth/codex/codex_auth.go (similar to those made for claude in internal/auth/claude/anthropic_auth.go) were not included in the patch set. This will result in a compilation error.
| } else if ok { | ||
| redirectURI = externalCallbackURL | ||
| useLocalForwarder = false | ||
| updatedURL, errReplace := replaceURLQueryValue(authURL, "redirect_uri", redirectURI) |
There was a problem hiding this comment.
There is an inconsistency in how the redirect_uri is updated for the codex provider compared to claude. While claude uses a new GenerateAuthURLWithRedirect method (added in this PR), codex relies on a manual query string replacement via replaceURLQueryValue. For better maintainability and consistency, the codex package should be updated with a similar WithRedirect method for URL generation, avoiding the need for manual URL manipulation in the handler.
| func replaceURLQueryValue(rawURL, key, value string) (string, error) { | ||
| parsed, err := url.Parse(strings.TrimSpace(rawURL)) | ||
| if err != nil { | ||
| return "", fmt.Errorf("parse url: %w", err) | ||
| } | ||
| query := parsed.Query() | ||
| query.Set(key, value) | ||
| parsed.RawQuery = query.Encode() | ||
| return parsed.String(), nil | ||
| } |
There was a problem hiding this comment.
The replaceURLQueryValue function is only used as a workaround for the codex provider's API inconsistency. If the codex package is updated to support a redirect URI during URL generation (as suggested in the other comment), this utility function becomes unnecessary and should be removed to keep the codebase clean.
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary
- Adds
remote-management.external-base-urlto support externally reachable OAuth callback URLs for WebUI flows. - Routes Codex/Claude/Gemini CLI/Antigravity WebUI OAuth through provider-specific external callback paths when configured, and skips localhost callback forwarders in that case.
- Adds tests to cover external callback URL resolution and preserve localhost fallback behavior.
Key findings
- Blocking: None.
- Non-blocking:
- Consider validating
external-base-urlscheme tohttp/httpsonly to prevent accidental misconfiguration. - Tests currently rely on
time.Sleep(600ms); polling with a timeout could reduce flakiness on slow runners.
- Consider validating
Test plan
go test ./internal/api/handlers/management ./internal/config ./internal/auth/claude
This is an automated Codex review result and still requires manual verification by a human reviewer.
2 participants
Summary
remote-management.external-base-urlso webui oauth callbacks can use a configured public base URL