diff --git a/litellm/proxy/_experimental/mcp_server/oauth_utils.py b/litellm/proxy/_experimental/mcp_server/oauth_utils.py index a30b5ee9e49..1ca2ffc703d 100644 --- a/litellm/proxy/_experimental/mcp_server/oauth_utils.py +++ b/litellm/proxy/_experimental/mcp_server/oauth_utils.py @@ -180,6 +180,22 @@ def well_known_root_suffix() -> str: return "" if root == "/" else root +def get_route_relative_request_path(scope: Scope) -> str: + """The request path the MCP route shapes are written against: the raw ASGI path with the + deployment's ``root_path`` removed. + + ``scope["path"]`` and ``_original_path`` are both raw request-line paths, so on a sub-path + deployment they still carry the ``SERVER_ROOT_PATH`` prefix (``/litellm/{server}/mcp``) while + every route shape compared against them is root-relative. Mirrors the segment-boundary strip in + :func:`litellm.proxy.auth.auth_utils.get_request_route`, which the rest of the MCP auth path + already routes through, so ``/litellmfoo`` is not truncated under ``root_path=/litellm``.""" + raw_path = str(scope.get("_original_path") or scope.get("path", "") or "") + root_path = str(scope.get("app_root_path") or scope.get("root_path") or "").rstrip("/") + if root_path and (raw_path == root_path or raw_path.startswith(f"{root_path}/")): + return raw_path[len(root_path) :] + return raw_path + + def get_passthrough_resource_metadata_url(scope: Scope, server_name: str) -> str: """The per-server protected-resource metadata URL matching the spelling the request arrived on, so a strict RFC 9728 client resolves the same route the proxy registered. @@ -188,7 +204,7 @@ def get_passthrough_resource_metadata_url(scope: Scope, server_name: str) -> str the route decorators insert it (see :func:`well_known_root_suffix`).""" request: Final = Request(scope) base_url: Final = get_request_base_url(request) - _path: Final = scope.get("_original_path") or scope.get("path", "") or "" + _path: Final = get_route_relative_request_path(scope) if _path.startswith(f"/{server_name}/mcp"): return f"{base_url}/.well-known/oauth-protected-resource{well_known_root_suffix()}/{server_name}/mcp" diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index 0dc85c0318c..3c6eb06bc71 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -51,6 +51,8 @@ from litellm.proxy._experimental.mcp_server.mcp_debug import MCPDebug from litellm.proxy._experimental.mcp_server.oauth_utils import ( _redact_mcp_resource_url, get_passthrough_www_authenticate, + get_route_relative_request_path, + well_known_root_suffix, ) from litellm.proxy._experimental.mcp_server.utils import ( LITELLM_MCP_SERVER_DESCRIPTION, @@ -3782,14 +3784,15 @@ if MCP_AVAILABLE: request = StarletteRequest(scope) base_url = get_request_base_url(request) - _path = scope.get("_original_path") or scope.get("path", "") or "" + _path = get_route_relative_request_path(scope) # Pick the well-known AS-metadata form that matches the inbound route # so strict RFC 9728 ยง3.2 clients can resolve it correctly. + as_metadata_root = f"{base_url}/.well-known/oauth-authorization-server{well_known_root_suffix()}" if _path.startswith(f"/mcp/{server_name}"): - _as_url = f"{base_url}/.well-known/oauth-authorization-server/mcp/{server_name}" + _as_url = f"{as_metadata_root}/mcp/{server_name}" else: - _as_url = f"{base_url}/.well-known/oauth-authorization-server/{server_name}" + _as_url = f"{as_metadata_root}/{server_name}" authorization_uri = f'Bearer authorization_uri="{_as_url}"' raise HTTPException( 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 418059be835..3bd615a6a33 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 @@ -6314,6 +6314,49 @@ class TestAggregateGatewayDcrChallenge: www_authenticate = (exc_info.value.headers or {})["WWW-Authenticate"] assert www_authenticate == f'Bearer resource_metadata="http://testserver{expected_metadata_path}"' + async def test_per_server_challenge_keeps_spelling_under_server_root_path(self): + """On a sub-path deployment the challenge must still advertise the spelling the client + used. ``_original_path`` is a raw request-line path, so under SERVER_ROOT_PATH it reads + ``/litellm/{server}/mcp``; matching that against the root-relative ``/{server}/mcp`` shape + used to fail, silently pointing a legacy-spelling client at the standard-pattern document + whose ``resource`` is ``{base}/mcp/{server}`` rather than the ``{base}/{server}/mcp`` URL it + called, which a strict RFC 9728 section 3 client rejects.""" + import os + + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + server = MCPServer( + server_id="gh-id", + name="github", + server_name="github", + url="https://upstream.example/mcp", + transport="http", + auth_type=MCPAuth.oauth2, + ) + for original_path, expected_metadata_path in ( + ("/litellm/mcp/github", "/litellm/.well-known/oauth-protected-resource/litellm/mcp/github"), + ("/litellm/github/mcp", "/litellm/.well-known/oauth-protected-resource/litellm/github/mcp"), + ): + scope = { + **self._scope(path="/mcp/github"), + "root_path": "/litellm", + "_original_path": original_path, + } + with ( + patch.dict(os.environ, {"SERVER_ROOT_PATH": "/litellm"}), + patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), + 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 = server + with pytest.raises(HTTPException) as exc_info: + await MCPRequestHandler.process_mcp_request(scope) + assert exc_info.value.status_code == 401 + www_authenticate = (exc_info.value.headers or {})["WWW-Authenticate"] + assert www_authenticate == f'Bearer resource_metadata="http://testserver{expected_metadata_path}"' + async def test_no_per_server_challenge_for_non_gateway_managed_targets(self): """The per-server challenge fires only for the server set the gateway's keyless flow serves: an OBO server and a multi-server CSV path keep the original admission error 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 aa6e63b7ca0..82f74cda835 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 @@ -1,5 +1,6 @@ import asyncio import contextvars +import os from datetime import datetime, timedelta from unittest.mock import AsyncMock, MagicMock, patch @@ -8094,6 +8095,54 @@ class TestPreemptive401ModeAware: assert exc.value.status_code == 401 assert "www-authenticate" in {k.lower() for k in exc.value.headers} + @pytest.mark.asyncio + @pytest.mark.parametrize( + "original_path, expected_as_path", + ( + ("/litellm/mcp/interactive", "/litellm/.well-known/oauth-authorization-server/litellm/mcp/interactive"), + ("/litellm/interactive/mcp", "/litellm/.well-known/oauth-authorization-server/litellm/interactive"), + ), + ) + async def test_gateway_as_metadata_challenge_under_server_root_path(self, original_path, expected_as_path): + """Under SERVER_ROOT_PATH the challenge must keep the spelling the client called and point at + a route the proxy registered, so it has to compare a route-relative path and carry the root suffix.""" + from litellm.proxy._experimental.mcp_server import server as server_module + + server = _make_oauth2_server("interactive", oauth2_flow="authorization_code") + scope = { + **self._scope(server.alias), + "root_path": "/litellm", + "_original_path": original_path, + "headers": [(b"host", b"testserver")], + } + with ( + patch.dict(os.environ, {"SERVER_ROOT_PATH": "/litellm"}), + patch.object( + server_module.global_mcp_server_manager, + "get_mcp_server_by_name", + return_value=server, + ), + patch.object( + server_module.global_mcp_server_manager, + "has_user_oauth_token", + new_callable=AsyncMock, + return_value=False, + ), + pytest.raises(HTTPException) as exc, + ): + await server_module._raise_preemptive_401_for_unauthenticated_servers( + scope=scope, + mcp_servers=[server.alias], + oauth2_headers=None, + mcp_server_auth_headers=None, + user_api_key_auth=UserAPIKeyAuth(api_key="sk-litellm-virtual-key"), + client_ip=None, + ) + + assert exc.value.status_code == 401 + headers = {k.lower(): v for k, v in (exc.value.headers or {}).items()} + assert headers["www-authenticate"] == f'Bearer authorization_uri="http://testserver{expected_as_path}"' + @pytest.mark.asyncio async def test_gateway_managed_interactive_no_token_challenges_with_authorization_bearer(self): """The bug fix: no stored token, key in Authorization (oauth2_headers