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 76589ff0734..bf88c0be5da 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,7 +12,6 @@ import litellm from litellm._logging import verbose_logger from litellm.proxy._experimental.mcp_server.oauth_utils import ( get_request_base_url, - is_mcp_gateway_dcr_enabled, ) from litellm.proxy._experimental.mcp_server.outbound_credentials.bridge_credentials import ( BridgeEnvelopeAdmitted, @@ -133,14 +132,12 @@ def _is_aggregate_gateway_dcr_challenge_scope( ) -> bool: """True when an unauthenticated request to the aggregate ``/mcp`` endpoint should receive the RFC 9728 401 challenge that advertises the gateway as - the authorization server (``mcp_gateway_dcr`` front door). + the authorization server. Fires only for a genuine 401 on the aggregate scope: any named target (path or ``x-mcp-servers``) belongs to the per-server challenge paths, and client-supplied MCP auth headers mean the caller is not a cold-start DCR client. Fails closed to the original admission error otherwise.""" - if not is_mcp_gateway_dcr_enabled(): - return False if not _is_litellm_auth_admission_error(exc): return False if mcp_servers: diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 0ad9a5e3ea2..39b84bdbd4e 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -42,7 +42,6 @@ from litellm.proxy._experimental.mcp_server.faults import ( from litellm.proxy._experimental.mcp_server.oauth_utils import ( TOKEN_NO_CACHE_HEADERS, get_request_base_url, - is_mcp_gateway_dcr_enabled, validate_trusted_redirect_uri, ) from litellm.proxy.auth.ip_address_utils import IPAddressUtils @@ -1641,12 +1640,6 @@ async def _build_oauth_protected_resource_response( global_mcp_server_manager, ) - # With the gateway-level DCR front door enabled, unnamed discovery - # describes the gateway itself as the authorization server for the - # aggregate /mcp resource instead of narrowing to one server. - if mcp_server_name is None and is_mcp_gateway_dcr_enabled(): - return _build_aggregate_protected_resource_response(request) - request_base_url = get_request_base_url(request) client_ip = IPAddressUtils.get_mcp_client_ip(request) @@ -1821,13 +1814,20 @@ def _build_aggregate_authorization_server_response(request: Request) -> dict: } -def _raise_404_unless_gateway_dcr_enabled() -> None: - """The aggregate well-known routes exist only under the gateway-level DCR - front door; flag-off they 404 exactly like the previously-absent routes so - discovery behavior is byte-identical for existing deployments.""" - if is_mcp_gateway_dcr_enabled(): - return - raise HTTPException(status_code=404, detail="Not Found") +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 @@ -1841,10 +1841,12 @@ def _raise_404_unless_gateway_dcr_enabled() -> None: ) async def oauth_protected_resource_aggregate(request: Request): """ - OAuth protected resource discovery for the aggregate /mcp endpoint - (gateway-level DCR front door; 404 when the flag is off). + OAuth protected resource discovery for the aggregate /mcp endpoint. + + The single-segment ``/mcp`` path does not collide with any per-server PRM pattern + (those are two-segment: ``/mcp/{server}`` or ``/{server}/mcp``), so this unambiguously + describes the aggregate resource. """ - _raise_404_unless_gateway_dcr_enabled() return _build_aggregate_protected_resource_response(request) @@ -1853,13 +1855,15 @@ async def oauth_protected_resource_aggregate(request: Request): ) 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 (gateway-level DCR front door; 404 when the flag - is off, indistinguishable from an unknown server name on the - parameterized route below). + 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. """ - _raise_404_unless_gateway_dcr_enabled() + 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) @@ -1922,11 +1926,6 @@ def _build_oauth_authorization_server_response( global_mcp_server_manager, ) - # With the gateway-level DCR front door enabled, unnamed discovery keeps - # advertising the gateway's own /authorize, /token, and /register. - if mcp_server_name is None and is_mcp_gateway_dcr_enabled(): - return _build_aggregate_authorization_server_response(request) - request_base_url = get_request_base_url(request) client_ip = IPAddressUtils.get_mcp_client_ip(request) diff --git a/litellm/proxy/_experimental/mcp_server/oauth_utils.py b/litellm/proxy/_experimental/mcp_server/oauth_utils.py index 74b56cf424b..6edb22dd858 100644 --- a/litellm/proxy/_experimental/mcp_server/oauth_utils.py +++ b/litellm/proxy/_experimental/mcp_server/oauth_utils.py @@ -70,29 +70,6 @@ def _origin_label(scheme: str, netloc: str) -> str: return f"{scheme}://{netloc}" if netloc else f"{scheme}://" -MCP_GATEWAY_DCR_SETTING = "mcp_gateway_dcr" - - -def is_mcp_gateway_dcr_enabled() -> bool: - """True when ``general_settings.mcp_gateway_dcr`` opts this deployment into - the gateway-level DCR front door for the aggregate ``/mcp`` endpoint: root - OAuth discovery advertises the gateway itself as the authorization server - (instead of resolving the single configured oauth2 server), and the - anonymous aggregate 401 carries the RFC 9728 ``resource_metadata`` - challenge so DCR clients (Claude Desktop, MCP Inspector) can start the - sign-in flow. Off by default; flag-off behavior is unchanged.""" - from litellm.proxy.proxy_server import general_settings # noqa: PLC0415 # circular import at module load - - if not isinstance(general_settings, dict): - return False - raw = general_settings.get(MCP_GATEWAY_DCR_SETTING) - if isinstance(raw, bool): - return raw - if isinstance(raw, str): - return raw.strip().lower() == "true" - return False - - def _resolve_proxy_base_url_env() -> Optional[str]: global _warned_invalid_proxy_base_url configured = os.environ.get("PROXY_BASE_URL", "").strip() 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 7625eec2f5c..2a28ff61271 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 @@ -5957,9 +5957,8 @@ class TestAggregateGatewayDcrChallenge: """The mcp_gateway_dcr front door: a 401 on the aggregate /mcp scope must carry the RFC 9728 resource_metadata challenge pointing at the gateway's own protected-resource metadata, and must NOT fire for named-server - targets, explicit litellm keys, non-401 failures, or with the flag off.""" + targets, explicit litellm keys, or non-401 failures.""" - _FLAG_PATCH_TARGET = "litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.is_mcp_gateway_dcr_enabled" _AUTH_PATCH_TARGET = "litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth" _EXPECTED_RESOURCE_METADATA = 'resource_metadata="http://testserver/.well-known/oauth-protected-resource/mcp"' @@ -5983,11 +5982,10 @@ class TestAggregateGatewayDcrChallenge: return _raise async def test_challenge_on_anonymous_aggregate_mcp(self): - """Anonymous request to the aggregate /mcp with the flag on: 401 plus + """Anonymous request to the aggregate /mcp: 401 plus the bare bearer challenge (no error attribute, RFC 6750 section 3.1).""" with ( patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), - patch(self._FLAG_PATCH_TARGET, return_value=True), ): with pytest.raises(HTTPException) as exc_info: await MCPRequestHandler.process_mcp_request(self._scope()) @@ -6001,7 +5999,6 @@ class TestAggregateGatewayDcrChallenge: so a spec client re-authorizes instead of retrying the dead token.""" with ( patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), - patch(self._FLAG_PATCH_TARGET, return_value=True), ): with pytest.raises(HTTPException) as exc_info: await MCPRequestHandler.process_mcp_request( @@ -6011,25 +6008,12 @@ 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_no_challenge_when_flag_off(self): - """Flag off: the original admission error propagates untouched, both - with and without a bearer.""" - for extra_headers in ((), ((b"authorization", b"Bearer some-token"),)): - with ( - patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), - patch(self._FLAG_PATCH_TARGET, return_value=False), - ): - with pytest.raises(ProxyException) as exc_info: - await MCPRequestHandler.process_mcp_request(self._scope(extra_headers=extra_headers)) - assert str(exc_info.value.code) == "401" - 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 would send SDKs into a sign-in flow.""" with ( patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), - patch(self._FLAG_PATCH_TARGET, return_value=True), ): with pytest.raises(ProxyException): await MCPRequestHandler.process_mcp_request( @@ -6041,7 +6025,6 @@ class TestAggregateGatewayDcrChallenge: own those, so the aggregate challenge must not fire.""" with ( patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), - patch(self._FLAG_PATCH_TARGET, return_value=True), ): with pytest.raises(ProxyException): await MCPRequestHandler.process_mcp_request( @@ -6053,7 +6036,6 @@ class TestAggregateGatewayDcrChallenge: fire even when that server does not resolve.""" with ( patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), - patch(self._FLAG_PATCH_TARGET, return_value=True), ): with pytest.raises(ProxyException): await MCPRequestHandler.process_mcp_request(self._scope(path="/mcp/github")) @@ -6063,7 +6045,6 @@ class TestAggregateGatewayDcrChallenge: not a cold-start DCR client; keep the original error.""" with ( patch(self._AUTH_PATCH_TARGET, side_effect=self._auth_401()), - patch(self._FLAG_PATCH_TARGET, return_value=True), ): with pytest.raises(ProxyException): await MCPRequestHandler.process_mcp_request( @@ -6078,7 +6059,6 @@ class TestAggregateGatewayDcrChallenge: with ( patch(self._AUTH_PATCH_TARGET, side_effect=_raise_500), - patch(self._FLAG_PATCH_TARGET, return_value=True), ): with pytest.raises(ProxyException) as exc_info: await MCPRequestHandler.process_mcp_request(self._scope()) 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 30a814c8ea4..af4b2caca72 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 @@ -7132,19 +7132,74 @@ async def test_token_exchange_unreadable_body_still_renders_oauth_fault(): assert body == {"error": "server_error", "error_description": "upstream token endpoint returned HTTP 400"} -def _patch_gateway_dcr_flag(enabled: bool): - return patch( - "litellm.proxy._experimental.mcp_server.discoverable_endpoints.is_mcp_gateway_dcr_enabled", - return_value=enabled, +def test_aggregate_wellknown_routes_serve_gateway_metadata(): + """Both path-appended aggregate routes serve the gateway documents. Exercises real + routing, so this also pins registration order: the parameterized + /.well-known/oauth-authorization-server/{name} route would otherwise capture the /mcp + suffix as a server name.""" + from fastapi import FastAPI + from fastapi.testclient import TestClient + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import router + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, ) + global_mcp_server_manager.registry.clear() + app = FastAPI() + app.include_router(router) + client = TestClient(app) + + prm = client.get("/.well-known/oauth-protected-resource/mcp") + asm = client.get("/.well-known/oauth-authorization-server/mcp") + + assert prm.status_code == 200 + assert prm.json()["resource"] == "http://testserver/mcp" + assert prm.json()["authorization_servers"] == ["http://testserver/mcp"] + + assert asm.status_code == 200 + assert asm.json()["issuer"] == "http://testserver/mcp" + assert asm.json()["authorization_endpoint"] == "http://testserver/authorize" + 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.""" + from fastapi import FastAPI + from fastapi.testclient import TestClient + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import router + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + + global_mcp_server_manager.registry.clear() + server_named_mcp = _create_oauth2_server(server_id="mcp_srv", name="mcp", server_name="mcp", alias="mcp") + global_mcp_server_manager.registry[server_named_mcp.server_id] = server_named_mcp + app = FastAPI() + app.include_router(router) + client = TestClient(app) + + 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"] + finally: + global_mcp_server_manager.registry.clear() + @pytest.mark.asyncio -async def test_gateway_dcr_root_discovery_describes_gateway_not_single_server(): - """Flag on: root discovery must keep describing the gateway as the - authorization server for the aggregate /mcp resource even when exactly one - OAuth2 server exists (flag off, resolution narrows to that server; that - behavior is pinned by test_discovery_root_includes_server_name_prefix).""" +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 + oauth2 server configured, the no-suffix /.well-known/oauth-{authorization-server, + protected-resource} still resolves THAT server, so an existing single-server deployment's + discovery is unchanged. The aggregate document lives only at the /mcp-suffixed routes.""" from fastapi import Request from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( @@ -7164,134 +7219,15 @@ async def test_gateway_dcr_root_discovery_describes_gateway_not_single_server(): mock_request.headers = {} try: - with _patch_gateway_dcr_flag(True): - authorization_response = _build_oauth_authorization_server_response( - request=mock_request, - mcp_server_name=None, - ) - resource_response = await _build_oauth_protected_resource_response( - request=mock_request, - mcp_server_name=None, - use_standard_pattern=True, - ) - - assert authorization_response["issuer"] == "https://llm.example.com/mcp" - assert authorization_response["authorization_endpoint"] == "https://llm.example.com/authorize" - assert authorization_response["token_endpoint"] == "https://llm.example.com/token" - assert authorization_response["registration_endpoint"] == "https://llm.example.com/register" - assert "none" in authorization_response["token_endpoint_auth_methods_supported"] - assert authorization_response["code_challenge_methods_supported"] == ["S256"] - assert authorization_response["scopes_supported"] == [] - - assert resource_response["resource"] == "https://llm.example.com/mcp" - assert resource_response["authorization_servers"] == ["https://llm.example.com/mcp"] - assert resource_response["scopes_supported"] == [] + authorization_response = _build_oauth_authorization_server_response( + request=mock_request, mcp_server_name=None + ) + resource_response = await _build_oauth_protected_resource_response( + request=mock_request, mcp_server_name=None, use_standard_pattern=True + ) + # per-server, not aggregate: the single server's name is in the endpoints + assert "/test_oauth/authorize" in authorization_response["authorization_endpoint"] + assert authorization_response["issuer"] == "https://llm.example.com" + assert resource_response["authorization_servers"] == ["https://llm.example.com/test_oauth"] finally: global_mcp_server_manager.registry.clear() - - -@pytest.mark.asyncio -async def test_gateway_dcr_named_discovery_unaffected_by_flag(): - """Flag on must not change named-server discovery: a named oauth2 server - still resolves to its own per-server document.""" - from fastapi import Request - - from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( - _build_oauth_authorization_server_response, - ) - from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( - global_mcp_server_manager, - ) - - global_mcp_server_manager.registry.clear() - oauth2_server = _create_oauth2_server() - global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server - - mock_request = MagicMock(spec=Request) - mock_request.base_url = "https://llm.example.com/" - mock_request.headers = {} - - try: - with _patch_gateway_dcr_flag(True): - response = _build_oauth_authorization_server_response( - request=mock_request, - mcp_server_name="test_oauth", - ) - assert "/test_oauth/authorize" in response["authorization_endpoint"] - assert response["scopes_supported"] == ["read", "write"] - finally: - global_mcp_server_manager.registry.clear() - - -def test_aggregate_wellknown_routes_404_when_flag_off(): - """Flag off, the aggregate well-known routes answer 404 exactly like the - previously-absent routes: discovery behavior is byte-identical for - existing deployments.""" - from fastapi import FastAPI - from fastapi.testclient import TestClient - - from litellm.proxy._experimental.mcp_server.discoverable_endpoints import router - - app = FastAPI() - app.include_router(router) - client = TestClient(app) - - with _patch_gateway_dcr_flag(False): - assert client.get("/.well-known/oauth-protected-resource/mcp").status_code == 404 - assert client.get("/.well-known/oauth-authorization-server/mcp").status_code == 404 - - -def test_aggregate_wellknown_routes_serve_gateway_metadata_when_flag_on(): - """Flag on, both path-appended aggregate routes serve the gateway - documents. Exercises real routing, so this also pins registration order: - /.well-known/oauth-authorization-server/{name} would otherwise capture - the /mcp suffix as a server name and 404.""" - from fastapi import FastAPI - from fastapi.testclient import TestClient - - from litellm.proxy._experimental.mcp_server.discoverable_endpoints import router - from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( - global_mcp_server_manager, - ) - - global_mcp_server_manager.registry.clear() - app = FastAPI() - app.include_router(router) - client = TestClient(app) - - with _patch_gateway_dcr_flag(True): - prm = client.get("/.well-known/oauth-protected-resource/mcp") - asm = client.get("/.well-known/oauth-authorization-server/mcp") - - assert prm.status_code == 200 - assert prm.json()["resource"] == "http://testserver/mcp" - assert prm.json()["authorization_servers"] == ["http://testserver/mcp"] - - assert asm.status_code == 200 - assert asm.json()["issuer"] == "http://testserver/mcp" - assert asm.json()["authorization_endpoint"] == "http://testserver/authorize" - - -def test_is_mcp_gateway_dcr_enabled_reads_general_settings(): - """The flag reader accepts YAML booleans and env-interpolated strings, and - fails closed on anything else.""" - from litellm.proxy._experimental.mcp_server.oauth_utils import ( - is_mcp_gateway_dcr_enabled, - ) - from litellm.proxy.proxy_server import general_settings - - for raw, expected in ( - (True, True), - (False, False), - ("true", True), - ("True", True), - ("false", False), - ("yes", False), - (1, False), - (None, False), - ): - with patch.dict(general_settings, {"mcp_gateway_dcr": raw}): - assert is_mcp_gateway_dcr_enabled() is expected, f"raw={raw!r}" - - with patch.dict(general_settings, {}, clear=True): - assert is_mcp_gateway_dcr_enabled() is False