mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
Merge pull request #33113 from BerriAI/litellm_mcp_oauth_error_relay
fix(mcp): relay upstream OAuth token and DCR rejections instead of a generic 500
This commit is contained in:
commit
384bbf7fc2
9 changed files with 1037 additions and 82 deletions
|
|
@ -24,6 +24,15 @@ from litellm.proxy._experimental.mcp_server.auth.token_endpoint_auth import (
|
|||
TokenEndpointAuthConfigError,
|
||||
build_token_endpoint_client_auth,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.faults import (
|
||||
CallerRejected,
|
||||
CredentialSource,
|
||||
UpstreamProtocolFault,
|
||||
classify_upstream_dcr_rejection,
|
||||
classify_upstream_token_rejection,
|
||||
dcr_fault_detail,
|
||||
render_token_fault,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.oauth_utils import (
|
||||
TOKEN_NO_CACHE_HEADERS,
|
||||
get_request_base_url,
|
||||
|
|
@ -1281,6 +1290,13 @@ def _finish_bridge_mint(
|
|||
return JSONResponse(body, headers=TOKEN_NO_CACHE_HEADERS)
|
||||
|
||||
|
||||
def _token_credential_source(mcp_server: MCPServer) -> CredentialSource:
|
||||
"""Mirrors the resolved-client rule in :func:`exchange_token_with_server`: when the server has a
|
||||
stored client_id the gateway presents its own credentials upstream, so a credential rejection is
|
||||
the operator's fault, not the caller's."""
|
||||
return "gateway_stored" if mcp_server.client_id else "caller_supplied"
|
||||
|
||||
|
||||
def _upstream_refresh_credential(token_response: object) -> "RefreshCredential | None":
|
||||
"""Extract the upstream refresh grant from a token response, or ``None`` when there is none to seal.
|
||||
Each field is isinstance-checked so nothing untyped reaches the refresh envelope; ``refresh_expires_in``
|
||||
|
|
@ -1338,21 +1354,6 @@ def _mint_refresh_envelope_value(
|
|||
return None
|
||||
|
||||
|
||||
def _upstream_oauth_error(response: httpx.Response) -> str | None:
|
||||
"""The RFC 6749 5.2 ``error`` code from an upstream token-endpoint error body, or ``None`` when the
|
||||
body is not a JSON object carrying a string ``error``. Reading the field beats substring-matching the
|
||||
raw text, which would false-match a code that only appears inside ``error_description`` (a false
|
||||
invalid_grant would trigger a needless authorization_code re-run)."""
|
||||
try:
|
||||
body = json.loads(response.text)
|
||||
except (ValueError, TypeError):
|
||||
return None
|
||||
if not isinstance(body, dict):
|
||||
return None
|
||||
error = body.get("error")
|
||||
return error if isinstance(error, str) else None
|
||||
|
||||
|
||||
async def exchange_token_with_server(
|
||||
request: Request,
|
||||
mcp_server: MCPServer,
|
||||
|
|
@ -1469,32 +1470,25 @@ async def exchange_token_with_server(
|
|||
return _bridge_mint_error_response(prepared)
|
||||
bridge_mint_ready = prepared
|
||||
async_client = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Check)
|
||||
response = await async_client.post(
|
||||
mcp_server.token_url,
|
||||
headers={"Accept": "application/json", **client_auth.headers},
|
||||
data=token_data,
|
||||
)
|
||||
if response is None:
|
||||
raise HTTPException(
|
||||
status_code=502,
|
||||
detail="MCP upstream token endpoint returned no response",
|
||||
)
|
||||
|
||||
try:
|
||||
response.raise_for_status()
|
||||
response = await async_client.post(
|
||||
mcp_server.token_url,
|
||||
headers={"Accept": "application/json", **client_auth.headers},
|
||||
data=token_data,
|
||||
)
|
||||
if response is not None:
|
||||
response.raise_for_status()
|
||||
except httpx.HTTPStatusError as exc:
|
||||
if "invalid_target" in exc.response.text:
|
||||
verbose_logger.warning(
|
||||
"MCP server %s: the upstream authorization server rejected the token request with "
|
||||
"invalid_target; it may require RFC 8707 resource indicators, which the gateway "
|
||||
"does not send yet (tracked as LIT-4339)",
|
||||
mcp_server.server_id,
|
||||
)
|
||||
fault = classify_upstream_token_rejection(
|
||||
exc.response,
|
||||
credential_source=_token_credential_source(mcp_server),
|
||||
log_context=mcp_server.server_id,
|
||||
)
|
||||
upstream_rejected_bridge_refresh = (
|
||||
is_bridge
|
||||
and grant_type == "refresh_token"
|
||||
and exc.response.status_code == 400
|
||||
and _upstream_oauth_error(exc.response) == "invalid_grant"
|
||||
and isinstance(fault, CallerRejected)
|
||||
and fault.code == "invalid_grant"
|
||||
)
|
||||
if upstream_rejected_bridge_refresh:
|
||||
verbose_logger.info(
|
||||
|
|
@ -1504,7 +1498,12 @@ async def exchange_token_with_server(
|
|||
mcp_server.server_id,
|
||||
)
|
||||
return _bridge_mint_error_response("invalid_refresh")
|
||||
raise
|
||||
return render_token_fault(fault)
|
||||
if response is None:
|
||||
raise HTTPException(
|
||||
status_code=502,
|
||||
detail="MCP upstream token endpoint returned no response",
|
||||
)
|
||||
token_response = response.json()
|
||||
|
||||
# Validate token response against server-configured rules before any storage.
|
||||
|
|
@ -1556,8 +1555,12 @@ async def exchange_token_with_server(
|
|||
minted = _finish_bridge_mint(bridge_mint_ready, mcp_server, token_response, datetime.now(timezone.utc))
|
||||
return minted if isinstance(minted, JSONResponse) else _bridge_mint_error_response(minted)
|
||||
|
||||
raw_access_token = token_response.get("access_token") if isinstance(token_response, dict) else None
|
||||
if not isinstance(raw_access_token, str) or not raw_access_token:
|
||||
return render_token_fault(UpstreamProtocolFault(note="the upstream token response has no usable access_token"))
|
||||
|
||||
result = {
|
||||
"access_token": token_response["access_token"],
|
||||
"access_token": raw_access_token,
|
||||
"token_type": token_response.get("token_type", "Bearer"),
|
||||
}
|
||||
|
||||
|
|
@ -1813,21 +1816,6 @@ async def _persist_dcr_client_registration(
|
|||
return "failed"
|
||||
|
||||
|
||||
_MAX_UPSTREAM_ERROR_CHARS = 500
|
||||
|
||||
|
||||
def _safe_upstream_error_detail(response: httpx.Response) -> str:
|
||||
"""Bounded plaintext summary of an upstream registration failure for the client.
|
||||
|
||||
RFC 7591 error bodies are small JSON objects (``error`` / ``error_description``); relaying the
|
||||
text lets the client read the real reason instead of a bare 500, and the length bound keeps a
|
||||
hostile or oversized upstream body from bloating the gateway response."""
|
||||
body = response.text
|
||||
if not body:
|
||||
return response.reason_phrase or "upstream registration failed"
|
||||
return body[:_MAX_UPSTREAM_ERROR_CHARS]
|
||||
|
||||
|
||||
async def register_client_with_server(
|
||||
request: Request,
|
||||
mcp_server: MCPServer,
|
||||
|
|
@ -1887,19 +1875,24 @@ async def register_client_with_server(
|
|||
}
|
||||
|
||||
async_client = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Register)
|
||||
response = await async_client.post(
|
||||
mcp_server.registration_url,
|
||||
headers=headers,
|
||||
json=register_data,
|
||||
)
|
||||
try:
|
||||
response = await async_client.post(
|
||||
mcp_server.registration_url,
|
||||
headers=headers,
|
||||
json=register_data,
|
||||
)
|
||||
if response is not None:
|
||||
response.raise_for_status()
|
||||
except httpx.HTTPStatusError as exc:
|
||||
status_code, detail = dcr_fault_detail(
|
||||
classify_upstream_dcr_rejection(exc.response, log_context=mcp_server.server_id)
|
||||
)
|
||||
raise HTTPException(status_code=status_code, detail=detail) from exc
|
||||
if response is None:
|
||||
raise HTTPException(
|
||||
status_code=502,
|
||||
detail="MCP upstream registration endpoint returned no response",
|
||||
)
|
||||
if bridge_relay and response.status_code >= 400:
|
||||
raise HTTPException(status_code=response.status_code, detail=_safe_upstream_error_detail(response))
|
||||
response.raise_for_status()
|
||||
|
||||
token_response = response.json()
|
||||
|
||||
|
|
|
|||
38
litellm/proxy/_experimental/mcp_server/faults/__init__.py
Normal file
38
litellm/proxy/_experimental/mcp_server/faults/__init__.py
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
"""Typed fault values for upstream OAuth/DCR failures (phase 1 of the MCP error-handling framework).
|
||||
|
||||
The invariant this package exists to enforce: an upstream failure is classified ONCE into a single
|
||||
fault value, and the response status, wire error code, and prose are all derived from that value.
|
||||
Deriving all three from one classification makes contradictory pairings (a caller-fault error code on
|
||||
a server-fault status) unrepresentable, and gives the trust-boundary rule one enforcement point:
|
||||
spec-defined machine fields may cross to callers, upstream prose and raw bodies go to server logs.
|
||||
"""
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.faults.classify import (
|
||||
classify_upstream_dcr_rejection,
|
||||
classify_upstream_token_rejection,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.faults.render_oauth import (
|
||||
dcr_fault_detail,
|
||||
render_token_fault,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.faults.types import (
|
||||
CallerRejected,
|
||||
CredentialSource,
|
||||
GatewayRejected,
|
||||
UpstreamOAuthFault,
|
||||
UpstreamProtocolFault,
|
||||
UpstreamReportedFault,
|
||||
)
|
||||
|
||||
__all__ = [
|
||||
"CallerRejected",
|
||||
"CredentialSource",
|
||||
"GatewayRejected",
|
||||
"UpstreamOAuthFault",
|
||||
"UpstreamProtocolFault",
|
||||
"UpstreamReportedFault",
|
||||
"classify_upstream_dcr_rejection",
|
||||
"classify_upstream_token_rejection",
|
||||
"dcr_fault_detail",
|
||||
"render_token_fault",
|
||||
]
|
||||
133
litellm/proxy/_experimental/mcp_server/faults/classify.py
Normal file
133
litellm/proxy/_experimental/mcp_server/faults/classify.py
Normal file
|
|
@ -0,0 +1,133 @@
|
|||
"""The single place that reads upstream OAuth/DCR failure responses.
|
||||
|
||||
Every accessor here is total: an upstream that lies about its content encoding, sends an undecodable
|
||||
body, or omits the spec fields yields a classified fault, never an exception. Nothing outside this
|
||||
module should touch a failed upstream response's body.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import httpx
|
||||
|
||||
from litellm._logging import verbose_logger
|
||||
from litellm.proxy._experimental.mcp_server.faults.types import (
|
||||
GATEWAY_CAPABILITY_CODES,
|
||||
GATEWAY_CREDENTIAL_CODES,
|
||||
MAX_WIRE_FIELD_CHARS,
|
||||
CallerRejected,
|
||||
CredentialSource,
|
||||
GatewayRejected,
|
||||
UpstreamOAuthFault,
|
||||
UpstreamProtocolFault,
|
||||
UpstreamReportedFault,
|
||||
)
|
||||
|
||||
|
||||
def _safe_text(response: httpx.Response) -> str:
|
||||
try:
|
||||
return response.text
|
||||
except Exception:
|
||||
return ""
|
||||
|
||||
|
||||
def _safe_json(response: httpx.Response) -> object:
|
||||
try:
|
||||
return response.json()
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
|
||||
def _bounded_field(value: object) -> str | None:
|
||||
if not isinstance(value, str) or not value:
|
||||
return None
|
||||
return value[:MAX_WIRE_FIELD_CHARS]
|
||||
|
||||
|
||||
def _log_out_of_contract(endpoint_kind: str, response: httpx.Response, log_context: str) -> None:
|
||||
verbose_logger.warning(
|
||||
"MCP upstream %s endpoint (%s) returned HTTP %s outside the OAuth error contract (first %s chars): %s",
|
||||
endpoint_kind,
|
||||
log_context,
|
||||
response.status_code,
|
||||
MAX_WIRE_FIELD_CHARS,
|
||||
_safe_text(response)[:MAX_WIRE_FIELD_CHARS],
|
||||
)
|
||||
|
||||
|
||||
def _classify_oauth_error_code(
|
||||
code: str,
|
||||
description: str | None,
|
||||
error_uri: str | None,
|
||||
credential_source: CredentialSource,
|
||||
log_context: str,
|
||||
) -> UpstreamOAuthFault:
|
||||
"""Blame assignment for a contract-conformant OAuth error code, shared by the token and DCR
|
||||
classifiers. Codes by which the upstream blames itself keep that blame; ``invalid_target`` is a
|
||||
gateway capability gap (RFC 8707 resource indicators, LIT-4339) no matter whose credentials were
|
||||
presented; credential-indicting codes follow the credential source; everything else, including
|
||||
codes we do not recognize, is the caller's to act on. The upstream's HTTP status is deliberately
|
||||
never consulted: status derives from this classification at render time, which is what keeps
|
||||
status and code from contradicting each other."""
|
||||
if code == "server_error" or code == "temporarily_unavailable":
|
||||
return UpstreamReportedFault(code=code)
|
||||
if code in GATEWAY_CAPABILITY_CODES:
|
||||
verbose_logger.warning(
|
||||
"MCP server %s: the upstream authorization server rejected the request with "
|
||||
"invalid_target; it may require RFC 8707 resource indicators, which the gateway "
|
||||
"does not send yet (tracked as LIT-4339)",
|
||||
log_context,
|
||||
)
|
||||
return GatewayRejected(code=code)
|
||||
if credential_source == "gateway_stored" and code in GATEWAY_CREDENTIAL_CODES:
|
||||
verbose_logger.warning(
|
||||
"MCP server %s: upstream authorization server rejected the gateway's configured client "
|
||||
"credentials (%s): %s",
|
||||
log_context,
|
||||
code,
|
||||
description or "<no description>",
|
||||
)
|
||||
return GatewayRejected(code=code)
|
||||
return CallerRejected(code=code, description=description, error_uri=error_uri)
|
||||
|
||||
|
||||
def classify_upstream_token_rejection(
|
||||
response: httpx.Response,
|
||||
credential_source: CredentialSource,
|
||||
log_context: str,
|
||||
) -> UpstreamOAuthFault:
|
||||
"""Classify a token-endpoint rejection into exactly one fault: a body with an RFC 6749 §5.2
|
||||
``error`` field goes through blame assignment (:func:`_classify_oauth_error_code`); anything
|
||||
without a usable ``error`` field is an upstream protocol fault."""
|
||||
parsed = _safe_json(response)
|
||||
fields = parsed if isinstance(parsed, dict) else {}
|
||||
code = _bounded_field(fields.get("error"))
|
||||
if code is None:
|
||||
_log_out_of_contract("token", response, log_context)
|
||||
return UpstreamProtocolFault(note=f"upstream token endpoint returned HTTP {response.status_code}")
|
||||
return _classify_oauth_error_code(
|
||||
code,
|
||||
description=_bounded_field(fields.get("error_description")),
|
||||
error_uri=_bounded_field(fields.get("error_uri")),
|
||||
credential_source=credential_source,
|
||||
log_context=log_context,
|
||||
)
|
||||
|
||||
|
||||
def classify_upstream_dcr_rejection(response: httpx.Response, log_context: str) -> UpstreamOAuthFault:
|
||||
"""Classify a dynamic-client-registration rejection. RFC 7591 §3.2.2 errors carry
|
||||
``error`` / ``error_description`` and go through the same blame assignment as token errors
|
||||
(registration sends no client credentials, so credential codes stay caller-actionable); anything
|
||||
without a usable ``error`` field is an upstream protocol fault."""
|
||||
parsed = _safe_json(response)
|
||||
fields = parsed if isinstance(parsed, dict) else {}
|
||||
code = _bounded_field(fields.get("error"))
|
||||
if code is None:
|
||||
_log_out_of_contract("registration", response, log_context)
|
||||
return UpstreamProtocolFault(note=f"upstream registration failed with HTTP {response.status_code}")
|
||||
return _classify_oauth_error_code(
|
||||
code,
|
||||
description=_bounded_field(fields.get("error_description")),
|
||||
error_uri=None,
|
||||
credential_source="caller_supplied",
|
||||
log_context=log_context,
|
||||
)
|
||||
|
|
@ -0,0 +1,89 @@
|
|||
"""Render upstream OAuth/DCR faults onto the wire. The only place that chooses statuses and bodies
|
||||
for these faults, so every consumer emits the same contract: RFC 6749 §5.2-shaped JSON with the §5.1
|
||||
no-store headers on token endpoints, HTTPException details on registration. Status, code, and prose
|
||||
all derive from the fault tag; exhaustive matches keep a new fault arm from shipping unrendered.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from fastapi.responses import JSONResponse
|
||||
from typing_extensions import assert_never
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.faults.types import UpstreamOAuthFault
|
||||
from litellm.proxy._experimental.mcp_server.oauth_utils import TOKEN_NO_CACHE_HEADERS
|
||||
|
||||
|
||||
def _gateway_rejected_description(code: str) -> str:
|
||||
if code == "invalid_target":
|
||||
return (
|
||||
"the upstream authorization server rejected the request (invalid_target); "
|
||||
"it may require RFC 8707 resource indicators, which the gateway does not send yet"
|
||||
)
|
||||
return (
|
||||
f"the upstream authorization server rejected the gateway's configured client credentials "
|
||||
f"({code}); verify the MCP server's client_id and client_secret"
|
||||
)
|
||||
|
||||
|
||||
def _upstream_reported_status_and_description(code: str) -> tuple[int, str]:
|
||||
if code == "temporarily_unavailable":
|
||||
return 503, "the upstream authorization server is temporarily unavailable; retry shortly"
|
||||
return 502, "the upstream authorization server reported an internal error"
|
||||
|
||||
|
||||
def render_token_fault(fault: UpstreamOAuthFault) -> JSONResponse:
|
||||
"""RFC 6749 §5.2 response for a token-endpoint fault. Caller-actionable rejections relay the
|
||||
upstream's code on the status that code implies (401 for invalid_client per §5.2, else 400);
|
||||
gateway-side faults are 502 ``server_error`` with gateway-authored prose so a caller is never
|
||||
blamed for, or shown the internals of, a failure only the operator can fix."""
|
||||
match fault.tag:
|
||||
case "caller_rejected":
|
||||
content = {
|
||||
"error": fault.code,
|
||||
**({"error_description": fault.description} if fault.description else {}),
|
||||
**({"error_uri": fault.error_uri} if fault.error_uri else {}),
|
||||
}
|
||||
status_code = 401 if fault.code == "invalid_client" else 400
|
||||
return JSONResponse(status_code=status_code, content=content, headers=TOKEN_NO_CACHE_HEADERS)
|
||||
case "gateway_rejected":
|
||||
return JSONResponse(
|
||||
status_code=502,
|
||||
content={
|
||||
"error": "server_error",
|
||||
"error_description": _gateway_rejected_description(fault.code),
|
||||
},
|
||||
headers=TOKEN_NO_CACHE_HEADERS,
|
||||
)
|
||||
case "upstream_reported_fault":
|
||||
status_code, description = _upstream_reported_status_and_description(fault.code)
|
||||
return JSONResponse(
|
||||
status_code=status_code,
|
||||
content={"error": fault.code, "error_description": description},
|
||||
headers=TOKEN_NO_CACHE_HEADERS,
|
||||
)
|
||||
case "upstream_protocol_fault":
|
||||
return JSONResponse(
|
||||
status_code=502,
|
||||
content={"error": "server_error", "error_description": fault.note},
|
||||
headers=TOKEN_NO_CACHE_HEADERS,
|
||||
)
|
||||
case _:
|
||||
assert_never(fault.tag)
|
||||
|
||||
|
||||
def dcr_fault_detail(fault: UpstreamOAuthFault) -> tuple[int, str]:
|
||||
"""Status and detail string for a registration fault, raised as HTTPException by the caller.
|
||||
RFC 7591 §3.2.2 defines registration errors as 400, so a contract-conformant rejection is 400
|
||||
regardless of the status the upstream chose; everything else is a 502 upstream fault."""
|
||||
match fault.tag:
|
||||
case "caller_rejected":
|
||||
detail = f"{fault.code}: {fault.description}" if fault.description else fault.code
|
||||
return 400, detail
|
||||
case "gateway_rejected":
|
||||
return 502, _gateway_rejected_description(fault.code)
|
||||
case "upstream_reported_fault":
|
||||
return _upstream_reported_status_and_description(fault.code)
|
||||
case "upstream_protocol_fault":
|
||||
return 502, fault.note
|
||||
case _:
|
||||
assert_never(fault.tag)
|
||||
79
litellm/proxy/_experimental/mcp_server/faults/types.py
Normal file
79
litellm/proxy/_experimental/mcp_server/faults/types.py
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
"""Fault taxonomy for upstream OAuth token and DCR registration failures.
|
||||
|
||||
Each fault is a frozen model on a ``tag`` literal. The tag alone decides the HTTP status, the wire
|
||||
error code, and whose prose the caller sees, so those three facts can never disagree the way they can
|
||||
when an upstream's status and error code are relayed independently.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Literal, TypeAlias
|
||||
|
||||
from pydantic import BaseModel, ConfigDict
|
||||
|
||||
MAX_WIRE_FIELD_CHARS = 500
|
||||
"""Bound on every upstream-derived string that crosses to a caller or into a log line."""
|
||||
|
||||
CredentialSource: TypeAlias = Literal["gateway_stored", "caller_supplied"]
|
||||
"""Whose client credentials the gateway presented upstream: the MCP server's stored configuration or
|
||||
credentials the caller supplied on the request. Decides whether a credential rejection is the
|
||||
caller's problem to fix or the gateway operator's."""
|
||||
|
||||
GATEWAY_CREDENTIAL_CODES: frozenset[str] = frozenset({"invalid_client", "unauthorized_client"})
|
||||
"""RFC 6749 error codes that indict the OAuth client's credentials or grant authorization. When the
|
||||
gateway presented its own stored credentials, these are gateway-side faults the caller cannot act on;
|
||||
when the caller supplied the credentials, they are the caller's to fix."""
|
||||
|
||||
GATEWAY_CAPABILITY_CODES: frozenset[str] = frozenset({"invalid_target"})
|
||||
"""Codes that indict a gateway capability regardless of whose credentials were presented:
|
||||
``invalid_target`` means the upstream wants RFC 8707 resource indicators, which the gateway does not
|
||||
send yet (LIT-4339). Never the caller's fault."""
|
||||
|
||||
UPSTREAM_FAULT_CODES: frozenset[str] = frozenset({"server_error", "temporarily_unavailable"})
|
||||
"""Codes by which the upstream blames itself. Relaying them as caller faults would invert blame, so
|
||||
they classify as upstream-reported faults and render on the 5xx their meaning implies."""
|
||||
|
||||
|
||||
class CallerRejected(BaseModel):
|
||||
"""The upstream spoke the OAuth error contract and the failure is actionable by our caller
|
||||
(e.g. ``invalid_grant``: re-run authorization). The code and its bounded prose relay on the
|
||||
4xx status the code itself implies."""
|
||||
|
||||
model_config = ConfigDict(frozen=True)
|
||||
tag: Literal["caller_rejected"] = "caller_rejected"
|
||||
code: str
|
||||
description: str | None = None
|
||||
error_uri: str | None = None
|
||||
|
||||
|
||||
class GatewayRejected(BaseModel):
|
||||
"""The upstream rejected the request for a cause only the gateway operator can address: the
|
||||
server's stored client credentials or a gateway capability gap. Not actionable by the caller:
|
||||
rendered as 502 with gateway-authored prose naming the code; the upstream's prose goes to
|
||||
server logs only."""
|
||||
|
||||
model_config = ConfigDict(frozen=True)
|
||||
tag: Literal["gateway_rejected"] = "gateway_rejected"
|
||||
code: str
|
||||
|
||||
|
||||
class UpstreamReportedFault(BaseModel):
|
||||
"""The upstream blamed itself in the OAuth vocabulary. Rendered on the 5xx the code implies
|
||||
(``server_error`` 502, ``temporarily_unavailable`` 503) so blame and status agree."""
|
||||
|
||||
model_config = ConfigDict(frozen=True)
|
||||
tag: Literal["upstream_reported_fault"] = "upstream_reported_fault"
|
||||
code: Literal["server_error", "temporarily_unavailable"]
|
||||
|
||||
|
||||
class UpstreamProtocolFault(BaseModel):
|
||||
"""The upstream broke the error contract: no JSON ``error`` field, an undecodable body, or a
|
||||
success response without a usable token. Rendered as 502 with a gateway-authored note; the
|
||||
upstream body never crosses to the caller."""
|
||||
|
||||
model_config = ConfigDict(frozen=True)
|
||||
tag: Literal["upstream_protocol_fault"] = "upstream_protocol_fault"
|
||||
note: str
|
||||
|
||||
|
||||
UpstreamOAuthFault: TypeAlias = CallerRejected | GatewayRejected | UpstreamReportedFault | UpstreamProtocolFault
|
||||
|
|
@ -0,0 +1,147 @@
|
|||
"""Classification matrix for upstream OAuth/DCR rejections: who is blamed depends only on the §5.2
|
||||
code and whose credentials the gateway presented, never on the upstream's HTTP status."""
|
||||
|
||||
import httpx
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.faults.classify import (
|
||||
classify_upstream_dcr_rejection,
|
||||
classify_upstream_token_rejection,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.faults.types import (
|
||||
CallerRejected,
|
||||
GatewayRejected,
|
||||
UpstreamProtocolFault,
|
||||
UpstreamReportedFault,
|
||||
)
|
||||
|
||||
|
||||
def _response(status_code: int, *, json_body: object = None, text_body: str = "", headers: dict = None) -> httpx.Response:
|
||||
request = httpx.Request("POST", "https://idp.example.com/token")
|
||||
if json_body is not None:
|
||||
return httpx.Response(status_code, json=json_body, request=request)
|
||||
return httpx.Response(status_code, text=text_body, headers=headers or {}, request=request)
|
||||
|
||||
|
||||
def test_caller_fault_code_classifies_as_caller_rejected_regardless_of_status():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(500, json_body={"error": "invalid_grant", "error_description": "Code expired."}),
|
||||
credential_source="gateway_stored",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, CallerRejected)
|
||||
assert fault.code == "invalid_grant"
|
||||
assert fault.description == "Code expired."
|
||||
|
||||
|
||||
def test_credential_code_with_gateway_stored_credentials_indicts_gateway():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(401, json_body={"error": "invalid_client", "error_description": "not found"}),
|
||||
credential_source="gateway_stored",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, GatewayRejected)
|
||||
assert fault.code == "invalid_client"
|
||||
|
||||
|
||||
def test_credential_code_with_caller_supplied_credentials_stays_caller_fault():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(401, json_body={"error": "invalid_client"}),
|
||||
credential_source="caller_supplied",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, CallerRejected)
|
||||
assert fault.code == "invalid_client"
|
||||
|
||||
|
||||
def test_unknown_code_relays_as_caller_rejected():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(400, json_body={"error": "slow_down", "error_description": "Polling too fast."}),
|
||||
credential_source="gateway_stored",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, CallerRejected)
|
||||
assert fault.code == "slow_down"
|
||||
|
||||
|
||||
def test_body_without_error_field_is_protocol_fault():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(404, text_body="<html>not here</html>"),
|
||||
credential_source="gateway_stored",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, UpstreamProtocolFault)
|
||||
assert fault.note == "upstream token endpoint returned HTTP 404"
|
||||
|
||||
|
||||
def test_unreadable_body_is_protocol_fault_not_exception():
|
||||
unreadable = httpx.Response(
|
||||
400,
|
||||
stream=httpx.ByteStream(b"\x1f\x8bnot-gzip"),
|
||||
headers={"content-encoding": "gzip"},
|
||||
request=httpx.Request("POST", "https://idp.example.com/token"),
|
||||
)
|
||||
fault = classify_upstream_token_rejection(unreadable, credential_source="gateway_stored", log_context="srv")
|
||||
assert isinstance(fault, UpstreamProtocolFault)
|
||||
|
||||
|
||||
def test_wire_fields_are_bounded():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(400, json_body={"error": "invalid_request", "error_description": "x" * 5000}),
|
||||
credential_source="gateway_stored",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, CallerRejected)
|
||||
assert len(fault.description) == 500
|
||||
|
||||
|
||||
def test_dcr_rejection_with_rfc7591_code_is_caller_rejected():
|
||||
fault = classify_upstream_dcr_rejection(
|
||||
_response(400, json_body={"error": "invalid_redirect_uri", "error_description": "not allowed"}),
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, CallerRejected)
|
||||
assert fault.code == "invalid_redirect_uri"
|
||||
|
||||
|
||||
def test_dcr_rejection_without_code_is_protocol_fault():
|
||||
fault = classify_upstream_dcr_rejection(_response(500, text_body="<html>trace</html>"), log_context="srv")
|
||||
assert isinstance(fault, UpstreamProtocolFault)
|
||||
assert fault.note == "upstream registration failed with HTTP 500"
|
||||
|
||||
|
||||
def test_upstream_self_blame_codes_stay_upstream_faults():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(400, json_body={"error": "server_error", "error_description": "boom"}),
|
||||
credential_source="caller_supplied",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, UpstreamReportedFault)
|
||||
assert fault.code == "server_error"
|
||||
|
||||
|
||||
def test_temporarily_unavailable_is_upstream_fault():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(503, json_body={"error": "temporarily_unavailable"}),
|
||||
credential_source="gateway_stored",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, UpstreamReportedFault)
|
||||
assert fault.code == "temporarily_unavailable"
|
||||
|
||||
|
||||
def test_invalid_target_is_gateway_fault_even_with_caller_credentials():
|
||||
fault = classify_upstream_token_rejection(
|
||||
_response(400, json_body={"error": "invalid_target"}),
|
||||
credential_source="caller_supplied",
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, GatewayRejected)
|
||||
assert fault.code == "invalid_target"
|
||||
|
||||
|
||||
def test_dcr_server_error_code_is_not_blamed_on_caller():
|
||||
fault = classify_upstream_dcr_rejection(
|
||||
_response(500, json_body={"error": "server_error"}),
|
||||
log_context="srv",
|
||||
)
|
||||
assert isinstance(fault, UpstreamReportedFault)
|
||||
|
|
@ -0,0 +1,96 @@
|
|||
"""Rendering contract: status, wire code, and prose all derive from the fault tag, so a caller-fault
|
||||
code can never ship on a server-fault status and gateway-side faults never carry provider prose."""
|
||||
|
||||
import json
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.faults.render_oauth import (
|
||||
dcr_fault_detail,
|
||||
render_token_fault,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.faults.types import (
|
||||
CallerRejected,
|
||||
GatewayRejected,
|
||||
UpstreamProtocolFault,
|
||||
UpstreamReportedFault,
|
||||
)
|
||||
|
||||
|
||||
def test_caller_rejected_renders_code_derived_status():
|
||||
response = render_token_fault(CallerRejected(code="invalid_grant", description="Code expired."))
|
||||
assert response.status_code == 400
|
||||
assert json.loads(response.body) == {"error": "invalid_grant", "error_description": "Code expired."}
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
|
||||
|
||||
def test_caller_rejected_invalid_client_renders_401():
|
||||
response = render_token_fault(CallerRejected(code="invalid_client"))
|
||||
assert response.status_code == 401
|
||||
assert json.loads(response.body) == {"error": "invalid_client"}
|
||||
|
||||
|
||||
def test_caller_rejected_includes_error_uri_only_when_present():
|
||||
response = render_token_fault(
|
||||
CallerRejected(code="invalid_scope", description="bad scope", error_uri="https://idp.example.com/e")
|
||||
)
|
||||
assert json.loads(response.body) == {
|
||||
"error": "invalid_scope",
|
||||
"error_description": "bad scope",
|
||||
"error_uri": "https://idp.example.com/e",
|
||||
}
|
||||
|
||||
|
||||
def test_gateway_rejected_renders_502_with_gateway_prose():
|
||||
response = render_token_fault(GatewayRejected(code="invalid_client"))
|
||||
assert response.status_code == 502
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "server_error"
|
||||
assert "invalid_client" in body["error_description"]
|
||||
assert "client_id and client_secret" in body["error_description"]
|
||||
|
||||
|
||||
def test_gateway_invalid_target_prose_names_resource_indicators():
|
||||
response = render_token_fault(GatewayRejected(code="invalid_target"))
|
||||
body = json.loads(response.body)
|
||||
assert response.status_code == 502
|
||||
assert "RFC 8707" in body["error_description"]
|
||||
|
||||
|
||||
def test_protocol_fault_renders_502_note():
|
||||
response = render_token_fault(UpstreamProtocolFault(note="upstream token endpoint returned HTTP 503"))
|
||||
assert response.status_code == 502
|
||||
assert json.loads(response.body) == {
|
||||
"error": "server_error",
|
||||
"error_description": "upstream token endpoint returned HTTP 503",
|
||||
}
|
||||
|
||||
|
||||
def test_dcr_caller_rejection_is_400_per_rfc7591_regardless_of_upstream_status():
|
||||
status_code, detail = dcr_fault_detail(CallerRejected(code="invalid_client_metadata", description="bad grant types"))
|
||||
assert status_code == 400
|
||||
assert detail == "invalid_client_metadata: bad grant types"
|
||||
|
||||
|
||||
def test_dcr_protocol_fault_is_502():
|
||||
status_code, detail = dcr_fault_detail(UpstreamProtocolFault(note="upstream registration failed with HTTP 500"))
|
||||
assert status_code == 502
|
||||
assert detail == "upstream registration failed with HTTP 500"
|
||||
|
||||
|
||||
def test_upstream_reported_server_error_renders_502_with_matching_code():
|
||||
response = render_token_fault(UpstreamReportedFault(code="server_error"))
|
||||
assert response.status_code == 502
|
||||
assert json.loads(response.body)["error"] == "server_error"
|
||||
|
||||
|
||||
def test_upstream_reported_temporarily_unavailable_renders_503_with_matching_code():
|
||||
response = render_token_fault(UpstreamReportedFault(code="temporarily_unavailable"))
|
||||
assert response.status_code == 503
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "temporarily_unavailable"
|
||||
assert "retry" in body["error_description"]
|
||||
|
||||
|
||||
def test_dcr_upstream_reported_fault_maps_to_5xx():
|
||||
status_code, detail = dcr_fault_detail(UpstreamReportedFault(code="server_error"))
|
||||
assert status_code == 502
|
||||
assert "internal error" in detail
|
||||
|
|
@ -4274,6 +4274,9 @@ async def test_register_bridge_relay_surfaces_upstream_error_not_500():
|
|||
error_response = MagicMock()
|
||||
error_response.status_code = 400
|
||||
error_response.text = '{"error":"invalid_redirect_uri","error_description":"redirect_uri not allowed"}'
|
||||
error_response.json = MagicMock(
|
||||
return_value={"error": "invalid_redirect_uri", "error_description": "redirect_uri not allowed"}
|
||||
)
|
||||
error_response.raise_for_status = MagicMock(
|
||||
side_effect=httpx.HTTPStatusError("bad", request=MagicMock(), response=error_response)
|
||||
)
|
||||
|
|
@ -4307,9 +4310,10 @@ async def test_register_bridge_relay_surfaces_upstream_error_not_500():
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_non_bridge_upstream_error_still_raises_500():
|
||||
"""Non-bridge DCR keeps its pre-change behavior: raise_for_status propagates so the flag-off
|
||||
contract is byte-identical; only the bridge relay arm relays the upstream status."""
|
||||
async def test_register_non_bridge_upstream_error_relays_status_not_500():
|
||||
"""A non-bridge DCR rejection must relay the upstream status and RFC 7591 error body just like
|
||||
the bridge relay arm; a raw HTTPStatusError would escape to the global handler and surface as an
|
||||
opaque 500 that hides the real reason from the create-flow UI."""
|
||||
import httpx
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
|
|
@ -4319,6 +4323,7 @@ async def test_register_non_bridge_upstream_error_still_raises_500():
|
|||
error_response = MagicMock()
|
||||
error_response.status_code = 400
|
||||
error_response.text = '{"error":"invalid_client_metadata"}'
|
||||
error_response.json = MagicMock(return_value={"error": "invalid_client_metadata"})
|
||||
error_response.raise_for_status = MagicMock(
|
||||
side_effect=httpx.HTTPStatusError("bad", request=MagicMock(), response=error_response)
|
||||
)
|
||||
|
|
@ -4338,7 +4343,7 @@ async def test_register_non_bridge_upstream_error_still_raises_500():
|
|||
return_value=False,
|
||||
),
|
||||
):
|
||||
with pytest.raises(httpx.HTTPStatusError):
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await register_client_with_server(
|
||||
request=_bridge_mock_request(),
|
||||
mcp_server=oauth2_server,
|
||||
|
|
@ -4348,6 +4353,9 @@ async def test_register_non_bridge_upstream_error_still_raises_500():
|
|||
token_endpoint_auth_method=None,
|
||||
)
|
||||
|
||||
assert exc.value.status_code == 400
|
||||
assert "invalid_client_metadata" in str(exc.value.detail)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_bridge_relay_never_persists():
|
||||
|
|
@ -5143,6 +5151,7 @@ async def test_bridge_refresh_upstream_invalid_grant_maps_to_invalid_grant():
|
|||
error_response = MagicMock()
|
||||
error_response.status_code = 400
|
||||
error_response.text = '{"error": "invalid_grant", "error_description": "refresh token expired"}'
|
||||
error_response.json = MagicMock(return_value={"error": "invalid_grant", "error_description": "refresh token expired"})
|
||||
error_response.raise_for_status = MagicMock(
|
||||
side_effect=httpx.HTTPStatusError("bad", request=MagicMock(), response=error_response)
|
||||
)
|
||||
|
|
@ -5178,10 +5187,10 @@ async def test_bridge_refresh_upstream_invalid_grant_maps_to_invalid_grant():
|
|||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bridge_refresh_upstream_error_detection_parses_json_not_substring():
|
||||
"""The upstream invalid_grant detection parses the RFC 6749 5.2 error field, not a substring of the
|
||||
body. An upstream error whose code is not invalid_grant (here invalid_client, with the string
|
||||
invalid_grant only inside error_description) must NOT be mistaken for a dead refresh token, so it
|
||||
propagates as the upstream error rather than triggering a spurious authorization_code re-run."""
|
||||
"""The upstream invalid_grant detection reads the classified RFC 6749 5.2 error code, not a substring
|
||||
of the body. An upstream error whose code is not invalid_grant (here invalid_client, with the string
|
||||
invalid_grant only inside error_description) must NOT be mistaken for a dead refresh token: it renders
|
||||
as the classified upstream rejection rather than triggering a spurious authorization_code re-run."""
|
||||
import httpx
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import exchange_token_with_server
|
||||
|
|
@ -5193,6 +5202,9 @@ async def test_bridge_refresh_upstream_error_detection_parses_json_not_substring
|
|||
error_response = MagicMock()
|
||||
error_response.status_code = 400
|
||||
error_response.text = '{"error": "invalid_client", "error_description": "this is not an invalid_grant problem"}'
|
||||
error_response.json = MagicMock(
|
||||
return_value={"error": "invalid_client", "error_description": "this is not an invalid_grant problem"}
|
||||
)
|
||||
error_response.raise_for_status = MagicMock(
|
||||
side_effect=httpx.HTTPStatusError("bad", request=MagicMock(), response=error_response)
|
||||
)
|
||||
|
|
@ -5210,18 +5222,21 @@ async def test_bridge_refresh_upstream_error_detection_parses_json_not_substring
|
|||
),
|
||||
patch("litellm.proxy.proxy_server.master_key", _BRIDGE_MASTER_KEY),
|
||||
):
|
||||
with pytest.raises(httpx.HTTPStatusError):
|
||||
await exchange_token_with_server(
|
||||
request=_bridge_mock_request(),
|
||||
mcp_server=server,
|
||||
grant_type="refresh_token",
|
||||
code=None,
|
||||
redirect_uri=None,
|
||||
client_id="dcr-client-123",
|
||||
client_secret=None,
|
||||
code_verifier=None,
|
||||
refresh_token=refresh_env,
|
||||
)
|
||||
response = await exchange_token_with_server(
|
||||
request=_bridge_mock_request(),
|
||||
mcp_server=server,
|
||||
grant_type="refresh_token",
|
||||
code=None,
|
||||
redirect_uri=None,
|
||||
client_id="dcr-client-123",
|
||||
client_secret=None,
|
||||
code_verifier=None,
|
||||
refresh_token=refresh_env,
|
||||
)
|
||||
|
||||
assert response.status_code == 401
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "invalid_client"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -6755,3 +6770,364 @@ async def test_token_exchange_pairs_client_secret_with_server_client_id():
|
|||
sent = mock_async_client.post.call_args.kwargs["data"]
|
||||
assert sent["client_id"] == "persisted-client"
|
||||
assert "client_secret" not in sent
|
||||
|
||||
|
||||
def _upstream_token_response(status_code: int, *, json_body: object = None, text_body: str = "") -> "httpx.Response":
|
||||
import httpx
|
||||
|
||||
request = httpx.Request("POST", "https://oauth2.googleapis.com/token")
|
||||
if json_body is not None:
|
||||
return httpx.Response(status_code, json=json_body, request=request)
|
||||
return httpx.Response(status_code, text=text_body, request=request)
|
||||
|
||||
|
||||
async def _exchange_with_upstream_response(upstream_response, *, server_client_id="web-client.apps.googleusercontent.com"):
|
||||
"""Run the raw (non-bridge) authorization_code exchange against a canned upstream token-endpoint
|
||||
response and return what the gateway would hand the client. ``server_client_id=None`` models the
|
||||
caller-supplied-credentials flow (no stored client on the server)."""
|
||||
from fastapi import Request
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
exchange_token_with_server,
|
||||
)
|
||||
from litellm.proxy._types import MCPTransport
|
||||
from litellm.types.mcp import MCPAuth
|
||||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
server = MCPServer(
|
||||
server_id="gcal",
|
||||
name="gcal",
|
||||
server_name="gcal",
|
||||
alias="gcal",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
client_id=server_client_id,
|
||||
authorization_url="https://accounts.google.com/o/oauth2/v2/auth",
|
||||
token_url="https://oauth2.googleapis.com/token",
|
||||
)
|
||||
mock_request = MagicMock(spec=Request)
|
||||
mock_request.base_url = "https://litellm.example.com/"
|
||||
mock_request.headers = {}
|
||||
mock_async_client = MagicMock()
|
||||
mock_async_client.post = AsyncMock(return_value=upstream_response)
|
||||
|
||||
with patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
|
||||
return_value=mock_async_client,
|
||||
):
|
||||
return await exchange_token_with_server(
|
||||
request=mock_request,
|
||||
mcp_server=server,
|
||||
grant_type="authorization_code",
|
||||
code="auth-code",
|
||||
redirect_uri="https://litellm.example.com/ui/mcp/oauth/callback",
|
||||
client_id="web-client.apps.googleusercontent.com",
|
||||
client_secret=None,
|
||||
code_verifier="verifier",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_gateway_credential_rejection_is_502_with_gateway_prose():
|
||||
"""When the gateway presented the server's stored client credentials and the IdP rejected them
|
||||
(Google refusing a secret-less or unknown client), the fault is the operator's, not the caller's:
|
||||
502 server_error with gateway-authored prose naming the code, and the IdP's own prose stays in
|
||||
server logs. Before the framework this either 500ed raw or relayed provider prose verbatim."""
|
||||
response = await _exchange_with_upstream_response(
|
||||
_upstream_token_response(
|
||||
401, json_body={"error": "invalid_client", "error_description": "The OAuth client was not found."}
|
||||
)
|
||||
)
|
||||
|
||||
assert response.status_code == 502
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "server_error"
|
||||
assert "invalid_client" in body["error_description"]
|
||||
assert "client_id and client_secret" in body["error_description"]
|
||||
assert "The OAuth client was not found." not in body["error_description"]
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_caller_supplied_credential_rejection_relays_code():
|
||||
"""When the caller supplied the client credentials themselves (no stored client on the server),
|
||||
an invalid_client rejection is theirs to act on: the §5.2 code relays on the 401 that code
|
||||
implies."""
|
||||
response = await _exchange_with_upstream_response(
|
||||
_upstream_token_response(
|
||||
401, json_body={"error": "invalid_client", "error_description": "The OAuth client was not found."}
|
||||
),
|
||||
server_client_id=None,
|
||||
)
|
||||
|
||||
assert response.status_code == 401
|
||||
body = json.loads(response.body)
|
||||
assert body == {"error": "invalid_client", "error_description": "The OAuth client was not found."}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_status_derives_from_error_code_not_upstream_status():
|
||||
"""An upstream that pairs a caller-fault code with a server-fault status (invalid_grant on a 500)
|
||||
must not produce a contradictory response: status derives from the classified fault, so the
|
||||
caller sees 400 invalid_grant and knows to re-authorize rather than blaming the gateway."""
|
||||
response = await _exchange_with_upstream_response(
|
||||
_upstream_token_response(500, json_body={"error": "invalid_grant", "error_description": "Code expired."})
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
body = json.loads(response.body)
|
||||
assert body == {"error": "invalid_grant", "error_description": "Code expired."}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_relays_only_rfc6749_error_fields():
|
||||
"""Only error / error_description / error_uri cross the gateway; any other upstream body field is
|
||||
dropped so an arbitrary rejection payload cannot ride the relay to the client."""
|
||||
response = await _exchange_with_upstream_response(
|
||||
_upstream_token_response(
|
||||
400,
|
||||
json_body={
|
||||
"error": "invalid_grant",
|
||||
"error_description": "Code was already redeemed.",
|
||||
"error_uri": "https://idp.example.com/errors/invalid_grant",
|
||||
"internal_trace": "should never reach the client",
|
||||
},
|
||||
)
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
body = json.loads(response.body)
|
||||
assert set(body.keys()) == {"error", "error_description", "error_uri"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_maps_out_of_contract_rejection_to_502():
|
||||
"""A rejection outside the §5.2 contract (no JSON error field, or a status the token-endpoint
|
||||
contract does not define) is an upstream fault; 502 keeps it from being misread as a caller
|
||||
mistake while the description still names the upstream status."""
|
||||
response = await _exchange_with_upstream_response(
|
||||
_upstream_token_response(503, text_body="<html>upstream maintenance</html>")
|
||||
)
|
||||
|
||||
assert response.status_code == 502
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "server_error"
|
||||
assert "HTTP 503" in body["error_description"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_bounds_relayed_error_fields():
|
||||
"""Relayed §5.2 fields are length-bounded so a hostile or broken upstream cannot bloat the
|
||||
gateway response."""
|
||||
response = await _exchange_with_upstream_response(
|
||||
_upstream_token_response(400, json_body={"error": "invalid_request", "error_description": "x" * 5000})
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
body = json.loads(response.body)
|
||||
assert len(body["error_description"]) == 500
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_200_without_access_token_is_502_not_keyerror():
|
||||
"""A 200 whose body has no usable access_token used to KeyError into a 500; the raw arm now
|
||||
answers 502 with the same wording as the bridge arm's no_upstream_token rejection."""
|
||||
response = await _exchange_with_upstream_response(
|
||||
_upstream_token_response(200, json_body={"token_type": "Bearer"})
|
||||
)
|
||||
|
||||
assert response.status_code == 502
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "server_error"
|
||||
assert "access_token" in body["error_description"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_relays_rejection_when_http_client_raises():
|
||||
"""litellm's AsyncHTTPHandler.post raise_for_status()es internally and raises MaskedHTTPStatusError
|
||||
at call time, so in production the rejection escapes from the post call itself rather than from the
|
||||
explicit raise_for_status; the relay must catch it there too (proven live: a mock returning the
|
||||
error response passed while the real proxy still 500ed)."""
|
||||
import httpx
|
||||
|
||||
rejection = _upstream_token_response(
|
||||
401, json_body={"error": "invalid_client", "error_description": "The OAuth client was not found."}
|
||||
)
|
||||
raising_client = MagicMock()
|
||||
raising_client.post = AsyncMock(
|
||||
side_effect=httpx.HTTPStatusError("Client error '401 Unauthorized'", request=rejection.request, response=rejection)
|
||||
)
|
||||
|
||||
from fastapi import Request
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
exchange_token_with_server,
|
||||
)
|
||||
from litellm.proxy._types import MCPTransport
|
||||
from litellm.types.mcp import MCPAuth
|
||||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
server = MCPServer(
|
||||
server_id="gcal",
|
||||
name="gcal",
|
||||
server_name="gcal",
|
||||
alias="gcal",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
client_id="web-client.apps.googleusercontent.com",
|
||||
authorization_url="https://accounts.google.com/o/oauth2/v2/auth",
|
||||
token_url="https://oauth2.googleapis.com/token",
|
||||
)
|
||||
mock_request = MagicMock(spec=Request)
|
||||
mock_request.base_url = "https://litellm.example.com/"
|
||||
mock_request.headers = {}
|
||||
|
||||
with patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
|
||||
return_value=raising_client,
|
||||
):
|
||||
response = await exchange_token_with_server(
|
||||
request=mock_request,
|
||||
mcp_server=server,
|
||||
grant_type="authorization_code",
|
||||
code="auth-code",
|
||||
redirect_uri="https://litellm.example.com/ui/mcp/oauth/callback",
|
||||
client_id="web-client.apps.googleusercontent.com",
|
||||
client_secret=None,
|
||||
code_verifier="verifier",
|
||||
)
|
||||
|
||||
assert response.status_code == 502
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "server_error"
|
||||
assert "invalid_client" in body["error_description"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_relays_rejection_when_http_client_raises():
|
||||
"""Same live mechanism as the token exchange: the DCR rejection escapes from the post call itself,
|
||||
so the register relay must catch it there, not only from the explicit raise_for_status."""
|
||||
import httpx
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
register_client_with_server,
|
||||
)
|
||||
|
||||
rejection = httpx.Response(
|
||||
400,
|
||||
json={"error": "invalid_client_metadata"},
|
||||
request=httpx.Request("POST", "https://idp.example.com/register"),
|
||||
)
|
||||
raising_client = MagicMock()
|
||||
raising_client.post = AsyncMock(
|
||||
side_effect=httpx.HTTPStatusError("Client error '400 Bad Request'", request=rejection.request, response=rejection)
|
||||
)
|
||||
|
||||
oauth2_server = _bridge_server(auth_type=MCPAuth.oauth2, dcr_bridge=None)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
|
||||
return_value=raising_client,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints._reuse_persisted_dcr_client_if_available",
|
||||
new_callable=AsyncMock,
|
||||
return_value=False,
|
||||
),
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await register_client_with_server(
|
||||
request=_bridge_mock_request(),
|
||||
mcp_server=oauth2_server,
|
||||
client_name="Claude",
|
||||
grant_types=None,
|
||||
response_types=None,
|
||||
token_endpoint_auth_method=None,
|
||||
)
|
||||
|
||||
assert exc.value.status_code == 400
|
||||
assert "invalid_client_metadata" in str(exc.value.detail)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_never_relays_out_of_contract_body_to_client():
|
||||
"""These endpoints serve unauthenticated OAuth clients, so a non-RFC6749 upstream body (HTML
|
||||
error page, proxy banner, stack trace) must stay in server logs; the client sees only the
|
||||
upstream status."""
|
||||
response = await _exchange_with_upstream_response(
|
||||
_upstream_token_response(404, text_body="<html>Error 404 stack trace: secret internals</html>")
|
||||
)
|
||||
|
||||
assert response.status_code == 502
|
||||
body = json.loads(response.body)
|
||||
assert body == {"error": "server_error", "error_description": "upstream token endpoint returned HTTP 404"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_never_relays_out_of_contract_body_to_client():
|
||||
"""Same trust boundary for DCR: a non-RFC7591 rejection body is logged server-side and the
|
||||
client detail names only the status."""
|
||||
import httpx
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
register_client_with_server,
|
||||
)
|
||||
|
||||
rejection = httpx.Response(
|
||||
500,
|
||||
text="<html>Tomcat stack trace with internals</html>",
|
||||
request=httpx.Request("POST", "https://idp.example.com/register"),
|
||||
)
|
||||
raising_client = MagicMock()
|
||||
raising_client.post = AsyncMock(
|
||||
side_effect=httpx.HTTPStatusError("Server error '500'", request=rejection.request, response=rejection)
|
||||
)
|
||||
|
||||
oauth2_server = _bridge_server(auth_type=MCPAuth.oauth2, dcr_bridge=None)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
|
||||
return_value=raising_client,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints._reuse_persisted_dcr_client_if_available",
|
||||
new_callable=AsyncMock,
|
||||
return_value=False,
|
||||
),
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await register_client_with_server(
|
||||
request=_bridge_mock_request(),
|
||||
mcp_server=oauth2_server,
|
||||
client_name="Claude",
|
||||
grant_types=None,
|
||||
response_types=None,
|
||||
token_endpoint_auth_method=None,
|
||||
)
|
||||
|
||||
assert exc.value.status_code == 502
|
||||
assert str(exc.value.detail) == "upstream registration failed with HTTP 500"
|
||||
assert "Tomcat" not in str(exc.value.detail)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_exchange_unreadable_body_still_renders_oauth_fault():
|
||||
"""An upstream whose failure body cannot be read (unconsumed stream, lying content-encoding)
|
||||
makes response.text/.json raise; the classifier must stay total so the caller still gets the
|
||||
§5.2-shaped 502 instead of the opaque 500 this change set out to remove."""
|
||||
import httpx
|
||||
|
||||
unreadable = httpx.Response(
|
||||
400,
|
||||
stream=httpx.ByteStream(b"\x1f\x8bnot-actually-gzip"),
|
||||
headers={"content-encoding": "gzip"},
|
||||
request=httpx.Request("POST", "https://oauth2.googleapis.com/token"),
|
||||
)
|
||||
|
||||
response = await _exchange_with_upstream_response(unreadable)
|
||||
|
||||
assert response.status_code == 502
|
||||
body = json.loads(response.body)
|
||||
assert body == {"error": "server_error", "error_description": "upstream token endpoint returned HTTP 400"}
|
||||
|
|
|
|||
|
|
@ -6847,7 +6847,11 @@ export const exchangeMcpOAuthToken = async ({
|
|||
|
||||
const data = await response.json();
|
||||
if (!response.ok) {
|
||||
const errorMessage = deriveErrorMessage(data) || data?.detail || "OAuth token exchange failed";
|
||||
const oauthErrorMessage =
|
||||
typeof data?.error === "string" && typeof data?.error_description === "string"
|
||||
? `${data.error}: ${data.error_description}`
|
||||
: undefined;
|
||||
const errorMessage = oauthErrorMessage || deriveErrorMessage(data) || data?.detail || "OAuth token exchange failed";
|
||||
throw new Error(errorMessage);
|
||||
}
|
||||
return data;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue