mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix(mcp): honor admin-entered OAuth URLs on authorize after issuer yield
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
f066b01b0a
commit
229e136783
4 changed files with 126 additions and 19 deletions
|
|
@ -663,6 +663,23 @@ def _endpoint_not_configured_detail(
|
|||
)
|
||||
|
||||
|
||||
async def _server_with_oauth_endpoints(mcp_server: MCPServer) -> MCPServer:
|
||||
"""Join deferred OAuth discovery only when this server still has no authorize URL.
|
||||
|
||||
Admin-entered endpoints live on ``configured_*`` after an anchored issuer empties the
|
||||
resolved fields. Those already let authorize/token run, so discovery is not awaited
|
||||
and cannot 503 over a leftover pin. A server with nothing configured still joins the
|
||||
deferred task; no slot is a no-op and the caller 400s.
|
||||
"""
|
||||
if mcp_server.effective_authorization_url is not None:
|
||||
return mcp_server
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( # noqa: PLC0415 # circular import with mcp_server_manager at module load
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
|
||||
return await global_mcp_server_manager.ensure_oauth_metadata_discovered(mcp_server)
|
||||
|
||||
|
||||
def _raise_unless_oauth2_discovery_server(
|
||||
mcp_server: MCPServer | None,
|
||||
mcp_server_name: str | None,
|
||||
|
|
@ -697,7 +714,7 @@ def _dcr_bridge_relays_client_registration(mcp_server: MCPServer) -> bool:
|
|||
returns directly to the client's redirect URI without transiting the gateway. Gateway-side
|
||||
redirect trust and the ``/callback`` state relay therefore only apply to the short-circuit
|
||||
arm, where the upstream only knows the gateway's own callback."""
|
||||
return mcp_server.is_dcr_bridge and bool(mcp_server.registration_url) and not mcp_server.client_id
|
||||
return mcp_server.is_dcr_bridge and bool(mcp_server.effective_registration_url) and not mcp_server.client_id
|
||||
|
||||
|
||||
def _require_s256_pkce(
|
||||
|
|
@ -745,7 +762,7 @@ def _redirect_to_upstream_authorize(
|
|||
**({"scope": scope_value} if scope_value else {}),
|
||||
**({"resource": upstream_resource} if upstream_resource else {}),
|
||||
}
|
||||
parsed_auth_url: Final = urlparse(mcp_server.authorization_url or "")
|
||||
parsed_auth_url: Final = urlparse(mcp_server.effective_authorization_url or "")
|
||||
merged_params: Final = {**dict(parse_qsl(parsed_auth_url.query)), **passthrough_params}
|
||||
return RedirectResponse(urlunparse(parsed_auth_url._replace(query=urlencode(merged_params))))
|
||||
|
||||
|
|
@ -812,11 +829,12 @@ async def authorize_with_server(
|
|||
ephemeral_dcr_client: "EphemeralDcrClient | None" = None,
|
||||
):
|
||||
_raise_if_not_oauth2(mcp_server)
|
||||
if mcp_server.authorization_url is None:
|
||||
resolved_server: Final = await _server_with_oauth_endpoints(mcp_server)
|
||||
if resolved_server.effective_authorization_url is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=_endpoint_not_configured_detail(
|
||||
mcp_server,
|
||||
resolved_server,
|
||||
"authorization url",
|
||||
"set Authorization URL and Token URL manually",
|
||||
"set Issuer to discover them from the identity provider (RFC 8414)",
|
||||
|
|
@ -913,7 +931,7 @@ async def authorize_with_server(
|
|||
if upstream_resource:
|
||||
params["resource"] = upstream_resource
|
||||
|
||||
parsed_auth_url: Final = urlparse(mcp_server.authorization_url)
|
||||
parsed_auth_url: Final = urlparse(resolved_server.effective_authorization_url)
|
||||
existing_params: Final = dict(parse_qsl(parsed_auth_url.query))
|
||||
existing_params.update(params)
|
||||
final_url: Final = urlunparse(parsed_auth_url._replace(query=urlencode(existing_params)))
|
||||
|
|
@ -946,11 +964,13 @@ async def exchange_token_with_server(
|
|||
if grant_type not in ("authorization_code", "refresh_token"):
|
||||
raise HTTPException(status_code=400, detail="Unsupported grant_type")
|
||||
|
||||
if mcp_server.token_url is None:
|
||||
resolved_server: Final = await _server_with_oauth_endpoints(mcp_server)
|
||||
token_url: Final = resolved_server.effective_token_url
|
||||
if token_url is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=_endpoint_not_configured_detail(
|
||||
mcp_server,
|
||||
resolved_server,
|
||||
"token url",
|
||||
"set Token URL manually",
|
||||
"set Issuer to discover it from the identity provider (RFC 8414)",
|
||||
|
|
@ -1067,7 +1087,7 @@ async def exchange_token_with_server(
|
|||
async_client: Final = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Check)
|
||||
try:
|
||||
response: Final = await async_client.post(
|
||||
mcp_server.token_url,
|
||||
token_url,
|
||||
headers={"Accept": "application/json", **token_request.headers},
|
||||
data=token_data,
|
||||
)
|
||||
|
|
@ -1551,7 +1571,8 @@ async def mint_ephemeral_dcr_client(request: Request, mcp_server: MCPServer) ->
|
|||
bounded by the server count even when the request origin varies) so parallel authorize requests
|
||||
cannot each register an upstream client; the cache stamps nothing onto the server record and
|
||||
correctness never depends on it because the sealed state carries the client through the flow."""
|
||||
if mcp_server.registration_url is None:
|
||||
registration_url: Final = mcp_server.effective_registration_url
|
||||
if registration_url is None:
|
||||
return None
|
||||
request_base_url: Final = get_request_base_url(request)
|
||||
cache_key: Final = f"mcp_ephemeral_dcr_client:{mcp_server.server_id}:{request_base_url}"
|
||||
|
|
@ -1571,7 +1592,7 @@ async def mint_ephemeral_dcr_client(request: Request, mcp_server: MCPServer) ->
|
|||
"token_endpoint_auth_method": "none",
|
||||
}
|
||||
response: Final = await _post_dcr_registration(
|
||||
registration_url=mcp_server.registration_url,
|
||||
registration_url=registration_url,
|
||||
register_data=register_data,
|
||||
server_id=mcp_server.server_id,
|
||||
)
|
||||
|
|
@ -1617,7 +1638,7 @@ async def resolve_ephemeral_dcr_client(
|
|||
usable to generate orphan IdP clients)."""
|
||||
if not (mcp_server.is_true_passthrough or (mcp_server.is_oauth_delegate and not mcp_server.is_dcr_bridge)):
|
||||
return None
|
||||
if mcp_server.authorization_url is None:
|
||||
if mcp_server.effective_authorization_url is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="MCP server authorization url is not set",
|
||||
|
|
@ -1661,21 +1682,23 @@ async def register_client_with_server(
|
|||
):
|
||||
return dummy_return
|
||||
|
||||
if mcp_server.authorization_url is None:
|
||||
resolved_server: Final = await _server_with_oauth_endpoints(mcp_server)
|
||||
if resolved_server.effective_authorization_url is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=_endpoint_not_configured_detail(
|
||||
mcp_server,
|
||||
resolved_server,
|
||||
"authorization url",
|
||||
"set Authorization URL and Token URL manually",
|
||||
"set Issuer to discover them from the identity provider (RFC 8414)",
|
||||
),
|
||||
)
|
||||
|
||||
if mcp_server.registration_url is None:
|
||||
registration_url: Final = resolved_server.effective_registration_url
|
||||
if registration_url is None:
|
||||
return dummy_return
|
||||
|
||||
bridge_relay: Final = _dcr_bridge_relays_client_registration(mcp_server)
|
||||
bridge_relay: Final = _dcr_bridge_relays_client_registration(resolved_server)
|
||||
if bridge_relay and not client_redirect_uris:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
|
|
@ -1690,7 +1713,7 @@ async def register_client_with_server(
|
|||
"token_endpoint_auth_method": token_endpoint_auth_method or ("none" if bridge_relay else ""),
|
||||
}
|
||||
response: Final = await _post_dcr_registration(
|
||||
registration_url=mcp_server.registration_url,
|
||||
registration_url=registration_url,
|
||||
register_data=register_data,
|
||||
server_id=mcp_server.server_id,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -523,7 +523,7 @@ def _oauth_endpoints_unresolved(server: MCPServer) -> bool:
|
|||
# can come from resource discovery, so a server that resolved its endpoints but no scopes is
|
||||
# still unresolved for its flow.
|
||||
return True
|
||||
if server.is_dcr_bridge and not server.client_id and server.registration_url is None:
|
||||
if server.is_dcr_bridge and not server.client_id and server.effective_registration_url is None:
|
||||
# A DCR bridge with no admin-configured client can only register callers through the
|
||||
# upstream's registration endpoint, so a build that resolved the authorize and token
|
||||
# endpoints but not registration_endpoint (partial metadata) is still unresolved for its
|
||||
|
|
@ -535,8 +535,8 @@ def _oauth_endpoints_unresolved(server: MCPServer) -> bool:
|
|||
return _flow_endpoints_missing(
|
||||
server.auth_type,
|
||||
MCPServerManager.effective_oauth2_flow(server),
|
||||
server.authorization_url,
|
||||
server.token_url,
|
||||
server.effective_authorization_url,
|
||||
server.effective_token_url,
|
||||
server.token_exchange_endpoint,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -183,6 +183,18 @@ class MCPServer(BaseModel):
|
|||
def __str__(self) -> str:
|
||||
return self.__repr__()
|
||||
|
||||
@property
|
||||
def effective_authorization_url(self) -> str | None:
|
||||
return self.authorization_url or self.configured_authorization_url
|
||||
|
||||
@property
|
||||
def effective_token_url(self) -> str | None:
|
||||
return self.token_url or self.configured_token_url
|
||||
|
||||
@property
|
||||
def effective_registration_url(self) -> str | None:
|
||||
return self.registration_url or self.configured_registration_url
|
||||
|
||||
@property
|
||||
def has_client_credentials(self) -> bool:
|
||||
"""True if this server should use the OAuth2 client_credentials (M2M) flow.
|
||||
|
|
|
|||
|
|
@ -8854,6 +8854,78 @@ async def test_authorize_wall_names_the_issuer_for_anchored_servers():
|
|||
assert "idp.example.com" not in detail_text
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_authorize_uses_admin_entered_github_oauth_urls_after_issuer_yield():
|
||||
"""GitHub MCP servers store Authorization URL and Token URL on the row. 1.99 can empty
|
||||
the resolved authorization_url when a leftover issuer is treated as a pin (RFC 8414
|
||||
yield). The UI authorize must still redirect to the admin-entered GitHub authorize URL
|
||||
instead of 400ing that discovery against api.githubcopilot.com failed."""
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
authorize_with_server,
|
||||
)
|
||||
from litellm.types.mcp import MCPAuth, MCPTransport
|
||||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
server = MCPServer(
|
||||
server_id="ecac50c4-8eca-438a-af80-9bdebadafc69",
|
||||
name="github_mcp",
|
||||
alias="github_mcp",
|
||||
server_name="github_mcp",
|
||||
url="https://api.githubcopilot.com/mcp/",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
oauth2_flow="authorization_code",
|
||||
client_id="github-app-client",
|
||||
authorization_url=None,
|
||||
token_url=None,
|
||||
issuer="https://github.com",
|
||||
issuer_is_anchored=True,
|
||||
configured_authorization_url="https://github.com/login/oauth/authorize",
|
||||
configured_token_url="https://github.com/login/oauth/access_token",
|
||||
)
|
||||
mock_request = MagicMock()
|
||||
mock_request.base_url = "https://litellm.example.com/"
|
||||
mock_request.headers = {}
|
||||
|
||||
with patch("litellm.proxy._experimental.mcp_server.discoverable_endpoints.encrypt_value_helper") as mock_encrypt:
|
||||
mock_encrypt.return_value = "mocked_encrypted_state"
|
||||
response = await authorize_with_server(
|
||||
request=mock_request,
|
||||
mcp_server=server,
|
||||
client_id="github-app-client",
|
||||
redirect_uri="http://127.0.0.1:60108/callback",
|
||||
state="state123",
|
||||
)
|
||||
|
||||
assert response.status_code == 307
|
||||
assert "https://github.com/login/oauth/authorize" in response.headers["location"]
|
||||
assert "client_id=github-app-client" in response.headers["location"]
|
||||
|
||||
|
||||
def test_oauth_endpoints_count_admin_entered_urls_as_resolved():
|
||||
"""A leftover issuer empties the resolved authorize/token fields but must not keep the
|
||||
server on the deferred-discovery retry path when the admin already stored those URLs."""
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
_oauth_endpoints_unresolved,
|
||||
)
|
||||
from litellm.types.mcp import MCPAuth, MCPTransport
|
||||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
server = MCPServer(
|
||||
server_id="github-configured",
|
||||
name="github_mcp",
|
||||
url="https://api.githubcopilot.com/mcp/",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
oauth2_flow="authorization_code",
|
||||
authorization_url=None,
|
||||
token_url=None,
|
||||
configured_authorization_url="https://github.com/login/oauth/authorize",
|
||||
configured_token_url="https://github.com/login/oauth/access_token",
|
||||
)
|
||||
assert _oauth_endpoints_unresolved(server) is False
|
||||
|
||||
|
||||
def test_passthrough_authorization_code_round_trips_and_rejects_hostile_input():
|
||||
"""The passthrough gateway code seals and recovers the ephemeral DCR client and upstream code,
|
||||
and is total over hostile input: a raw upstream code opens to None, and a tampered or
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue