fix: proxy /oauth/* through the BFF and stop guessing redirect_uri - #101
Open
marekdano wants to merge 1 commit into
Open
fix: proxy /oauth/* through the BFF and stop guessing redirect_uri#101marekdano wants to merge 1 commit into
marekdano wants to merge 1 commit into
Conversation
Signed-off-by: Marek Dano <mk.dano@gmail.com>
marekdano
requested review from
a-effort,
cafalchio,
gcgoncalves and
vishu-bh
September 3, 2026 13:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6458
Summary
The OAuth authorization-code popup had no reliable home. Two root causes:
/oauth/*proxy on the BFF. The popup's first hop (window.open("/oauth/authorize/{id}?popup=true")) is a raw browser navigation with no/apiprefix, so it fell through to the SPA's catch-all 404 handler instead of reaching the gateway - the popup opened the React shell instead of redirecting to the provider.redirect_uriderived fromwindow.location.origin. The web UI's own origin isn't necessarily where the gateway serves/oauth/callbackin a split deployment, so the value registered with the OAuth provider could silently diverge from what the gateway/APP_DOMAIN actually use.Changes
server/src/routes/proxy/oauth-authorize.ts(new): authenticated proxy for the popup's first hop — injects the session's bearer token (same ascatch-all.tsdoes for/api/*), forwards the provider redirect untouched, and rejects cross-site requests via the sameisForbiddenCrossOriginguardlogin.ts/proxy-sse.tsalready use (this route isn't idempotent — it can run DCR registration and DB writes - and can't rely on a CSRF token sincewindow.opensets no headers).server/src/routes/proxy/oauth-callback.ts(new): unauthenticated proxy for the second hop, needed both for gateways with a pre-existingredirect_uripointing at the web UI's origin, and so the flow works when the gateway isn't independently internet-reachable (common split deployment).server/src/lib/oauth-upstream-forward.ts(new): shared fetch/timeout/error/header-forwarding logic between the two proxy routes.src/components/mcp-servers/OAuth2Auth.tsx: stopped deriving/submittingredirect_urifromwindow.location.origin. When no value is stored, the form now shows an explicit "determined automatically by the server" placeholder instead of a guess, letting the gateway's ownAPP_DOMAIN-based default apply.onRedirectUriChangeprop chain throughAdvancedSettings.tsx/MCPServerForm.tsx.oauth-authorize.test.ts,oauth-callback.test.ts, including a cross-origin rejection test), updatedOAuth2Auth.test.tsx, and a newe2e/oauth-authorization.spec.tsdriving the full popup flow through a real browser (success and error paths) with the popup's network mocked at the browser-context level.Test plan
npm run typecheck(root +server/)npm run lintnpx vitest run— 95 BFF + 3304 frontend tests passingnpx playwright test— new OAuth specs + fullservers.spec.tspassingTesting locally (split-origin deployment)
The bug only reproduces when the web UI and the gateway are on genuinely different origins, so
localhostfor both isn't enough. Using/etc/hostsaliases avoids needing a second device or exposing anything beyond loopback:sudo sh -c 'echo "127.0.0.1 web.local" >> /etc/hosts'
sudo sh -c 'echo "127.0.0.1 api.local" >> /etc/hosts'
mcp-context-forge/.env— the gateway defaults to binding127.0.0.1only, so it must be opened up to accept requests addressed toapi.local:HOST=0.0.0.0
APP_DOMAIN=http://api.local:8000
contextforge-web-ui/.env— for local HTTP testing:COOKIE_SECURE=false
(
CONTEXTFORGE_URLcan stayhttp://127.0.0.1:8000— that hop is server-to-server, same machine, origin doesn't matter there.)http://web.local:3000— notlocalhost:3000— sowindow.location.origingenuinely differs fromAPP_DOMAIN.http://api.local:8000/oauth/callback, and delete/recreate the gateway in the UI soredirect_uristarts unset (picks up the newAPP_DOMAIN- based default rather than a stale value from before this fix).api.local:8000/oauth/callback, and close itself with a success notification in the opener.