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

Merge pull request #33756 from BerriAI/litellm_mcp_dcr_client_redirect_33699

Browse source 
fix(mcp): return the DCR client's own redirect_uris to stop the /callback self-redirect loop
This commit is contained in:
tin-berri 2026-07-20 19:16:45 -07:00 • committed by GitHub
commit 7df76fc0d8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 429 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py
View file

@ -1215,6 +1215,18 @@ async def _persist_dcr_client_registration(
return "failed"
def _client_supplied_redirect_uris(value: object) -> list[str] | None:
"""RFC 7591 redirect_uris must be a non-empty array of URI strings. Any other shape (not a list,
an empty list, or a list holding a non-string or empty-string element) yields None so every
register arm falls back to the gateway callback instead of echoing a malformed value back to the
client as its redirect_uris. The redirect actually used is trust-validated later at /authorize by
validate_trusted_redirect_uri; this guard only keeps the client-facing echo well-typed."""
if not isinstance(value, list) or not value:
return None
uris = [uri for uri in value if isinstance(uri, str) and uri]
return uris if len(uris) == len(value) else None
async def register_client_with_server(
request: Request,
mcp_server: MCPServer,
@ -1224,15 +1236,16 @@ async def register_client_with_server(
token_endpoint_auth_method: Optional[str],
fallback_client_id: Optional[str] = None,
persist_credentials: bool = False,
client_redirect_uris: Optional[list] = None,
client_redirect_uris: list[str] | None = None,
):
_raise_if_not_oauth2(mcp_server)
request_base_url = get_request_base_url(request)
current_redirect_uri = f"{request_base_url}/callback"
client_facing_redirect_uris = client_redirect_uris or [current_redirect_uri]
dummy_return = {
"client_id": fallback_client_id or mcp_server.server_name,
"client_secret": "dummy",
"redirect_uris": [current_redirect_uri],
"redirect_uris": client_facing_redirect_uris,
}
if mcp_server.client_id and not (
@ -1300,6 +1313,9 @@ async def register_client_with_server(
if persistence_result == "reused":
return dummy_return
if client_redirect_uris and not bridge_relay and isinstance(token_response, dict):
token_response = {**token_response, "redirect_uris": client_facing_redirect_uris}
return JSONResponse(token_response)
@ -2121,11 +2137,12 @@ async def register_client(request: Request, mcp_server_name: Optional[str] = Non
request_data = await _read_request_body(request=request)
data: dict = {**request_data}
client_redirect_uris = _client_supplied_redirect_uris(data.get("redirect_uris"))
dummy_return = {
"client_id": mcp_server_name or "dummy_client",
"client_secret": "dummy",
"redirect_uris": [f"{request_base_url}/callback"],
"redirect_uris": client_redirect_uris or [f"{request_base_url}/callback"],
}
client_ip = IPAddressUtils.get_mcp_client_ip(request)
if not mcp_server_name:
@ -2139,7 +2156,7 @@ async def register_client(request: Request, mcp_server_name: Optional[str] = Non
response_types=data.get("response_types", []),
token_endpoint_auth_method=data.get("token_endpoint_auth_method", ""),
fallback_client_id=resolved.server_name or resolved.name,
client_redirect_uris=data.get("redirect_uris"),
client_redirect_uris=client_redirect_uris,
)
return dummy_return
@ -2154,5 +2171,5 @@ async def register_client(request: Request, mcp_server_name: Optional[str] = Non
response_types=data.get("response_types", []),
token_endpoint_auth_method=data.get("token_endpoint_auth_method", ""),
fallback_client_id=mcp_server_name,
client_redirect_uris=data.get("redirect_uris"),
client_redirect_uris=client_redirect_uris,
)

5
litellm/proxy/_experimental/mcp_server/oauth_utils.py
View file

@ -465,7 +465,10 @@ def _raise_trusted_redirect_uri_rejected(
"Align the proxy public URL with the browser URL. Set PROXY_BASE_URL to your "
"HTTPS origin (e.g. https://litellm.example.com), or enable "
"general_settings.use_x_forwarded_for with mcp_trusted_proxy_ranges for your "
"ingress. Verify: curl https://<host>/.well-known/oauth-authorization-server "
"ingress. If the redirect_uri is a legitimate separate-origin OAuth client "
"(e.g. a web app registering with the proxy from another host via dynamic client "
f"registration), add its origin to {_TRUSTED_REDIRECT_ORIGINS_ENV}. "
"Verify: curl https://<host>/.well-known/oauth-authorization-server "
"| jq .issuer — issuer must match window.location.origin in the UI."
)

403
tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py
View file

@ -572,6 +572,409 @@ async def test_register_client_remote_registration_success():
assert call_args.kwargs["json"]["token_endpoint_auth_method"] == request_payload["token_endpoint_auth_method"]
@pytest.mark.asyncio
async def test_register_client_non_bridge_returns_client_redirect_not_gateway_callback():
"""Regression for the DCR self-redirect loop (#33699). A plain oauth2 DCR server relays the
gateway's own /callback upstream, which is correct for the relay leg, but the client-facing
/register response must echo the CLIENT's own redirect_uris. A Rovo-style upstream echoes back
whatever redirect_uris it was registered with (here the gateway callback); returning that
verbatim makes a spec-compliant DCR client adopt /callback as its own redirect and loop."""
from fastapi import Request
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import register_client
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
from litellm.proxy._types import MCPTransport
from litellm.types.mcp import MCPAuth
from litellm.types.mcp_server.mcp_server_manager import MCPServer
global_mcp_server_manager.registry.clear()
oauth2_server = MCPServer(
server_id="rovo_like",
name="rovo_like",
server_name="rovo_like",
alias="rovo_like",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
client_id=None,
client_secret=None,
authorization_url="https://provider.example/oauth/authorize",
token_url="https://provider.example/oauth/token",
registration_url="https://provider.example/oauth/register",
)
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.base_url = "https://proxy.litellm.example/"
mock_request.headers = {}
client_redirect = "https://open-webui.example/oauth/oidc/callback"
request_payload = {
"client_name": "Open WebUI",
"grant_types": ["authorization_code", "refresh_token"],
"response_types": ["code"],
"redirect_uris": [client_redirect],
}
mock_response = MagicMock()
mock_response.json.return_value = {
"client_id": "upstream-generated-client-id",
"client_secret": "upstream-generated-secret",
"redirect_uris": ["https://proxy.litellm.example/callback"],
}
mock_response.raise_for_status = MagicMock()
mock_async_client = MagicMock()
mock_async_client.post = AsyncMock(return_value=mock_response)
try:
with (
patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints._read_request_body",
new=AsyncMock(return_value=request_payload),
),
patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
return_value=mock_async_client,
),
):
response = await register_client(request=mock_request, mcp_server_name=oauth2_server.server_name)
finally:
global_mcp_server_manager.registry.clear()
payload = json.loads(response.body.decode("utf-8"))
assert payload["redirect_uris"] == [client_redirect]
assert payload["client_id"] == "upstream-generated-client-id"
assert mock_async_client.post.call_args.kwargs["json"]["redirect_uris"] == [
"https://proxy.litellm.example/callback"
]
@pytest.mark.asyncio
async def test_register_client_admin_client_id_echoes_client_redirect_uris():
"""A server with an admin-configured client_id short-circuits registration to a placeholder
response, which must still echo the client's own redirect_uris so a DCR client does not adopt
the gateway /callback and self-redirect loop (#33699)."""
from fastapi import Request
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import register_client
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
from litellm.proxy._types import MCPTransport
from litellm.types.mcp import MCPAuth
from litellm.types.mcp_server.mcp_server_manager import MCPServer
global_mcp_server_manager.registry.clear()
oauth2_server = MCPServer(
server_id="stored_server",
name="stored_server",
server_name="stored_server",
alias="stored_server",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
client_id="existing-client",
client_secret="existing-secret",
authorization_url="https://provider.example/oauth/authorize",
token_url="https://provider.example/oauth/token",
)
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.base_url = "https://proxy.litellm.example/"
mock_request.headers = {}
client_redirect = "https://open-webui.example/oauth/oidc/callback"
try:
with patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints._read_request_body",
new=AsyncMock(return_value={"redirect_uris": [client_redirect]}),
):
result = await register_client(request=mock_request, mcp_server_name=oauth2_server.server_name)
finally:
global_mcp_server_manager.registry.clear()
assert result == {
"client_id": "stored_server",
"client_secret": "dummy",
"redirect_uris": [client_redirect],
}
@pytest.mark.asyncio
async def test_dcr_full_loop_lands_on_client_redirect_not_gateway_callback(monkeypatch):
"""End-to-end regression for #33699. A DCR client registers, then completes /authorize and
/callback. With the fix the client registers and authorizes with its OWN redirect, so /callback
delivers the code to the client's real endpoint instead of looping back into the gateway
/callback (whose decrypt of the client's opaque state failed as 'Incorrect padding'). The
client's separate origin is trusted via MCP_TRUSTED_REDIRECT_ORIGINS."""
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,
register_client,
)
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
from litellm.proxy._types import MCPTransport
from litellm.types.mcp import MCPAuth
from litellm.types.mcp_server.mcp_server_manager import MCPServer
monkeypatch.setenv("LITELLM_SALT_KEY", "sk-test-salt-33699")
monkeypatch.setenv("MCP_TRUSTED_REDIRECT_ORIGINS", "open-webui.example")
client_redirect = "https://open-webui.example/oauth/oidc/callback"
client_state = "client-opaque-state-777"
global_mcp_server_manager.registry.clear()
server = MCPServer(
server_id="rovo_like",
name="rovo_like",
server_name="rovo_like",
alias="rovo_like",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
client_id=None,
client_secret=None,
authorization_url="https://provider.example/oauth/authorize",
token_url="https://provider.example/oauth/token",
registration_url="https://provider.example/oauth/register",
)
global_mcp_server_manager.registry[server.server_id] = server
reg_request = MagicMock(spec=Request)
reg_request.base_url = "https://proxy.example.com/"
reg_request.headers = {}
mock_response = MagicMock()
mock_response.json.return_value = {
"client_id": "upstream-generated-client-id",
"client_secret": "upstream-generated-secret",
"redirect_uris": ["https://proxy.example.com/callback"],
}
mock_response.raise_for_status = MagicMock()
mock_async_client = MagicMock()
mock_async_client.post = AsyncMock(return_value=mock_response)
try:
with (
patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints._read_request_body",
new=AsyncMock(
return_value={
"client_name": "Open WebUI",
"redirect_uris": [client_redirect],
"grant_types": ["authorization_code", "refresh_token"],
"response_types": ["code"],
}
),
),
patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
return_value=mock_async_client,
),
):
reg_response = await register_client(request=reg_request, mcp_server_name=server.server_name)
reg_payload = json.loads(reg_response.body.decode("utf-8"))
assert reg_payload["redirect_uris"] == [client_redirect]
registered_redirect = reg_payload["redirect_uris"][0]
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-generated-client-id",
redirect_uri=registered_redirect,
state=client_state,
code_challenge="challenge",
code_challenge_method="S256",
)
finally:
global_mcp_server_manager.registry.clear()
assert authorize_response.status_code == 307
location = authorize_response.headers["location"]
upstream_state = parse_qs(urlparse(location).query)["state"][0]
assert upstream_state != client_state
assert "redirect_uri=https%3A%2F%2Fproxy.example.com%2Fcallback" in location
jar = SimpleCookie()
jar.load(authorize_response.headers["set-cookie"])
cookie_name = _oauth_state_cookie_name(upstream_state)
morsel = jar[cookie_name]
callback_request = MagicMock(spec=Request)
callback_request.base_url = "https://proxy.example.com/"
callback_request.headers = {}
callback_request.cookies = {cookie_name: morsel.value}
callback_response = await callback(
request=callback_request,
code="upstream-auth-code",
state=upstream_state,
)
assert callback_response.status_code == 302
final = urlparse(callback_response.headers["location"])
assert f"{final.scheme}://{final.netloc}{final.path}" == client_redirect
final_query = parse_qs(final.query)
assert final_query["code"] == ["upstream-auth-code"]
assert final_query["state"] == [client_state]
@pytest.mark.asyncio
async def test_authorize_rejects_untrusted_cross_origin_redirect_with_allowlist_hint(monkeypatch):
"""Once the client uses its own separate-origin redirect (#33699 fix), an untrusted origin is
rejected at /authorize. The rejection must point the operator to MCP_TRUSTED_REDIRECT_ORIGINS,
the mechanism a legitimate separate-origin DCR client needs, not only to PROXY_BASE_URL."""
from fastapi import Request
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import authorize
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
from litellm.proxy._types import MCPTransport
from litellm.types.mcp import MCPAuth
from litellm.types.mcp_server.mcp_server_manager import MCPServer
monkeypatch.delenv("MCP_TRUSTED_REDIRECT_ORIGINS", raising=False)
global_mcp_server_manager.registry.clear()
oauth2_server = MCPServer(
server_id="rovo_like",
name="rovo_like",
server_name="rovo_like",
alias="rovo_like",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
client_id="upstream-client",
authorization_url="https://provider.example/oauth/authorize",
token_url="https://provider.example/oauth/token",
)
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.base_url = "https://proxy.example.com/"
mock_request.headers = {}
try:
with pytest.raises(HTTPException) as exc_info:
await authorize(
request=mock_request,
client_id="upstream-client",
mcp_server_name="rovo_like",
redirect_uri="https://open-webui.example/oauth/oidc/callback",
state="s",
)
finally:
global_mcp_server_manager.registry.clear()
assert exc_info.value.status_code == 400
assert "MCP_TRUSTED_REDIRECT_ORIGINS" in exc_info.value.detail["hint"]
@pytest.mark.asyncio
@pytest.mark.parametrize(
"malformed_redirect_uris",
[
"https://evil.example/cb",
["https://ok.example/cb", None],
["https://ok.example/cb", 123],
["https://ok.example/cb", {"nested": "object"}],
[""],
[],
],
)
async def test_register_client_malformed_redirect_uris_falls_back_to_gateway_callback(malformed_redirect_uris):
"""RFC 7591 redirect_uris is a non-empty array of URI strings. A client that sends any other shape
(a bare string, a list holding a non-string or empty-string element, or an empty list) must not
have that value echoed back as its redirect_uris; the register response falls back to the gateway
callback so downstream never iterates a string as URIs or leaks non-string element types (#33699)."""
from fastapi import Request
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import register_client
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
from litellm.proxy._types import MCPTransport
from litellm.types.mcp import MCPAuth
from litellm.types.mcp_server.mcp_server_manager import MCPServer
global_mcp_server_manager.registry.clear()
oauth2_server = MCPServer(
server_id="stored_server",
name="stored_server",
server_name="stored_server",
alias="stored_server",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
client_id="existing-client",
client_secret="existing-secret",
authorization_url="https://provider.example/oauth/authorize",
token_url="https://provider.example/oauth/token",
)
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.base_url = "https://proxy.litellm.example/"
mock_request.headers = {}
try:
with patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints._read_request_body",
new=AsyncMock(return_value={"redirect_uris": malformed_redirect_uris}),
):
result = await register_client(request=mock_request, mcp_server_name=oauth2_server.server_name)
finally:
global_mcp_server_manager.registry.clear()
assert result["redirect_uris"] == ["https://proxy.litellm.example/callback"]
@pytest.mark.asyncio
async def test_register_client_valid_multi_redirect_uris_all_echoed():
"""A well-formed client sending several valid redirect URI strings gets all of them echoed back
unchanged, so the element-type guard does not narrow a legitimate multi-entry list (#33699)."""
from fastapi import Request
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import register_client
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
from litellm.proxy._types import MCPTransport
from litellm.types.mcp import MCPAuth
from litellm.types.mcp_server.mcp_server_manager import MCPServer
global_mcp_server_manager.registry.clear()
oauth2_server = MCPServer(
server_id="stored_server",
name="stored_server",
server_name="stored_server",
alias="stored_server",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
client_id="existing-client",
client_secret="existing-secret",
authorization_url="https://provider.example/oauth/authorize",
token_url="https://provider.example/oauth/token",
)
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.base_url = "https://proxy.litellm.example/"
mock_request.headers = {}
client_redirects = ["https://app.example/cb", "http://127.0.0.1:6274/callback"]
try:
with patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints._read_request_body",
new=AsyncMock(return_value={"redirect_uris": client_redirects}),
):
result = await register_client(request=mock_request, mcp_server_name=oauth2_server.server_name)
finally:
global_mcp_server_manager.registry.clear()
assert result["redirect_uris"] == client_redirects
@pytest.mark.asyncio
async def test_register_client_persists_dcr_client_identity():
"""A dynamic client registration (RFC 7591) must persist the issued client_id /