Repository navigation
fix(mcp): surface upstream 401 for token-forwarding MCP servers #27847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
7165d6f
2982355
25ec4b1
5b13913
d25cb57
b211144
d4d6155
73e41f6
ade8ca1
6e3d676
1925335
e88c5ab
851d9f6
560c22b
c07b62a
28eda2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,13 +51,17 @@ | |
| get_server_prefix, | ||
| iter_known_server_prefixes, | ||
| ) | ||
| from litellm.llms.custom_httpx.http_handler import ( | ||
| get_async_httpx_client, | ||
| httpxSpecialProvider, | ||
| ) | ||
| from litellm.proxy._types import UserAPIKeyAuth | ||
| from litellm.proxy.auth.ip_address_utils import IPAddressUtils | ||
| from litellm.proxy.litellm_pre_call_utils import ( | ||
| LiteLLMProxyRequestSetup, | ||
| get_chain_id_from_headers, | ||
| ) | ||
| from litellm.types.mcp import MCPAuth | ||
| from litellm.types.mcp import MCPAuth, MCPSpecVersion | ||
| from litellm.types.mcp_server.mcp_server_manager import MCPInfo, MCPServer | ||
| from litellm.types.utils import CallTypes, StandardLoggingMCPToolCall | ||
| from litellm.utils import Rules, client, function_setup | ||
|
|
@@ -2754,6 +2758,115 @@ async def _apply_toolset_scope( | |
| ) | ||
| return user_api_key_auth.model_copy(update={"object_permission": updated_op}) | ||
|
|
||
| def _get_forwarded_auth_from_scope(scope: Scope) -> Optional[str]: | ||
| """Return the raw Authorization header value from the ASGI scope, or None.""" | ||
| for key, value in scope.get("headers", []): | ||
| if key.lower() == b"authorization": | ||
| return value.decode("latin-1") | ||
| return None | ||
|
|
||
| async def _probe_upstream_auth( | ||
| url: str, | ||
| auth_header: str, | ||
| timeout: float = 5.0, | ||
| ) -> tuple: | ||
| """JSON-RPC initialize-probe the upstream URL to check whether the token is accepted. | ||
|
|
||
| Uses POST so StreamableHTTP MCP servers run the same auth path as a | ||
| real client request. Returns (status_code, www_authenticate). | ||
| Fails-open with (200, None) on network errors so a transient hiccup | ||
| does not block valid requests. | ||
| """ | ||
| try: | ||
| client = get_async_httpx_client( | ||
| llm_provider=httpxSpecialProvider.MCP, | ||
| params={"timeout": timeout}, | ||
| ) | ||
| probe_payload = { | ||
| "jsonrpc": "2.0", | ||
| "id": "litellm-mcp-auth-probe", | ||
| "method": "initialize", | ||
| "params": { | ||
| "protocolVersion": MCPSpecVersion.jun_2025.value, | ||
| "capabilities": {}, | ||
| "clientInfo": { | ||
| "name": "litellm-mcp-auth-probe", | ||
| "version": "1.0.0", | ||
| }, | ||
| }, | ||
| } | ||
| resp = await client.client.post( # type: ignore[attr-defined] | ||
| url, | ||
| headers={ | ||
| "Authorization": auth_header, | ||
| "Accept": "application/json, text/event-stream", | ||
| }, | ||
| json=probe_payload, | ||
| ) | ||
|
cursor[bot] marked this conversation as resolved.
cursor[bot] marked this conversation as resolved.
|
||
| return resp.status_code, resp.headers.get("www-authenticate") | ||
| except Exception as exc: | ||
| verbose_logger.debug( | ||
| f"_probe_upstream_auth: probe to {url} failed ({exc}), allowing request through" | ||
| ) | ||
| return 200, None | ||
|
Sameerlite marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The correct fix is to use the public
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 560c22b — the probe now uses the public |
||
|
|
||
| async def _check_passthrough_upstream_auth( | ||
| scope: Scope, | ||
| user_api_key_auth: Optional[UserAPIKeyAuth], | ||
| mcp_servers: Optional[List[str]], | ||
| client_ip: Optional[str], | ||
| ) -> None: | ||
| """Probe pass-through upstream servers in parallel before the MCP session starts. | ||
|
|
||
| Only servers the caller's key is already authorized to reach are probed — | ||
| the list is derived from _get_allowed_mcp_servers so that a user cannot | ||
| trigger an upstream probe against a server their key is not permitted for. | ||
|
|
||
| The MCP SDK commits HTTP 200 headers before invoking handlers, so a 401 | ||
| can only be returned before that point. This function raises HTTPException(401) | ||
| with a WWW-Authenticate header if any upstream rejects the client token. | ||
| Fails-open: network errors are logged and the request is allowed through. | ||
| """ | ||
| forwarded_auth = _get_forwarded_auth_from_scope(scope) | ||
| if not forwarded_auth: | ||
| return | ||
|
|
||
| # Use the authorized server set, not the raw user-supplied names, so that | ||
| # a caller cannot force a probe to a server their key is not allowed to use. | ||
| allowed_servers = await _get_allowed_mcp_servers( | ||
| user_api_key_auth=user_api_key_auth, | ||
| mcp_servers=mcp_servers, | ||
| client_ip=client_ip, | ||
| ) | ||
| passthrough_servers = [ | ||
| srv | ||
| for srv in allowed_servers | ||
| if srv.extra_headers | ||
|
veria-ai[bot] marked this conversation as resolved.
|
||
| and any(h.lower() == "authorization" for h in srv.extra_headers) | ||
|
veria-ai[bot] marked this conversation as resolved.
|
||
| ] | ||
| if not passthrough_servers: | ||
| return | ||
|
|
||
| probe_results = await asyncio.gather( | ||
| *[ | ||
| _probe_upstream_auth(srv.url or "", forwarded_auth) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High: Proxy API key forwarded upstream
|
||
| for srv in passthrough_servers | ||
| ] | ||
| ) | ||
| request = StarletteRequest(scope) | ||
| base_url = get_request_base_url(request) | ||
| for srv, (probe_status, _) in zip(passthrough_servers, probe_results): | ||
| if probe_status in (401, 403): | ||
| authorization_uri = ( | ||
| f"Bearer authorization_uri=" | ||
| f"{base_url}/.well-known/oauth-authorization-server/{srv.name}" | ||
| ) | ||
| raise HTTPException( | ||
| status_code=401, | ||
| detail="Unauthorized", | ||
| headers={"WWW-Authenticate": authorization_uri}, | ||
| ) | ||
|
Sameerlite marked this conversation as resolved.
Outdated
|
||
|
|
||
| async def handle_streamable_http_mcp( | ||
| scope: Scope, receive: Receive, send: Send | ||
| ) -> None: | ||
|
|
@@ -2827,6 +2940,13 @@ async def handle_streamable_http_mcp( | |
| user_api_key_auth, active_toolset_id | ||
| ) | ||
|
|
||
| # Pre-flight auth check for pass-through servers. Must run after | ||
| # toolset scoping so the probe list is derived from the fully-authorized | ||
| # server set, not the raw user-supplied names. | ||
| await _check_passthrough_upstream_auth( | ||
| scope, user_api_key_auth, mcp_servers, _client_ip | ||
| ) | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| # Inject masked debug headers when client sends x-litellm-mcp-debug: true | ||
| _debug_headers = MCPDebug.maybe_build_debug_headers( | ||
| raw_headers=raw_headers, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3256,3 +3256,79 @@ async def capture_create_mcp_client(*args, **kwargs): | |
| ), "P2 API consistency issue: expected None for empty extra_headers, got: " + str( | ||
| captured_extra_headers | ||
| ) | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Pre-flight upstream auth check tests | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_probe_upstream_auth_returns_upstream_status(): | ||
| """_probe_upstream_auth forwards the status code from the upstream server.""" | ||
| from litellm.proxy._experimental.mcp_server.server import _probe_upstream_auth | ||
|
|
||
| mock_response = MagicMock() | ||
| mock_response.status_code = 401 | ||
| mock_response.headers = {"www-authenticate": 'Bearer realm="test"'} | ||
|
|
||
| mock_client = AsyncMock() | ||
| mock_client.client.post = AsyncMock(return_value=mock_response) | ||
|
|
||
| with patch( | ||
| "litellm.proxy._experimental.mcp_server.server.get_async_httpx_client", | ||
| return_value=mock_client, | ||
| ): | ||
| status, www_auth = await _probe_upstream_auth( | ||
| "http://upstream/mcp", "Bearer some-token" | ||
| ) | ||
|
|
||
| assert status == 401 | ||
| assert www_auth == 'Bearer realm="test"' | ||
| mock_client.client.post.assert_awaited_once() | ||
| _, kwargs = mock_client.client.post.call_args | ||
| assert kwargs["headers"]["Authorization"] == "Bearer some-token" | ||
| assert kwargs["json"]["method"] == "initialize" | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_probe_upstream_auth_fails_open_on_network_error(): | ||
| """_probe_upstream_auth returns (200, None) when the network call fails.""" | ||
| from litellm.proxy._experimental.mcp_server.server import _probe_upstream_auth | ||
|
|
||
| mock_client = AsyncMock() | ||
| mock_client.client.post = AsyncMock(side_effect=Exception("connection refused")) | ||
|
|
||
| with patch( | ||
| "litellm.proxy._experimental.mcp_server.server.get_async_httpx_client", | ||
| return_value=mock_client, | ||
| ): | ||
| status, www_auth = await _probe_upstream_auth( | ||
| "http://upstream/mcp", "Bearer some-token" | ||
| ) | ||
|
|
||
| assert status == 200 | ||
| assert www_auth is None | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Both probe tests set up For For To correctly test the 401 case,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
|
|
||
| def test_get_forwarded_auth_from_scope_extracts_header(): | ||
| """_get_forwarded_auth_from_scope returns the Authorization value.""" | ||
| from litellm.proxy._experimental.mcp_server.server import ( | ||
| _get_forwarded_auth_from_scope, | ||
| ) | ||
|
|
||
| scope = { | ||
| "headers": [ | ||
| (b"content-type", b"application/json"), | ||
| (b"authorization", b"Bearer my-token"), | ||
| ] | ||
| } | ||
| assert _get_forwarded_auth_from_scope(scope) == "Bearer my-token" | ||
|
|
||
|
|
||
| def test_get_forwarded_auth_from_scope_returns_none_when_missing(): | ||
| from litellm.proxy._experimental.mcp_server.server import ( | ||
| _get_forwarded_auth_from_scope, | ||
| ) | ||
|
|
||
| assert _get_forwarded_auth_from_scope({"headers": []}) is None | ||
Uh oh!
There was an error while loading. Please reload this page.