[v1.x] fix(auth): coerce empty-string optional URL fields to None in OAuthClientMetadata (#2405)

This commit is contained in:
Felix Weinberger 2026-04-13 18:53:58 +01:00 • committed by GitHub
commit 73d458baac
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 99 additions and 1 deletions

View file

@ -71,6 +71,24 @@ class OAuthClientMetadata(BaseModel):
software_id: str | None = None
software_version: str | None = None
@field_validator(
"client_uri",
"logo_uri",
"tos_uri",
"policy_uri",
"jwks_uri",
mode="before",
)
@classmethod
def _empty_string_optional_url_to_none(cls, v: object) -> object:
# RFC 7591 §2 marks these URL fields OPTIONAL. Some authorization servers
# echo omitted metadata back as "" instead of dropping the keys, which
# AnyHttpUrl would otherwise reject — throwing away an otherwise valid
# registration response. Treat "" as absent.
if v == "":
return None
return v
def validate_scope(self, requested_scope: str | None) -> list[str] | None:
if requested_scope is None:
return None

View file

@ -1,6 +1,9 @@
"""Tests for OAuth 2.0 shared code."""
from mcp.shared.auth import OAuthMetadata
import pytest
from pydantic import ValidationError
from mcp.shared.auth import OAuthClientInformationFull, OAuthClientMetadata, OAuthMetadata
class TestOAuthMetadata:
@ -59,3 +62,80 @@ class TestOAuthMetadata:
"token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
}
)
# RFC 7591 §2 marks client_uri/logo_uri/tos_uri/policy_uri/jwks_uri as OPTIONAL.
# Some authorization servers echo the client's omitted metadata back as ""
# instead of dropping the keys; without coercion, AnyHttpUrl rejects "" and
# the whole registration response is thrown away even though the server
# returned a valid client_id.
@pytest.mark.parametrize(
"empty_field",
["client_uri", "logo_uri", "tos_uri", "policy_uri", "jwks_uri"],
)
def test_optional_url_empty_string_coerced_to_none(empty_field: str):
data = {
"redirect_uris": ["https://example.com/callback"],
empty_field: "",
}
metadata = OAuthClientMetadata.model_validate(data)
assert getattr(metadata, empty_field) is None
def test_all_optional_urls_empty_together():
data = {
"redirect_uris": ["https://example.com/callback"],
"client_uri": "",
"logo_uri": "",
"tos_uri": "",
"policy_uri": "",
"jwks_uri": "",
}
metadata = OAuthClientMetadata.model_validate(data)
assert metadata.client_uri is None
assert metadata.logo_uri is None
assert metadata.tos_uri is None
assert metadata.policy_uri is None
assert metadata.jwks_uri is None
def test_valid_url_passes_through_unchanged():
data = {
"redirect_uris": ["https://example.com/callback"],
"client_uri": "https://udemy.com/",
}
metadata = OAuthClientMetadata.model_validate(data)
assert str(metadata.client_uri) == "https://udemy.com/"
def test_information_full_inherits_coercion():
"""OAuthClientInformationFull subclasses OAuthClientMetadata, so the
same coercion applies to DCR responses parsed via the full model."""
data = {
"client_id": "abc123",
"redirect_uris": ["https://example.com/callback"],
"client_uri": "",
"logo_uri": "",
"tos_uri": "",
"policy_uri": "",
"jwks_uri": "",
}
info = OAuthClientInformationFull.model_validate(data)
assert info.client_id == "abc123"
assert info.client_uri is None
assert info.logo_uri is None
assert info.tos_uri is None
assert info.policy_uri is None
assert info.jwks_uri is None
def test_invalid_non_empty_url_still_rejected():
"""Coercion must only touch empty strings — garbage URLs still raise."""
data = {
"redirect_uris": ["https://example.com/callback"],
"client_uri": "not a url",
}
with pytest.raises(ValidationError):
OAuthClientMetadata.model_validate(data)