From e9ff79c96af392594092fc192e76fd5178095321 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 21 May 2026 17:43:31 +0000 Subject: [PATCH] fix(mcp): admit and forward Authorization for passthrough OAuth return For pass-through MCP servers (auth_type=none with delegate_auth_to_upstream) the RFC 9728 cold-start flow sends the client back with only "Authorization: Bearer " after upstream OAuth discovery. Previously this path 1) was rejected in process_mcp_request because the oauth2_headers fallback only covered auth_type=oauth2 targets, and 2) had the Authorization header stripped by _prepare_mcp_server_headers when no x-litellm-api-key was present, treating the upstream token as a potential LiteLLM key leak. - Extend the elif oauth2_headers fallback to also admit anonymously when every target is a pass-through server. - Pass user_api_key_auth into _prepare_mcp_server_headers so it can forward Authorization for pass-through servers when admission did not consume the bearer as a LiteLLM key (api_key is unset). Co-authored-by: Yassin Kortam --- .../mcp_server/auth/user_api_key_auth_mcp.py | 38 +++++++-- .../proxy/_experimental/mcp_server/server.py | 39 +++++++-- .../auth/test_user_api_key_auth_mcp.py | 56 ++++++++++++- .../mcp_server/test_mcp_server.py | 83 +++++++++++++++++++ 4 files changed, 200 insertions(+), 16 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py index 4388275cb4f..7a6177e15df 100644 --- a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py +++ b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py @@ -252,21 +252,45 @@ class MCPRequestHandler: # than coercing (``int("None")`` would raise ValueError and # rewrite the auth error as a 500). status = e.status_code if isinstance(e, HTTPException) else e.code - if status in ( - 401, - 403, - "401", - "403", - ) and MCPRequestHandler._target_servers_use_oauth2( + is_auth_error = status in (401, 403, "401", "403") + client_ip = IPAddressUtils.get_mcp_client_ip(request) + if is_auth_error and MCPRequestHandler._target_servers_use_oauth2( path=request.url.path, mcp_servers=mcp_servers, - client_ip=IPAddressUtils.get_mcp_client_ip(request), + client_ip=client_ip, ): verbose_logger.debug( "MCP OAuth2: target server is OAuth2-mode, treating " "Authorization as upstream OAuth2 token passthrough" ) validated_user_api_key_auth = UserAPIKeyAuth() + elif is_auth_error: + # Pass-through cold-start return: per RFC 9728 / MCP + # Authorization spec the client completes upstream OAuth + # discovery and returns with ``Authorization: Bearer + # ``. For ``auth_type=none`` passthrough + # servers that bearer is not a LiteLLM key (auth above + # failed) but is meant to be forwarded upstream + # unchanged. Fall back to anonymous admission so the + # caller is not rejected for following the discovery + # flow without also setting ``x-litellm-api-key``. + mcp_servers_from_path = _parse_mcp_server_names_from_path( + request.url.path + ) + if ( + mcp_servers_from_path is not None + and _is_mcp_passthrough_cold_start( + mcp_servers_from_path, client_ip=client_ip + ) + ): + verbose_logger.debug( + "MCP pass-through return: target server is " + "passthrough, treating Authorization as " + "upstream OAuth token for delegated auth" + ) + validated_user_api_key_auth = UserAPIKeyAuth() + else: + raise else: raise else: diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index a93363be4bd..124a9d06439 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -1148,6 +1148,7 @@ if MCP_AVAILABLE: mcp_auth_header: Optional[str], oauth2_headers: Optional[Dict[str, str]], raw_headers: Optional[Dict[str, str]], + user_api_key_auth: Optional[UserAPIKeyAuth] = None, ) -> Tuple[Optional[Union[Dict[str, str], str]], Optional[Dict[str, str]]]: """Build auth and extra headers for a server.""" server_auth_header: Optional[Union[Dict[str, str], str]] = None @@ -1177,6 +1178,17 @@ if MCP_AVAILABLE: has_explicit_litellm_admission_header = ( normalized_raw_headers.get("x-litellm-api-key") is not None ) + # Admission consumed ``Authorization`` as a LiteLLM key only when + # auth produced a validated ``api_key`` AND the caller did not + # supply ``x-litellm-api-key``. When admission was anonymous + # (e.g. pass-through cold-start return per RFC 9728), the bearer + # in ``Authorization`` is the upstream OAuth token and must be + # forwarded — not stripped — for the delegated flow to work. + admission_consumed_authorization_as_litellm_key = ( + user_api_key_auth is not None + and bool(getattr(user_api_key_auth, "api_key", None)) + and not has_explicit_litellm_admission_header + ) for header in server.extra_headers: if not isinstance(header, str): @@ -1189,13 +1201,20 @@ if MCP_AVAILABLE: continue # Transparent OAuth pass-through: forward the caller's # Authorization header only when LiteLLM admission used - # a different header (`x-litellm-api-key`). Without an - # explicit admission header, `Authorization` may itself - # be the LiteLLM key — strip it to avoid leaking the - # gateway credential upstream. - if ( - server.is_oauth_passthrough - and not has_explicit_litellm_admission_header + # a different header (`x-litellm-api-key`) or when + # admission was anonymous (delegated to upstream). + # Without that signal `Authorization` may itself be the + # LiteLLM key — strip it to avoid leaking the gateway + # credential upstream. The legacy ``user_api_key_auth + # is None`` callers keep the conservative pre-PR + # behavior of stripping when no explicit admission + # header was supplied. + if server.is_oauth_passthrough and ( + admission_consumed_authorization_as_litellm_key + or ( + user_api_key_auth is None + and not has_explicit_litellm_admission_header + ) ): continue header_value = normalized_raw_headers.get(header.lower()) @@ -1391,6 +1410,7 @@ if MCP_AVAILABLE: mcp_auth_header=mcp_auth_header, oauth2_headers=oauth2_headers, raw_headers=raw_headers, + user_api_key_auth=user_api_key_auth, ) # Prefer server-stored per-user OAuth when configured, so a stale @@ -1565,6 +1585,7 @@ if MCP_AVAILABLE: mcp_auth_header=mcp_auth_header, oauth2_headers=oauth2_headers, raw_headers=raw_headers, + user_api_key_auth=user_api_key_auth, ) try: @@ -1622,6 +1643,7 @@ if MCP_AVAILABLE: mcp_auth_header=mcp_auth_header, oauth2_headers=oauth2_headers, raw_headers=raw_headers, + user_api_key_auth=user_api_key_auth, ) try: @@ -1677,6 +1699,7 @@ if MCP_AVAILABLE: mcp_auth_header=mcp_auth_header, oauth2_headers=oauth2_headers, raw_headers=raw_headers, + user_api_key_auth=user_api_key_auth, ) try: @@ -2512,6 +2535,7 @@ if MCP_AVAILABLE: mcp_auth_header=mcp_auth_header, oauth2_headers=oauth2_headers, raw_headers=raw_headers, + user_api_key_auth=user_api_key_auth, ) return await global_mcp_server_manager.get_prompt_from_server( @@ -2562,6 +2586,7 @@ if MCP_AVAILABLE: mcp_auth_header=mcp_auth_header, oauth2_headers=oauth2_headers, raw_headers=raw_headers, + user_api_key_auth=user_api_key_auth, ) return await global_mcp_server_manager.read_resource_from_server( diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py b/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py index ff1d2eea608..984052e9b16 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py @@ -1170,9 +1170,14 @@ class TestMCPOAuth2FallbackTargetGating: """ @staticmethod - def _make_server(auth_type): + def _make_server(auth_type, is_oauth_passthrough=False): server = MagicMock() server.auth_type = auth_type + # MagicMock would otherwise auto-create truthy stand-ins for any + # attribute access (including ``is_oauth_passthrough``), which + # would silently flip the passthrough fallback gate on. Pin the + # boolean explicitly so non-passthrough fixtures stay non-passthrough. + server.is_oauth_passthrough = is_oauth_passthrough return server async def test_fallback_blocked_when_target_is_not_oauth2(self): @@ -1274,6 +1279,48 @@ class TestMCPOAuth2FallbackTargetGating: (auth_result, *_rest) = await MCPRequestHandler.process_mcp_request(scope) assert isinstance(auth_result, UserAPIKeyAuth) + async def test_fallback_allowed_when_target_is_passthrough(self): + """ + Cold-start return per RFC 9728 / MCP Authorization spec: client + discovered the upstream IdP via the gateway's protected-resource + metadata, completed OAuth, and is returning with + ``Authorization: Bearer ``. The bearer is not a + LiteLLM key but the target is a pass-through server, so admission + falls back to anonymous and forwards the bearer upstream. + """ + from fastapi import HTTPException + + from litellm.types.mcp import MCPAuth + + scope = { + "type": "http", + "method": "POST", + "path": "/mcp/passthrough_server", + "headers": [(b"authorization", b"Bearer upstream-token-xyz")], + } + + async def mock_user_api_key_auth_fails(api_key, request): + raise HTTPException(status_code=401, detail="Invalid API key") + + with ( + patch( + "litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth", + side_effect=mock_user_api_key_auth_fails, + ), + patch( + "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager" + ) as mock_mgr, + ): + mock_mgr.get_mcp_server_by_name.return_value = ( + TestMCPOAuth2FallbackTargetGating._make_server( + auth_type=MCPAuth.none, + is_oauth_passthrough=True, + ) + ) + (auth_result, *_rest) = await MCPRequestHandler.process_mcp_request(scope) + assert isinstance(auth_result, UserAPIKeyAuth) + assert auth_result.api_key is None + async def test_fallback_blocked_when_client_ip_hides_oauth2_target(self): from fastapi import HTTPException @@ -1305,7 +1352,12 @@ class TestMCPOAuth2FallbackTargetGating: await MCPRequestHandler.process_mcp_request(scope) assert exc_info.value.status_code == 401 - mock_mgr.get_mcp_server_by_name.assert_called_once_with( + # Lookup may run twice — once for the oauth2-target fallback gate + # and once for the passthrough-target fallback gate. Both must + # resolve to ``None`` (hidden by client IP) so neither bypass + # opens. Use ``assert_any_call`` to assert the IP-scoped lookup + # happened without locking the count. + mock_mgr.get_mcp_server_by_name.assert_any_call( "hidden_oauth2_server", client_ip="203.0.113.10" ) 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 a07e0ead253..ed63b2ac07c 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 @@ -171,6 +171,87 @@ def test_prepare_mcp_server_headers_passthrough_strips_authorization_without_adm assert extra_headers == {"x-request-id": "req-789"} +def test_prepare_mcp_server_headers_passthrough_forwards_authorization_for_anonymous_admission(): + """Cold-start return per RFC 9728: client admits anonymously through + the pass-through fallback in :meth:`MCPRequestHandler.process_mcp_request` + (``user_api_key_auth.api_key is None``) and the ``Authorization`` bearer + is the upstream OAuth token — it must be forwarded, not stripped.""" + try: + from litellm.proxy._experimental.mcp_server.server import ( + _prepare_mcp_server_headers, + ) + except ImportError: + pytest.skip("MCP server not available") + + from litellm.proxy._types import UserAPIKeyAuth + + server = MCPServer( + server_id="server-passthrough-anon-admission", + name="server", + 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( + server=server, + mcp_server_auth_headers=None, + mcp_auth_header=None, + oauth2_headers=None, + raw_headers={ + "authorization": "Bearer upstream-oauth-token", + "x-request-id": "req-790", + }, + user_api_key_auth=UserAPIKeyAuth(), + ) + + assert server_auth_header is None + assert extra_headers == { + "Authorization": "Bearer upstream-oauth-token", + "x-request-id": "req-790", + } + + +def test_prepare_mcp_server_headers_passthrough_strips_authorization_for_authenticated_admission(): + """When admission validated ``Authorization`` as a LiteLLM key + (``user_api_key_auth.api_key`` is set, no explicit ``x-litellm-api-key``), + the bearer must still be stripped to avoid leaking the gateway key + upstream.""" + try: + from litellm.proxy._experimental.mcp_server.server import ( + _prepare_mcp_server_headers, + ) + except ImportError: + pytest.skip("MCP server not available") + + from litellm.proxy._types import UserAPIKeyAuth + + server = MCPServer( + server_id="server-passthrough-authenticated", + name="server", + 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( + server=server, + mcp_server_auth_headers=None, + mcp_auth_header=None, + oauth2_headers=None, + raw_headers={ + "authorization": "Bearer sk-litellm-key", + "x-request-id": "req-791", + }, + user_api_key_auth=UserAPIKeyAuth(api_key="sk-litellm-key"), + ) + + assert server_auth_header is None + assert extra_headers == {"x-request-id": "req-791"} + + def test_prepare_mcp_server_headers_oauth2_m2m_omits_litellm_caller_authorization(): """M2M OAuth must not put caller Bearer (LiteLLM API key) into extra_headers (#23652).""" try: @@ -547,6 +628,7 @@ async def test_mcp_get_prompt_success(): mcp_auth_header=None, oauth2_headers=None, raw_headers=None, + user_api_key_auth=user_api_key_auth, ) mock_manager.get_prompt_from_server.assert_awaited_once_with( server=server, @@ -608,6 +690,7 @@ async def test_mcp_read_resource_success(): mcp_auth_header=None, oauth2_headers=None, raw_headers=None, + user_api_key_auth=user_api_key_auth, ) mock_manager.read_resource_from_server.assert_awaited_once_with( server=server,