From 0511b76adadfc112aa05e24409a07b880ee467ae Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 21 May 2026 16:17:16 +0000 Subject: [PATCH] Gate MCP OAuth pass-through on delegate_auth_to_upstream flag Sameer's review on #28356/#28008 flagged that the new pass-through behaviors (preemptive 401 challenges, /.well-known/oauth-protected- resource proxying, upstream 401/403 propagation as MCPUpstreamAuthError, and Authorization-stripping when no x-litellm-api-key is supplied) were implicitly enabled for every server with auth_type=none plus Authorization in extra_headers. Existing users doing static bearer pass-through for non-OAuth reasons would have silently regressed. Make the detection rule explicit: extend the existing delegate_auth_to_upstream flag (previously oauth2-only) to also gate is_oauth_passthrough. Now requires flag + auth_type=None + Authorization in extra_headers, per Sameer's suggested detection rule. The UI toggle now appears for both modes (oauth2 PKCE passthrough and auth_type=none OAuth pass-through) with mode-appropriate copy. Update test fixtures to set the flag where the test intent is to exercise OAuth pass-through behavior, and add negative tests covering the new default-false case. --- .../types/mcp_server/mcp_server_manager.py | 33 ++++++++++++--- .../mcp_server/test_mcp_oauth_passthrough.py | 37 ++++++++++++++++ .../test_mcp_oauth_passthrough_cold_start.py | 1 + .../test_mcp_oauth_passthrough_tools.py | 2 + .../mcp_server/test_mcp_server.py | 1 + .../mcp_tools/MCPPermissionManagement.tsx | 42 +++++++++++++++---- .../components/mcp_tools/mcp_server_edit.tsx | 27 ++++++++---- 7 files changed, 121 insertions(+), 22 deletions(-) diff --git a/litellm/types/mcp_server/mcp_server_manager.py b/litellm/types/mcp_server/mcp_server_manager.py index cadd41c3e46..438c2e283e4 100644 --- a/litellm/types/mcp_server/mcp_server_manager.py +++ b/litellm/types/mcp_server/mcp_server_manager.py @@ -68,11 +68,23 @@ class MCPServer(BaseModel): access_groups: Optional[List[str]] = None allow_all_keys: bool = False available_on_public_internet: bool = True - # When True AND auth_type == oauth2, MCP requests targeting this server - # bypass LiteLLM API-key/SSO auth (and the pre-emptive 401) so the client - # completes PKCE directly with the upstream MCP server. Honored only for - # auth_type=oauth2; ignored for any other auth_type. See - # MCPRequestHandler._target_servers_delegate_auth_to_upstream. + # Explicit opt-in to upstream-delegated authentication. Two distinct modes + # depending on ``auth_type``: + # + # * ``auth_type == oauth2``: MCP requests bypass LiteLLM API-key/SSO + # auth (and the pre-emptive 401) so the client completes PKCE + # directly with the upstream MCP server. See + # ``MCPRequestHandler._target_servers_delegate_auth_to_upstream``. + # * ``auth_type in (None, MCPAuth.none)`` AND ``extra_headers`` contains + # ``Authorization``: enables OAuth pass-through (see + # ``is_oauth_passthrough``). The gateway proxies upstream + # ``/.well-known/oauth-protected-resource`` metadata, emits + # spec-compliant 401 challenges when no bearer is supplied, and + # propagates upstream 401/403 responses instead of swallowing them. + # + # Ignored for any other ``auth_type``. The flag must be set explicitly to + # avoid silently changing behavior for servers that forward + # ``Authorization`` for non-OAuth reasons (e.g. static bearer tokens). delegate_auth_to_upstream: bool = False is_byok: bool = False byok_description: List[str] = [] @@ -144,12 +156,19 @@ class MCPServer(BaseModel): (discovery + 401s) rather than participating as an authorization server itself. - A server is pass-through for OAuth purposes when both conditions hold: + A server is pass-through for OAuth purposes when ALL three conditions + hold: 1. ``auth_type`` is ``None`` or ``MCPAuth.none`` (the gateway does not manage OAuth for this server). 2. ``extra_headers`` includes ``Authorization`` — the admin has opted this server into forwarding the client's bearer token straight to the upstream MCP server. + 3. ``delegate_auth_to_upstream`` is ``True`` — the admin has + explicitly opted into upstream-delegated OAuth semantics for + this server. This is the explicit detection flag: without it, + a server that merely forwards ``Authorization`` (e.g. for + static bearer tokens or custom auth schemes) keeps the + pre-PR behavior and is not treated as OAuth pass-through. This is intentionally narrower than ``requires_per_user_auth``, which also covers PATs (``x-api-key``, ``api-key``, ``apikey``). @@ -160,6 +179,8 @@ class MCPServer(BaseModel): return False if not self.extra_headers: return False + if self.delegate_auth_to_upstream is not True: + return False return any(h.lower() == "authorization" for h in self.extra_headers) @property diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough.py index 17fc029030b..015b23c2fdf 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough.py @@ -70,6 +70,7 @@ def test_is_oauth_passthrough_true_when_none_auth_and_authorization_header(): transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) assert server.is_oauth_passthrough is True @@ -81,6 +82,7 @@ def test_is_oauth_passthrough_true_when_auth_type_none_and_mixed_case_header(): transport=MCPTransport.http, auth_type=None, extra_headers=["authorization", "x-request-id"], + delegate_auth_to_upstream=True, ) assert server.is_oauth_passthrough is True @@ -92,6 +94,7 @@ def test_is_oauth_passthrough_false_for_oauth2_server(): transport=MCPTransport.http, auth_type=MCPAuth.oauth2, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) assert server.is_oauth_passthrough is False @@ -103,6 +106,7 @@ def test_is_oauth_passthrough_false_without_authorization_header(): transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["x-api-key"], + delegate_auth_to_upstream=True, ) assert server.is_oauth_passthrough is False @@ -113,6 +117,34 @@ def test_is_oauth_passthrough_false_without_extra_headers(): name="s1", transport=MCPTransport.http, auth_type=MCPAuth.none, + delegate_auth_to_upstream=True, + ) + assert server.is_oauth_passthrough is False + + +def test_is_oauth_passthrough_false_without_delegate_flag(): + """The detection flag must be set explicitly. Without it, the legacy + behavior is preserved for servers that forward Authorization for + non-OAuth reasons (static bearer tokens, custom auth schemes).""" + server = MCPServer( + server_id="s1", + name="s1", + transport=MCPTransport.http, + auth_type=MCPAuth.none, + extra_headers=["Authorization"], + # delegate_auth_to_upstream defaults to False + ) + assert server.is_oauth_passthrough is False + + +def test_is_oauth_passthrough_false_when_delegate_flag_explicitly_false(): + server = MCPServer( + server_id="s1", + name="s1", + transport=MCPTransport.http, + auth_type=MCPAuth.none, + extra_headers=["Authorization"], + delegate_auth_to_upstream=False, ) assert server.is_oauth_passthrough is False @@ -138,6 +170,7 @@ async def test_oauth_protected_resource_passthrough_proxies_upstream_metadata(): transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) global_mcp_server_manager.registry[passthrough_server.server_id] = ( passthrough_server @@ -188,6 +221,7 @@ async def test_oauth_protected_resource_passthrough_cache_hit(): transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) global_mcp_server_manager.registry[passthrough_server.server_id] = ( passthrough_server @@ -280,6 +314,7 @@ async def test_oauth_metadata_cache_expired_entry_is_refetched(): transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) _OAUTH_METADATA_CACHE[(passthrough_server.server_id, passthrough_server.url)] = ( 0, @@ -321,6 +356,7 @@ async def test_oauth_protected_resource_passthrough_network_error_returns_502(): transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) global_mcp_server_manager.registry[passthrough_server.server_id] = ( passthrough_server @@ -353,6 +389,7 @@ async def test_fetch_upstream_metadata_returns_none_when_not_all_candidates_netw transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) not_found_response = MagicMock() diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_cold_start.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_cold_start.py index 139f3bad4a3..f0f32a65708 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_cold_start.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_cold_start.py @@ -60,6 +60,7 @@ def test_passthrough_cold_start_emits_401_with_matching_resource_metadata( transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) global_mcp_server_manager.registry[passthrough_server.server_id] = ( passthrough_server diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_tools.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_tools.py index 59029339583..0e00443d15d 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_tools.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_tools.py @@ -60,6 +60,7 @@ async def test_fetch_tools_from_passthrough_raises_on_upstream_401(): transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) response = httpx.Response( @@ -97,6 +98,7 @@ async def test_fetch_tools_from_passthrough_returns_tools_on_success(): transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization"], + delegate_auth_to_upstream=True, ) tool = MagicMock() diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py index 060eb29e63b..a07e0ead253 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py @@ -153,6 +153,7 @@ def test_prepare_mcp_server_headers_passthrough_strips_authorization_without_adm transport=MCPTransport.http, auth_type=MCPAuth.none, extra_headers=["Authorization", "x-request-id"], + delegate_auth_to_upstream=True, ) server_auth_header, extra_headers = _prepare_mcp_server_headers( diff --git a/ui/litellm-dashboard/src/components/mcp_tools/MCPPermissionManagement.tsx b/ui/litellm-dashboard/src/components/mcp_tools/MCPPermissionManagement.tsx index 58848df39a0..4aaca2a274b 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/MCPPermissionManagement.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/MCPPermissionManagement.tsx @@ -25,6 +25,19 @@ const MCPPermissionManagement: React.FC = ({ const form = Form.useFormInstance(); const watchedAuthType = Form.useWatch("auth_type", form); const isOAuth2 = watchedAuthType === AUTH_TYPE.OAUTH2; + const isNoneAuth = watchedAuthType === AUTH_TYPE.NONE || watchedAuthType == null; + const watchedExtraHeaders = Form.useWatch("extra_headers", form); + const hasAuthorizationHeader = Array.isArray(watchedExtraHeaders) + && watchedExtraHeaders.some( + (h) => typeof h === "string" && h.toLowerCase() === "authorization", + ); + // Two modes that honor delegate_auth_to_upstream server-side: + // - oauth2 servers (PKCE passthrough — bypass LiteLLM admission) + // - auth_type=none + Authorization in extra_headers (OAuth pass-through: + // proxy upstream oauth-protected-resource, emit 401 challenges, + // propagate upstream 401/403) + const isOAuthPassthrough = isNoneAuth && hasAuthorizationHeader; + const canDelegateAuth = isOAuth2 || isOAuthPassthrough; const watchedDelegateAuth = Form.useWatch("delegate_auth_to_upstream", form); const watchedPublicInternet = Form.useWatch("available_on_public_internet", form); const showInternalDelegatePkceWarning = @@ -58,14 +71,15 @@ const MCPPermissionManagement: React.FC = ({ } }, [mcpServer, form]); - // delegate_auth_to_upstream is only honored server-side when auth_type=oauth2. - // Force it back to false whenever the user switches away from oauth2 so a - // stale toggle value doesn't get persisted with another auth type. + // delegate_auth_to_upstream is only honored server-side for oauth2 servers + // or auth_type=none servers that forward Authorization to upstream. Force + // it back to false whenever the user switches to any other configuration + // so a stale toggle value doesn't get persisted unexpectedly. useEffect(() => { - if (!isOAuth2) { + if (!canDelegateAuth) { form.setFieldValue("delegate_auth_to_upstream", false); } - }, [isOAuth2, form]); + }, [canDelegateAuth, form]); return ( @@ -126,17 +140,27 @@ const MCPPermissionManagement: React.FC = ({ - {isOAuth2 && ( + {canDelegateAuth && (
- Delegate auth to upstream (PKCE passthrough) - + {isOAuth2 + ? "Delegate auth to upstream (PKCE passthrough)" + : "Delegate auth to upstream (OAuth pass-through)"} +

- Bypass LiteLLM auth so clients authenticate directly with the upstream OAuth MCP server. + {isOAuth2 + ? "Bypass LiteLLM auth so clients authenticate directly with the upstream OAuth MCP server." + : "Forward upstream OAuth discovery and 401 challenges so clients negotiate OAuth directly with the upstream MCP server."}

= ({ allow_all_keys: Boolean(allowAllKeysRaw ?? mcpServer.allow_all_keys), available_on_public_internet: Boolean(availableOnPublicInternetRaw ?? mcpServer.available_on_public_internet), // ``delegate_auth_to_upstream`` is only honored server-side for - // ``auth_type=oauth2``. The Form.Item is conditionally rendered so the - // value drops out of the form on auth_type change; force false for any - // non-oauth2 server to avoid persisting a stale ``true`` that would - // silently re-activate if auth_type is later switched back to oauth2. - delegate_auth_to_upstream: - restValues.auth_type === AUTH_TYPE.OAUTH2 + // ``auth_type=oauth2`` (PKCE passthrough) or ``auth_type=none`` with + // ``Authorization`` in ``extra_headers`` (OAuth pass-through). The + // Form.Item is conditionally rendered so the value drops out of the + // form on auth_type change; force false for any other configuration + // to avoid persisting a stale ``true`` that would silently + // re-activate if the configuration is later switched back. + delegate_auth_to_upstream: (() => { + const isOauth2 = restValues.auth_type === AUTH_TYPE.OAUTH2; + const isNoneAuth = + restValues.auth_type === AUTH_TYPE.NONE || restValues.auth_type == null; + const extraHeaders = Array.isArray(restValues.extra_headers) + ? restValues.extra_headers + : []; + const hasAuthorizationHeader = extraHeaders.some( + (h: unknown) => typeof h === "string" && h.toLowerCase() === "authorization", + ); + const eligible = isOauth2 || (isNoneAuth && hasAuthorizationHeader); + return eligible ? Boolean(delegateAuthToUpstreamRaw ?? mcpServer.delegate_auth_to_upstream) - : false, + : false; + })(), // Include token_validation when it is set (non-null) or when clearing an existing value ...(tokenValidation !== null || mcpServer.token_validation ? { token_validation: tokenValidation }