fix(mcp): accept issuers from configured OAuth origins

Restore MCP OAuth callbacks when both endpoints are manually configured and no issuer is known. Save the configured authorization origin in OAuth state only while both configured endpoints remain in use

Compare known issuers exactly. For an unknown issuer, validate the returned URL and require its origin to match the saved authorization origin. Apply the same check to successful responses and provider errors

Share the existing origin helper and add callback regression coverage. The origin fallback intentionally accepts every tenant path on the configured origin
This commit is contained in:
shrey kharbanda 2026-10-26 16:08:15 -07:00
parent 1c022d1d06
commit f6115191c8
6 changed files with 301 additions and 35 deletions

View file

@ -91,6 +91,7 @@ from litellm.proxy._experimental.mcp_server.oauth_utils import (
TOKEN_NO_CACHE_HEADERS,
build_upstream_authorize_url,
build_upstream_oauth2_token_request,
canonical_origin,
configured_proxy_base_url,
get_cimd_client_id,
get_cimd_document_url,
@ -232,6 +233,7 @@ def encode_state_with_base_url(
oauth_nonce: str | None = None,
managed_scope: str | None = None,
scope_user_id: str | None = None,
configured_authorization_origin: str | None = None,
) -> str:
"""
Encode the base_url, original state, and PKCE parameters using encryption.
@ -276,6 +278,7 @@ def encode_state_with_base_url(
"dcr_client_secret": dcr_client_secret,
"dcr_token_endpoint_auth_method": dcr_token_endpoint_auth_method,
"expected_issuer": expected_issuer,
"configured_authorization_origin": configured_authorization_origin,
"authorization_response_iss_parameter_supported": authorization_response_iss_parameter_supported,
}
state_json: Final = json.dumps(state_data, sort_keys=True)
@ -1173,6 +1176,7 @@ async def authorize_with_server(
if ephemeral_dcr_client
else None,
expected_issuer=resolved_server.issuer,
configured_authorization_origin=_configured_authorization_origin(resolved_server),
authorization_response_iss_parameter_supported=resolved_server.authorization_response_iss_parameter_supported,
)
relay_state: Final = secrets.token_urlsafe(_OAUTH_STATE_HANDLE_BYTES)
@ -2671,11 +2675,46 @@ def _render_oauth_error_html(error: str, description: str | None) -> HTMLRespons
return HTMLResponse(body, status_code=400)
def _configured_authorization_origin(server: MCPServer) -> str | None:
"""Discovery without an issuer cannot vouch for the authorization server's identity."""
configured_url: Final = server.configured_authorization_url
if configured_url is None or configured_url != server.effective_authorization_url:
return None
configured_token_url: Final = server.configured_token_url
if not configured_token_url or configured_token_url != server.effective_token_url:
return None
return canonical_origin(configured_url)
def _is_origin_comparable_issuer(issuer: str) -> bool:
"""Reject issuer syntax that URL parsers normalize or interpret inconsistently."""
if not all(char.isprintable() and not char.isspace() for char in issuer) or "?" in issuer or "#" in issuer:
return False
try:
parsed: Final = urlparse(issuer)
return (
parsed.scheme in ("http", "https")
and bool(parsed.hostname)
and "@" not in parsed.netloc
and (parsed.port is None or 0 <= parsed.port <= 65535)
)
except ValueError:
return False
def _authorization_response_issuer_is_trusted(response_issuer: str | None, state_data: Mapping[str, object]) -> bool:
"""Known issuers match exactly. Without one, the configured origin covers every tenant path on that host."""
expected_issuer: Final = state_data.get("expected_issuer")
if response_issuer is None:
return state_data.get("authorization_response_iss_parameter_supported") is not True
return isinstance(expected_issuer, str) and bool(expected_issuer) and response_issuer == expected_issuer
if isinstance(expected_issuer, str) and expected_issuer:
return response_issuer == expected_issuer
configured_origin: Final = state_data.get("configured_authorization_origin")
return (
isinstance(configured_origin, str)
and _is_origin_comparable_issuer(response_issuer)
and canonical_origin(response_issuer) == configured_origin
)
@router.get("/callback")
@ -2696,7 +2735,8 @@ async def callback(
forwarded back to the validated client ``redirect_uri`` with the
original (un-wrapped) ``state``, once the RFC 9207 ``iss`` (when the
authorization server sent one) matches the issuer /authorize sealed
into the state.
into the state. Without a known issuer, it must match the sealed origin of
the configured authorization URL.
- An error response (``error``[+``error_description``/``error_uri``]), per
RFC 6749 §4.1.2.1. When ``state`` is present and decodes to a trusted
``redirect_uri``, the error params are propagated back to the client so
@ -2774,9 +2814,10 @@ async def callback(
if not _authorization_response_issuer_is_trusted(iss, issuer_state):
verbose_logger.warning(
"MCP /callback rejected an authorization response: RFC 9207 iss=%r does not match the "
"issuer this flow was sent to (%r)",
"issuer this flow was sent to (%r) or, with none known, the configured authorization origin (%r)",
iss,
issuer_state.get("expected_issuer"),
issuer_state.get("configured_authorization_origin"),
)
issuer_error_response: Final = _render_oauth_error_html(
"invalid_issuer",

View file

@ -37,10 +37,9 @@ runs at most once per row.
"""
from typing import Final, Protocol
from urllib.parse import urlparse
from litellm._logging import verbose_proxy_logger
from litellm.proxy._experimental.mcp_server.oauth_utils import canonicalize_url_identity
from litellm.proxy._experimental.mcp_server.oauth_utils import canonical_origin
from litellm.proxy.db.db_span import db_span
from litellm.proxy.utils import PrismaClient
@ -54,18 +53,6 @@ _BACKFILL_ACTOR: Final = "mcp_oauth_issuer_stamp_backfill"
_AUTH_TYPES_WITH_ISSUER_ANCHORING: Final = ("oauth2", "true_passthrough", "oauth_delegate")
def _origin(url: str) -> str | None:
"""The scheme-and-authority identity of ``url``, or ``None`` when it has none.
Built on the shared URL canonicalizer so the lowercase-host and default-port rules match the
RFC 8414 issuer comparison the resolution path uses, instead of being re-derived here.
"""
parsed: Final = urlparse(canonicalize_url_identity(url))
if not parsed.scheme or not parsed.netloc:
return None
return f"{parsed.scheme}://{parsed.netloc}"
class _MCPServerRow(Protocol):
"""The MCP server row fields this heal reads, so the untyped DB record is narrowed once here."""
@ -100,8 +87,8 @@ def _is_stamped_issuer_row(row: _MCPServerRow) -> bool:
)
if not configured:
return False
issuer_origin: Final = _origin(row.issuer or "")
return issuer_origin is not None and all(_origin(endpoint) == issuer_origin for endpoint in configured)
issuer_origin: Final = canonical_origin(row.issuer or "")
return issuer_origin is not None and all(canonical_origin(endpoint) == issuer_origin for endpoint in configured)
async def backfill_discovery_stamped_issuers(prisma_client: PrismaClient) -> int:

View file

@ -744,6 +744,18 @@ def canonicalize_url_identity(url: str) -> str:
return urlunparse((scheme, netloc, parsed.path.rstrip("/"), "", "", ""))
def canonical_origin(url: str) -> str | None:
"""The scheme-and-authority identity of ``url``, or ``None`` when it has none.
Built on the shared URL canonicalizer so the lowercase-host and default-port rules match the
RFC 8414 issuer comparison the resolution path uses, instead of being re-derived here.
"""
parsed: Final = urlparse(canonicalize_url_identity(url))
if not parsed.scheme or not parsed.netloc:
return None
return f"{parsed.scheme}://{parsed.netloc}"
def oauth_client_registration_matches(
registered_issuer: str | None,
registered_url: str | None,

View file

@ -40080,7 +40080,7 @@
},
"/callback": {
"get": {
"description": "OAuth 2.0 authorization response handler for MCP loopback clients.\n\nAccepts either:\n\n- A successful authorization response (``code`` + ``state``), which is\n forwarded back to the validated client ``redirect_uri`` with the\n original (un-wrapped) ``state``, once the RFC 9207 ``iss`` (when the\n authorization server sent one) matches the issuer /authorize sealed\n into the state.\n- An error response (``error``[+``error_description``/``error_uri``]), per\n RFC 6749 \u00a74.1.2.1. When ``state`` is present and decodes to a trusted\n ``redirect_uri``, the error params are propagated back to the client so\n its OAuth library can surface them. Otherwise we render an HTML error\n page so the user is not left on an opaque 422 / blank screen.",
"description": "OAuth 2.0 authorization response handler for MCP loopback clients.\n\nAccepts either:\n\n- A successful authorization response (``code`` + ``state``), which is\n forwarded back to the validated client ``redirect_uri`` with the\n original (un-wrapped) ``state``, once the RFC 9207 ``iss`` (when the\n authorization server sent one) matches the issuer /authorize sealed\n into the state. Without a known issuer, it must match the sealed origin of\n the configured authorization URL.\n- An error response (``error``[+``error_description``/``error_uri``]), per\n RFC 6749 \u00a74.1.2.1. When ``state`` is present and decodes to a trusted\n ``redirect_uri``, the error params are propagated back to the client so\n its OAuth library can surface them. Otherwise we render an HTML error\n page so the user is not left on an opaque 422 / blank screen.",
"operationId": "callback_callback_get",
"parameters": [
{

View file

@ -4851,7 +4851,7 @@ async def test_callback_error_path_reads_cookie_and_clears_it(monkeypatch):
assert cleared[cookie_name]["max-age"] == "0"
def _issuer_anchored_oauth_server(issuer: str | None = "https://idp.example.com"):
def _issuer_anchored_oauth_server(issuer: str | None = "https://idp.example.com") -> "MCPServer":
from litellm.proxy._types import MCPTransport
from litellm.types.mcp import MCPAuth
from litellm.types.mcp_server.mcp_server_manager import MCPServer
@ -4870,23 +4870,30 @@ def _issuer_anchored_oauth_server(issuer: str | None = "https://idp.example.com"
)
async def _authorize_then_callback(server, iss, monkeypatch, expected_issuer_override=..., error=None):
async def _authorize_then_callback(
server: "MCPServer",
iss: str | None,
monkeypatch: pytest.MonkeyPatch,
error: str | None = None,
drop_state_keys: tuple[str, ...] = (),
) -> tuple[Response, dict[str, object]]:
"""Run /authorize for ``server``, then feed the resulting flow back through /callback with the
RFC 9207 ``iss`` the authorization server supposedly returned. Returns (callback_response,
sealed_state_data)."""
from http.cookies import SimpleCookie
from urllib.parse import parse_qs, urlparse
from fastapi import Request
from pydantic import TypeAdapter
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
_oauth_state_cookie_name,
authorize_with_server,
callback,
decode_state_hash,
encode_state_with_base_url,
)
from litellm.proxy.common_utils.encrypt_decrypt_utils import encrypt_value_helper
monkeypatch.setenv("LITELLM_SALT_KEY", "sk-test-salt-for-LIT-5940")
client_redirect_uri = "http://127.0.0.1:6274/oauth/callback/debug"
@ -4907,15 +4914,15 @@ async def _authorize_then_callback(server, iss, monkeypatch, expected_issuer_ove
jar = SimpleCookie()
jar.load(authorize_response.headers["set-cookie"])
cookie_name = _oauth_state_cookie_name(handle)
sealed_state = jar[cookie_name].value
if expected_issuer_override is not ...:
# A state minted before the issuer was sealed into it: same shape, key absent.
sealed_state = encode_state_with_base_url(
base_url=client_redirect_uri,
original_state="client-original-state-9207",
client_redirect_uri=client_redirect_uri,
expected_issuer=expected_issuer_override,
original_state: Final = jar[cookie_name].value
state_data: Final = TypeAdapter(dict[str, object]).validate_python(decode_state_hash(original_state))
sealed_state: Final = (
encrypt_value_helper(
json.dumps({key: value for key, value in state_data.items() if key not in drop_state_keys})
)
if drop_state_keys
else original_state
)
callback_request = MagicMock(spec=Request)
callback_request.base_url = "https://proxy.example.com/"
@ -4929,7 +4936,7 @@ async def _authorize_then_callback(server, iss, monkeypatch, expected_issuer_ove
state=handle,
iss=iss,
)
return response, decode_state_hash(sealed_state)
return response, TypeAdapter(dict[str, object]).validate_python(decode_state_hash(sealed_state))
@pytest.mark.asyncio
@ -5015,11 +5022,229 @@ async def test_callback_legacy_state_requires_absent_issuer(monkeypatch, iss, ex
_issuer_anchored_oauth_server(),
iss=iss,
monkeypatch=monkeypatch,
expected_issuer_override=None,
drop_state_keys=("expected_issuer", "configured_authorization_origin"),
)
assert response.status_code == expected_status
@pytest.mark.asyncio
@pytest.mark.parametrize(
("authorization_url", "iss"),
(
("https://idp.example.com/oauth/authorize", "https://idp.example.com"),
("https://idp.example.com/tenant-a/authorize", "https://idp.example.com/tenant-a"),
("https://IDP.example.com:443/authorize?prompt=consent", "https://idp.example.com"),
("https://idp.example.com/authorize", "https://IDP.example.com:443"),
("http://127.0.0.1:54582/authorize", "http://127.0.0.1:54582"),
("http://localhost/authorize", "http://localhost:80"),
("http://[::1]:54582/authorize", "http://[::1]:54582"),
),
)
async def test_callback_accepts_an_issuer_on_the_configured_authorization_origin(
monkeypatch: pytest.MonkeyPatch, authorization_url: str, iss: str
) -> None:
from urllib.parse import parse_qs, urlparse
server: Final = _issuer_anchored_oauth_server(issuer=None).model_copy(
update={
"authorization_url": authorization_url,
"configured_authorization_url": authorization_url,
"token_url": "https://tokens.example.com/token",
"configured_token_url": "https://tokens.example.com/token",
}
)
response, _ = await _authorize_then_callback(server, iss, monkeypatch)
assert response.status_code == 302
destination: Final = urlparse(response.headers["location"])
assert destination._replace(query="").geturl() == "http://127.0.0.1:6274/oauth/callback/debug"
assert parse_qs(destination.query) == {"code": ["upstream-auth-code"], "state": ["client-original-state-9207"]}
@pytest.mark.asyncio
@pytest.mark.parametrize(
"iss",
(
"https://attacker.example.com",
"https://idp.example.com.evil.com",
"http://idp.example.com",
"ftp://idp.example.com",
"https://idp.example.com:8443",
"https://idp.example.com.",
"https://idp.example.com%40evil.com",
"https://idp.example.com?tenant=a",
"https://idp.example.com?",
"https://idp.example.com#fragment",
"https://idp.example.com#",
"https://idp.example.com@evil.com",
"https://user@idp.example.com",
"https://evil.com\\@idp.example.com",
" https://idp.example.com",
"https://idp.example.com\n",
"https://idp.exa\tmple.com",
"https://idp.example.com/\x00",
"idp.example.com",
"",
"https://[idp.example.com",
"https://[not-ipv6]/",
"https://idp.example.com:+443",
"https://idp.example.com:٤٤٣",
"https://idp.example.com:-443",
"https://idp.example.com:65536",
),
)
async def test_callback_rejects_an_issuer_off_the_configured_authorization_origin(
monkeypatch: pytest.MonkeyPatch, iss: str
) -> None:
server: Final = _issuer_anchored_oauth_server(issuer=None).model_copy(
update={
"configured_authorization_url": "https://idp.example.com/oauth/authorize",
"configured_token_url": "https://idp.example.com/oauth/token",
}
)
response, _ = await _authorize_then_callback(server, iss, monkeypatch)
assert response.status_code == 400
assert "location" not in response.headers
assert b"invalid_issuer" in response.body
assert b"upstream-auth-code" not in response.body
@pytest.mark.asyncio
@pytest.mark.parametrize(
("authorization_url", "iss"),
(
("http://[::1]:54582/authorize", "http://[0:0::1]:54582"),
("https://xn--bcher-kva.example/authorize", "https://bücher.example"),
),
)
async def test_callback_rejects_an_issuer_whose_host_is_spelled_differently(
monkeypatch: pytest.MonkeyPatch, authorization_url: str, iss: str
) -> None:
server: Final = _issuer_anchored_oauth_server(issuer=None).model_copy(
update={
"authorization_url": authorization_url,
"configured_authorization_url": authorization_url,
"token_url": "https://tokens.example.com/token",
"configured_token_url": "https://tokens.example.com/token",
}
)
response, _ = await _authorize_then_callback(server, iss, monkeypatch)
assert response.status_code == 400
assert "location" not in response.headers
assert b"invalid_issuer" in response.body
@pytest.mark.asyncio
@pytest.mark.parametrize(
"configured_url", (None, "https://other.example.com/authorize", "https://idp.example.com/manual-authorize")
)
async def test_callback_rejects_an_issuer_when_the_authorization_url_was_discovered(
monkeypatch: pytest.MonkeyPatch, configured_url: str | None
) -> None:
server: Final = _issuer_anchored_oauth_server(issuer=None).model_copy(
update={
"configured_authorization_url": configured_url,
"configured_token_url": "https://idp.example.com/oauth/token",
}
)
response, _ = await _authorize_then_callback(server, "https://idp.example.com", monkeypatch)
assert response.status_code == 400
assert "location" not in response.headers
assert b"invalid_issuer" in response.body
@pytest.mark.asyncio
@pytest.mark.parametrize("iss, expected_status", ((None, 302), ("https://idp.example.com", 400)))
async def test_callback_legacy_state_without_a_configured_origin(
monkeypatch: pytest.MonkeyPatch, iss: str | None, expected_status: int
) -> None:
server: Final = _issuer_anchored_oauth_server(issuer=None).model_copy(
update={
"configured_authorization_url": "https://idp.example.com/oauth/authorize",
"configured_token_url": "https://idp.example.com/oauth/token",
}
)
response, _ = await _authorize_then_callback(
server, iss, monkeypatch, drop_state_keys=("configured_authorization_origin",)
)
assert response.status_code == expected_status
@pytest.mark.asyncio
@pytest.mark.parametrize("iss, expected_status", (("https://idp.example.com", 302), ("https://other.example.com", 400)))
async def test_callback_relays_an_error_only_from_the_configured_authorization_origin(
monkeypatch: pytest.MonkeyPatch, iss: str, expected_status: int
) -> None:
from urllib.parse import parse_qs, urlparse
server: Final = _issuer_anchored_oauth_server(issuer=None).model_copy(
update={
"configured_authorization_url": "https://idp.example.com/oauth/authorize",
"configured_token_url": "https://idp.example.com/oauth/token",
}
)
response, _ = await _authorize_then_callback(server, iss, monkeypatch, error="access_denied")
assert response.status_code == expected_status
if expected_status == 302:
assert parse_qs(urlparse(response.headers["location"]).query) == {
"error": ["access_denied"],
"state": ["client-original-state-9207"],
}
else:
assert "location" not in response.headers
assert b"invalid_issuer" in response.body
@pytest.mark.asyncio
async def test_callback_holds_a_known_issuer_to_exact_match_on_the_configured_origin(
monkeypatch: pytest.MonkeyPatch,
) -> None:
server: Final = _issuer_anchored_oauth_server().model_copy(
update={
"configured_authorization_url": "https://idp.example.com/oauth/authorize",
"configured_token_url": "https://idp.example.com/oauth/token",
}
)
response, _ = await _authorize_then_callback(server, "https://idp.example.com/", monkeypatch)
assert response.status_code == 400
assert "location" not in response.headers
assert b"invalid_issuer" in response.body
@pytest.mark.asyncio
@pytest.mark.parametrize("error", (None, "access_denied"))
@pytest.mark.parametrize(
("configured_token_url", "token_url"),
(
(None, None),
(None, "https://collector.example.com/token"),
("https://idp.example.com/oauth/token", "https://collector.example.com/token"),
),
)
async def test_callback_rejects_an_unknown_issuer_when_the_token_endpoint_is_not_configured(
monkeypatch: pytest.MonkeyPatch, configured_token_url: str | None, token_url: str | None, error: str | None
) -> None:
server: Final = _issuer_anchored_oauth_server(issuer=None).model_copy(
update={
"configured_authorization_url": "https://idp.example.com/oauth/authorize",
"configured_token_url": configured_token_url,
"token_url": token_url,
}
)
response, _ = await _authorize_then_callback(server, "https://idp.example.com", monkeypatch, error=error)
assert response.status_code == 400
assert "location" not in response.headers
assert b"invalid_issuer" in response.body
assert b"upstream-auth-code" not in response.body
@pytest.mark.asyncio
async def test_oauth_authorize_includes_scopes_from_server_config():
"""Test that authorize endpoint includes scopes from server configuration."""

View file

@ -2275,7 +2275,8 @@ export interface paths {
* forwarded back to the validated client ``redirect_uri`` with the
* original (un-wrapped) ``state``, once the RFC 9207 ``iss`` (when the
* authorization server sent one) matches the issuer /authorize sealed
* into the state.
* into the state. Without a known issuer, it must match the sealed origin of
* the configured authorization URL.
* - An error response (``error``[+``error_description``/``error_uri``]), per
* RFC 6749 §4.1.2.1. When ``state`` is present and decodes to a trusted
* ``redirect_uri``, the error params are propagated back to the client so