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 815be2229fc..f1fcc95c532 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 @@ -12,6 +12,7 @@ import litellm from litellm._logging import verbose_logger from litellm.proxy._experimental.mcp_server.oauth_utils import ( get_request_base_url, + well_known_root_suffix, ) from litellm.proxy._experimental.mcp_server.outbound_credentials.bridge_credentials import ( BridgeEnvelopeAdmitted, @@ -157,7 +158,9 @@ def _aggregate_gateway_dcr_challenge(request: Request, invalid_token: bool) -> H spec-compliant clients to re-authorize rather than retry; a request with no credentials at all gets the bare challenge per RFC 6750 section 3.1.""" error_attr = 'error="invalid_token", ' if invalid_token else "" - resource_metadata_url = f"{get_request_base_url(request)}/.well-known/oauth-protected-resource/mcp" + resource_metadata_url = ( + f"{get_request_base_url(request)}/.well-known/oauth-protected-resource{well_known_root_suffix()}/mcp" + ) return HTTPException( status_code=401, detail={ diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 08827accf3c..9ea452b9aa8 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -43,6 +43,7 @@ from litellm.proxy._experimental.mcp_server.oauth_utils import ( TOKEN_NO_CACHE_HEADERS, get_request_base_url, validate_trusted_redirect_uri, + well_known_root_suffix, ) from litellm.proxy.auth.ip_address_utils import IPAddressUtils from litellm.proxy.common_utils.encrypt_decrypt_utils import ( @@ -50,7 +51,6 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import ( encrypt_value_helper, ) from litellm.proxy.common_utils.http_parsing_utils import _read_request_body -from litellm.proxy.utils import get_server_root_path from litellm.types.mcp import MCPAuth, MCPCredentials from litellm.types.mcp_server.mcp_server_manager import MCPServer @@ -1882,31 +1882,13 @@ def _build_aggregate_authorization_server_response(request: Request) -> dict: } -def _mcp_named_server_exists(request: Request) -> bool: - """True when a server literally named ``mcp`` is configured and visible to this caller. - - Its per-server authorization-server document is served at - ``/.well-known/oauth-authorization-server/mcp``, a single segment that collides with the - aggregate path. When such a server exists the real server wins the route, so that - deployment keeps its per-server discovery regardless of whether the aggregate front door - is on.""" - from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( # noqa: PLC0415 # circular import with mcp_server_manager at module load - global_mcp_server_manager, - ) - - client_ip = IPAddressUtils.get_mcp_client_ip(request) - return global_mcp_server_manager.get_mcp_server_by_name("mcp", client_ip=client_ip) is not None - - # RFC 9728 path-appended discovery for the aggregate /mcp endpoint. A client # pointed at {base}/mcp inserts the well-known segment before the resource # path, so this exact route must exist for aggregate discovery to work at all. # Declared before the parameterized well-known routes below: Starlette matches # in registration order, and /.well-known/oauth-authorization-server/{name} # would otherwise capture the "/mcp" suffix as a server name. -@router.get( - f"/.well-known/oauth-protected-resource{'' if get_server_root_path() == '/' else get_server_root_path()}/mcp" -) +@router.get(f"/.well-known/oauth-protected-resource{well_known_root_suffix()}/mcp") async def oauth_protected_resource_aggregate(request: Request): """ OAuth protected resource discovery for the aggregate /mcp endpoint. @@ -1918,28 +1900,26 @@ async def oauth_protected_resource_aggregate(request: Request): return _build_aggregate_protected_resource_response(request) -@router.get( - f"/.well-known/oauth-authorization-server{'' if get_server_root_path() == '/' else get_server_root_path()}/mcp" -) +@router.get(f"/.well-known/oauth-authorization-server{well_known_root_suffix()}/mcp") async def oauth_authorization_server_aggregate(request: Request): """ OAuth authorization server discovery for the aggregate /mcp endpoint, the RFC 8414 path-inserted form for a client that treats {base}/mcp as its authorization base URL. - This single-segment path collides with the parameterized ``/{mcp_server_name}`` route - below, so a server literally named ``mcp`` wins it and keeps its per-server discovery; - only when no such server exists is the aggregate document served. + The single-segment /mcp is reserved for the aggregate so the discovery chain stays + consistent: the aggregate protected-resource document advertises {base}/mcp as its + authorization server, so the document served here must have issuer {base}/mcp. A server + literally named ``mcp`` therefore does not take this route; it keeps its standard + two-segment discovery at /.well-known/oauth-authorization-server/mcp/mcp. Letting the + per-server row win here instead would serve an issuer of {base} against a resource that + advertised {base}/mcp, which fails the RFC 8414 issuer check and breaks the front door. """ - if _mcp_named_server_exists(request): - return _build_oauth_authorization_server_response(request=request, mcp_server_name="mcp") return _build_aggregate_authorization_server_response(request) # Standard MCP pattern: /.well-known/oauth-protected-resource/mcp/{server_name} # This is the pattern expected by standard MCP clients (mcp-inspector, VSCode Copilot) -@router.get( - f"/.well-known/oauth-protected-resource{'' if get_server_root_path() == '/' else get_server_root_path()}/mcp/{{mcp_server_name}}" -) +@router.get(f"/.well-known/oauth-protected-resource{well_known_root_suffix()}/mcp/{{mcp_server_name}}") async def oauth_protected_resource_mcp_standard(request: Request, mcp_server_name: str): """ OAuth protected resource discovery endpoint using standard MCP URL pattern. @@ -1959,9 +1939,7 @@ async def oauth_protected_resource_mcp_standard(request: Request, mcp_server_nam # LiteLLM legacy pattern: /.well-known/oauth-protected-resource/{server_name}/mcp # Kept for backward compatibility with existing deployments -@router.get( - f"/.well-known/oauth-protected-resource{'' if get_server_root_path() == '/' else get_server_root_path()}/{{mcp_server_name}}/mcp" -) +@router.get(f"/.well-known/oauth-protected-resource{well_known_root_suffix()}/{{mcp_server_name}}/mcp") @router.get("/.well-known/oauth-protected-resource") async def oauth_protected_resource_mcp(request: Request, mcp_server_name: Optional[str] = None): """ @@ -2031,9 +2009,7 @@ def _build_oauth_authorization_server_response( # Standard MCP pattern: /.well-known/oauth-authorization-server/mcp/{server_name} -@router.get( - f"/.well-known/oauth-authorization-server{'' if get_server_root_path() == '/' else get_server_root_path()}/mcp/{{mcp_server_name}}" -) +@router.get(f"/.well-known/oauth-authorization-server{well_known_root_suffix()}/mcp/{{mcp_server_name}}") async def oauth_authorization_server_mcp_standard(request: Request, mcp_server_name: str): """ OAuth authorization server discovery endpoint using standard MCP URL pattern. @@ -2048,9 +2024,7 @@ async def oauth_authorization_server_mcp_standard(request: Request, mcp_server_n # LiteLLM legacy pattern and root endpoint -@router.get( - f"/.well-known/oauth-authorization-server{'' if get_server_root_path() == '/' else get_server_root_path()}/{{mcp_server_name}}" -) +@router.get(f"/.well-known/oauth-authorization-server{well_known_root_suffix()}/{{mcp_server_name}}") @router.get("/.well-known/oauth-authorization-server") async def oauth_authorization_server_mcp(request: Request, mcp_server_name: Optional[str] = None): """ diff --git a/litellm/proxy/_experimental/mcp_server/oauth_utils.py b/litellm/proxy/_experimental/mcp_server/oauth_utils.py index 6edb22dd858..ccee3fc8ac0 100644 --- a/litellm/proxy/_experimental/mcp_server/oauth_utils.py +++ b/litellm/proxy/_experimental/mcp_server/oauth_utils.py @@ -132,6 +132,18 @@ def get_request_base_url(request: Request) -> str: return urlunparse((scheme, _strip_default_port(scheme, netloc), parsed.path, "", "", "")) +def well_known_root_suffix() -> str: + """The ``SERVER_ROOT_PATH`` segment inserted into a ``.well-known`` path (RFC 8414 / 9728 + path insertion), empty for a root-mounted proxy or an explicit ``/``. + + The discovery route registrations and the 401 challenges that advertise those routes both + derive their path from this one function, so the ``resource_metadata`` URL a client is told + to fetch cannot drift from the route that actually serves it. + """ + root = os.getenv("SERVER_ROOT_PATH", "") + return "" if root == "/" else root + + def validate_loopback_redirect_uri(redirect_uri: str) -> None: """Require a loopback ``redirect_uri`` (OAuth 2.1 §4.1.2.1 + RFC 8252 §7.3 native-app pattern). MCP clients are native apps that listen on diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index a8ab0937124..a9840bdc02f 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -48,6 +48,7 @@ from litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp import ( ) from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( get_request_base_url, + well_known_root_suffix, ) from litellm.proxy._experimental.mcp_server.exceptions import ( MCPToolResultError, @@ -3525,9 +3526,10 @@ if MCP_AVAILABLE: base_url = get_request_base_url(request) _path = scope.get("_original_path") or scope.get("path", "") or "" + suffix = well_known_root_suffix() if _path.startswith(f"/{server_name}/mcp"): - return f"{base_url}/.well-known/oauth-protected-resource/{server_name}/mcp" - return f"{base_url}/.well-known/oauth-protected-resource/mcp/{server_name}" + return f"{base_url}/.well-known/oauth-protected-resource{suffix}/{server_name}/mcp" + return f"{base_url}/.well-known/oauth-protected-resource{suffix}/mcp/{server_name}" def _get_passthrough_www_authenticate( scope: Scope, 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 568081e0673..7b05b8c9dd0 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 @@ -6189,6 +6189,23 @@ class TestAggregateGatewayDcrChallenge: www_authenticate = (exc_info.value.headers or {})["WWW-Authenticate"] assert www_authenticate == f'Bearer error="invalid_token", {self._EXPECTED_RESOURCE_METADATA}' + async def test_challenge_inserts_server_root_path(self): + """With SERVER_ROOT_PATH set the resource_metadata URL must carry the same path-inserted + root segment the aggregate PRM route is registered with (both derive it from + well_known_root_suffix), so a DCR client behind a sub-path is pointed at a route that + exists instead of a 404. Regression: the challenge used to hard-code /mcp and omit the + root path the route inserts.""" + import os + + with ( + patch.dict(os.environ, {"SERVER_ROOT_PATH": "/litellm"}), + patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), + ): + with pytest.raises(HTTPException) as exc_info: + await MCPRequestHandler.process_mcp_request(self._scope()) + www_authenticate = (exc_info.value.headers or {})["WWW-Authenticate"] + assert 'resource_metadata="http://testserver/.well-known/oauth-protected-resource/litellm/mcp"' in www_authenticate + async def test_no_challenge_for_explicit_litellm_key(self): """An explicit x-litellm-api-key declares a litellm-key client; a typo there must surface the real auth error, never a DCR challenge that diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index af4b2caca72..7e9ff4692b5 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -7163,11 +7163,13 @@ def test_aggregate_wellknown_routes_serve_gateway_metadata(): assert "none" in asm.json()["token_endpoint_auth_methods_supported"] -def test_as_aggregate_route_prefers_a_real_server_named_mcp(): - """A server literally named ``mcp`` wins the single-segment - /.well-known/oauth-authorization-server/mcp route (it collides with the parameterized - /{server_name} route) and keeps its per-server discovery; the aggregate document is - served only when no such server exists.""" +def test_as_aggregate_route_reserves_mcp_for_the_aggregate(): + """The single-segment /.well-known/oauth-authorization-server/mcp is reserved for the + aggregate even when a server is literally named ``mcp``. The aggregate protected-resource + document advertises {base}/mcp as its authorization server, so the document served here + must carry issuer {base}/mcp for the RFC 8414 issuer check to pass. Letting the per-server + row win (issuer {base}) breaks that chain, so the aggregate wins and the mcp-named server + keeps its standard two-segment discovery at /.well-known/oauth-authorization-server/mcp/mcp.""" from fastapi import FastAPI from fastapi.testclient import TestClient @@ -7186,14 +7188,39 @@ def test_as_aggregate_route_prefers_a_real_server_named_mcp(): try: asm = client.get("/.well-known/oauth-authorization-server/mcp") assert asm.status_code == 200 - # the real server's own document (issuer is the bare origin, endpoint is /mcp/authorize), - # not the aggregate one (whose issuer would be {base}/mcp) - assert asm.json()["issuer"] == "http://testserver" - assert "/mcp/authorize" in asm.json()["authorization_endpoint"] + # the aggregate document, whose issuer matches what the aggregate PRM advertises + assert asm.json()["issuer"] == "http://testserver/mcp" + + prm = client.get("/.well-known/oauth-protected-resource/mcp") + assert prm.status_code == 200 + assert prm.json()["authorization_servers"] == [asm.json()["issuer"]] + + # the mcp-named server keeps its own document on the standard two-segment route + per_server = client.get("/.well-known/oauth-authorization-server/mcp/mcp") + assert per_server.status_code == 200 + assert "/mcp/authorize" in per_server.json()["authorization_endpoint"] finally: global_mcp_server_manager.registry.clear() +def test_well_known_root_suffix_reflects_server_root_path(): + """The single path segment both the discovery routes and the 401 challenges insert for RFC + 8414/9728 path insertion: empty for a root-mounted proxy or an explicit ``/``, the configured + path otherwise. Sharing this one function is what keeps the advertised resource_metadata URL + equal to the route that serves it.""" + import os + from unittest.mock import patch + + from litellm.proxy._experimental.mcp_server.oauth_utils import well_known_root_suffix + + with patch.dict(os.environ, {"SERVER_ROOT_PATH": ""}): + assert well_known_root_suffix() == "" + with patch.dict(os.environ, {"SERVER_ROOT_PATH": "/"}): + assert well_known_root_suffix() == "" + with patch.dict(os.environ, {"SERVER_ROOT_PATH": "/litellm"}): + assert well_known_root_suffix() == "/litellm" + + @pytest.mark.asyncio async def test_bare_origin_discovery_resolves_single_server_not_aggregate(): """The always-on aggregate front door must not change bare-origin discovery: with one