fix(mcp): keep params, query and fragment significant in issuer comparison

The shared canonicalizer drops all three, so two issuers differing only outside the path compared equal and a response from another tenant's authorization server would have continued through the flow.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

litellm/proxy/_experimental/mcp_server/oauth_utils.py

@@ -636,7 +636,15 @@ def canonicalize_url_identity(url: str) -> str:
 def issuer_identities_match(claimed_issuer: str, expected_issuer: str) -> bool:
     """Issuer equality tolerant only of URL-insignificant differences (scheme/host case, the default
     port, a trailing slash), through the shared canonicalizer. Used for RFC 8414 §3.3 metadata
-    anchoring and for the RFC 9207 ``iss`` an authorization response carries."""
+    anchoring and for the RFC 9207 ``iss`` an authorization response carries.
+
+    An RFC 8414 issuer identifier carries no params, query or fragment, and the canonicalizer drops
+    all three, so two issuers differing only there would compare equal. That difference is compared
+    on the raw URLs first, keeping a tenant that a deployment encoded outside the path distinct."""
+    claimed: Final = urlparse(claimed_issuer)
+    expected: Final = urlparse(expected_issuer)
+    if (claimed.params, claimed.query, claimed.fragment) != (expected.params, expected.query, expected.fragment):
+        return False
     return canonicalize_url_identity(claimed_issuer) == canonicalize_url_identity(expected_issuer)
 
 

tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py

@@ -4139,6 +4139,28 @@ async def test_callback_forwards_when_issuer_is_unknown_or_iss_absent(monkeypatc
     assert unanchored_response.status_code == 302
 
 
+@pytest.mark.asyncio
+async def test_callback_rejects_an_issuer_differing_only_outside_the_path(monkeypatch):
+    """A tenant a deployment encoded in a query string is part of that issuer's identity, so the
+    comparison must not canonicalize it away and let another tenant's response through."""
+    from litellm.proxy._experimental.mcp_server.oauth_utils import issuer_identities_match
+
+    assert not issuer_identities_match("https://idp.example.com/?tenant=b", "https://idp.example.com/?tenant=a")
+    assert not issuer_identities_match("https://idp.example.com/#b", "https://idp.example.com/#a")
+    assert issuer_identities_match("https://IDP.example.com:443/?tenant=a", "https://idp.example.com/?tenant=a")
+
+    response, state_data = await _authorize_then_callback(
+        _issuer_anchored_oauth_server(issuer="https://idp.example.com/?tenant=a"),
+        iss="https://idp.example.com/?tenant=b",
+        monkeypatch=monkeypatch,
+    )
+
+    assert state_data["expected_issuer"] == "https://idp.example.com/?tenant=a"
+    assert response.status_code == 400
+    assert "location" not in response.headers
+    assert b"upstream-auth-code" not in response.body
+
+
 @pytest.mark.asyncio
 async def test_callback_accepts_states_minted_before_the_issuer_was_sealed(monkeypatch):
     """An authorization in flight across the upgrade carries no sealed issuer and must still land."""