mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
feat(mcp): advertise the SDK's latest spec revision and validate the RFC 9207 iss
MCPSpecVersion stopped at 2025-06-18 while the pinned SDK negotiates 2025-11-25, and the version LiteLLM puts on its own outbound initialize was a hardcoded historical member. Add the missing revision, name the highest revision we speak once, and pin it to the SDK's LATEST_PROTOCOL_VERSION with a test so the two cannot drift apart silently. /authorize now seals the issuer it sent the user to into the OAuth state, and /callback holds the authorization response's RFC 9207 iss against it, refusing to forward a code that came back from an authorization server we never sent the user to. An absent iss, an unanchored server row and a state minted before the seal all keep their current behavior. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
e17988f4fe
commit
8bcfb1a080
7 changed files with 244 additions and 5 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
34
tests/test_litellm/types/test_mcp.py
Normal file
34
tests/test_litellm/types/test_mcp.py
Normal file
|
|
@ -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)
|
||||
Loading…
Add table
Reference in a new issue