scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-15 23:31:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 165

fix(mcp): reject the gateway's own /callback as a DCR client redirect_uri

Browse source 
A DCR client that holds a registration whose redirect_uris echoed the gateway callback authorizes with LiteLLM's own /callback as its redirect. That URI is same-origin, so the trust policy accepted it, /callback then redirected the authorization response to itself, and the second hit failed to decrypt the client's opaque state, surfacing "Authentication incomplete" and leaving the tool call at 401.

The redirect trust check now rejects that URI at /authorize, before the browser leaves for the upstream, and at the /callback sink for states minted earlier, with a hint that names re-registration and MCP_TRUSTED_REDIRECT_ORIGINS.
This commit is contained in:
Devin AI 2026-07-27 11:51:50 +00:00
parent 24123269cc
commit 45245c6191
4 changed files with 134 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -54,6 +54,7 @@ from litellm.proxy._experimental.mcp_server.gateway_dcr_flow import (
from litellm.proxy._experimental.mcp_server.oauth_utils import (
TOKEN_NO_CACHE_HEADERS,
build_upstream_oauth2_token_request,
gateway_callback_url,
get_request_base_url,
resolve_upstream_resource,
validate_trusted_redirect_uri,
@ -831,7 +832,7 @@ async def authorize_with_server(
params = {
"client_id": mcp_server.client_id if mcp_server.client_id else client_id,
"redirect_uri": f"{request_base_url}/callback",
"redirect_uri": gateway_callback_url(request_base_url),
"state": relay_state,
"response_type": response_type or "code",
}
@ -983,7 +984,7 @@ async def exchange_token_with_server(
),
)
proxy_base_url = get_request_base_url(request)
resolved_redirect_uri = redirect_uri if bridge_token_relay else f"{proxy_base_url}/callback"
resolved_redirect_uri = redirect_uri if bridge_token_relay else gateway_callback_url(proxy_base_url)
token_data = {
"grant_type": "authorization_code",
"code": code,
@ -1501,7 +1502,7 @@ async def mint_ephemeral_dcr_client(request: Request, mcp_server: MCPServer) ->
return cached_after_wait
register_data: dict[str, object] = {
"client_name": mcp_server.server_name or mcp_server.server_id,
"redirect_uris": [f"{request_base_url}/callback"],
"redirect_uris": [gateway_callback_url(request_base_url)],
"grant_types": ["authorization_code", "refresh_token"],
"response_types": ["code"],
"token_endpoint_auth_method": "none",
@ -1576,7 +1577,7 @@ async def register_client_with_server(
):
_raise_if_not_oauth2(mcp_server)
request_base_url = get_request_base_url(request)
current_redirect_uri = f"{request_base_url}/callback"
current_redirect_uri = gateway_callback_url(request_base_url)
client_facing_redirect_uris = client_redirect_uris or [current_redirect_uri]
dummy_return = {
"client_id": fallback_client_id or mcp_server.server_name,
@ -2525,7 +2526,7 @@ async def register_client(request: Request, mcp_server_name: Optional[str] = Non
dummy_return = {
"client_id": mcp_server_name or "dummy_client",
"client_secret": "dummy",
"redirect_uris": client_redirect_uris or [f"{request_base_url}/callback"],
"redirect_uris": client_redirect_uris or [gateway_callback_url(request_base_url)],
}
client_ip = IPAddressUtils.get_mcp_client_ip(request)
if not mcp_server_name:

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

@ -555,6 +555,41 @@ def _raise_trusted_redirect_uri_rejected(
)
def gateway_callback_url(base_url: str) -> str:
"""The proxy's own MCP OAuth callback, spelled once so the value the relay registers upstream
and the value :func:`_raise_if_gateway_callback_redirect` rejects for a client cannot diverge."""
return f"{base_url}/callback"
def _raise_if_gateway_callback_redirect(redirect_uri: str, proxy_base: str | None) -> None:
"""Reject a client ``redirect_uri`` that is the gateway's own ``/callback``.
Such a URI is same-origin, so the trust policy would accept it, and the flow then dies in a
way that looks like anything but a redirect loop: ``/callback`` decodes the relay state,
redirects the authorization response to the client's redirect_uri, which is itself, and the
second hit tries to decrypt the client's opaque ``state`` as a relay handle and surfaces an
"Incorrect padding" decrypt error. A DCR client adopts this URI when it holds a registration
that echoed the gateway callback back as its own ``redirect_uris``, so the actionable failure
belongs here, before the browser leaves for the upstream.
"""
if not proxy_base:
return
if canonicalize_url_identity(redirect_uri) != canonicalize_url_identity(gateway_callback_url(proxy_base)):
return
_oauth_invalid_request(
"redirect_uri is the proxy's own MCP OAuth callback, so the authorization response "
"would be redirected back into the proxy instead of to your client.",
hint=(
"The client is using the proxy's callback as its own redirect_uri; that happens when it "
"still holds a dynamic client registration whose redirect_uris echoed the gateway "
"callback. Delete the client's stored registration for this server and register again "
"so it sends its own redirect_uri, then add that client's origin to "
f"{_TRUSTED_REDIRECT_ORIGINS_ENV} if it is hosted on another host."
),
redirect_uri=redirect_uri,
)
def validate_trusted_redirect_uri(request: Request, redirect_uri: str) -> None:
"""Accept ``redirect_uri`` when it is (a) same-origin with the
proxy's own request origin, (b) loopback, (c) listed in the
@ -583,6 +618,7 @@ def validate_trusted_redirect_uri(request: Request, redirect_uri: str) -> None:
return
redirect_netloc = _strip_default_port(parsed.scheme, parsed.netloc)
proxy_base = _resolve_proxy_base_for_redirect(request)
_raise_if_gateway_callback_redirect(redirect_uri, proxy_base)
if _trusted_redirect_uri_is_allowed(parsed, redirect_netloc, proxy_base):
return
_raise_trusted_redirect_uri_rejected(request, redirect_uri, parsed, redirect_netloc, proxy_base)

3
litellm/proxy/management_endpoints/mcp_management_endpoints.py
View file

@ -138,6 +138,7 @@ if MCP_AVAILABLE:
authorize_with_server,
client_supplied_redirect_uris,
exchange_token_with_server,
gateway_callback_url,
get_request_base_url,
redeem_passthrough_authorization_code,
register_client_with_server,
@ -1777,7 +1778,7 @@ if MCP_AVAILABLE:
resolved_code = sealed_code.upstream_code if sealed_code else code
# A sealed flow ran the gateway /callback as its upstream redirect (bridge short-circuit
# or plain flow alike), so the exchange must present that binding, not the browser page.
resolved_redirect_uri = f"{get_request_base_url(request)}/callback" if sealed_code else redirect_uri
resolved_redirect_uri = gateway_callback_url(get_request_base_url(request)) if sealed_code else redirect_uri
caller_client_id = sealed_code.client_id if sealed_code else client_id
caller_client_secret = sealed_code.client_secret if sealed_code else client_secret
resolved_client_id = mcp_server.client_id or caller_client_id or ""

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

@ -975,6 +975,96 @@ async def test_register_client_valid_multi_redirect_uris_all_echoed():
assert result["redirect_uris"] == client_redirects
@pytest.mark.parametrize(
"gateway_callback_variant",
[
"https://proxy.litellm.example/callback",
"https://proxy.litellm.example/callback/",
"https://proxy.litellm.example:443/callback",
"https://PROXY.litellm.example/callback",
"https://proxy.litellm.example/callback?client=open-webui",
],
)
def test_authorize_rejects_the_gateway_callback_as_a_client_redirect_uri(gateway_callback_variant, monkeypatch):
"""Regression for the reopened DCR self-redirect loop (#34771, #33699). A DCR client holding a
registration that echoed the gateway callback back as its own redirect_uris authorizes with
LiteLLM's /callback as its redirect. That URI is same-origin, so the trust policy used to accept
it, /callback then redirected the authorization response to itself, and the second hit failed to
decrypt the client's opaque state ("Incorrect padding"). The flow must fail at /authorize with an
actionable error instead, and must not send the browser upstream."""
from fastapi import FastAPI
from fastapi.testclient import TestClient
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import router
from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager
monkeypatch.setenv("PROXY_BASE_URL", "https://proxy.litellm.example")
monkeypatch.delenv("MCP_TRUSTED_REDIRECT_ORIGINS", raising=False)
global_mcp_server_manager.registry.clear()
oauth2_server = _create_oauth2_server(server_id="atlassian", name="atlassian", server_name="atlassian")
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
app = FastAPI()
app.include_router(router)
try:
response = TestClient(app).get(
"/atlassian/authorize",
params={
"response_type": "code",
"client_id": "upstream-issued-client-id",
"redirect_uri": gateway_callback_variant,
"state": "client-opaque-state",
"code_challenge": "c" * 43,
"code_challenge_method": "S256",
},
follow_redirects=False,
)
finally:
global_mcp_server_manager.registry.clear()
assert response.status_code == 400
detail = response.json()["detail"]
assert detail["error"] == "invalid_request"
assert "own MCP OAuth callback" in detail["error_description"]
assert "dynamic client registration" in detail["hint"]
assert "MCP_TRUSTED_REDIRECT_ORIGINS" in detail["hint"]
def test_callback_refuses_to_redirect_an_authorization_response_to_itself(monkeypatch):
"""The /callback sink is the second half of the same loop: a state minted before the /authorize
guard (they never expire) still carries the gateway callback as the client redirect. Forwarding
the code there re-enters this handler with the client's own state and dies in the decrypt path,
so the sink must reject it too (#34771)."""
from fastapi import FastAPI
from fastapi.testclient import TestClient
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
encode_state_with_base_url,
router,
)
monkeypatch.setenv("PROXY_BASE_URL", "https://proxy.litellm.example")
monkeypatch.setenv("LITELLM_SALT_KEY", "sk-test-salt-for-lit34771")
monkeypatch.setattr("litellm.proxy.proxy_server.master_key", "sk-test-salt-for-lit34771", raising=False)
stale_state = encode_state_with_base_url(
base_url="https://proxy.litellm.example/callback",
original_state="client-opaque-state",
client_redirect_uri="https://proxy.litellm.example/callback",
)
app = FastAPI()
app.include_router(router)
response = TestClient(app).get(
"/callback",
params={"code": "upstream-code", "state": stale_state},
follow_redirects=False,
)
assert response.status_code == 400
assert "own MCP OAuth callback" in response.json()["detail"]["error_description"]
@pytest.mark.asyncio
async def test_register_client_persists_dcr_client_identity():
"""A dynamic client registration (RFC 7591) must persist the issued client_id /