mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
fix(mcp): challenge gateway-owned per-server authentication
This commit is contained in:
parent
f529d6d6bd
commit
1bcb587d33
5 changed files with 75 additions and 70 deletions
|
|
@ -179,16 +179,7 @@ def _gateway_dcr_challenge_target(
|
|||
mcp_servers: list[str] | None,
|
||||
client_ip: str | None,
|
||||
) -> str | None:
|
||||
"""The single path-named server this request targets, iff it resolves to a
|
||||
gateway-managed oauth2 server — the one per-server shape the gateway's own keyless
|
||||
DCR flow serves end to end, so the 401 challenge may advertise the per-server
|
||||
protected-resource metadata (whose ``authorization_servers`` names the gateway).
|
||||
|
||||
Multi-server CSV paths, header/path mismatches, unknown names, and every
|
||||
client-forwarded or delegated mode return ``None``: those cells keep their existing
|
||||
challenge (or absence of one), and a challenge is never emitted for a name the
|
||||
public discovery routes would 404, so this reveals exactly the server set the
|
||||
per-server protected-resource metadata already reveals."""
|
||||
"""Resolve a single path target whose sign-in metadata advertises the gateway."""
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
|
|
@ -217,7 +208,7 @@ def _is_gateway_dcr_challenge_scope(
|
|||
the caller is not a cold-start DCR client), on the scopes the gateway's keyless
|
||||
flow serves: the aggregate ``/mcp`` endpoint, an ``x-mcp-servers``-scoped request
|
||||
(the resource the client configured is still ``/mcp``), or a per-server path whose
|
||||
single target is a gateway-managed oauth2 server. Every other named target keeps
|
||||
single target advertises gateway-owned sign-in. Every other named target keeps
|
||||
its existing behavior, failing closed to the original admission error."""
|
||||
if not _is_litellm_auth_admission_error(exc):
|
||||
return False
|
||||
|
|
@ -236,7 +227,7 @@ def _gateway_dcr_challenge(
|
|||
) -> HTTPException:
|
||||
"""The RFC 9728 challenge pointing the client at the protected-resource metadata
|
||||
matching the scope it requested: the per-server document (same URL spelling the
|
||||
request arrived on) when the single target is a gateway-managed oauth2 server,
|
||||
request arrived on) when the single target advertises gateway-owned sign-in,
|
||||
else the gateway's aggregate document. Either way the client discovers the gateway
|
||||
as its authorization server and starts the same sign-in flow.
|
||||
|
||||
|
|
|
|||
|
|
@ -2310,8 +2310,7 @@ async def _build_oauth_protected_resource_response(
|
|||
it. Only the legacy ``is_oauth_passthrough`` opt-in rewrites ``resource`` to
|
||||
the gateway's own URL so clients present the bearer token back to the gateway.
|
||||
|
||||
An explicitly named gateway-managed oauth2 server (interactive with
|
||||
gateway-vaulted per-user tokens, or M2M) advertises the gateway's own
|
||||
An explicitly named server with gateway-owned sign-in advertises the gateway's own
|
||||
authorization server (``{base}/mcp``): a keyless DCR client that configured the
|
||||
per-server URL completes the same sign-in flow the aggregate ``/mcp`` endpoint
|
||||
supports and is admitted with a gateway session bearer. The per-server relay
|
||||
|
|
@ -2401,11 +2400,6 @@ async def _build_oauth_protected_resource_response(
|
|||
if obo_response is not None:
|
||||
return obo_response
|
||||
|
||||
# An OBO server with no configured issuer falls through to the gateway default so discovery still
|
||||
# returns metadata; every other non-oauth2 named server 404s to avoid enumeration.
|
||||
if mcp_server is None or mcp_server.auth_type != MCPAuth.oauth2_token_exchange:
|
||||
_raise_unless_oauth2_discovery_server(mcp_server, mcp_server_name, "not an OAuth-protected resource")
|
||||
|
||||
if explicitly_named and mcp_server is not None and mcp_server.advertises_gateway_authorization_server:
|
||||
return {
|
||||
"authorization_servers": [f"{request_base_url}/mcp"],
|
||||
|
|
@ -2413,6 +2407,9 @@ async def _build_oauth_protected_resource_response(
|
|||
"scopes_supported": (mcp_server.scopes if mcp_server.scopes else []),
|
||||
}
|
||||
|
||||
if mcp_server is None or mcp_server.auth_type != MCPAuth.oauth2_token_exchange:
|
||||
_raise_unless_oauth2_discovery_server(mcp_server, mcp_server_name, "not an OAuth-protected resource")
|
||||
|
||||
return {
|
||||
"authorization_servers": [
|
||||
(f"{request_base_url}/{mcp_server_name}" if mcp_server_name else f"{request_base_url}")
|
||||
|
|
|
|||
|
|
@ -250,7 +250,23 @@ class MCPServer(BaseModel):
|
|||
@property
|
||||
def advertises_gateway_authorization_server(self) -> bool:
|
||||
"""Whether named discovery should advertise the aggregate gateway authorization server."""
|
||||
return self.is_gateway_managed_oauth2 and not self.uses_per_server_oauth_relay
|
||||
if self.auth_type == MCPAuth.oauth2:
|
||||
return self.is_gateway_managed_oauth2 and not self.uses_per_server_oauth_relay
|
||||
if self.auth_type not in (
|
||||
None,
|
||||
MCPAuth.none,
|
||||
MCPAuth.api_key,
|
||||
MCPAuth.bearer_token,
|
||||
MCPAuth.basic,
|
||||
MCPAuth.authorization,
|
||||
MCPAuth.token,
|
||||
MCPAuth.aws_sigv4,
|
||||
):
|
||||
return False
|
||||
return not any(
|
||||
header.lower() in ("authorization", "x-api-key", "api-key", "apikey")
|
||||
for header in (self.extra_headers or ())
|
||||
)
|
||||
|
||||
@property
|
||||
def is_true_passthrough(self) -> bool:
|
||||
|
|
|
|||
|
|
@ -7193,14 +7193,13 @@ class TestAggregateGatewayDcrChallenge:
|
|||
www_authenticate = (exc_info.value.headers or {})["WWW-Authenticate"]
|
||||
assert www_authenticate == f"Bearer {self._EXPECTED_RESOURCE_METADATA}"
|
||||
|
||||
async def test_per_server_challenge_for_gateway_managed_oauth2(self):
|
||||
"""Anonymous request to a per-server path whose single target is a gateway-managed
|
||||
oauth2 server: 401 plus the RFC 9728 challenge advertising the PER-SERVER
|
||||
protected-resource metadata in the same URL spelling the request used, so a keyless
|
||||
DCR client configured with either per-server spelling discovers the gateway as the
|
||||
authorization server (LIT-4864). Covers interactive and M2M, which the gateway can
|
||||
both serve end to end."""
|
||||
from litellm.types.mcp import MCPAuth
|
||||
@pytest.mark.parametrize(
|
||||
"auth_type",
|
||||
(None, "none", "api_key", "bearer_token", "basic", "aws_sigv4", "authorization", "token", "oauth2"),
|
||||
)
|
||||
@pytest.mark.parametrize("bearer_presented", (False, True))
|
||||
async def test_per_server_challenge_for_gateway_owned_auth(self, auth_type, bearer_presented):
|
||||
"""Gateway admission challenges are independent of upstream authentication."""
|
||||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
server = MCPServer(
|
||||
|
|
@ -7209,7 +7208,7 @@ class TestAggregateGatewayDcrChallenge:
|
|||
server_name="github",
|
||||
url="https://upstream.example/mcp",
|
||||
transport="http",
|
||||
auth_type=MCPAuth.oauth2,
|
||||
auth_type=auth_type,
|
||||
)
|
||||
for path, expected_metadata_path in (
|
||||
("/mcp/github", "/.well-known/oauth-protected-resource/mcp/github"),
|
||||
|
|
@ -7223,10 +7222,16 @@ class TestAggregateGatewayDcrChallenge:
|
|||
):
|
||||
mock_mgr.get_mcp_server_by_name.return_value = server
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await MCPRequestHandler.process_mcp_request(self._scope(path=path))
|
||||
await MCPRequestHandler.process_mcp_request(
|
||||
self._scope(
|
||||
path=path,
|
||||
extra_headers=((b"authorization", b"Bearer invalid-key"),) if bearer_presented else (),
|
||||
)
|
||||
)
|
||||
assert exc_info.value.status_code == 401
|
||||
www_authenticate = (exc_info.value.headers or {})["WWW-Authenticate"]
|
||||
assert www_authenticate == f'Bearer resource_metadata="http://testserver{expected_metadata_path}"'
|
||||
error = 'error="invalid_token", ' if bearer_presented else ""
|
||||
assert www_authenticate == f'Bearer {error}resource_metadata="http://testserver{expected_metadata_path}"'
|
||||
|
||||
async def test_per_server_challenge_keeps_spelling_under_server_root_path(self):
|
||||
"""On a sub-path deployment the challenge must still advertise the spelling the client
|
||||
|
|
@ -7303,10 +7308,7 @@ class TestAggregateGatewayDcrChallenge:
|
|||
)
|
||||
|
||||
def test_challenge_target_excludes_every_non_gateway_managed_mode(self):
|
||||
"""Unit pin of the challenge-target owner: only a resolved gateway-managed oauth2
|
||||
target (interactive or M2M) yields a per-server challenge; delegate-auth oauth2
|
||||
(whose keyless flow is upstream PKCE via the relay), every client-forwarded auth
|
||||
type, OBO, api_key, unknown names, and CSV paths yield None (LIT-4864)."""
|
||||
"""Gateway challenges exclude unresolved, delegated, and client-forwarded targets."""
|
||||
from litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp import (
|
||||
_gateway_dcr_challenge_target,
|
||||
)
|
||||
|
|
@ -7333,7 +7335,11 @@ class TestAggregateGatewayDcrChallenge:
|
|||
(_server(MCPAuth.true_passthrough), None),
|
||||
(_server(MCPAuth.oauth_delegate), None),
|
||||
(_server(MCPAuth.oauth_delegate, dcr_bridge=True), None),
|
||||
(_server(MCPAuth.api_key), None),
|
||||
(_server(MCPAuth.api_key), "srv"),
|
||||
(_server(MCPAuth.none, extra_headers=["Authorization"]), None),
|
||||
(_server(None, extra_headers=["X-API-Key"]), None),
|
||||
(_server(MCPAuth.none, extra_headers=["Authorization"], oauth_passthrough=True), None),
|
||||
(_server(MCPAuth.oauth2_id_jag), None),
|
||||
(None, None),
|
||||
]
|
||||
for resolved, expected in cases:
|
||||
|
|
|
|||
|
|
@ -7651,12 +7651,6 @@ async def test_token_endpoint_client_secret_basic_without_secret_returns_400():
|
|||
assert exc_info.value.status_code == 400
|
||||
|
||||
|
||||
# -------------------------------------------------------------------
|
||||
# Non-oauth2 (auth_type=none, access-group gated) servers must not be
|
||||
# driven through the gateway OAuth authorize/token/register/discovery
|
||||
# flow, and must not be advertised as OAuth-protected in discovery docs.
|
||||
# -------------------------------------------------------------------
|
||||
|
||||
|
||||
def _access_group_none_server(server_name="access_group_server"):
|
||||
"""A non-oauth2, access-group gated MCP server: no client_id, no OAuth."""
|
||||
|
|
@ -7794,35 +7788,38 @@ async def test_register_client_rejects_non_oauth2_server():
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_oauth_protected_resource_404_for_non_oauth2_server():
|
||||
"""Discovery must not advertise a none-auth server as an OAuth-protected resource."""
|
||||
try:
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
_build_oauth_protected_resource_response,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
except ImportError:
|
||||
pytest.skip("MCP discoverable endpoints not available")
|
||||
@pytest.mark.parametrize(
|
||||
"auth_type", (None, "none", "api_key", "bearer_token", "basic", "aws_sigv4", "authorization", "token")
|
||||
)
|
||||
@pytest.mark.parametrize("use_standard_pattern", (False, True))
|
||||
async def test_oauth_protected_resource_for_gateway_owned_auth(auth_type, use_standard_pattern):
|
||||
from starlette.requests import Request
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
_build_oauth_protected_resource_response,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
|
||||
server = _access_group_none_server().model_copy(update={"auth_type": auth_type})
|
||||
request = Request(
|
||||
{"type": "http", "scheme": "https", "path": "/", "headers": [(b"host", b"litellm.example.com")]}
|
||||
)
|
||||
global_mcp_server_manager.registry.clear()
|
||||
server = _access_group_none_server()
|
||||
global_mcp_server_manager.registry[server.server_id] = server
|
||||
|
||||
mock_request = MagicMock()
|
||||
mock_request.base_url = "https://litellm.example.com/"
|
||||
mock_request.headers = {}
|
||||
|
||||
try:
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await _build_oauth_protected_resource_response(
|
||||
request=mock_request,
|
||||
mcp_server_name="access_group_server",
|
||||
use_standard_pattern=False,
|
||||
)
|
||||
assert exc_info.value.status_code == 404
|
||||
assert "not an OAuth-protected resource" in str(exc_info.value.detail)
|
||||
response = await _build_oauth_protected_resource_response(
|
||||
request=request,
|
||||
mcp_server_name="access_group_server",
|
||||
use_standard_pattern=use_standard_pattern,
|
||||
)
|
||||
resource_path = "/mcp/access_group_server" if use_standard_pattern else "/access_group_server/mcp"
|
||||
assert response == {
|
||||
"resource": f"https://litellm.example.com{resource_path}",
|
||||
"authorization_servers": ["https://litellm.example.com/mcp"],
|
||||
"scopes_supported": [],
|
||||
}
|
||||
finally:
|
||||
global_mcp_server_manager.registry.clear()
|
||||
|
||||
|
|
@ -7914,9 +7911,7 @@ async def test_oauth_protected_resource_passthrough_none_auth_not_404():
|
|||
|
||||
@pytest.mark.asyncio
|
||||
async def test_oauth_protected_resource_404_for_unknown_server_name():
|
||||
"""A discovery request for an unknown server name returns the same 404 as a non-oauth2
|
||||
server (not a 200 metadata doc with broken URLs), so the well-known paths cannot be used
|
||||
to enumerate non-OAuth server names."""
|
||||
"""Unknown server names must not produce metadata advertising nonexistent resources."""
|
||||
try:
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
_build_oauth_protected_resource_response,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue