mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-13 08:52:02 +00:00
fix(mcp): accept comma-separated vendor scope grants on the connect check (#50717)
This commit is contained in:
parent
3539c0afc9
commit
e6db6206d1
3 changed files with 150 additions and 3 deletions
|
|
@ -105,9 +105,11 @@ from litellm.proxy.mcp_server.oauth_utils import (
|
|||
get_cimd_client_id,
|
||||
get_cimd_document_url,
|
||||
get_request_base_url,
|
||||
grant_covers_scopes,
|
||||
needs_cimd_discovery,
|
||||
oauth_client_registration_matches,
|
||||
resolve_upstream_resource,
|
||||
server_scope_names,
|
||||
validate_trusted_redirect_uri,
|
||||
well_known_root_suffix,
|
||||
)
|
||||
|
|
@ -2537,6 +2539,9 @@ async def _vendor_credential_state(
|
|||
get_user_oauth_credential,
|
||||
oauth_grant_state,
|
||||
)
|
||||
from litellm.proxy.mcp_server.mcp_server_manager import ( # noqa: PLC0415 # circular import at module load
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
from litellm.proxy.proxy_server import prisma_client # noqa: PLC0415 # circular import at module load
|
||||
|
||||
if prisma_client is None:
|
||||
|
|
@ -2548,7 +2553,12 @@ async def _vendor_credential_state(
|
|||
if oauth_grant_state(credential) == "absent":
|
||||
return "absent"
|
||||
granted_scopes: Final = credential.get("scopes") if credential else None
|
||||
return "present" if set(requested_scopes).issubset(granted_scopes or ()) else "absent"
|
||||
covered: Final = grant_covers_scopes(
|
||||
tuple(granted_scopes or ()),
|
||||
requested_scopes,
|
||||
lambda: server_scope_names(global_mcp_server_manager.get_mcp_server_by_id(server_id)),
|
||||
)
|
||||
return "present" if covered else "absent"
|
||||
|
||||
|
||||
@router.get("/authorize/flow")
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
|
||||
import os
|
||||
import re
|
||||
from collections.abc import Mapping
|
||||
from collections.abc import Callable, Mapping
|
||||
from dataclasses import dataclass
|
||||
from ipaddress import ip_address
|
||||
from types import MappingProxyType
|
||||
|
|
@ -11,6 +11,7 @@ from typing import TYPE_CHECKING, Final, NoReturn
|
|||
from urllib.parse import ParseResult, parse_qsl, urlencode, urlparse, urlsplit, urlunparse, urlunsplit
|
||||
|
||||
from fastapi import HTTPException, Request
|
||||
from pydantic import TypeAdapter
|
||||
from starlette.types import Scope
|
||||
|
||||
from litellm._logging import verbose_logger
|
||||
|
|
@ -131,6 +132,34 @@ def build_upstream_authorize_url(authorization_url: str, params: Mapping[str, st
|
|||
return urlunparse(parsed._replace(query=urlencode(merged)))
|
||||
|
||||
|
||||
def grant_covers_scopes(
|
||||
granted: tuple[str, ...], requested: tuple[str, ...], server_scopes: Callable[[], frozenset[str]]
|
||||
) -> bool:
|
||||
"""Whether a stored grant holds every requested scope. A grant stored as one comma-joined token, the form
|
||||
Slack and GitHub reply with, counts as its parts only when every part is a requested or server scope."""
|
||||
if all(scope in granted for scope in requested):
|
||||
return True
|
||||
if len(granted) != 1 or "," not in granted[0]:
|
||||
return False
|
||||
parts: Final = frozenset(granted[0].split(","))
|
||||
known: Final = frozenset(requested) | server_scopes()
|
||||
return parts <= known and all(scope in granted or scope in parts for scope in requested)
|
||||
|
||||
|
||||
_PUBLISHED_SCOPES: Final = TypeAdapter[tuple[str, ...] | None](tuple[str, ...] | None)
|
||||
|
||||
|
||||
def server_scope_names(server: "MCPServer | None") -> frozenset[str]:
|
||||
if server is None:
|
||||
return frozenset()
|
||||
published: Final = (
|
||||
_PUBLISHED_SCOPES.validate_python(server.authorization_server_metadata.get("scopes_supported"))
|
||||
if server.authorization_server_metadata is not None
|
||||
else None
|
||||
)
|
||||
return frozenset((*(server.scopes or ()), *(published or ())))
|
||||
|
||||
|
||||
def proxy_base_url_from_env() -> str | None:
|
||||
"""The gateway's fixed public URL from ``PROXY_BASE_URL``, ``None`` when unset or malformed."""
|
||||
return _resolve_proxy_base_url_env()
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ from litellm.proxy._types import LiteLLM_MCPServerTable
|
|||
from litellm.types.mcp import MCPAuth
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Iterator
|
||||
from collections.abc import Iterator, Mapping
|
||||
|
||||
import httpx
|
||||
from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey
|
||||
|
|
@ -15278,6 +15278,9 @@ async def test_managed_resource_metadata_excludes_refresh_permission(monkeypatch
|
|||
(["tools.read", "tools.write"], ("tools.write",), "present"),
|
||||
(None, ("tools.write",), "absent"),
|
||||
(["tools.read"], (), "present"),
|
||||
(["channels:read,chat:write"], ("channels:read", "chat:write"), "present"),
|
||||
(["channels:read,chat:write"], ("channels:read", "files:write"), "absent"),
|
||||
(["channels:read"], ("channels:read", "chat:write"), "absent"),
|
||||
],
|
||||
)
|
||||
async def test_connect_requires_the_requested_vendor_grant(monkeypatch, granted, required, expected):
|
||||
|
|
@ -15293,6 +15296,111 @@ async def test_connect_requires_the_requested_vendor_grant(monkeypatch, granted,
|
|||
read.assert_awaited_once_with(prisma, "owner", "server")
|
||||
|
||||
|
||||
_SLACK_SCOPES: Final = frozenset(("channels:read", "chat:write", "files:write"))
|
||||
_COMMA_NAMED_SCOPES: Final = frozenset(("files:read", "files:read,write"))
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("granted", "requested", "known", "expected"),
|
||||
[
|
||||
(("channels:read,chat:write",), ("channels:read", "chat:write"), _SLACK_SCOPES, True),
|
||||
(("channels:read,chat:write",), ("channels:read", "files:write"), _SLACK_SCOPES, False),
|
||||
(("channels:read,chat:write,files:read",), ("channels:read",), _SLACK_SCOPES, False),
|
||||
(("x:unknown,chat:write",), ("chat:write",), _SLACK_SCOPES, False),
|
||||
(("files:read,write",), ("files:read",), _COMMA_NAMED_SCOPES, False),
|
||||
(("files:read,write",), ("files:read,write",), _COMMA_NAMED_SCOPES, True),
|
||||
(("channels:read,chat:write",), ("read",), _SLACK_SCOPES | {"read"}, False),
|
||||
(("read", "write,admin"), ("admin",), frozenset(("read", "write", "admin")), False),
|
||||
(("write,admin", "read"), ("admin",), frozenset(("read", "write", "admin")), False),
|
||||
(("read", "write"), ("read", "write"), frozenset[str](), True),
|
||||
((), ("read",), frozenset(("read",)), False),
|
||||
((), (), frozenset[str](), True),
|
||||
],
|
||||
)
|
||||
def test_grant_covers_scopes_splits_a_lone_token_only_into_known_scopes(
|
||||
granted: tuple[str, ...], requested: tuple[str, ...], known: frozenset[str], expected: bool
|
||||
) -> None:
|
||||
from litellm.proxy.mcp_server.oauth_utils import grant_covers_scopes
|
||||
|
||||
assert grant_covers_scopes(granted, requested, lambda: known) is expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("granted", "requested", "expected"),
|
||||
[
|
||||
(("channels:read",), ("channels:read",), True),
|
||||
(("read", "write"), ("read", "write"), True),
|
||||
(("read", "write"), ("admin",), False),
|
||||
(("read",), ("admin",), False),
|
||||
((), (), True),
|
||||
],
|
||||
)
|
||||
def test_grant_covers_scopes_answers_like_a_subset_check_without_reading_server_scopes(
|
||||
granted: tuple[str, ...], requested: tuple[str, ...], expected: bool
|
||||
) -> None:
|
||||
from litellm.proxy.mcp_server.oauth_utils import grant_covers_scopes
|
||||
|
||||
def unreachable_server_scopes() -> frozenset[str]:
|
||||
raise AssertionError("server scopes read for a grant with no comma token")
|
||||
|
||||
assert grant_covers_scopes(granted, requested, unreachable_server_scopes) is expected
|
||||
|
||||
|
||||
def test_server_scope_names_joins_configured_and_published_scopes() -> None:
|
||||
from litellm.proxy.mcp_server.oauth_utils import server_scope_names
|
||||
|
||||
server: Final = _cimd_oauth_server().model_copy(
|
||||
update={
|
||||
"scopes": ["channels:read"],
|
||||
"authorization_server_metadata": {
|
||||
"issuer": "https://idp.example.com",
|
||||
"authorization_endpoint": "https://idp.example.com/authorize",
|
||||
"token_endpoint": "https://idp.example.com/token",
|
||||
"response_types_supported": ["code"],
|
||||
"scopes_supported": ["chat:write", "files:write"],
|
||||
},
|
||||
}
|
||||
)
|
||||
assert server_scope_names(server) == frozenset(("channels:read", "chat:write", "files:write"))
|
||||
assert server_scope_names(_cimd_oauth_server()) == frozenset[str]()
|
||||
assert server_scope_names(None) == frozenset[str]()
|
||||
|
||||
|
||||
_PUBLISHED_SLACK_METADATA: Final = {
|
||||
"issuer": "https://idp.example.com",
|
||||
"authorization_endpoint": "https://idp.example.com/authorize",
|
||||
"token_endpoint": "https://idp.example.com/token",
|
||||
"response_types_supported": ["code"],
|
||||
"scopes_supported": ["channels:read", "chat:write", "files:read"],
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"server_update",
|
||||
[
|
||||
{"scopes": ["channels:read", "chat:write", "files:read"]},
|
||||
{"authorization_server_metadata": _PUBLISHED_SLACK_METADATA},
|
||||
],
|
||||
)
|
||||
async def test_connect_reads_a_comma_grant_against_the_server_scopes(
|
||||
monkeypatch: pytest.MonkeyPatch, server_update: "Mapping[str, object]"
|
||||
) -> None:
|
||||
from litellm.proxy.mcp_server import db
|
||||
from litellm.proxy.mcp_server import discoverable_endpoints as endpoints
|
||||
from litellm.proxy.mcp_server.mcp_server_manager import global_mcp_server_manager
|
||||
|
||||
server: Final = _cimd_oauth_server().model_copy(update=server_update)
|
||||
credential: Final = {**_stored_grant(), "scopes": ["channels:read,chat:write,files:read"]}
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", MagicMock())
|
||||
monkeypatch.setattr(db, "get_user_oauth_credential", AsyncMock(return_value=credential))
|
||||
assert await endpoints._vendor_credential_state("owner", server.server_id, ("channels:read",)) == "absent"
|
||||
monkeypatch.setitem(global_mcp_server_manager.registry, server.server_id, server)
|
||||
assert await endpoints._vendor_credential_state("owner", server.server_id, ("channels:read",)) == "present"
|
||||
assert await endpoints._vendor_credential_state("owner", server.server_id, ("files:write",)) == "absent"
|
||||
assert await endpoints._vendor_credential_state("owner", server.server_id, ()) == "present"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("header", "expected"),
|
||||
[
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue