From f2d20baf9d84134a00a2d97f2d721f25b2b85898 Mon Sep 17 00:00:00 2001 From: Ishaan Jaffer Date: Sat, 7 Mar 2026 11:13:16 -0800 Subject: [PATCH] fix: use LITELLM_PROXY_BASE_URL for redirect_uri, close popup on unmount, no handleError in poll - Add _get_callback_base_url() helper that prefers LITELLM_PROXY_BASE_URL env var over X-Forwarded-Host-derived URL, avoiding header spoofing in the security-sensitive OAuth2 redirect_uri construction. - OAuth2ConnectButton.tsx: close popup on component unmount so orphaned popups don't persist when the user navigates away. - getMcpOAuth2Status: remove handleError() call before throwing so the polling loop's catch block is the single error surface and no unexpected UI notifications fire on transient 2-second poll failures. --- .../mcp_server/openapi_oauth2_endpoints.py | 20 +++++++++++++++++-- .../mcp_tools/OAuth2ConnectButton.tsx | 7 ++++++- .../src/components/networking.tsx | 7 ++++--- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/openapi_oauth2_endpoints.py b/litellm/proxy/_experimental/mcp_server/openapi_oauth2_endpoints.py index 9fd1be888b7..4cc8ec1e502 100644 --- a/litellm/proxy/_experimental/mcp_server/openapi_oauth2_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/openapi_oauth2_endpoints.py @@ -59,6 +59,22 @@ router = APIRouter(tags=["mcp"]) # --------------------------------------------------------------------------- +def _get_callback_base_url(request: "Request") -> str: + """Return the base URL to use for the OAuth2 redirect_uri. + + Prefers a statically-configured base URL (via the LITELLM_PROXY_BASE_URL + environment variable) over the request-derived URL to avoid trusting + potentially-spoofable X-Forwarded-Host headers in the OAuth2 security + context. Falls back to the request-derived URL when the env var is unset. + """ + import os + + static_base = os.environ.get("LITELLM_PROXY_BASE_URL", "").rstrip("/") + if static_base: + return static_base + return get_request_base_url(request) + + def _purge_expired_states() -> None: now = time.time() expired = [k for k, v in _pending_oauth2_states.items() if v["expires_at"] < now] @@ -206,7 +222,7 @@ async def openapi_oauth2_connect( "expires_at": timestamp + _STATE_TTL_SECONDS, } - base_url = get_request_base_url(request) + base_url = _get_callback_base_url(request) callback_url = f"{base_url}/v1/mcp/oauth2/callback" # NOTE: PKCE (RFC 7636 / OAuth 2.1) is not implemented here because this is @@ -316,7 +332,7 @@ async def openapi_oauth2_callback( status_code=500, ) - base_url = get_request_base_url(request) + base_url = _get_callback_base_url(request) callback_url = f"{base_url}/v1/mcp/oauth2/callback" token_request_data = { diff --git a/ui/litellm-dashboard/src/components/mcp_tools/OAuth2ConnectButton.tsx b/ui/litellm-dashboard/src/components/mcp_tools/OAuth2ConnectButton.tsx index 9d62a09b9ed..cd56469dc96 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/OAuth2ConnectButton.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/OAuth2ConnectButton.tsx @@ -107,10 +107,15 @@ export const OAuth2ConnectButton: React.FC = ({ } }; - // Clean up on unmount + // Clean up on unmount: stop polling and close any open popup so it doesn't + // stay open after the user navigates away from the page. useEffect(() => { return () => { stopPolling(); + if (popupRef.current && !popupRef.current.closed) { + popupRef.current.close(); + } + popupRef.current = null; }; }, []); diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 4f63cbe2d92..42a1a64b796 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -6442,15 +6442,16 @@ export const getMcpOAuth2Status = async ( }); if (!response.ok) { - const errorData = await response.json(); + // Do NOT call handleError here: this function is used inside a polling + // loop that catches and ignores errors. Calling handleError would cause + // UI notifications to fire on every failed poll tick. + const errorData = await response.json().catch(() => ({})); const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); throw new Error(errorMessage); } return await response.json(); } catch (error) { - console.error("Failed to get MCP OAuth2 status:", error); throw error; } };