diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 2994f98f309..3e5762b3935 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -58,6 +58,7 @@ from litellm.proxy._experimental.mcp_server.oauth_utils import ( TOKEN_NO_CACHE_HEADERS, build_upstream_oauth2_token_request, get_request_base_url, + issuer_identities_match, resolve_upstream_resource, validate_trusted_redirect_uri, well_known_root_suffix, @@ -135,6 +136,7 @@ def encode_state_with_base_url( dcr_client_id: str | None = None, dcr_client_secret: str | None = None, dcr_token_endpoint_auth_method: MCPTokenEndpointAuthMethod | None = None, + expected_issuer: str | None = None, ) -> str: """ Encode the base_url, original state, and PKCE parameters using encryption. @@ -160,6 +162,8 @@ def encode_state_with_base_url( response granted the minted client, sealed alongside the credentials so the exchange authenticates the way the upstream expects instead of falling back to the server row's configured method + expected_issuer: Issuer identifier of the authorization server this flow is being sent to, + sealed so /callback can hold the RFC 9207 ``iss`` of the response against it Returns: An encrypted string that encodes all values @@ -175,6 +179,7 @@ def encode_state_with_base_url( "dcr_client_id": dcr_client_id, "dcr_client_secret": dcr_client_secret, "dcr_token_endpoint_auth_method": dcr_token_endpoint_auth_method, + "expected_issuer": expected_issuer, } state_json: Final = json.dumps(state_data, sort_keys=True) encrypted_state: Final = encrypt_value_helper(state_json) @@ -833,6 +838,7 @@ async def authorize_with_server( dcr_token_endpoint_auth_method=ephemeral_dcr_client.token_endpoint_auth_method if ephemeral_dcr_client else None, + expected_issuer=mcp_server.issuer, ) relay_state: Final = secrets.token_urlsafe(_OAUTH_STATE_HANDLE_BYTES) @@ -1892,11 +1898,29 @@ def _render_oauth_error_html(error: str, description: str | None) -> HTMLRespons return HTMLResponse(body, status_code=400) +def _authorization_response_issuer_is_trusted(response_issuer: str | None, state_data: Mapping[str, object]) -> bool: + """RFC 9207: an authorization response may name the authorization server that issued it in + ``iss``. When /authorize sealed the issuer this flow was sent to, an ``iss`` naming a different + issuer means the response came back from an authorization server we never sent the user to, which + is the mix-up attack the parameter exists to catch, so the code must not be forwarded. + + An absent ``iss`` is trusted: most authorization servers still do not send it, and RFC 9207 only + requires rejecting its absence when the server's metadata advertises support, which is not known + at this point in the flow. A state minted before this was sealed carries no expected issuer and + is likewise trusted, so in-flight authorizations survive the upgrade. + """ + expected_issuer: Final = state_data.get("expected_issuer") + if not isinstance(expected_issuer, str) or not expected_issuer or response_issuer is None: + return True + return issuer_identities_match(response_issuer, expected_issuer) + + @router.get("/callback") async def callback( request: Request, code: str | None = None, state: str | None = None, + iss: str | None = None, error: str | None = None, error_description: str | None = None, error_uri: str | None = None, @@ -1907,7 +1931,9 @@ async def callback( - A successful authorization response (``code`` + ``state``), which is forwarded back to the validated client ``redirect_uri`` with the - original (un-wrapped) ``state``. + original (un-wrapped) ``state``, once the RFC 9207 ``iss`` (when the + authorization server sent one) matches the issuer /authorize sealed + into the state. - An error response (``error``[+``error_description``/``error_uri``]), per RFC 6749 §4.1.2.1. When ``state`` is present and decodes to a trusted ``redirect_uri``, the error params are propagated back to the client so @@ -1974,6 +2000,21 @@ async def callback( # states while permitting same-origin / allowlisted clients. redirect_uri = _get_validated_client_redirect_uri(request, state_data) + if not _authorization_response_issuer_is_trusted(iss, state_data): + verbose_logger.warning( + "MCP /callback rejected an authorization response: RFC 9207 iss=%r does not match the " + "issuer this flow was sent to (%r)", + iss, + state_data.get("expected_issuer"), + ) + response = _render_oauth_error_html( + "invalid_issuer", + "This authorization response came from a different identity provider than the one this " + "MCP server is configured to use.", + ) + _clear_oauth_state_cookie(response, request, state) + return response + # Interactive dcr_bridge oauth_delegate: the state carries the litellm user the authorize step # captured. Instead of forwarding the raw upstream code (which the client would present at the # token endpoint with no way to prove who signed in), seal the user and the upstream code into a diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index dbe97dd5bce..79f52bab407 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -76,6 +76,7 @@ from litellm.proxy._experimental.mcp_server.oauth2_token_cache import ( from litellm.proxy._experimental.mcp_server.oauth_utils import ( _redact_mcp_resource_url, canonicalize_url_identity, + issuer_identities_match, ) from litellm.proxy._experimental.mcp_server.outbound_credentials import ( Error, @@ -476,7 +477,7 @@ def _issuer_matches(claimed_issuer: object, configured_issuer: str) -> bool: """ if not isinstance(claimed_issuer, str) or not claimed_issuer: return False - return _normalized_authorize_endpoint(claimed_issuer) == _normalized_authorize_endpoint(configured_issuer) + return issuer_identities_match(claimed_issuer, configured_issuer) def _flow_endpoints_missing( diff --git a/litellm/proxy/_experimental/mcp_server/oauth_utils.py b/litellm/proxy/_experimental/mcp_server/oauth_utils.py index a30b5ee9e49..676a356991d 100644 --- a/litellm/proxy/_experimental/mcp_server/oauth_utils.py +++ b/litellm/proxy/_experimental/mcp_server/oauth_utils.py @@ -633,6 +633,13 @@ def canonicalize_url_identity(url: str) -> str: return urlunparse((scheme, netloc, parsed.path.rstrip("/"), "", "", "")) +def issuer_identities_match(claimed_issuer: str, expected_issuer: str) -> bool: + """Issuer equality tolerant only of URL-insignificant differences (scheme/host case, the default + port, a trailing slash), through the shared canonicalizer. Used for RFC 8414 §3.3 metadata + anchoring and for the RFC 9207 ``iss`` an authorization response carries.""" + return canonicalize_url_identity(claimed_issuer) == canonicalize_url_identity(expected_issuer) + + def canonical_resource_uri(url: str) -> str | None: """Canonicalize an upstream MCP server URL into an RFC 8707 resource identifier. diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index 0dc85c0318c..435b8bdc460 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -75,7 +75,7 @@ from litellm.proxy.litellm_pre_call_utils import ( LiteLLMProxyRequestSetup, get_chain_id_from_headers, ) -from litellm.types.mcp import MCPAuth, MCPSpecVersion +from litellm.types.mcp import MCP_LATEST_SUPPORTED_SPEC_VERSION, MCPAuth from litellm.types.mcp_server.mcp_server_manager import MCPInfo, MCPServer from litellm.types.utils import CallTypes, StandardLoggingMCPToolCall from litellm.utils import Rules, client, function_setup @@ -3980,7 +3980,7 @@ if MCP_AVAILABLE: "id": "litellm-mcp-auth-probe", "method": "initialize", "params": { - "protocolVersion": MCPSpecVersion.jun_2025.value, + "protocolVersion": MCP_LATEST_SUPPORTED_SPEC_VERSION.value, "capabilities": {}, "clientInfo": { "name": "litellm-mcp-auth-probe", diff --git a/litellm/types/mcp.py b/litellm/types/mcp.py index 57437ea7e54..ab5e6538057 100644 --- a/litellm/types/mcp.py +++ b/litellm/types/mcp.py @@ -26,6 +26,13 @@ class MCPSpecVersion(str, enum.Enum): nov_2024 = "2024-11-05" mar_2025 = "2025-03-26" jun_2025 = "2025-06-18" + nov_2025 = "2025-11-25" + + +# The highest MCP spec revision LiteLLM speaks, kept in lockstep with the pinned SDK's +# LATEST_PROTOCOL_VERSION (tests/test_litellm/types/test_mcp.py fails when they diverge). Outbound +# MCP requests LiteLLM builds itself advertise this instead of a hardcoded historical revision. +MCP_LATEST_SUPPORTED_SPEC_VERSION: Final = MCPSpecVersion.nov_2025 class MCPAuth(str, enum.Enum): @@ -51,7 +58,9 @@ DEFAULT_SUBJECT_TOKEN_TYPE: Final = "urn:ietf:params:oauth:token-type:access_tok # MCP Literals MCPTransportType = Literal[MCPTransport.sse, MCPTransport.http, MCPTransport.stdio] -MCPSpecVersionType = Literal[MCPSpecVersion.nov_2024, MCPSpecVersion.mar_2025, MCPSpecVersion.jun_2025] +MCPSpecVersionType = Literal[ + MCPSpecVersion.nov_2024, MCPSpecVersion.mar_2025, MCPSpecVersion.jun_2025, MCPSpecVersion.nov_2025 +] MCPAuthType = ( Literal[ MCPAuth.none, 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 b4d3782ba43..5ab825774b8 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 @@ -4004,6 +4004,153 @@ async def test_callback_error_path_reads_cookie_and_clears_it(monkeypatch): assert cleared[cookie_name]["max-age"] == "0" +def _issuer_anchored_oauth_server(issuer: str | None = "https://idp.example.com"): + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + return MCPServer( + server_id="rfc9207_server", + name="rfc9207", + server_name="rfc9207", + alias="rfc9207", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + client_id="upstream-client-id", + issuer=issuer, + authorization_url="https://idp.example.com/oauth/authorize", + token_url="https://idp.example.com/oauth/token", + ) + + +async def _authorize_then_callback(server, iss, monkeypatch, expected_issuer_override=...): + """Run /authorize for ``server``, then feed the resulting flow back through /callback with the + RFC 9207 ``iss`` the authorization server supposedly returned. Returns (callback_response, + sealed_state_data).""" + from http.cookies import SimpleCookie + from urllib.parse import parse_qs, urlparse + + from fastapi import Request + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _oauth_state_cookie_name, + authorize_with_server, + callback, + decode_state_hash, + encode_state_with_base_url, + ) + + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-test-salt-for-LIT-5940") + client_redirect_uri = "http://127.0.0.1:6274/oauth/callback/debug" + + authorize_request = MagicMock(spec=Request) + authorize_request.base_url = "https://proxy.example.com/" + authorize_request.headers = {} + + authorize_response = await authorize_with_server( + request=authorize_request, + mcp_server=server, + client_id="upstream-client-id", + redirect_uri=client_redirect_uri, + state="client-original-state-9207", + code_challenge="challenge", + code_challenge_method="S256", + ) + handle = parse_qs(urlparse(authorize_response.headers["location"]).query)["state"][0] + jar = SimpleCookie() + jar.load(authorize_response.headers["set-cookie"]) + cookie_name = _oauth_state_cookie_name(handle) + sealed_state = jar[cookie_name].value + if expected_issuer_override is not ...: + # A state minted before the issuer was sealed into it: same shape, key absent. + sealed_state = encode_state_with_base_url( + base_url=client_redirect_uri, + original_state="client-original-state-9207", + client_redirect_uri=client_redirect_uri, + expected_issuer=expected_issuer_override, + ) + + callback_request = MagicMock(spec=Request) + callback_request.base_url = "https://proxy.example.com/" + callback_request.headers = {} + callback_request.cookies = {cookie_name: sealed_state} + + response = await callback( + request=callback_request, + code="upstream-auth-code", + state=handle, + iss=iss, + ) + return response, decode_state_hash(sealed_state) + + +@pytest.mark.asyncio +async def test_authorize_seals_the_issuer_and_callback_accepts_a_matching_rfc9207_iss(monkeypatch): + """LIT-5940: /authorize seals the issuer it sent the user to, and an authorization response + naming that same issuer (modulo URL-insignificant differences) is forwarded to the client.""" + from urllib.parse import parse_qs, urlparse + + response, state_data = await _authorize_then_callback( + _issuer_anchored_oauth_server(), + iss="https://IDP.example.com:443/", + monkeypatch=monkeypatch, + ) + + assert state_data["expected_issuer"] == "https://idp.example.com" + assert response.status_code == 302 + query = parse_qs(urlparse(response.headers["location"]).query) + assert query["code"] == ["upstream-auth-code"] + assert query["state"] == ["client-original-state-9207"] + + +@pytest.mark.asyncio +async def test_callback_rejects_authorization_response_from_a_different_issuer(monkeypatch): + """LIT-5940 / RFC 9207 §2.4: an ``iss`` naming an authorization server we never sent the user to + is a mix-up attack, so the code must not reach the client's redirect_uri.""" + response, _ = await _authorize_then_callback( + _issuer_anchored_oauth_server(), + iss="https://attacker-idp.example.com", + monkeypatch=monkeypatch, + ) + + assert response.status_code == 400 + assert "location" not in response.headers + assert b"upstream-auth-code" not in response.body + assert b"invalid_issuer" in response.body + + +@pytest.mark.asyncio +async def test_callback_forwards_when_issuer_is_unknown_or_iss_absent(monkeypatch): + """Neither an authorization server that omits ``iss`` nor a server row with no issuer configured + can be validated, so both keep the pre-RFC-9207 behavior instead of failing closed.""" + no_iss_response, _ = await _authorize_then_callback( + _issuer_anchored_oauth_server(), + iss=None, + monkeypatch=monkeypatch, + ) + assert no_iss_response.status_code == 302 + + unanchored_response, state_data = await _authorize_then_callback( + _issuer_anchored_oauth_server(issuer=None), + iss="https://whatever-idp.example.com", + monkeypatch=monkeypatch, + ) + assert state_data["expected_issuer"] is None + assert unanchored_response.status_code == 302 + + +@pytest.mark.asyncio +async def test_callback_accepts_states_minted_before_the_issuer_was_sealed(monkeypatch): + """An authorization in flight across the upgrade carries no sealed issuer and must still land.""" + response, _ = await _authorize_then_callback( + _issuer_anchored_oauth_server(), + iss="https://some-idp.example.com", + monkeypatch=monkeypatch, + expected_issuer_override=None, + ) + assert response.status_code == 302 + + @pytest.mark.asyncio async def test_oauth_authorize_includes_scopes_from_server_config(): """Test that authorize endpoint includes scopes from server configuration.""" diff --git a/tests/test_litellm/types/test_mcp.py b/tests/test_litellm/types/test_mcp.py new file mode 100644 index 00000000000..ca768dc79b4 --- /dev/null +++ b/tests/test_litellm/types/test_mcp.py @@ -0,0 +1,34 @@ +"""The advertised MCP spec revisions must stay in lockstep with the pinned MCP SDK. + +``MCP_LATEST_SUPPORTED_SPEC_VERSION`` is what LiteLLM puts on the wire for the MCP requests it +builds itself, and the SDK owns negotiation for everything else, so a revision the SDK gained +without ``MCPSpecVersion`` gaining it means LiteLLM is advertising a version it no longer leads +with. +""" + +from typing import Final, get_args + +from mcp.shared.version import SUPPORTED_PROTOCOL_VERSIONS +from mcp.types import LATEST_PROTOCOL_VERSION + +from litellm.types.mcp import ( + MCP_LATEST_SUPPORTED_SPEC_VERSION, + MCPSpecVersion, + MCPSpecVersionType, +) + + +def test_spec_version_enum_covers_every_sdk_supported_revision(): + known: Final = {member.value for member in MCPSpecVersion} + assert set(SUPPORTED_PROTOCOL_VERSIONS) <= known, ( + "the pinned MCP SDK negotiates a spec revision MCPSpecVersion does not know about; " + "add it to the enum and MCPSpecVersionType" + ) + + +def test_latest_supported_spec_version_is_the_sdk_latest(): + assert MCP_LATEST_SUPPORTED_SPEC_VERSION.value == LATEST_PROTOCOL_VERSION + + +def test_spec_version_literal_mirrors_the_enum(): + assert set(get_args(MCPSpecVersionType)) == set(MCPSpecVersion)