Skip to content

Commit fef29a5

Browse files
fix(auth): coerce empty-string optional URL fields to None in OAuthClientMetadata
1 parent 941089e commit fef29a5

File tree

2 files changed

+92
-1
lines changed

2 files changed

+92
-1
lines changed

src/mcp/shared/auth.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,24 @@ class OAuthClientMetadata(BaseModel):
6767
software_id: str | None = None
6868
software_version: str | None = None
6969

70+
@field_validator(
71+
"client_uri",
72+
"logo_uri",
73+
"tos_uri",
74+
"policy_uri",
75+
"jwks_uri",
76+
mode="before",
77+
)
78+
@classmethod
79+
def _empty_string_optional_url_to_none(cls, v: object) -> object:
80+
# RFC 7591 §2 marks these URL fields OPTIONAL. Some authorization servers
81+
# echo omitted metadata back as "" instead of dropping the keys, which
82+
# AnyHttpUrl would otherwise reject — throwing away an otherwise valid
83+
# registration response. Treat "" as absent.
84+
if v == "":
85+
return None
86+
return v
87+
7088
def validate_scope(self, requested_scope: str | None) -> list[str] | None:
7189
if requested_scope is None:
7290
return None

tests/client/test_auth.py

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import httpx
99
import pytest
1010
from inline_snapshot import Is, snapshot
11-
from pydantic import AnyHttpUrl, AnyUrl
11+
from pydantic import AnyHttpUrl, AnyUrl, ValidationError
1212

1313
from mcp.client.auth import OAuthClientProvider, PKCEParameters
1414
from mcp.client.auth.exceptions import OAuthFlowError
@@ -985,6 +985,79 @@ def text(self):
985985
assert "Registration failed: 400" in str(exc_info.value)
986986

987987

988+
class TestOAuthClientMetadataEmptyUrlCoercion:
989+
"""RFC 7591 §2 marks client_uri/logo_uri/tos_uri/policy_uri/jwks_uri as OPTIONAL.
990+
Some authorization servers echo the client's omitted metadata back as ""
991+
instead of dropping the keys; without coercion, AnyHttpUrl rejects "" and
992+
the whole registration response is thrown away even though the server
993+
returned a valid client_id."""
994+
995+
@pytest.mark.parametrize(
996+
"empty_field",
997+
["client_uri", "logo_uri", "tos_uri", "policy_uri", "jwks_uri"],
998+
)
999+
def test_optional_url_empty_string_coerced_to_none(self, empty_field: str):
1000+
data = {
1001+
"redirect_uris": ["https://example.com/callback"],
1002+
empty_field: "",
1003+
}
1004+
metadata = OAuthClientMetadata.model_validate(data)
1005+
assert getattr(metadata, empty_field) is None
1006+
1007+
def test_all_optional_urls_empty_together(self):
1008+
data = {
1009+
"redirect_uris": ["https://example.com/callback"],
1010+
"client_uri": "",
1011+
"logo_uri": "",
1012+
"tos_uri": "",
1013+
"policy_uri": "",
1014+
"jwks_uri": "",
1015+
}
1016+
metadata = OAuthClientMetadata.model_validate(data)
1017+
assert metadata.client_uri is None
1018+
assert metadata.logo_uri is None
1019+
assert metadata.tos_uri is None
1020+
assert metadata.policy_uri is None
1021+
assert metadata.jwks_uri is None
1022+
1023+
def test_valid_url_passes_through_unchanged(self):
1024+
data = {
1025+
"redirect_uris": ["https://example.com/callback"],
1026+
"client_uri": "https://udemy.com/",
1027+
}
1028+
metadata = OAuthClientMetadata.model_validate(data)
1029+
assert str(metadata.client_uri) == "https://udemy.com/"
1030+
1031+
def test_information_full_inherits_coercion(self):
1032+
"""OAuthClientInformationFull subclasses OAuthClientMetadata, so the
1033+
same coercion applies to DCR responses parsed via the full model."""
1034+
data = {
1035+
"client_id": "abc123",
1036+
"redirect_uris": ["https://example.com/callback"],
1037+
"client_uri": "",
1038+
"logo_uri": "",
1039+
"tos_uri": "",
1040+
"policy_uri": "",
1041+
"jwks_uri": "",
1042+
}
1043+
info = OAuthClientInformationFull.model_validate(data)
1044+
assert info.client_id == "abc123"
1045+
assert info.client_uri is None
1046+
assert info.logo_uri is None
1047+
assert info.tos_uri is None
1048+
assert info.policy_uri is None
1049+
assert info.jwks_uri is None
1050+
1051+
def test_invalid_non_empty_url_still_rejected(self):
1052+
"""Coercion must only touch empty strings — garbage URLs still raise."""
1053+
data = {
1054+
"redirect_uris": ["https://example.com/callback"],
1055+
"client_uri": "not a url",
1056+
}
1057+
with pytest.raises(ValidationError):
1058+
OAuthClientMetadata.model_validate(data)
1059+
1060+
9881061
class TestCreateClientRegistrationRequest:
9891062
"""Test client registration request creation."""
9901063

0 commit comments

Comments
 (0)