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 f76b0c8acc8..e2204ea7a21 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 @@ -23,14 +23,29 @@ def _parse_mcp_server_names_from_path(path: str) -> Optional[List[str]]: Multi-server CSV paths like ``/mcp/server1,server2`` also return ``None`` — the cold-start bypass must not activate when any of the co-targeted servers - might not be passthrough-eligible. The regex stops at ``/?#`` only; the - comma check is handled explicitly below.""" - m = re.match(r"^/mcp/([^/?#]+)", path) - if m: - segment = m.group(1) - if "," in segment: + might not be passthrough-eligible. + + Server names may contain a single slash (e.g. ``custom_solutions/user_123``), + so the ``/mcp/...`` form must mirror + :meth:`MCPRequestHandler._extract_target_server_names_from_path` rather than + stop at the first ``/``. Otherwise the cold-start lookup would miss + slashed-name servers and fall through to the generic admission error + instead of the spec-compliant 401 challenge.""" + mcp_path_match = re.match(r"^/mcp/([^?#]+)", path) + if mcp_path_match: + servers_and_path = mcp_path_match.group(1) + if not servers_and_path: return None - return [segment] + if "," in servers_and_path: + return None + # Server name may contain at most one slash; strip any trailing + # path segments so the registry lookup uses the canonical name. + single_server_match = re.match( + r"^([^/]+(?:/[^/]+)?)(?:/.*)?$", servers_and_path + ) + if single_server_match: + return [single_server_match.group(1)] + return [servers_and_path] m = re.match(r"^/([^/,?#]+)/mcp", path) if m: return [m.group(1)] @@ -38,7 +53,7 @@ def _parse_mcp_server_names_from_path(path: str) -> Optional[List[str]]: def _is_mcp_passthrough_cold_start( - scope: Scope, mcp_servers: Optional[List[str]], client_ip: Optional[str] + mcp_servers: Optional[List[str]], client_ip: Optional[str] ) -> bool: """True only when EVERY targeted server is a pass-through server with no auth headers — the cold-start OAuth discovery case per RFC 9728 / MCP @@ -272,7 +287,7 @@ class MCPRequestHandler: ) and _is_litellm_auth_admission_error(exc) and _is_mcp_passthrough_cold_start( - scope, mcp_servers_from_path, client_ip=client_ip + mcp_servers_from_path, client_ip=client_ip ) ): verbose_logger.debug( diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index 9f20ea81879..071c6f35ad3 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -3043,17 +3043,32 @@ if MCP_AVAILABLE: ) request = StarletteRequest(scope) base_url = get_request_base_url(request) + _path = scope.get("_original_path") or scope.get("path", "") or "" for srv, (probe_status, _) in zip(passthrough_servers, probe_results): if probe_status == 401: - # Token is missing or expired — direct the client to re-authorize. - authorization_uri = ( - f"Bearer authorization_uri=" - f"{base_url}/.well-known/oauth-authorization-server/{srv.name}" - ) + # Token is missing or expired — direct the client at the + # gateway's oauth-protected-resource well-known URL (which + # proxies the upstream IdP's metadata), matching the + # pre-emptive 401 path in + # _raise_preemptive_401_for_unauthenticated_servers. The + # gateway is not the authorization server for pass-through + # servers, so emitting ``authorization_uri=`` would point + # clients at the wrong AS metadata. + if _path.startswith(f"/{srv.name}/mcp"): + resource_metadata_url = ( + f"{base_url}/.well-known/oauth-protected-resource/" + f"{srv.name}/mcp" + ) + else: + resource_metadata_url = ( + f"{base_url}/.well-known/oauth-protected-resource/mcp/" + f"{srv.name}" + ) + www_authenticate = f'Bearer resource_metadata="{resource_metadata_url}"' raise HTTPException( status_code=401, detail="Unauthorized", - headers={"WWW-Authenticate": authorization_uri}, + headers={"WWW-Authenticate": www_authenticate}, ) if probe_status == 403: # Token is valid but the caller lacks permission — do not hint diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_cold_start.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_cold_start.py index bcfb442ba23..139f3bad4a3 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_cold_start.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_cold_start.py @@ -72,7 +72,7 @@ def test_passthrough_cold_start_emits_401_with_matching_resource_metadata( scope["_original_path"] = route servers = _parse_mcp_server_names_from_path(scope.get("path", "")) - assert _is_mcp_passthrough_cold_start(scope, servers, client_ip=None) is True + assert _is_mcp_passthrough_cold_start(servers, client_ip=None) is True server_name = "sample_docs" base_url = "http://localhost:4000" @@ -117,8 +117,7 @@ def test_is_mcp_passthrough_cold_start_false_for_oauth2_server(): ) global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server - scope = _make_scope("/mcp/keycloak_whoami") - result = _is_mcp_passthrough_cold_start(scope, ["keycloak_whoami"], client_ip=None) + result = _is_mcp_passthrough_cold_start(["keycloak_whoami"], client_ip=None) assert result is False @@ -128,16 +127,19 @@ def test_is_mcp_passthrough_cold_start_false_for_empty_servers(): _is_mcp_passthrough_cold_start, ) - scope = _make_scope("/mcp") - assert _is_mcp_passthrough_cold_start(scope, None, client_ip=None) is False - assert _is_mcp_passthrough_cold_start(scope, [], client_ip=None) is False + assert _is_mcp_passthrough_cold_start(None, client_ip=None) is False + assert _is_mcp_passthrough_cold_start([], client_ip=None) is False @pytest.mark.parametrize( "path,expected", [ ("/mcp/sample_docs", ["sample_docs"]), - ("/mcp/sample_docs/tools/list", ["sample_docs"]), + # Server names may contain at most one slash (mirrors + # ``_extract_target_server_names_from_path``), so when more than two + # segments follow ``/mcp/`` the first two are treated as the name. + ("/mcp/sample_docs/tools/list", ["sample_docs/tools"]), + ("/mcp/custom_solutions/user_123", ["custom_solutions/user_123"]), ("/sample_docs/mcp", ["sample_docs"]), ("/sample_docs/mcp/tools/list", ["sample_docs"]), ("/mcp", None),