diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index c87e900aa2a..4fd47a97066 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -465,8 +465,19 @@ async def _store_per_user_token_server_side( def _raise_if_not_oauth2(mcp_server: MCPServer) -> None: - """Reject a non-oauth2 server from the gateway's OAuth authorize/token/register flow.""" - if mcp_server.auth_type == MCPAuth.oauth2: + """Reject a server without upstream OAuth from the gateway's authorize/token/register flow. + + The client-forwarded token modes (``true_passthrough`` / ``oauth_delegate``) are allowed + through: the caller owns the upstream token, and this relayed flow is how a browser obtains + one against the upstream IdP (the admin UI's browser-only Authorize uses it). The minted + token is upstream-audienced and held by the caller; the gateway persists nothing for these + modes (DCR persistence is opt-in and never enabled on this path). + """ + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( # noqa: PLC0415 # circular import with mcp_server_manager at module load + _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES, + ) + + if mcp_server.auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES: return raise HTTPException( status_code=400, @@ -515,8 +526,7 @@ async def authorize_with_server( response_type: Optional[str] = None, scope: Optional[str] = None, ): - if mcp_server.auth_type != "oauth2": - raise HTTPException(status_code=400, detail="MCP server is not OAuth2") + _raise_if_not_oauth2(mcp_server) if mcp_server.authorization_url is None: raise HTTPException(status_code=400, detail="MCP server authorization url is not set") diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index c4ad673b88f..356ed7a2729 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -171,6 +171,16 @@ _user_env_vars_cache: dict[tuple[str, str], tuple[dict[str, str], float]] = {} _USER_ENV_VARS_CACHE_TTL = 60 # seconds _USER_ENV_VARS_CACHE_MAX_SIZE = 4096 # cap to prevent unbounded growth +# Auth types whose upstream OAuth endpoints (protected-resource + authorization-server metadata) the +# gateway discovers from the upstream itself: interactive oauth2 and the two client-forwarded modes. +# OBO/M2M endpoint discovery is decided separately via _obo_needs_endpoint_discovery. Shared by the +# config-YAML and DB server loaders so the two paths cannot drift on which modes trigger discovery. +_UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES: tuple[MCPAuth, ...] = ( + MCPAuth.oauth2, + MCPAuth.true_passthrough, + MCPAuth.oauth_delegate, +) + def invalidate_user_env_vars_cache(user_id: str, server_id: str) -> None: """Drop a cached entry after the user stores or clears their env var values @@ -984,7 +994,7 @@ class MCPServerManager: auth_type = server_config.get("auth_type", None) if server_url and ( - auth_type == MCPAuth.oauth2 + auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES or self._obo_needs_endpoint_discovery( auth_type, server_config.get("token_exchange_endpoint"), @@ -993,7 +1003,7 @@ class MCPServerManager: ): mcp_oauth_metadata = await self._descovery_metadata( server_url=server_url, - allow_origin_fallback=auth_type == MCPAuth.oauth2, + allow_origin_fallback=auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES, ) else: mcp_oauth_metadata = None @@ -1385,7 +1395,7 @@ class MCPServerManager: auth_type = cast(MCPAuthType, mcp_server.auth_type) server_url = mcp_server.url needs_discovery = bool(server_url) and ( - (auth_type == MCPAuth.oauth2 and not mcp_server.authorization_url) + (auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES and not mcp_server.authorization_url) or self._obo_needs_endpoint_discovery( auth_type, mcp_server.token_exchange_endpoint @@ -1396,7 +1406,7 @@ class MCPServerManager: mcp_oauth_metadata = ( await self._descovery_metadata( server_url=server_url, # type: ignore[arg-type] - allow_origin_fallback=auth_type == MCPAuth.oauth2, + allow_origin_fallback=auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES, ) if needs_discovery else None diff --git a/litellm/proxy/_experimental/mcp_server/rest_endpoints.py b/litellm/proxy/_experimental/mcp_server/rest_endpoints.py index b917530dd52..74f0b488d20 100644 --- a/litellm/proxy/_experimental/mcp_server/rest_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/rest_endpoints.py @@ -69,6 +69,7 @@ if MCP_AVAILABLE: from mcp.types import Tool as MCPTool from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES, global_mcp_server_manager, ) from litellm.proxy._experimental.mcp_server.oauth_utils import ( @@ -1321,8 +1322,13 @@ if MCP_AVAILABLE: if isinstance(credentials, dict): mcp_auth_header = credentials.get("auth_value") + # Authorization doubles as the admission fallback (LITELLM_API_KEY_HEADER_NAME_SECONDARY): + # when the primary x-litellm-api-key header is absent, the Authorization value is the + # caller's LiteLLM key, not an upstream token, and must never be forwarded upstream. oauth2_headers: Optional[Dict[str, str]] = None - if new_mcp_server_request.auth_type == MCPAuth.oauth2: + if new_mcp_server_request.auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES and headers.get( + MCPRequestHandler.LITELLM_API_KEY_HEADER_NAME_PRIMARY + ): oauth2_headers = MCPRequestHandler._get_oauth2_headers_from_headers(headers) async def _list_tools_operation(client): diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index 55a0fa083c0..3550237dd65 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -27,6 +27,7 @@ from typing import ( Union, cast, ) +from urllib.parse import urlsplit, urlunsplit import httpx from fastapi import FastAPI, HTTPException @@ -105,6 +106,27 @@ _MAX_STATEFUL_SESSIONS_PER_OWNER = 100 _MCP_ROUTING_PEEK_MAX_BYTES = 4096 +def _redact_mcp_resource_url(url: Optional[str]) -> Optional[str]: + """Reduce an MCP server URL to its origin (scheme + host + port) for logging. + + Everything else is dropped: userinfo (``user:pass@``), the query string, the + fragment, and the path, because hosted MCP servers routinely embed the + credential in the path (e.g. ``/mcp/s/``) and this value is persisted + in spend-log metadata that a caller who can invoke the tool can read back. + Returns None when the URL has no host to identify (nothing safe to log). + """ + if not isinstance(url, str) or not url: + return None + try: + parts = urlsplit(url) + except ValueError: + return None + if not parts.hostname: + return None + netloc = f"{parts.hostname}:{parts.port}" if parts.port else parts.hostname + return urlunsplit((parts.scheme, netloc, "", "", "")) or None + + def _invalidate_byok_cred_cache(user_id: str, server_id: str) -> None: """Remove a (user_id, server_id) entry from the BYOK credential cache. @@ -3064,6 +3086,8 @@ if MCP_AVAILABLE: mcp_server_logo_url=mcp_info.get("logo_url"), namespaced_tool_name=namespaced_tool_name, mcp_session_id=session_id, + mcp_auth_mode=mcp_server.auth_type, + mcp_server_resource=_redact_mcp_resource_url(mcp_server.url), ) else: return StandardLoggingMCPToolCall( diff --git a/litellm/types/utils.py b/litellm/types/utils.py index 908f5b76424..6b99cfa3314 100644 --- a/litellm/types/utils.py +++ b/litellm/types/utils.py @@ -2523,6 +2523,22 @@ class StandardLoggingMCPToolCall(TypedDict, total=False): the client is driving a stateful session. Absent for stateless calls. """ + mcp_auth_mode: Optional[str] + """ + The server's auth_type for this call (e.g. `true_passthrough`, `oauth_delegate`, + `oauth2`). For the client-forwarded token modes this records that the caller's own + upstream token was relayed, so an audit can attribute a relayed request to its mode + without logging any credential. + """ + + mcp_server_resource: Optional[str] + """ + The upstream MCP server resource identifier (scheme + host + path) the tool call was + forwarded to. Redacted for logging: userinfo, query string, and fragment are stripped so an + upstream URL carrying an embedded token or secret query parameter never reaches log metadata. + Records which upstream received a relayed request; never a credential. + """ + class StandardLoggingVectorStoreRequest(TypedDict, total=False): """ 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 19d030f17c4..9de342eabd7 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 @@ -6,6 +6,8 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from fastapi import HTTPException +from litellm.types.mcp import MCPAuth + # Fixture to mock IP address check for all MCP tests # This prevents tests from failing due to IP-based access control @@ -119,6 +121,61 @@ async def test_authorize_endpoint_includes_response_type(): assert "scope=read+write" in response.headers["location"] +@pytest.mark.asyncio +@pytest.mark.parametrize("auth_type_value", ["true_passthrough", "oauth_delegate"]) +async def test_authorize_endpoint_allows_client_forwarded_modes(auth_type_value): + """The browser-only Authorize relays the gateway authorize flow for the client-forwarded + token modes; the oauth2-only gate must let them through and redirect to the upstream IdP.""" + try: + from fastapi import Request + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + authorize, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + + server = MCPServer( + server_id="test_cf_server", + name="test_cf", + server_name="test_cf", + alias="test_cf", + transport=MCPTransport.http, + auth_type=MCPAuth(auth_type_value), + # Discovery stamps these onto the in-memory registry entry at build time. + authorization_url="https://provider.com/oauth/authorize", + token_url="https://provider.com/oauth/token", + ) + global_mcp_server_manager.registry[server.server_id] = server + + mock_request = MagicMock(spec=Request) + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + with patch("litellm.proxy._experimental.mcp_server.discoverable_endpoints.encrypt_value_helper") as mock_encrypt: + mock_encrypt.return_value = "mocked_encrypted_state" + + response = await authorize( + request=mock_request, + client_id="dcr_client_id", + mcp_server_name="test_cf", + redirect_uri="http://127.0.0.1:60108/callback", + state="test_state", + ) + + assert response.status_code == 307 + assert "https://provider.com/oauth/authorize" in response.headers["location"] + assert "client_id=dcr_client_id" in response.headers["location"] + + @pytest.mark.asyncio async def test_authorize_endpoint_preserves_existing_query_params(): """Test that authorize endpoint merges OAuth params with existing query params in authorization_url""" @@ -3348,6 +3405,83 @@ async def test_token_exchange_passes_through_upstream_expires_in(): assert body["expires_in"] == 43200 +async def _exchange_persistence_attempted_for_auth_type(auth_type) -> bool: + """Run exchange_token_with_server for a server of ``auth_type`` and report whether it attempted + to persist the exchanged token server-side. The client-forwarded token modes must not persist: + their contract is that the upstream token stays browser-held, minted/stored/refreshed nowhere.""" + from fastapi import Request + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + exchange_token_with_server, + ) + from litellm.proxy._types import MCPTransport + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + server = MCPServer( + server_id="t", + name="t", + server_name="t", + alias="t", + transport=MCPTransport.http, + auth_type=auth_type, + client_id="cid", + client_secret="cs", + authorization_url="https://provider.com/oauth/authorize", + token_url="https://provider.com/oauth/token", + ) + mock_request = MagicMock(spec=Request) + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + fake_http_response = MagicMock() + fake_http_response.json.return_value = {"access_token": "tok", "refresh_token": "r", "token_type": "Bearer"} + fake_http_response.raise_for_status = MagicMock() + fake_http_client = MagicMock() + fake_http_client.post = AsyncMock(return_value=fake_http_response) + + with ( + patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client", + return_value=fake_http_client, + ), + patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints._extract_user_id_from_request", + new_callable=AsyncMock, + return_value="admin-user", + ), + patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints._store_per_user_token_server_side", + new_callable=AsyncMock, + ) as mock_store, + ): + await exchange_token_with_server( + request=mock_request, + mcp_server=server, + grant_type="authorization_code", + code="c", + redirect_uri="http://127.0.0.1:3000/cb", + client_id="cid", + client_secret=None, + code_verifier=None, + ) + return mock_store.await_count > 0 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("auth_type", [MCPAuth.true_passthrough, MCPAuth.oauth_delegate]) +async def test_token_exchange_does_not_persist_for_client_forwarded_modes(auth_type): + """The browser-only Authorize for true_passthrough / oauth_delegate must not write the upstream + token to the DB: these modes forward a browser-held token and persist nothing server-side.""" + assert await _exchange_persistence_attempted_for_auth_type(auth_type) is False + + +@pytest.mark.asyncio +async def test_token_exchange_persists_for_oauth2(): + """Guard the test's own discriminator: a genuine oauth2 (authorization_code) server DOES persist, + so the passthrough no-persist assertion above is meaningful and not vacuously true.""" + assert await _exchange_persistence_attempted_for_auth_type(MCPAuth.oauth2) is True + + # ------------------------------------------------------------------- # OBO (token_exchange) Protected Resource Metadata: discovery must name the # JWT-auth issuer the client SSOs with, not the gateway. 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 e9dccdcd4ad..bba0eb31cfb 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 @@ -6854,3 +6854,27 @@ async def test_call_tool_with_legacy_db_m2m_server_resolves_oauth2_flow(): resolved = captured_servers["allowed"] assert resolved and resolved[0].oauth2_flow == "client_credentials" assert resolved[0].has_client_credentials is True + + +@pytest.mark.parametrize( + "url, expected", + [ + # only the origin may be logged: userinfo, query, fragment, and the PATH are all stripped, + # because hosted MCP servers routinely embed the credential in the path (e.g. /mcp/s/) + ("https://user:s3cr3t@mcp.example.com/mcp?token=abcd1234&x=1", "https://mcp.example.com"), + ("https://mcp.example.com/mcp#frag", "https://mcp.example.com"), + ("https://host:8443/a/b?q=1", "https://host:8443"), + ("https://mcp.zapier.com/api/mcp/s/NDgzcret-token/mcp", "https://mcp.zapier.com"), + ("https://mcp.notion.com/mcp", "https://mcp.notion.com"), + (None, None), + ("", None), + ("not a url", None), + ], +) +def test_redact_mcp_resource_url_strips_credentials(url, expected): + """The MCP tool-call log records the upstream resource, so the URL must be redacted to + scheme+host+path: userinfo, query string, and fragment (which can carry embedded tokens or + secret parameters) must never reach spend-log metadata or logging callbacks.""" + from litellm.proxy._experimental.mcp_server.server import _redact_mcp_resource_url + + assert _redact_mcp_resource_url(url) == expected diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index 12e22de195b..e8fca9ac6ab 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -833,6 +833,39 @@ class TestMCPServerManager: assert spec is not None and isinstance(spec.config, TokenExchangeConfig) assert spec.config.profile == "entra_obo" + @pytest.mark.asyncio + @pytest.mark.parametrize("auth_type", [MCPAuth.true_passthrough, MCPAuth.oauth_delegate]) + async def test_build_from_table_discovers_upstream_oauth_for_client_forwarded_modes(self, auth_type): + """The gateway's relayed authorize flow (used by the browser-only Authorize) needs the + upstream's authorization_url on the registry entry, and these rows never persist one, so + the DB build must discover it the same way oauth2 rows do.""" + from types import SimpleNamespace + + manager = MCPServerManager() + row = LiteLLM_MCPServerTable( + server_id="cf-db-1", + alias="cf_db", + description="client-forwarded from db", + url="https://up.example.com/mcp", + transport=MCPTransport.http, + auth_type=auth_type, + created_at=datetime.now(), + updated_at=datetime.now(), + ) + + metadata = SimpleNamespace( + authorization_url="https://idp.example.com/authorize", + token_url="https://idp.example.com/token", + registration_url="https://idp.example.com/register", + scopes=None, + ) + with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)) as mock_discovery: + built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False) + + mock_discovery.assert_awaited_once() + assert built.authorization_url == "https://idp.example.com/authorize" + assert built.token_url == "https://idp.example.com/token" + async def _capture_subject_token(self, call) -> Optional[str]: """Run a manager method (via ``call(manager)``) and return the subject_token it threaded into ``_create_mcp_client``.""" diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py index 3d9afd8f250..465cebcc12c 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py @@ -37,10 +37,7 @@ def _build_request( body_bytes = body else: body_bytes = b"" - raw_headers = [ - (key.lower().encode("latin-1"), value.encode("latin-1")) - for key, value in headers.items() - ] + raw_headers = [(key.lower().encode("latin-1"), value.encode("latin-1")) for key, value in headers.items()] scope = { "type": "http", "http_version": "1.1", @@ -62,25 +59,18 @@ def _build_request( def _get_route(path: str, method: str): for route in rest_endpoints.router.routes: - if getattr(route, "path", None) == path and method in getattr( - route, "methods", set() - ): + if getattr(route, "path", None) == path and method in getattr(route, "methods", set()): return route raise AssertionError(f"Route {method} {path} not found") def _route_has_dependency(route, dependency) -> bool: - if any( - getattr(dep, "dependency", None) == dependency - for dep in getattr(route, "dependencies", []) - ): + if any(getattr(dep, "dependency", None) == dependency for dep in getattr(route, "dependencies", [])): return True dependant = getattr(route, "dependant", None) if dependant is None: return False - return any( - getattr(dep, "call", None) == dependency for dep in dependant.dependencies - ) + return any(getattr(dep, "call", None) == dependency for dep in dependant.dependencies) class TestExecuteWithMcpClient: @@ -104,9 +94,7 @@ class TestExecuteWithMcpClient: auth_type=MCPAuth.none, ) - result = await rest_endpoints._execute_with_mcp_client( - payload, failing_operation - ) + result = await rest_endpoints._execute_with_mcp_client(payload, failing_operation) assert result["status"] == "error" assert "stack_trace" not in result @@ -267,15 +255,10 @@ class TestExecuteWithMcpClient: assert result["status"] == "ok" # The incoming Authorization must be dropped — extra_headers should # contain no oauth2 headers (only static_headers, which are None here). - assert ( - captured["extra_headers"] is None - or "Authorization" not in captured["extra_headers"] - ) + assert captured["extra_headers"] is None or "Authorization" not in captured["extra_headers"] @pytest.mark.asyncio - async def test_interactive_oauth_resolves_forwarded_token_via_presented_store( - self, monkeypatch - ): + async def test_interactive_oauth_resolves_forwarded_token_via_presented_store(self, monkeypatch): """Interactive authorization_code preview (oauth2, no client credentials): the forwarded just-authorized token is resolved THROUGH the v2 resolver via a one-shot presented store (cred_provider), not the caller-override path. The bare token (Bearer stripped) is the @@ -433,9 +416,7 @@ class TestExecuteWithMcpClient: return None async def fake_create_client(*args, **kwargs): - raise BaseExceptionGroup( - "test group", [RuntimeError("Cancelled via cancel scope")] - ) + raise BaseExceptionGroup("test group", [RuntimeError("Cancelled via cancel scope")]) monkeypatch.setattr( rest_endpoints.global_mcp_server_manager, @@ -497,9 +478,7 @@ class TestTestToolsList: "message": "Successfully retrieved tools", } - monkeypatch.setattr( - rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False - ) + monkeypatch.setattr(rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False) oauth_call_counter = {"count": 0} @@ -555,9 +534,7 @@ class TestTestToolsList: "message": "Successfully retrieved tools", } - monkeypatch.setattr( - rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False - ) + monkeypatch.setattr(rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False) oauth_headers = {"Authorization": "Bearer oauth"} oauth_call_counter = {"count": 0} @@ -573,7 +550,7 @@ class TestTestToolsList: raising=False, ) - request = _build_request({"authorization": "Bearer incoming"}) + request = _build_request({"authorization": "Bearer incoming", "x-litellm-api-key": "sk-admission"}) payload = NewMCPServerRequest( server_name="example", url="https://example.com", @@ -593,6 +570,101 @@ class TestTestToolsList: assert captured["oauth2_headers"] == oauth_headers assert oauth_call_counter["count"] == 1 + @pytest.mark.parametrize("auth_type", [MCPAuth.true_passthrough, MCPAuth.oauth_delegate]) + async def test_extracts_oauth2_headers_for_client_forwarded_modes(self, monkeypatch, auth_type): + """The browser-only authorize flow sends the upstream token as Authorization; the preview + must thread it through for the client-forwarded token modes so the passthrough arm can + forward it, instead of probing the upstream unauthenticated.""" + + captured: dict = {} + + async def fake_execute( + request, + operation, + mcp_auth_header=None, + oauth2_headers=None, + raw_headers=None, + ): + captured["mcp_auth_header"] = mcp_auth_header + captured["oauth2_headers"] = oauth2_headers + return { + "tools": [], + "error": None, + "message": "Successfully retrieved tools", + } + + monkeypatch.setattr(rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False) + + oauth_headers = {"Authorization": "Bearer upstream-token"} + + monkeypatch.setattr( + auth_mcp.MCPRequestHandler, + "_get_oauth2_headers_from_headers", + staticmethod(lambda headers: oauth_headers), + raising=False, + ) + + request = _build_request({"authorization": "Bearer upstream-token", "x-litellm-api-key": "sk-admission"}) + payload = NewMCPServerRequest( + server_name="example", + url="https://example.com", + auth_type=auth_type, + ) + + from litellm.proxy._types import LitellmUserRoles + + result = await rest_endpoints.test_tools_list( + request, + payload, + user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN), + ) + + assert result["message"] == "Successfully retrieved tools" + assert captured["mcp_auth_header"] is None + assert captured["oauth2_headers"] == oauth_headers + + @pytest.mark.parametrize("auth_type", [MCPAuth.oauth2, MCPAuth.true_passthrough, MCPAuth.oauth_delegate]) + async def test_does_not_forward_authorization_that_satisfied_admission(self, monkeypatch, auth_type): + """Authorization is also the admission fallback: with no x-litellm-api-key on the request, + the Authorization value is the caller's LiteLLM key, so forwarding it would send the + admission credential to the upstream.""" + + captured: dict = {} + + async def fake_execute( + request, + operation, + mcp_auth_header=None, + oauth2_headers=None, + raw_headers=None, + ): + captured["oauth2_headers"] = oauth2_headers + return { + "tools": [], + "error": None, + "message": "Successfully retrieved tools", + } + + monkeypatch.setattr(rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False) + + request = _build_request({"authorization": "Bearer sk-litellm-admission-key"}) + payload = NewMCPServerRequest( + server_name="example", + url="https://example.com", + auth_type=auth_type, + ) + + from litellm.proxy._types import LitellmUserRoles + + result = await rest_endpoints.test_tools_list( + request, + payload, + user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN), + ) + + assert result["message"] == "Successfully retrieved tools" + assert captured["oauth2_headers"] is None + class TestListToolsRestAPI: pytestmark = pytest.mark.asyncio @@ -722,9 +794,7 @@ class TestListToolsRestAPI: stub_server = StubServer() captured = {} - async def fake_get_tools( - server, server_auth_header, *args, apply_tool_filters=True, **kwargs - ): + async def fake_get_tools(server, server_auth_header, *args, apply_tool_filters=True, **kwargs): captured["apply_tool_filters"] = apply_tool_filters return ["tool-1"] @@ -772,9 +842,7 @@ class TestListToolsRestAPI: assert captured["apply_tool_filters"] is True @pytest.mark.parametrize("upstream_status", [401, 403]) - async def test_upstream_auth_failure_surfaces_status_and_challenge( - self, monkeypatch, upstream_status - ): + async def test_upstream_auth_failure_surfaces_status_and_challenge(self, monkeypatch, upstream_status): """A single-server pass-through request whose upstream rejects the token must surface the upstream status (401 or 403) plus its WWW-Authenticate challenge, not collapse into a 200 ``unexpected_error`` body.""" @@ -1362,9 +1430,7 @@ class TestListToolsRestAPI: oauth_headers = {"Authorization": "Bearer user-oauth-token"} - async def fake_get_user_oauth_extra_headers( - server, user_api_key_dict, prefetched_creds=None - ): + async def fake_get_user_oauth_extra_headers(server, user_api_key_dict, prefetched_creds=None): return oauth_headers captured = {} @@ -1608,9 +1674,7 @@ class TestGetToolsForSingleServer: pytestmark = pytest.mark.asyncio - async def test_filters_tools_by_object_permission_mcp_tool_permissions( - self, monkeypatch - ): + async def test_filters_tools_by_object_permission_mcp_tool_permissions(self, monkeypatch): """Test that tools are filtered by user_api_key_auth.object_permission.mcp_tool_permissions""" from litellm.proxy._experimental.mcp_server.server import MCPServer from litellm.proxy._types import LiteLLM_ObjectPermissionTable @@ -1773,9 +1837,7 @@ class TestGetToolsForSingleServer: # All tools should be returned assert len(result) == 2 - async def test_no_filtering_when_server_not_in_mcp_tool_permissions( - self, monkeypatch - ): + async def test_no_filtering_when_server_not_in_mcp_tool_permissions(self, monkeypatch): """Test that all tools are returned when server is not in mcp_tool_permissions""" from litellm.proxy._experimental.mcp_server.server import MCPServer from litellm.proxy._types import LiteLLM_ObjectPermissionTable @@ -1828,9 +1890,7 @@ class TestGetToolsForSingleServer: # All tools should be returned since server is not in permissions assert len(result) == 2 - async def test_combines_server_allowed_tools_and_object_permission_filters( - self, monkeypatch - ): + async def test_combines_server_allowed_tools_and_object_permission_filters(self, monkeypatch): """Test that both server.allowed_tools and object_permission.mcp_tool_permissions filters are applied""" from litellm.proxy._experimental.mcp_server.server import MCPServer from litellm.proxy._types import LiteLLM_ObjectPermissionTable @@ -2148,9 +2208,7 @@ class TestPreviewOpenAPITools: "paths": { "/repos/{owner}/{repo}/actions/jobs/{job_id}/logs": { "get": { - "operationId": ( - "actions/download-job-logs-for-workflow-run" - ), + "operationId": ("actions/download-job-logs-for-workflow-run"), "summary": "Download job logs", } }, @@ -2193,9 +2251,7 @@ class TestPreviewOpenAPITools: names = [t["name"] for t in result["tools"]] anthropic_re = re.compile(r"^[a-zA-Z0-9_-]{1,128}$") for name in names: - assert anthropic_re.match( - name - ), f"preview tool name {name!r} violates ^[a-zA-Z0-9_-]+$" + assert anthropic_re.match(name), f"preview tool name {name!r} violates ^[a-zA-Z0-9_-]+$" assert "actions_download-job-logs-for-workflow-run" in names assert "pulls_list-files" in names @@ -2252,9 +2308,7 @@ class TestPreviewOpenAPITools: registered_summary_to_name: dict = {} - def fake_create_tool_function( - path, method, operation, base_url - ): # noqa: ANN001 + def fake_create_tool_function(path, method, operation, base_url): # noqa: ANN001 def _f(): return None @@ -2267,9 +2321,7 @@ class TestPreviewOpenAPITools: ) class _StubRegistry: - def register_tool( - self, name, description, input_schema, handler - ): # noqa: ANN001 + def register_tool(self, name, description, input_schema, handler): # noqa: ANN001 registered_summary_to_name[description] = name monkeypatch.setattr( @@ -2278,9 +2330,7 @@ class TestPreviewOpenAPITools: _StubRegistry(), ) - openapi_to_mcp_generator.register_tools_from_openapi( - spec, base_url="https://example.invalid" - ) + openapi_to_mcp_generator.register_tools_from_openapi(spec, base_url="https://example.invalid") assert preview_summary_to_name == registered_summary_to_name, ( f"preview {preview_summary_to_name} != " @@ -2308,15 +2358,11 @@ class TestConnectionErrorMessage: assert secret not in message def test_connect_error_points_at_reachability(self): - message = rest_endpoints._connection_error_message( - httpx.ConnectError("All connection attempts failed") - ) + message = rest_endpoints._connection_error_message(httpx.ConnectError("All connection attempts failed")) assert "unreachable" in message.lower() def test_timeout_error_message(self): - message = rest_endpoints._connection_error_message( - httpx.ConnectTimeout("timed out") - ) + message = rest_endpoints._connection_error_message(httpx.ConnectTimeout("timed out")) assert "unreachable" in message.lower() def test_http_status_error_includes_status_code(self): diff --git a/ui/litellm-dashboard/eslint-metrics.json b/ui/litellm-dashboard/eslint-metrics.json index fcf60934f64..d69b3e1f729 100644 --- a/ui/litellm-dashboard/eslint-metrics.json +++ b/ui/litellm-dashboard/eslint-metrics.json @@ -1,7 +1,7 @@ { "@typescript-eslint/no-explicit-any": 1980, "complexity": 128, - "local/no-large-inline-object-arg": 519, + "local/no-large-inline-object-arg": 512, "local/no-long-condition-chain": 233, "max-depth": 59, "no-console": 15 diff --git a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx new file mode 100644 index 00000000000..af81f2713ae --- /dev/null +++ b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx @@ -0,0 +1,75 @@ +import React from "react"; +import { Button, Form, Input } from "antd"; +import { isClientForwardedTokenMode } from "./types"; + +interface PassthroughOAuthFlow { + startOAuthFlow: () => void | Promise; + status: string; + error: string | null; + tokenResponse: { access_token?: string; expires_in?: number } | null; +} + +/** + * Browser-only Authorize & Fetch for the client-forwarded token modes + * (true_passthrough / oauth_delegate). LiteLLM never stores upstream + * credentials for these modes, so the token obtained here lives in this + * browser session only: it is forwarded per-server for the tools preview and + * allowlist configuration, and is never written to the server row or the + * per-user credential store. The optional client credentials cover IdPs + * without dynamic client registration (e.g. a pre-registered Slack app) and + * ride the temporary authorize session only. + */ +export default function PassthroughAuthorizeSection({ + authType, + oauthFlow, +}: { + authType?: string | null; + oauthFlow: PassthroughOAuthFlow; +}) { + if (!isClientForwardedTokenMode(authType)) return null; + const authorizeButtonLabels: Record = { + authorizing: "Waiting for authorization...", + exchanging: "Exchanging authorization code...", + }; + const authorizeButtonLabel = authorizeButtonLabels[oauthFlow.status] ?? "Authorize & Fetch Tools (browser-only)"; + return ( +
+

+ Callers bring their own upstream token for this auth type, so LiteLLM stores no upstream credentials. To preview + tools and configure the tool allowlist, authorize against the upstream here: the token stays in this browser + session only and is never saved to LiteLLM. +

+ OAuth Client ID (optional, not saved)} + name={["credentials", "client_id"]} + extra="Only needed when the upstream does not support dynamic client registration (e.g. a pre-registered Slack app). Used for this browser authorization only." + > + + + OAuth Client Secret (optional, not saved)} + name={["credentials", "client_secret"]} + > + + + + {oauthFlow.error &&

{oauthFlow.error}

} + {oauthFlow.status === "success" && oauthFlow.tokenResponse?.access_token && ( +

+ Token held for this browser session. Tools can now be previewed and configured; nothing was saved to LiteLLM. +

+ )} +
+ ); +} diff --git a/ui/litellm-dashboard/src/components/mcp_tools/TruePassthroughWarning.tsx b/ui/litellm-dashboard/src/components/mcp_tools/TruePassthroughWarning.tsx new file mode 100644 index 00000000000..b52d3f4c672 --- /dev/null +++ b/ui/litellm-dashboard/src/components/mcp_tools/TruePassthroughWarning.tsx @@ -0,0 +1,21 @@ +import React from "react"; +import { Alert } from "antd"; +import { AUTH_TYPE } from "./types"; + +/** + * Warning shown in the create/edit MCP server forms when auth_type + * true_passthrough is selected: the gateway performs no admission auth for + * that server, so callers reach the upstream without a LiteLLM identity. + */ +export default function TruePassthroughWarning({ authType }: { authType?: string | null }) { + if (authType !== AUTH_TYPE.TRUE_PASSTHROUGH) return null; + return ( + + ); +} diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx index 36af2f8d9fc..eecbc253b6b 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx @@ -30,6 +30,7 @@ const oauthHook = vi.hoisted(() => ({ onTokenReceived: null as | ((token: Record | null, registeredClient?: { clientId?: string; clientSecret?: string }) => void) | null, + getCredentials: null as (() => Record | undefined) | null, })); vi.mock("@/hooks/useMcpOAuthFlow", () => ({ useMcpOAuthFlow: (opts: { @@ -37,8 +38,10 @@ vi.mock("@/hooks/useMcpOAuthFlow", () => ({ token: Record | null, registeredClient?: { clientId?: string; clientSecret?: string }, ) => void; + getCredentials?: () => Record | undefined; }) => { oauthHook.onTokenReceived = opts.onTokenReceived; + oauthHook.getCredentials = opts.getCredentials ?? null; return { startOAuthFlow: vi.fn(), status: "idle", @@ -164,6 +167,57 @@ describe("CreateMCPServer", () => { }); }); + it("should warn that LiteLLM auth is disabled when True Passthrough is selected", async () => { + await selectHttpTransport(); + + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + await waitFor(() => { + expect( + screen.getByText("True Passthrough disables LiteLLM authentication for this server"), + ).toBeInTheDocument(); + }); + }); + + it("should not show the True Passthrough warning when OAuth Delegate is selected", async () => { + await selectHttpTransport(); + + await selectAntOption("Authentication", "OAuth Delegate (client-supplied upstream token)"); + + await waitFor(() => { + expect(screen.getAllByText("OAuth Delegate (client-supplied upstream token)").length).toBeGreaterThan(0); + }); + expect( + screen.queryByText("True Passthrough disables LiteLLM authentication for this server"), + ).not.toBeInTheDocument(); + }); + + it.each([["True Passthrough (no LiteLLM auth)"], ["OAuth Delegate (client-supplied upstream token)"]])( + "should show the browser-only authorize section when %s is selected", + async (optionLabel) => { + await selectHttpTransport(); + + await selectAntOption("Authentication", optionLabel); + + await waitFor(() => { + expect(screen.getByRole("button", { name: "Authorize & Fetch Tools (browser-only)" })).toBeInTheDocument(); + }); + expect(screen.getByText("OAuth Client ID (optional, not saved)")).toBeInTheDocument(); + expect(screen.getByText("OAuth Client Secret (optional, not saved)")).toBeInTheDocument(); + }, + ); + + it("should not show the browser-only authorize section for API Key auth", async () => { + await selectHttpTransport(); + + await selectAntOption("Authentication", "API Key"); + + await waitFor(() => { + expect(screen.getByText("Authentication Value")).toBeInTheDocument(); + }); + expect(screen.queryByRole("button", { name: "Authorize & Fetch Tools (browser-only)" })).not.toBeInTheDocument(); + }); + it("should not require auth value when creating a server with API Key auth type", async () => { await selectHttpTransport(); @@ -298,6 +352,28 @@ describe("CreateMCPServer", () => { expect(payload.credentials).toEqual({ auth_value: "my-secret-key" }); }); + it("does not write the browser-authorized token into form.credentials for true_passthrough", async () => { + await selectHttpTransport(); + + const user = userEvent.setup({ delay: null }); + await user.type(getServerNameInput(), "PT_Server"); + await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp"); + + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + // Simulate the browser Authorize & Fetch flow handing back an upstream token. + await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy()); + await act(async () => { + oauthHook.onTokenReceived!({ access_token: "upstream-tok", token_type: "Bearer" }, undefined); + }); + + // For a browser-only mode the token must never land in form.credentials, which the OAuth flow's + // getCredentials reads for preview requests and the redirect-persist cache serializes. Without + // the guard, onTokenReceived writes it here and this returns { access_token: "upstream-tok" }. + const credentials = oauthHook.getCredentials?.() ?? {}; + expect(credentials.access_token).toBeUndefined(); + }); + it("should not show auth value field when None auth type is selected", async () => { await selectHttpTransport(); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index 10668468c15..0b39add234c 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -14,8 +14,11 @@ import { getMcpOAuthMode, MCP_OAUTH2_FLOW_M2M, MCP_OAUTH2_FLOW_INTERACTIVE, + isClientForwardedTokenMode, } from "./types"; import OAuthFormFields from "./OAuthFormFields"; +import TruePassthroughWarning from "./TruePassthroughWarning"; +import PassthroughAuthorizeSection from "./PassthroughAuthorizeSection"; import TokenExchangeFormFields from "./TokenExchangeFormFields"; import MCPServerCostConfig from "./mcp_server_cost_config"; import MCPConnectionStatus from "./mcp_connection_status"; @@ -134,20 +137,18 @@ const CreateMCPServer: React.FC = ({ } try { const values = form.getFieldsValue(true); - setSecureItem( - CREATE_OAUTH_UI_STATE_KEY, - JSON.stringify({ - modalVisible: isModalVisible, - formValues: values, - transportType, - costConfig, - allowedTools, - hasToolAllowlistInteraction, - searchValue, - aliasManuallyEdited, - logoUrl, - }), - ); + const uiState = { + modalVisible: isModalVisible, + formValues: values, + transportType, + costConfig, + allowedTools, + hasToolAllowlistInteraction, + searchValue, + aliasManuallyEdited, + logoUrl, + }; + setSecureItem(CREATE_OAUTH_UI_STATE_KEY, JSON.stringify(uiState)); } catch (err) { console.warn("Failed to persist MCP create state", err); } @@ -182,7 +183,7 @@ const CreateMCPServer: React.FC = ({ description: values.description, url, transport: transport === TRANSPORT.OPENAPI ? "http" : transport, - auth_type: AUTH_TYPE.OAUTH2, + auth_type: isClientForwardedTokenMode(values.auth_type) ? values.auth_type : AUTH_TYPE.OAUTH2, credentials: values.credentials, authorization_url: values.authorization_url, token_url: values.token_url, @@ -197,23 +198,36 @@ const CreateMCPServer: React.FC = ({ onTokenReceived: (token, registeredClient) => { setOauthAccessToken(token?.access_token ?? null); - if (token?.access_token) { - const credentials = { - access_token: token.access_token, - ...(token.refresh_token && { refresh_token: token.refresh_token }), - ...(token.expires_in && { expires_in: token.expires_in }), - ...(token.scope && { scope: token.scope }), - ...(registeredClient?.clientId && { client_id: registeredClient.clientId }), - ...(registeredClient?.clientSecret && { client_secret: registeredClient.clientSecret }), - }; - - form.setFieldsValue({ credentials }); - setAuthorizedUrl(getOAuthAuthorizationTarget(form.getFieldsValue(true))); - - NotificationsManager.success( - "OAuth authorization successful! Please click 'Create MCP Server' to save the configuration.", - ); + if (!token?.access_token) { + return; } + + if (isClientForwardedTokenMode(form.getFieldValue("auth_type"))) { + // Browser-only modes: the token is held in local state (oauthAccessToken) for tool preview + // and committed to sessionStorage on submit; it must never be written into form.credentials, + // which would persist it as server-level credentials on the created server row. Mirrors the + // edit form's onTokenReceived early return. + NotificationsManager.success( + "Token held for this browser session. Tools can now be previewed and configured; nothing will be saved to LiteLLM.", + ); + return; + } + + const credentials = { + access_token: token.access_token, + ...(token.refresh_token && { refresh_token: token.refresh_token }), + ...(token.expires_in && { expires_in: token.expires_in }), + ...(token.scope && { scope: token.scope }), + ...(registeredClient?.clientId && { client_id: registeredClient.clientId }), + ...(registeredClient?.clientSecret && { client_secret: registeredClient.clientSecret }), + }; + + form.setFieldsValue({ credentials }); + setAuthorizedUrl(getOAuthAuthorizationTarget(form.getFieldsValue(true))); + + NotificationsManager.success( + "OAuth authorization successful! Please click 'Create MCP Server' to save the configuration.", + ); }, onBeforeRedirect: persistCreateUiState, flowSource: "create", @@ -494,23 +508,21 @@ const CreateMCPServer: React.FC = ({ }); if (oauthMode === "authorization_code") { const scope = oauthTokenResponse.scope; - await storeMCPOAuthUserCredential(accessToken, response.server_id, { + const oauthCredentialPayload = { access_token: oauthTokenResponse.access_token, refresh_token: oauthTokenResponse.refresh_token, expires_in: oauthTokenResponse.expires_in, scopes: typeof scope === "string" && scope ? scope.split(" ") : undefined, - }); + }; + await storeMCPOAuthUserCredential(accessToken, response.server_id, oauthCredentialPayload); } else { - setToken( - response.server_id, - { - access_token: oauthTokenResponse.access_token, - expires_in: oauthTokenResponse.expires_in, - refresh_token: oauthTokenResponse.refresh_token, - token_type: oauthTokenResponse.token_type, - }, - userID, - ); + const browserHeldToken = { + access_token: oauthTokenResponse.access_token, + expires_in: oauthTokenResponse.expires_in, + refresh_token: oauthTokenResponse.refresh_token, + token_type: oauthTokenResponse.token_type, + }; + setToken(response.server_id, browserHeldToken, userID); } } @@ -970,9 +982,25 @@ const CreateMCPServer: React.FC = ({ OAuth OAuth Token Exchange (OBO) AWS SigV4 (Bedrock AgentCore MCPs) + True Passthrough (no LiteLLM auth) + + OAuth Delegate (client-supplied upstream token) + + + + + {shouldShowAuthValueField && ( ({ }, })); -const mockOauth: { tokenResponse: any } = { tokenResponse: null }; +const mockOauth: { + tokenResponse: any; + getTemporaryPayload: (() => Record | null) | null; +} = { tokenResponse: null, getTemporaryPayload: null }; vi.mock("@/hooks/useMcpOAuthFlow", () => ({ - useMcpOAuthFlow: () => ({ - startOAuthFlow: vi.fn(), - status: "idle", - error: null, - tokenResponse: mockOauth.tokenResponse, - }), + useMcpOAuthFlow: (opts: { getTemporaryPayload?: () => Record | null }) => { + mockOauth.getTemporaryPayload = opts?.getTemporaryPayload ?? null; + return { + startOAuthFlow: vi.fn(), + status: "idle", + error: null, + tokenResponse: mockOauth.tokenResponse, + }; + }, })); vi.mock("./mcp_server_cost_config", () => ({ @@ -307,6 +313,69 @@ describe("MCPServerEdit (delegate auth)", () => { }); }); +describe("MCPServerEdit (true passthrough warning)", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + const renderWithAuthType = (authType: string) => + render( + , + ); + + it("warns that LiteLLM auth is disabled for a true_passthrough server", async () => { + renderWithAuthType("true_passthrough"); + + await waitFor(() => { + expect(screen.getByText("True Passthrough disables LiteLLM authentication for this server")).toBeInTheDocument(); + }); + }); + + it("does not warn for an oauth_delegate server", async () => { + renderWithAuthType("oauth_delegate"); + + await waitFor(() => { + expect(screen.getAllByRole("button", { name: "Save Changes" }).length).toBeGreaterThan(0); + }); + expect( + screen.queryByText("True Passthrough disables LiteLLM authentication for this server"), + ).not.toBeInTheDocument(); + }); + + it("browser-authorize temp payload uses the selected auth_type, not the stored one", async () => { + // Stored server is oauth2; the admin switches the dropdown to true_passthrough before saving. + // The temp OAuth-relay payload must reflect the selection so the exchange is treated as + // browser-held (no DB persistence), matching onTokenReceived and the create form. + render( + , + ); + + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + await waitFor(() => { + expect(mockOauth.getTemporaryPayload).toBeTruthy(); + }); + const payload = mockOauth.getTemporaryPayload!(); + expect(payload).toBeTruthy(); + expect(payload?.auth_type).toBe("true_passthrough"); + }); +}); + describe("MCPServerEdit (auth type switch)", () => { beforeEach(() => { vi.clearAllMocks(); @@ -883,6 +952,55 @@ describe("MCPServerEdit (tool list fetch)", () => { expect(mockGetToken).toHaveBeenCalledWith("oauth_server_1", "user-1"); }); + it("forwards the sessionStorage token as the x-mcp header for an oauth_delegate server", async () => { + mockIsTokenValid.mockReturnValue(true); + mockGetToken.mockReturnValue({ access_token: "browser-token" }); + + render( + , + ); + + await waitFor(() => { + expect(networking.listMCPTools).toHaveBeenCalledWith( + "access-token", + "oauth_server_1", + { "x-mcp-oauth_server-authorization": "Bearer browser-token" }, + true, + ); + }); + expect(mockGetToken).toHaveBeenCalledWith("oauth_server_1", "user-1"); + }); + + it("prompts for the browser-only authorize when a true_passthrough server has no token", async () => { + mockIsTokenValid.mockReturnValue(false); + + render( + , + ); + + await waitFor(() => { + expect(screen.getByTestId("mcp-tool-config").getAttribute("data-external-error")).toContain( + "Authorize with the upstream (browser-only", + ); + }); + expect(networking.listMCPTools).not.toHaveBeenCalled(); + expect(screen.getByRole("button", { name: "Authorize & Fetch Tools (browser-only)" })).toBeInTheDocument(); + }); + it("uses the staged OAuth token to load passthrough tools after authorize", async () => { const passthroughServer = { ...interactiveOAuthServer, delegate_auth_to_upstream: true }; mockIsTokenValid.mockReturnValue(false); @@ -1055,6 +1173,76 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => { expect(onSuccess).not.toHaveBeenCalled(); }); + it.each([["true_passthrough"], ["oauth_delegate"]])( + "persists the staged token to sessionStorage on save for the %s mode", + async (authType) => { + // Regression: the save path classified the staged token with getMcpOAuthMode, which returns + // null for the client-forwarded modes, so setToken was never called and the browser-held + // token was dropped on save; the create form's submit path already committed it. + mockOauth.tokenResponse = { access_token: "cf-tok", expires_in: 1800, token_type: "bearer" }; + vi.mocked(networking.updateMCPServer).mockResolvedValue({ + ...interactiveOAuthServer, + auth_type: authType, + }); + + render( + , + ); + + await act(async () => { + fireEvent.click(screen.getAllByRole("button", { name: "Save Changes" })[0]); + }); + + await waitFor(() => { + expect(mockSetToken).toHaveBeenCalledWith( + "oauth_server_1", + expect.objectContaining({ access_token: "cf-tok" }), + "user-1", + ); + }); + expect(networking.storeMCPOAuthUserCredential).not.toHaveBeenCalled(); + const [, payload] = vi.mocked(networking.updateMCPServer).mock.calls[0]; + expect(payload.credentials).toBeUndefined(); + }, + ); + + it("forwards a newly authorized browser-held token for tool loading before the form is saved", async () => { + // Regression: fetchTools keyed the browser-held decision off the saved mcpServer.auth_type, so + // after switching the form to true_passthrough and authorizing, the fresh token was not sent as + // the x-mcp header until the server was saved. + vi.mocked(networking.listMCPTools).mockResolvedValue({ tools: [], error: null }); + mockIsTokenValid.mockReturnValue(false); + + render( + , + ); + + await selectAntOption("Authentication", "OAuth Delegate (client-supplied upstream token)"); + mockOauth.tokenResponse = { access_token: "fresh-tok", token_type: "bearer" }; + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + await waitFor(() => { + const withHeaders = vi + .mocked(networking.listMCPTools) + .mock.calls.find(([, , headers]) => headers && JSON.stringify(headers).includes("fresh-tok")); + expect(withHeaders).toBeTruthy(); + }); + }); + it("persists the passthrough token to sessionStorage on save after authorize", async () => { mockOauth.tokenResponse = { access_token: "pt-tok", expires_in: 1800, token_type: "bearer" }; vi.mocked(networking.updateMCPServer).mockResolvedValue({ diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx index 70632b459fc..ee7938d2904 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx @@ -4,6 +4,7 @@ import { InfoCircleOutlined } from "@ant-design/icons"; import { Button, TabGroup, TabList, Tab, TabPanels, TabPanel } from "@tremor/react"; import { AUTH_TYPE, + isClientForwardedTokenMode, OAUTH_FLOW, MCP_OAUTH2_FLOW_M2M, MCP_OAUTH2_FLOW_INTERACTIVE, @@ -18,6 +19,8 @@ import { getToken, isTokenValid, setToken } from "@/utils/mcpTokenStore"; import { buildMcpPassthroughAuthHeader } from "@/utils/mcpHeaderUtils"; import MCPServerCostConfig from "./mcp_server_cost_config"; import MCPPermissionManagement from "./MCPPermissionManagement"; +import TruePassthroughWarning from "./TruePassthroughWarning"; +import PassthroughAuthorizeSection from "./PassthroughAuthorizeSection"; import MCPToolConfiguration from "./mcp_tool_configuration"; import StdioConfiguration from "./StdioConfiguration"; import TokenExchangeFormFields from "./TokenExchangeFormFields"; @@ -128,6 +131,11 @@ const MCPServerEdit: React.FC = ({ } }; + // The auth mode every decision must key off: the admin's in-flight form selection wins over the + // saved record, so authorizing, loading tools, and saving all agree with what the form shows. Paths + // that read only mcpServer.auth_type go stale the moment the admin switches modes in the form. + const getEffectiveAuthType = () => form.getFieldValue("auth_type") ?? mcpServer.auth_type; + const { startOAuthFlow, status: oauthStatus, @@ -161,7 +169,7 @@ const MCPServerEdit: React.FC = ({ description: values.description || mcpServer.description, url, transport, - auth_type: AUTH_TYPE.OAUTH2, + auth_type: isClientForwardedTokenMode(values.auth_type) ? values.auth_type : AUTH_TYPE.OAUTH2, credentials: values.credentials, mcp_access_groups: values.mcp_access_groups || mcpServer.mcp_access_groups, static_headers: staticHeaders, @@ -171,20 +179,36 @@ const MCPServerEdit: React.FC = ({ }; }, onTokenReceived: (token) => { - if (token?.access_token) { - const credentials = { - access_token: token.access_token, - ...(token.refresh_token && { refresh_token: token.refresh_token }), - ...(token.expires_in && { expires_in: token.expires_in }), - ...(token.scope && { scope: token.scope }), - }; - - form.setFieldsValue({ credentials }); - - NotificationsManager.success( - "OAuth authorization successful! Please click 'Update MCP Server' to save the credentials.", - ); + if (!token?.access_token) { + return; } + + if (isClientForwardedTokenMode(getEffectiveAuthType())) { + const browserHeldToken = { + access_token: token.access_token, + expires_in: token.expires_in, + refresh_token: token.refresh_token, + token_type: token.token_type, + }; + setToken(mcpServer.server_id, browserHeldToken, userID); + NotificationsManager.success( + "Token held for this browser session. Tools can now be loaded and configured; nothing was saved to LiteLLM.", + ); + return; + } + + const credentials = { + access_token: token.access_token, + ...(token.refresh_token && { refresh_token: token.refresh_token }), + ...(token.expires_in && { expires_in: token.expires_in }), + ...(token.scope && { scope: token.scope }), + }; + + form.setFieldsValue({ credentials }); + + NotificationsManager.success( + "OAuth authorization successful! Please click 'Update MCP Server' to save the credentials.", + ); }, onBeforeRedirect: persistEditUiState, flowSource: "edit", @@ -368,7 +392,8 @@ const MCPServerEdit: React.FC = ({ oauth2_flow: mcpServer.oauth2_flow, delegate_auth_to_upstream: mcpServer.delegate_auth_to_upstream, }) === "passthrough"; - if (isPassthrough) { + const isBrowserHeldTokenMode = isClientForwardedTokenMode(getEffectiveAuthType()); + if (isPassthrough || isBrowserHeldTokenMode) { const token = oauthTokenResponse?.access_token ?? (isTokenValid(mcpServer.server_id, userID) @@ -376,7 +401,11 @@ const MCPServerEdit: React.FC = ({ : null); if (!token) { setTools([]); - setToolsError("Authenticate with this server in the Tools tab to load and configure its tools."); + setToolsError( + isBrowserHeldTokenMode + ? "Authorize with the upstream (browser-only, in the Authentication section) to load and configure this server's tools." + : "Authenticate with this server in the Tools tab to load and configure its tools.", + ); return; } customHeaders = buildMcpPassthroughAuthHeader(mcpServer.alias, token); @@ -439,7 +468,7 @@ const MCPServerEdit: React.FC = ({ const handleTransportChange = (value: string) => { // Clear fields that are not relevant for the selected transport. if (value === "stdio") { - form.setFieldsValue({ + const clearedForStdio = { url: undefined, spec_path: undefined, auth_type: undefined, @@ -447,15 +476,17 @@ const MCPServerEdit: React.FC = ({ authorization_url: undefined, token_url: undefined, registration_url: undefined, - }); + }; + form.setFieldsValue(clearedForStdio); } else if (value === TRANSPORT.OPENAPI) { - form.setFieldsValue({ + const clearedForOpenapi = { url: undefined, command: undefined, args: undefined, env_json: undefined, stdio_config: undefined, - }); + }; + form.setFieldsValue(clearedForOpenapi); } else { form.setFieldsValue({ spec_path: undefined, @@ -722,8 +753,9 @@ const MCPServerEdit: React.FC = ({ const updated = await updateMCPServer(accessToken, payload); // Persist the token staged via "Authorize & Fetch" (mirrors the create flow's - // commit-on-submit): OBO writes the per-user token to the DB, passthrough keeps - // it in sessionStorage. M2M/static auth resolve server-side and need neither. + // commit-on-submit): OBO writes the per-user token to the DB; legacy passthrough and the + // client-forwarded modes (true_passthrough / oauth_delegate) keep it in sessionStorage and + // never in the server row. M2M/static auth resolve server-side and need neither. if (oauthTokenResponse?.access_token) { const oauthMode = getMcpOAuthMode({ auth_type: restValues.auth_type, @@ -733,23 +765,21 @@ const MCPServerEdit: React.FC = ({ try { if (oauthMode === "authorization_code") { const scope = oauthTokenResponse.scope; - await storeMCPOAuthUserCredential(accessToken, mcpServer.server_id, { + const oauthCredentialPayload = { access_token: oauthTokenResponse.access_token, refresh_token: oauthTokenResponse.refresh_token, expires_in: oauthTokenResponse.expires_in, scopes: typeof scope === "string" && scope ? scope.split(" ") : undefined, - }); - } else if (oauthMode === "passthrough") { - setToken( - mcpServer.server_id, - { - access_token: oauthTokenResponse.access_token, - expires_in: oauthTokenResponse.expires_in, - refresh_token: oauthTokenResponse.refresh_token, - token_type: oauthTokenResponse.token_type, - }, - userID, - ); + }; + await storeMCPOAuthUserCredential(accessToken, mcpServer.server_id, oauthCredentialPayload); + } else if (oauthMode === "passthrough" || isClientForwardedTokenMode(restValues.auth_type)) { + const browserHeldToken = { + access_token: oauthTokenResponse.access_token, + expires_in: oauthTokenResponse.expires_in, + refresh_token: oauthTokenResponse.refresh_token, + token_type: oauthTokenResponse.token_type, + }; + setToken(mcpServer.server_id, browserHeldToken, userID); } } catch (error: unknown) { const message = error instanceof Error ? error.message : ""; @@ -874,18 +904,34 @@ const MCPServerEdit: React.FC = ({ {/* Authentication - for HTTP, SSE, and OpenAPI */} {!isStdioTransport && ( - - - + <> + + + + + + )} {isStdioTransport && ( diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx index 3346bc342f3..2e8fa901f6d 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx @@ -91,6 +91,37 @@ describe("MCPToolsViewer auth gate routing", () => { expect(screen.queryByText(GATE_TEXT)).not.toBeInTheDocument(); }); + it.each([["true_passthrough"], ["oauth_delegate"]])( + "shows the Authorize gate for a %s server without a browser token and does not list tools", + async (authType) => { + renderViewer({ auth_type: authType, oauth2_flow: null, delegate_auth_to_upstream: false }); + + expect(await screen.findByText(GATE_TEXT)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Authorize" })).toBeInTheDocument(); + expect(vi.mocked(listMCPTools)).not.toHaveBeenCalled(); + expect(vi.mocked(getMCPOAuthUserCredentialStatus)).not.toHaveBeenCalled(); + }, + ); + + it.each([["true_passthrough"], ["oauth_delegate"]])( + "forwards the session token via the x-mcp header for a %s server that has one", + async (authType) => { + vi.mocked(isTokenValid).mockReturnValue(true); + vi.mocked(getToken).mockReturnValue({ access_token: "upstream-tok" } as ReturnType); + + renderViewer({ auth_type: authType, oauth2_flow: null, delegate_auth_to_upstream: false }); + + await waitFor(() => + expect(vi.mocked(listMCPTools)).toHaveBeenCalledWith( + "litellm-key", + "srv-1", + expect.objectContaining({ "x-mcp-slack-authorization": "Bearer upstream-tok" }), + ), + ); + expect(screen.queryByText(GATE_TEXT)).not.toBeInTheDocument(); + }, + ); + it("lists tools for an OBO server when the user has a DB credential, with no x-mcp header", async () => { renderViewer({ oauth2_flow: null, delegate_auth_to_upstream: false }); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx index e436732ba3b..928a2e3c6bb 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx @@ -2,7 +2,14 @@ import React, { useCallback, useEffect, useState } from "react"; import { useQuery, useMutation } from "@tanstack/react-query"; import { ToolTestPanel } from "./ToolTestPanel"; import { resolveLogoSrc } from "@/lib/assetPaths"; -import { MCPTool, MCPToolsViewerProps, MCPContent, CallMCPToolResponse, getMcpOAuthMode } from "./types"; +import { + isClientForwardedTokenMode, + MCPTool, + MCPToolsViewerProps, + MCPContent, + CallMCPToolResponse, + getMcpOAuthMode, +} from "./types"; import { listMCPTools, callMCPTool, getMCPOAuthUserCredentialStatus } from "../networking"; import { isTokenValid, getToken, removeToken } from "@/utils/mcpTokenStore"; import { sanitizeMcpAliasForHeader, buildMcpPassthroughAuthHeader } from "@/utils/mcpHeaderUtils"; @@ -42,19 +49,23 @@ const MCPToolsViewer = ({ // service token and needs no gate. const oauthMode = getMcpOAuthMode({ auth_type, oauth2_flow, delegate_auth_to_upstream }); const isPassthrough = oauthMode === "passthrough"; + // The client-forwarded token modes gate the same way as PKCE passthrough: the + // browser session token (established via the browser-only Authorize in the + // create/edit forms, or right here) is the upstream credential. + const usesBrowserHeldToken = isPassthrough || isClientForwardedTokenMode(auth_type); const isAuthorizationCode = oauthMode === "authorization_code"; const [oauthToken, setOauthToken] = useState(() => - isPassthrough && isTokenValid(serverId, userID) ? getToken(serverId, userID)?.access_token ?? null : null, + usesBrowserHeldToken && isTokenValid(serverId, userID) ? getToken(serverId, userID)?.access_token ?? null : null, ); // Re-sync token when serverId/userID changes (useState initializer only runs on mount). useEffect(() => { - if (!isPassthrough) { + if (!usesBrowserHeldToken) { setOauthToken(null); return; } setOauthToken(isTokenValid(serverId, userID) ? getToken(serverId, userID)?.access_token ?? null : null); - }, [serverId, userID, isPassthrough]); + }, [serverId, userID, usesBrowserHeldToken]); const { startOAuthFlow, @@ -109,7 +120,7 @@ const MCPToolsViewer = ({ // x-mcp-{alias}-{header} pattern and forwards it to the upstream MCP server. // When no alias is available, fall back to x-mcp-auth (legacy but still supported). // Passthrough only: authorization_code/token_exchange/M2M tokens are attached server-side, not from the browser. - if (isPassthrough && oauthToken) { + if (usesBrowserHeldToken && oauthToken) { Object.assign(customHeaders, buildMcpPassthroughAuthHeader(serverAlias, oauthToken)); } @@ -164,7 +175,8 @@ const MCPToolsViewer = ({ // Passthrough blocks until a browser session token exists; authorization_code blocks until // the user has a valid DB credential (else the backend returns no tools). enabled: - !!accessToken && (isPassthrough ? oauthToken !== null : isAuthorizationCode ? hasAuthorizationCodeCred : true), + !!accessToken && + (usesBrowserHeldToken ? oauthToken !== null : isAuthorizationCode ? hasAuthorizationCodeCred : true), staleTime: 30000, // Consider data fresh for 30 seconds retry: (failureCount, error: any) => { // Don't retry on 401 — token is invalid, user must re-authenticate @@ -253,7 +265,8 @@ const MCPToolsViewer = ({ // passthrough needs a browser token; authorization_code needs a stored DB credential or a // still-valid one — a 401 from the list call means the backend has none even // after attempting a refresh, so re-authorization is required. - const authGateActive = (isPassthrough && !oauthToken) || authorizationCodeNeedsAuth || authorizationCodeTokenRejected; + const authGateActive = + (usesBrowserHeldToken && !oauthToken) || authorizationCodeNeedsAuth || authorizationCodeTokenRejected; // Treat authorization_code credential-status loading as "tools loading" so the empty state // doesn't flash before we know whether the user needs to authorize. const toolsAreaLoading = isLoadingTools || authorizationCodeStatusLoading; @@ -359,7 +372,7 @@ const MCPToolsViewer = ({ {/* Passthrough auth gate — browser session token absent */} - {isPassthrough && !oauthToken && ( + {usesBrowserHeldToken && !oauthToken && (

Authentication required

diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx index 9469d7bd89e..dca9e574e22 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx @@ -41,8 +41,17 @@ export const AUTH_TYPE = { OAUTH2: "oauth2", OAUTH2_TOKEN_EXCHANGE: "oauth2_token_exchange", AWS_SIGV4: "aws_sigv4", + TRUE_PASSTHROUGH: "true_passthrough", + OAUTH_DELEGATE: "oauth_delegate", }; +// The two client-forwarded token modes: the caller supplies the upstream Authorization (forwarded +// verbatim for true_passthrough, alongside LiteLLM admission for oauth_delegate). The dashboard holds +// their token in sessionStorage instead of persisting it, and the browser-authorize temp payload keeps +// their real auth_type so the backend does not treat them as needing a stored per-user token. +export const isClientForwardedTokenMode = (authType?: string | null): boolean => + authType === AUTH_TYPE.TRUE_PASSTHROUGH || authType === AUTH_TYPE.OAUTH_DELEGATE; + export const OAUTH_FLOW = { INTERACTIVE: "interactive", M2M: "m2m", diff --git a/ui/litellm-dashboard/src/hooks/useTestMCPConnection.tsx b/ui/litellm-dashboard/src/hooks/useTestMCPConnection.tsx index 055e15350ca..d27e1ca8bf9 100644 --- a/ui/litellm-dashboard/src/hooks/useTestMCPConnection.tsx +++ b/ui/litellm-dashboard/src/hooks/useTestMCPConnection.tsx @@ -1,6 +1,6 @@ import { useState, useEffect, useCallback } from "react"; import { testMCPToolsListRequest } from "../components/networking"; -import { AUTH_TYPE, OAUTH_FLOW, TRANSPORT } from "@/components/mcp_tools/types"; +import { AUTH_TYPE, OAUTH_FLOW, TRANSPORT, isClientForwardedTokenMode } from "@/components/mcp_tools/types"; interface MCPServerConfig { server_id?: string; @@ -56,7 +56,8 @@ export const useTestMCPConnection = ({ // Check if we have the minimum required fields to fetch tools const isM2MOAuth = formValues.auth_type === AUTH_TYPE.OAUTH2 && formValues.oauth_flow_type === OAUTH_FLOW.M2M; - const requiresOAuthToken = formValues.auth_type === AUTH_TYPE.OAUTH2 && !isM2MOAuth; + const isBrowserHeldTokenMode = isClientForwardedTokenMode(formValues.auth_type); + const requiresOAuthToken = (formValues.auth_type === AUTH_TYPE.OAUTH2 && !isM2MOAuth) || isBrowserHeldTokenMode; const isOpenAPITransport = formValues.transport === TRANSPORT.OPENAPI; const hasEndpoint = isOpenAPITransport ? !!formValues.spec_path : !!formValues.url;