fix: proxy /oauth/* through the BFF and stop guessing redirect_uri - #101
fix: proxy /oauth/* through the BFF and stop guessing redirect_uri#101marekdano wants to merge 1 commit into
Conversation
Signed-off-by: Marek Dano <mk.dano@gmail.com>
cafalchio
left a comment
There was a problem hiding this comment.
LGTM
I will trust your test, but I am not qualified for reviewing the ts.
There was a problem hiding this comment.
Verified on 6458-oauth-callback-path: client and server tsc clean, server suite 95 passed, client suite green.
The proxy pair reads right. forwardOAuthGet forwards only status, Location and Content-Type, so upstream Set-Cookie can never reach the browser; the authorize hop injects the session bearer and a browser-supplied Authorization cannot override it; redirect: "manual" keeps the 302 aimed at the provider. I also checked the CSRF reasoning, since a top-level window.open GET sends no Origin header: isForbiddenCrossOrigin falls through to Sec-Fetch-Site (origin-guard.ts:38-42), which reads cross-site for the hostile-opener case and same-origin for the real popup, so the guard does hold for the case the header comment describes.
Potential blocker: proxy-level failures leave the popup showing JSON and the form waiting indefinitely:
Every failure these two routes can produce is a JSON body: 401 from sessionAuth, 403 from the origin guard, 502 from forwardOAuthGet's catch. The popup is a bare window.open, so the browser just renders that JSON and nothing posts a message. triggerOAuthAuthorization (src/api/servers.ts:214-243) settles only on a postMessage or on authWindow.closed, so the form sits on "Waiting for OAuth authorization in the popup window" until the user closes the popup by hand, and then reports "OAuth authorization was cancelled", which is not what happened.
The 502 is the plausible one: the 30s timeout is explicitly sized for the DCR round trip, so a slow or unreachable registration endpoint lands exactly here. Returning the same HTML shape the callback hop returns, postMessage an oauth_callback error then window.close(), would close the loop for all three.
Copy change request
redirectUriAutoHelp reads "The gateway fills this in from its own configured public URL (APP_DOMAIN)". We should move away from using this term (other than the product description as an "AI Gateway".
Change to something like "Set automatically from the server's public URL".
Same field: redirectUriLocalWarning says "The server's public URL is not configured. Redirect URIs derived from localhost will not work", but it now fires only for an explicitly stored localhost URI, so nothing is derived and the public URL is configured, just to localhost.
|
Filed the sibling case this PR does not cover: IBM/mcp-context-forge#6632.
|
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.