mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(mcp): optional broker auth + temp-session authorize test
Resolve proxy credentials on OAuth broker /authorize and /token when Authorization is present so allowlist and non-admin temp-cache rules apply. Add HTTP test that seeds a temp MCP server and asserts unauthenticated GET /authorize redirects to the upstream IdP. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
a969c974d9
commit
38c7cc3aab
2 changed files with 94 additions and 3 deletions
|
|
@ -1448,6 +1448,29 @@ if MCP_AVAILABLE:
|
|||
|
||||
return _redact_mcp_credentials(temp_record)
|
||||
|
||||
async def _try_resolve_mcp_oauth_broker_user(
|
||||
request: Request,
|
||||
) -> Optional[UserAPIKeyAuth]:
|
||||
"""
|
||||
Optional proxy credentials for ``/authorize`` and ``/token``.
|
||||
|
||||
When absent, unauthenticated access is still allowed for **temp-cache**
|
||||
servers only (browser OAuth). When present, global-registry access
|
||||
follows admin / allowlist rules via ``_get_cached_temporary_mcp_server_or_404``.
|
||||
"""
|
||||
authorization = (
|
||||
request.headers.get("authorization")
|
||||
or request.headers.get("Authorization")
|
||||
or ""
|
||||
).strip()
|
||||
if not authorization:
|
||||
return None
|
||||
from litellm.proxy.auth.user_api_key_auth import (
|
||||
user_api_key_auth_from_request_headers,
|
||||
)
|
||||
|
||||
return await user_api_key_auth_from_request_headers(request)
|
||||
|
||||
async def _get_cached_temporary_mcp_server_or_404(
|
||||
server_id: str,
|
||||
user_api_key_dict: Optional[UserAPIKeyAuth] = None,
|
||||
|
|
@ -1535,8 +1558,9 @@ if MCP_AVAILABLE:
|
|||
response_type: Optional[str] = None,
|
||||
scope: Optional[str] = None,
|
||||
):
|
||||
user_api_key_dict = await _try_resolve_mcp_oauth_broker_user(request)
|
||||
mcp_server = await _get_cached_temporary_mcp_server_or_404(
|
||||
server_id, request=request
|
||||
server_id, user_api_key_dict=user_api_key_dict, request=request
|
||||
)
|
||||
# Use the server's stored client_id when the caller doesn't supply one
|
||||
resolved_client_id = mcp_server.client_id or client_id or ""
|
||||
|
|
@ -1579,8 +1603,9 @@ if MCP_AVAILABLE:
|
|||
refresh_token: Optional[str] = Form(None),
|
||||
scope: Optional[str] = Form(None),
|
||||
):
|
||||
user_api_key_dict = await _try_resolve_mcp_oauth_broker_user(request)
|
||||
mcp_server = await _get_cached_temporary_mcp_server_or_404(
|
||||
server_id, request=request
|
||||
server_id, user_api_key_dict=user_api_key_dict, request=request
|
||||
)
|
||||
resolved_client_id = mcp_server.client_id or client_id or ""
|
||||
if not resolved_client_id:
|
||||
|
|
|
|||
|
|
@ -295,7 +295,8 @@ async def test_token_exchange_applies_token_validation_rules(
|
|||
# not be able to invoke the OAuth broker for globally configured servers,
|
||||
# as doing so would use the proxy's stored client_secret)
|
||||
# 3. Temp-session server + no API key → passes access check (browser OAuth
|
||||
# flow; temp sessions are admin-created and scoped to this flow)
|
||||
# flow; temp sessions are admin-created and scoped to this flow).
|
||||
# Covered by test_management_broker_authorize_unauthenticated_temp_session_passes.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
|
@ -415,3 +416,68 @@ async def test_management_broker_rejects_unauthenticated_access_to_global_regist
|
|||
)
|
||||
finally:
|
||||
global_mcp_server_manager.registry.pop("global-oauth-srv", None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_management_broker_authorize_unauthenticated_temp_session_passes(
|
||||
management_asgi_app: FastAPI,
|
||||
) -> None:
|
||||
"""
|
||||
Positive path: a temp-cached MCP OAuth server must allow GET /authorize with
|
||||
no API key (browser redirect), yielding a redirect to the upstream IdP — not
|
||||
401/403 from the broker gate.
|
||||
"""
|
||||
from litellm.proxy.management_endpoints import mcp_management_endpoints as mcp_mod
|
||||
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
|
||||
_cache_temporary_mcp_server,
|
||||
)
|
||||
|
||||
server_id = "temp-broker-oauth-success-001"
|
||||
server = MCPServer(
|
||||
server_id=server_id,
|
||||
name="temp_oauth",
|
||||
server_name="temp_oauth",
|
||||
alias="temp_oauth",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
client_id="upstream-client",
|
||||
client_secret="upstream-secret",
|
||||
authorization_url="https://idp.example/oauth/authorize",
|
||||
token_url="https://idp.example/oauth/token",
|
||||
)
|
||||
_cache_temporary_mcp_server(server, ttl_seconds=300)
|
||||
try:
|
||||
transport = ASGITransport(app=management_asgi_app)
|
||||
async with httpx.AsyncClient(
|
||||
transport=transport,
|
||||
base_url="http://test",
|
||||
follow_redirects=False,
|
||||
) as client:
|
||||
r = await client.get(
|
||||
f"/v1/mcp/server/oauth/{server_id}/authorize",
|
||||
params={
|
||||
"redirect_uri": "http://127.0.0.1:8080/callback",
|
||||
"state": "browser-oauth-state",
|
||||
"response_type": "code",
|
||||
"code_challenge": "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk",
|
||||
"code_challenge_method": "S256",
|
||||
"client_id": "upstream-client",
|
||||
},
|
||||
)
|
||||
assert r.status_code != 401, (
|
||||
"Unauthenticated temp-session authorize must not hit master-key 401. "
|
||||
f"body={r.text}"
|
||||
)
|
||||
assert r.status_code != 403, (
|
||||
"Unauthenticated temp-session authorize must not be blocked as global. "
|
||||
f"body={r.text}"
|
||||
)
|
||||
assert r.status_code in (
|
||||
302,
|
||||
303,
|
||||
307,
|
||||
), f"expected redirect to upstream IdP, got {r.status_code}: {r.text}"
|
||||
location = r.headers.get("location") or ""
|
||||
assert "idp.example" in location
|
||||
finally:
|
||||
mcp_mod._temporary_mcp_servers.pop(server_id, None)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue