Preserve empty issuer/resource paths on AuthSettings (#2987)
This commit is contained in:
parent
e9cd1692b0
commit
f41a5193f3
3 changed files with 41 additions and 2 deletions
|
|
@ -1373,6 +1373,14 @@ match redirect URIs by exact string comparison, so if you registered such a URI
|
||||||
release (with the trailing slash) and the registration is persisted in `TokenStorage`, re-register
|
release (with the trailing slash) and the registration is persisted in `TokenStorage`, re-register
|
||||||
the client so the stored value matches what the SDK now transmits.
|
the client so the stored value matches what the SDK now transmits.
|
||||||
|
|
||||||
|
`AuthSettings` now sets `url_preserve_empty_path=True` for the same reason: a path-less
|
||||||
|
`issuer_url` (or `resource_server_url`) passed as a string keeps its empty path, so the authorization
|
||||||
|
server advertises `issuer` as `https://as.example.com` rather than `https://as.example.com/` in its
|
||||||
|
metadata. Previously the trailing slash was added before the model saw the value, leaving the served
|
||||||
|
issuer inconsistent with what clients compare against under RFC 8414 / RFC 9207. Passing an
|
||||||
|
already-built `AnyHttpUrl` object still normalizes at construction; pass a string to get the
|
||||||
|
preserved form.
|
||||||
|
|
||||||
### Lowlevel `Server`: `subscribe` capability now correctly reported
|
### Lowlevel `Server`: `subscribe` capability now correctly reported
|
||||||
|
|
||||||
Previously, the lowlevel `Server` hardcoded `subscribe=False` in resource capabilities even when a `subscribe_resource()` handler was registered. The `subscribe` capability is now dynamically set to `True` when an `on_subscribe_resource` handler is provided. Clients that previously didn't see `subscribe: true` in capabilities will now see it when a handler is registered, which may change client behavior.
|
Previously, the lowlevel `Server` hardcoded `subscribe=False` in resource capabilities even when a `subscribe_resource()` handler was registered. The `subscribe` capability is now dynamically set to `True` when an `on_subscribe_resource` handler is provided. Clients that previously didn't see `subscribe: true` in capabilities will now see it when a handler is registered, which may change client behavior.
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
from pydantic import AnyHttpUrl, BaseModel, Field
|
from pydantic import AnyHttpUrl, BaseModel, ConfigDict, Field
|
||||||
|
|
||||||
|
|
||||||
class ClientRegistrationOptions(BaseModel):
|
class ClientRegistrationOptions(BaseModel):
|
||||||
|
|
@ -13,6 +13,12 @@ class RevocationOptions(BaseModel):
|
||||||
|
|
||||||
|
|
||||||
class AuthSettings(BaseModel):
|
class AuthSettings(BaseModel):
|
||||||
|
# Preserve empty URL paths so a path-less issuer/resource passed as a string keeps its
|
||||||
|
# canonical form (no trailing slash). RFC 8414/9207 issuer comparison is exact string
|
||||||
|
# comparison, so a spurious trailing slash would break it. See PR #2925 for the metadata
|
||||||
|
# models; this applies the same to the server's own configured URLs.
|
||||||
|
model_config = ConfigDict(url_preserve_empty_path=True)
|
||||||
|
|
||||||
issuer_url: AnyHttpUrl = Field(
|
issuer_url: AnyHttpUrl = Field(
|
||||||
...,
|
...,
|
||||||
description="OAuth authorization server URL that issues tokens for this resource server.",
|
description="OAuth authorization server URL that issues tokens for this resource server.",
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,8 @@
|
||||||
import pytest
|
import pytest
|
||||||
from pydantic import AnyHttpUrl
|
from pydantic import AnyHttpUrl
|
||||||
|
|
||||||
from mcp.server.auth.routes import validate_issuer_url
|
from mcp.server.auth.routes import build_metadata, validate_issuer_url
|
||||||
|
from mcp.server.auth.settings import AuthSettings, ClientRegistrationOptions, RevocationOptions
|
||||||
|
|
||||||
|
|
||||||
def test_validate_issuer_url_https_allowed():
|
def test_validate_issuer_url_https_allowed():
|
||||||
|
|
@ -45,3 +46,27 @@ def test_validate_issuer_url_fragment_rejected():
|
||||||
def test_validate_issuer_url_query_rejected():
|
def test_validate_issuer_url_query_rejected():
|
||||||
with pytest.raises(ValueError, match="query"):
|
with pytest.raises(ValueError, match="query"):
|
||||||
validate_issuer_url(AnyHttpUrl("https://example.com/path?q=1"))
|
validate_issuer_url(AnyHttpUrl("https://example.com/path?q=1"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_auth_settings_preserves_path_less_issuer():
|
||||||
|
"""A path-less issuer passed as a string keeps its canonical form (no trailing slash)."""
|
||||||
|
settings = AuthSettings(
|
||||||
|
issuer_url="https://as.example.com", # type: ignore[arg-type]
|
||||||
|
resource_server_url="https://rs.example.com", # type: ignore[arg-type]
|
||||||
|
)
|
||||||
|
assert str(settings.issuer_url) == "https://as.example.com"
|
||||||
|
assert str(settings.resource_server_url) == "https://rs.example.com"
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_metadata_serves_issuer_without_trailing_slash():
|
||||||
|
"""The served issuer matches the configured one exactly (RFC 8414/9207 string comparison)."""
|
||||||
|
settings = AuthSettings(
|
||||||
|
issuer_url="https://as.example.com", # type: ignore[arg-type]
|
||||||
|
resource_server_url="https://rs.example.com", # type: ignore[arg-type]
|
||||||
|
)
|
||||||
|
metadata = build_metadata(settings.issuer_url, None, ClientRegistrationOptions(), RevocationOptions())
|
||||||
|
|
||||||
|
served = metadata.model_dump(mode="json", exclude_none=True)
|
||||||
|
assert served["issuer"] == "https://as.example.com"
|
||||||
|
assert served["authorization_endpoint"] == "https://as.example.com/authorize"
|
||||||
|
assert served["token_endpoint"] == "https://as.example.com/token"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue