scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-08 03:08:45 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 115

fix(mcp): keep scope selection resource-driven, not authorization-server-driven

Browse source 
Reverts the over-correction that restricted a pinned-authorization_url server's
discovered scopes to the authorization server's own scopes_supported. Per the MCP
authorization spec Scope Selection Strategy and RFC 9700 §2.3, the scopes a client
requests are resource-driven: the WWW-Authenticate 401 challenge scope, else the
RFC 9728 protected-resource scopes_supported. The authorization server's RFC 8414
scopes_supported is a non-exhaustive capability list (the server MAY omit supported
scopes) and is never the selection source; scope inflation by a compromised resource
is bounded by the authorization server and user consent (RFC 6749 §3.3), not by the
client restricting the request. The corroboration gate now rejects only the
uncorroborated token_url/registration_url (the RFC 9700 endpoint mix-up) and leaves
scopes untouched. Removes the now-unused authorization_server_scopes field.
This commit is contained in:
Tin Chi Lo 2026-07-15 16:27:15 -07:00
parent feedab214e
commit e4a6516b49
3 changed files with 60 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
View file

@ -284,24 +284,26 @@ def _restrict_discovery_to_corroborated_authorization_server(
server_identifier: str,
is_dcr_bridge: bool,
) -> MCPOAuthMetadata | None:
"""Bound what freshly discovered metadata may backfill into a manually pinned config.
"""Reject discovered token/registration endpoints a manually pinned authorize endpoint cannot
vouch for (the RFC 9700 authorization-server mix-up).
Discovery is rooted at the MCP resource, so provenance is a property of the whole metadata
document, not per field: a compromised upstream can advertise both an attacker ``token_endpoint``
(the RFC 9700 mix-up) and inflated ``scopes`` (tricking the user into granting a broader token
that then flows to the upstream). Both are closed by one rule. When ``authorization_url`` is
admin-pinned, the discovered ``token_url`` and ``registration_url`` are kept only if the document
corroborates the pin (its ``authorization_endpoint`` matches), and scopes are taken from the
authorization server's own ``scopes_supported`` (``authorization_server_scopes``, trusted tier)
rather than the resource-advertised ``scopes`` a compromised upstream controls. A document that
does not corroborate backfills nothing. With no pin there is no trust anchor to protect and the
authorize endpoint comes from the same chain as everything else, so discovery is returned as-is.
Discovery is rooted at the MCP resource, so a compromised upstream can advertise an attacker
``token_endpoint``: with ``authorization_url`` admin-pinned but ``token_url`` blank, the merge
would pair the trusted authorize endpoint with that attacker token endpoint, and the gateway would
post the authorization code and client secret there. So the discovered ``token_url`` and
``registration_url`` are kept only if the document corroborates the pin (its
``authorization_endpoint`` matches). ``scopes`` are deliberately NOT gated here: per the MCP
authorization spec Scope Selection Strategy and RFC 9700 §2.3, the scopes a client requests are
resource-driven (the WWW-Authenticate challenge or the RFC 9728 protected-resource
``scopes_supported``), and scope inflation by a compromised resource is bounded by the
authorization server and user consent (RFC 6749 §3.3), not by the client second-guessing the
request. With no pin there is no trust anchor to protect, so discovery is returned as-is.
"""
if metadata is None or not (manual_authorization_url and manual_authorization_url.strip()):
return metadata
if _endpoints_corroborate_authorization_url(metadata.authorization_url, manual_authorization_url):
return metadata.model_copy(update={"scopes": metadata.authorization_server_scopes})
if not metadata.token_url and not metadata.registration_url and not metadata.scopes:
return metadata
if not metadata.token_url and not metadata.registration_url:
return metadata
bridge_note = (
" The discovered registration_url is rejected with it, so this dcr_bridge server stays on the"
@ -311,15 +313,15 @@ def _restrict_discovery_to_corroborated_authorization_server(
)
verbose_logger.warning(
"MCP OAuth discovery for server %s advertised authorization_endpoint %s, which does not match the "
"manually configured authorization_url %s; rejecting the discovered token_url/registration_url/scopes "
"so authorization codes, client credentials, and granted scopes only follow the configured "
"authorization server. Configure Token URL and Scopes manually if the mismatch is intentional.%s",
"manually configured authorization_url %s; rejecting the discovered token_url/registration_url so "
"authorization codes and client credentials only follow the configured authorization server. "
"Configure Token URL manually if the mismatch is intentional.%s",
server_identifier,
_normalized_authorize_endpoint(metadata.authorization_url) if metadata.authorization_url else "<absent>",
_normalized_authorize_endpoint(manual_authorization_url),
bridge_note,
)
return metadata.model_copy(update={"token_url": None, "registration_url": None, "scopes": None})
return metadata.model_copy(update={"token_url": None, "registration_url": None})
def invalidate_user_env_vars_cache(user_id: str, server_id: str) -> None:
@ -3394,7 +3396,6 @@ class MCPServerManager:
authorization_url=data.get("authorization_endpoint"),
token_url=data.get("token_endpoint"),
registration_url=data.get("registration_endpoint"),
authorization_server_scopes=scopes,
)
if any(

15
litellm/types/mcp_server/mcp_server_manager.py
View file

@ -17,18 +17,15 @@ MCPInfo = Dict[str, Any]
class MCPOAuthMetadata(BaseModel):
scopes: Optional[List[str]] = None
"""Effective scopes, resource-preferred: the RFC 9728 protected-resource advertisement or the
WWW-Authenticate challenge when the resource supplied one, else the authorization server's
``scopes_supported``. A compromised resource server can influence this, so it must not expand a
manually pinned ``authorization_url`` (see ``authorization_server_scopes``)."""
"""Resource-driven scopes for the authorization request: the RFC 9728 protected-resource
``scopes_supported``, or the ``scope`` from the WWW-Authenticate 401 challenge when the resource
supplied one, else the authorization server's ``scopes_supported``. This is the scope value a
client requests per the MCP authorization spec Scope Selection Strategy; scope minimization and
inflation control are the authorization server's and user's job at consent (RFC 6749 §3.3), not
the client's."""
authorization_url: Optional[str] = None
token_url: Optional[str] = None
registration_url: Optional[str] = None
authorization_server_scopes: Optional[List[str]] = None
"""The ``scopes_supported`` enumerated by the authorization-server metadata document itself
(RFC 8414), independent of anything the resource server advertised. This is the only scope
source trusted to backfill a manually pinned ``authorization_url``, because it shares provenance
with the ``authorization_endpoint`` used to corroborate that pin."""
from_origin_fallback: bool = False
"""True when the metadata came from guessing the resource origin as its authorization
server rather than from an RFC 9728/8414-advertised document. Guessed endpoints are

77
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py
View file

@ -357,18 +357,18 @@ class TestMCPServerManager:
assert server.needs_user_oauth_token is True
@pytest.mark.asyncio
async def test_load_servers_from_config_rejects_uncorroborated_discovery_including_scopes(self):
"""The config loader always runs discovery and or-merges per field, so a yaml server with a
manual authorization_url has the same config-time mix-up exposure as a DB row: a document
advertising a different authorize endpoint backfills nothing, neither its token_url nor its
scopes."""
async def test_load_servers_from_config_rejects_uncorroborated_endpoints_but_keeps_resource_scopes(self):
"""A yaml server with a manual authorization_url has the same config-time mix-up exposure as a
DB row: a document advertising a different authorize endpoint has its token_url rejected. The
resource-driven scopes are kept, because scope selection is resource-driven (MCP Scope
Selection Strategy) and scope inflation is bounded by the authorization server at consent, not
by dropping scopes when an endpoint mismatches."""
manager = MCPServerManager()
metadata = MCPOAuthMetadata(
authorization_url="https://attacker.example.com/authorize",
token_url="https://attacker.example.com/token",
scopes=["read", "admin"],
authorization_server_scopes=["read", "admin"],
)
config = self._oauth2_config(
oauth2_flow="authorization_code",
@ -381,20 +381,20 @@ class TestMCPServerManager:
server = next(iter(manager.config_mcp_servers.values()))
assert server.authorization_url == "https://idp.example.com/authorize"
assert server.token_url is None
assert server.scopes is None
assert server.scopes == ["read", "admin"]
@pytest.mark.asyncio
async def test_load_servers_from_config_fills_token_url_when_metadata_corroborates_manual_authorization_url(self):
"""Corroborated metadata keeps the self-heal on the config path: when the discovered
document advertises the same authorize endpoint the admin pinned, its token_url fills the
blank field and scopes come from the authorization server's own scopes_supported."""
"""Corroborated metadata keeps the self-heal on the config path: when the discovered document
advertises the same authorize endpoint the admin pinned, its token_url fills the blank field
and scopes come through resource-driven (the discovered document's resource-preferred scopes),
not the authorization server's own capability list."""
manager = MCPServerManager()
metadata = MCPOAuthMetadata(
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
scopes=["read", "admin"],
authorization_server_scopes=["read"],
)
config = self._oauth2_config(
oauth2_flow="authorization_code",
@ -406,7 +406,7 @@ class TestMCPServerManager:
server = next(iter(manager.config_mcp_servers.values()))
assert server.token_url == "https://idp.example.com/token"
assert server.scopes == ["read"]
assert server.scopes == ["read", "admin"]
@pytest.mark.asyncio
@pytest.mark.parametrize("blank_authorization_url", ["", " "])
@ -422,7 +422,6 @@ class TestMCPServerManager:
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
scopes=["read"],
authorization_server_scopes=["read"],
)
config = self._oauth2_config(
oauth2_flow="authorization_code",
@ -1134,12 +1133,13 @@ class TestMCPServerManager:
assert built.token_url == "https://idp.example.com/token"
@pytest.mark.asyncio
async def test_build_from_table_backfills_scopes_from_authorization_server_not_resource(self):
"""When authorization_url is admin-pinned, scopes backfill from the authorization server's
own scopes_supported (trusted tier), never from the resource-advertised scopes a compromised
upstream controls. Here the corroborating document carries an inflated resource `scopes`
(`admin`) alongside the real authorization_server_scopes; only the latter may be requested,
otherwise a hostile resource could trick the user into granting a broader token."""
async def test_build_from_table_backfills_resource_driven_scopes_for_pinned_authorization_url(self):
"""When authorization_url is admin-pinned and corroborated, scopes backfill as the
resource-driven value (the WWW-Authenticate challenge scope, else the RFC 9728
protected-resource scopes_supported), per the MCP authorization spec Scope Selection Strategy.
The client does not restrict scopes to the authorization server's own scopes_supported; scope
minimization and inflation control are the authorization server's and user's job at consent
(RFC 6749 §3.3)."""
manager = MCPServerManager()
row = LiteLLM_MCPServerTable(
server_id="manual-auth-url-1",
@ -1157,7 +1157,6 @@ class TestMCPServerManager:
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
scopes=["read", "admin"],
authorization_server_scopes=["read", "write"],
)
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)
@ -1165,7 +1164,7 @@ class TestMCPServerManager:
mock_discovery.assert_awaited_once()
assert built.authorization_url == "https://idp.example.com/authorize"
assert built.token_url == "https://idp.example.com/token"
assert built.scopes == ["read", "write"]
assert built.scopes == ["read", "admin"]
@pytest.mark.asyncio
async def test_build_from_table_whitespace_authorization_url_is_not_a_pin(self):
@ -1190,7 +1189,6 @@ class TestMCPServerManager:
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
scopes=["read"],
authorization_server_scopes=["read"],
)
with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)):
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
@ -1223,7 +1221,6 @@ class TestMCPServerManager:
token_url="https://idp.example.com/token",
registration_url="https://idp.example.com/register",
scopes=["read"],
authorization_server_scopes=["read"],
)
with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)):
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
@ -1238,15 +1235,17 @@ class TestMCPServerManager:
"advertised_authorization_url",
["https://attacker.example.com/authorize", None],
)
async def test_build_from_table_rejects_uncorroborated_discovery_including_scopes(
async def test_build_from_table_rejects_uncorroborated_endpoints_but_keeps_resource_scopes(
self, advertised_authorization_url
):
"""Resource-rooted discovery lets a compromised upstream advertise its own authorization
server. With a manual authorization_url pinned, a document that does not corroborate it
backfills nothing: accepting its token_url would send the code, client secret, and PKCE
verifier to the attacker (config-time RFC 9700 mix-up), and accepting its scopes would let
the upstream inflate the granted token. Both the in-memory merge and the persisted metadata
must drop the uncorroborated token_url, registration_url, and scopes."""
server. With a manual authorization_url pinned, a document that does not corroborate it has
its token_url and registration_url dropped: accepting them would send the code, client secret,
and PKCE verifier to the attacker (config-time RFC 9700 mix-up). The resource-driven scopes
are kept, because scope selection is resource-driven (MCP Scope Selection Strategy) and scope
inflation is bounded by the authorization server at consent (RFC 6749 §3.3), not by dropping
scopes on an endpoint mismatch. Both the in-memory merge and the persisted metadata drop only
the uncorroborated endpoints."""
manager = MCPServerManager()
row = LiteLLM_MCPServerTable(
server_id="manual-auth-url-3",
@ -1265,7 +1264,6 @@ class TestMCPServerManager:
token_url="https://attacker.example.com/token",
registration_url="https://attacker.example.com/register",
scopes=["read", "admin"],
authorization_server_scopes=["read", "admin"],
)
with (
patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)),
@ -1276,11 +1274,11 @@ class TestMCPServerManager:
assert built.authorization_url == "https://idp.example.com/authorize"
assert built.token_url is None
assert built.registration_url is None
assert built.scopes is None
assert built.scopes == ["read", "admin"]
persisted_metadata = mock_persist.await_args.kwargs["metadata"]
assert persisted_metadata.token_url is None
assert persisted_metadata.registration_url is None
assert persisted_metadata.scopes is None
assert persisted_metadata.scopes == ["read", "admin"]
@pytest.mark.asyncio
async def test_build_from_table_skips_discovery_when_all_upstream_oauth_fields_present(self):
@ -2384,11 +2382,11 @@ class TestMCPServerManager:
assert result.from_origin_fallback is False
@pytest.mark.asyncio
async def test_descovery_metadata_preserves_authorization_server_scopes_under_resource_override(self):
"""The effective `scopes` field is resource-preferred (RFC 9728 / WWW-Authenticate), but the
authorization server's own scopes_supported must survive on `authorization_server_scopes` so
the pinned-config backfill can request the trusted-tier scopes instead of resource-advertised
ones. This is the provenance split the scope-inflation defense depends on."""
async def test_descovery_metadata_scopes_are_resource_driven(self):
"""The effective `scopes` are resource-driven: the RFC 9728 protected-resource advertisement
(or WWW-Authenticate challenge) overrides the authorization server's own scopes_supported. This
is the MCP Scope Selection Strategy: the client requests what the resource needs, not the AS's
full capability list."""
manager = MCPServerManager()
mock_response = MagicMock()
@ -2400,7 +2398,6 @@ class TestMCPServerManager:
scopes=["as.read", "as.write"],
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
authorization_server_scopes=["as.read", "as.write"],
)
with (
@ -2423,7 +2420,6 @@ class TestMCPServerManager:
assert result is not None
assert result.scopes == ["resource.only"]
assert result.authorization_server_scopes == ["as.read", "as.write"]
@pytest.mark.asyncio
async def test_fetch_single_authorization_server_metadata_supports_azure_issuer_path(
@ -2465,9 +2461,6 @@ class TestMCPServerManager:
assert result.authorization_url == "https://login.microsoftonline.com/test-tenant-id/oauth2/v2.0/authorize"
assert result.token_url == "https://login.microsoftonline.com/test-tenant-id/oauth2/v2.0/token"
assert result.scopes == ["api://some-scope/.default"]
# The authorization server's own scopes_supported is retained under a dedicated field so a
# later resource-scope override cannot erase the trusted-tier value used to backfill a pin.
assert result.authorization_server_scopes == ["api://some-scope/.default"]
@pytest.mark.asyncio
async def test_fetch_single_authorization_server_metadata_derives_azure_metadata(