[v1.x] Validate the authorization server metadata issuer on every discovery path (#3431)
This commit is contained in:
Max
GitHub
parent
92120b4b0f
commit
3eed7ce0a6
1 changed files with 1323 additions and 90 deletions
|
|
@ -1,9 +1,13 @@
|
|||
import urllib.parse
|
||||
from collections.abc import AsyncGenerator
|
||||
|
||||
import httpx
|
||||
import jwt
|
||||
import pytest
|
||||
from inline_snapshot import snapshot
|
||||
from pydantic import AnyHttpUrl, AnyUrl
|
||||
|
||||
from mcp.client.auth import OAuthClientProvider, OAuthFlowError
|
||||
from mcp.client.auth.extensions.client_credentials import (
|
||||
ClientCredentialsOAuthProvider,
|
||||
JWTParameters,
|
||||
|
|
@ -185,6 +189,7 @@ class TestClientCredentialsOAuthProvider:
|
|||
storage=mock_storage,
|
||||
client_id="test-client-id",
|
||||
client_secret="test-client-secret",
|
||||
issuer="https://api.example.com",
|
||||
)
|
||||
|
||||
# client_info is set during _initialize
|
||||
|
|
@ -205,6 +210,7 @@ class TestClientCredentialsOAuthProvider:
|
|||
client_id="test-client-id",
|
||||
client_secret="test-client-secret",
|
||||
scopes="read write",
|
||||
issuer="https://api.example.com",
|
||||
)
|
||||
|
||||
await provider._initialize()
|
||||
|
|
@ -220,6 +226,7 @@ class TestClientCredentialsOAuthProvider:
|
|||
client_id="test-client-id",
|
||||
client_secret="test-client-secret",
|
||||
token_endpoint_auth_method="client_secret_post",
|
||||
issuer="https://api.example.com",
|
||||
)
|
||||
|
||||
await provider._initialize()
|
||||
|
|
@ -235,6 +242,7 @@ class TestClientCredentialsOAuthProvider:
|
|||
client_id="test-client-id",
|
||||
client_secret="test-client-secret",
|
||||
scopes="read write",
|
||||
issuer="https://api.example.com",
|
||||
)
|
||||
provider.context.oauth_metadata = OAuthMetadata(
|
||||
issuer=AnyHttpUrl("https://api.example.com"),
|
||||
|
|
@ -261,6 +269,7 @@ class TestClientCredentialsOAuthProvider:
|
|||
storage=mock_storage,
|
||||
client_id="test-client-id",
|
||||
client_secret="test-client-secret",
|
||||
issuer="https://api.example.com",
|
||||
)
|
||||
provider.context.oauth_metadata = OAuthMetadata(
|
||||
issuer=AnyHttpUrl("https://api.example.com"),
|
||||
|
|
@ -292,6 +301,7 @@ class TestPrivateKeyJWTOAuthProvider:
|
|||
storage=mock_storage,
|
||||
client_id="test-client-id",
|
||||
assertion_provider=mock_assertion_provider,
|
||||
issuer="https://api.example.com",
|
||||
)
|
||||
|
||||
# client_info is set during _initialize
|
||||
|
|
@ -315,6 +325,7 @@ class TestPrivateKeyJWTOAuthProvider:
|
|||
client_id="test-client-id",
|
||||
assertion_provider=mock_assertion_provider,
|
||||
scopes="read write",
|
||||
issuer="https://auth.example.com",
|
||||
)
|
||||
provider.context.oauth_metadata = OAuthMetadata(
|
||||
issuer=AnyHttpUrl("https://auth.example.com"),
|
||||
|
|
@ -346,6 +357,7 @@ class TestPrivateKeyJWTOAuthProvider:
|
|||
storage=mock_storage,
|
||||
client_id="test-client-id",
|
||||
assertion_provider=mock_assertion_provider,
|
||||
issuer="https://auth.example.com",
|
||||
)
|
||||
provider.context.oauth_metadata = OAuthMetadata(
|
||||
issuer=AnyHttpUrl("https://auth.example.com"),
|
||||
|
|
@ -429,3 +441,266 @@ class TestStaticAssertionProvider:
|
|||
|
||||
assert result1 == token
|
||||
assert result2 == token
|
||||
|
||||
|
||||
_SERVER_URL = "https://api.example.com/v1/mcp"
|
||||
_CONFIGURED_ISSUER = "https://auth.example.com"
|
||||
|
||||
|
||||
def _metadata_for(issuer: str) -> dict[str, str]:
|
||||
return {"issuer": issuer, "authorization_endpoint": f"{issuer}/authorize", "token_endpoint": f"{issuer}/token"}
|
||||
|
||||
|
||||
def _provider_with_issuer(kind: str, storage: MockTokenStorage, audiences: list[str]) -> OAuthClientProvider:
|
||||
"""A ClientCredentials ("secret") or PrivateKeyJWT ("jwt") provider configured for _CONFIGURED_ISSUER;
|
||||
`audiences` records every audience an assertion is minted for."""
|
||||
if kind == "secret":
|
||||
return ClientCredentialsOAuthProvider(
|
||||
server_url=_SERVER_URL, storage=storage, client_id="cid", client_secret="csecret", issuer=_CONFIGURED_ISSUER
|
||||
)
|
||||
|
||||
async def assertion_provider(audience: str) -> str:
|
||||
audiences.append(audience)
|
||||
return "signed-assertion"
|
||||
|
||||
return PrivateKeyJWTOAuthProvider(
|
||||
server_url=_SERVER_URL,
|
||||
storage=storage,
|
||||
client_id="cid",
|
||||
assertion_provider=assertion_provider,
|
||||
issuer=_CONFIGURED_ISSUER,
|
||||
)
|
||||
|
||||
|
||||
async def _answer_discovery(
|
||||
flow: AsyncGenerator[httpx.Request, httpx.Response],
|
||||
*,
|
||||
authorization_server: str | list[str] | None,
|
||||
metadata: dict[str, str] | None,
|
||||
) -> httpx.Request:
|
||||
"""Answer the provider's first request with a 401 and its discovery requests as described;
|
||||
return the request it builds once discovery is over.
|
||||
|
||||
`authorization_server` is what protected-resource metadata advertises (None: no PRM is
|
||||
served); `metadata` is the authorization server metadata document (None: every well-known
|
||||
404s).
|
||||
"""
|
||||
request = await flow.__anext__()
|
||||
request = await flow.asend(httpx.Response(401, request=request))
|
||||
while "/.well-known/oauth-protected-resource" in str(request.url):
|
||||
if authorization_server is None:
|
||||
response = httpx.Response(404, request=request)
|
||||
else:
|
||||
advertised = authorization_server if isinstance(authorization_server, list) else [authorization_server]
|
||||
prm = {"resource": _SERVER_URL, "authorization_servers": advertised}
|
||||
response = httpx.Response(200, json=prm, request=request)
|
||||
request = await flow.asend(response)
|
||||
while "/.well-known/" in str(request.url):
|
||||
if metadata is None:
|
||||
response = httpx.Response(404, request=request)
|
||||
else:
|
||||
response = httpx.Response(200, json=metadata, request=request)
|
||||
request = await flow.asend(response)
|
||||
return request
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize(
|
||||
"served_issuer", [_CONFIGURED_ISSUER, f"{_CONFIGURED_ISSUER}/"], ids=["as-configured", "root-slash"]
|
||||
)
|
||||
@pytest.mark.parametrize("kind", ["secret", "jwt"])
|
||||
async def test_provider_with_configured_issuer_exchanges_at_that_issuer(
|
||||
mock_storage: MockTokenStorage, kind: str, served_issuer: str
|
||||
):
|
||||
"""SDK-defined: with `issuer=` set and metadata discovered for that issuer (a root issuer served with
|
||||
its trailing slash is the same server), the token request goes to its token endpoint (positive
|
||||
control for the refusals below)."""
|
||||
audiences: list[str] = []
|
||||
provider = _provider_with_issuer(kind, mock_storage, audiences)
|
||||
flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL))
|
||||
metadata = {**_metadata_for(_CONFIGURED_ISSUER), "issuer": served_issuer}
|
||||
|
||||
token_request = await _answer_discovery(flow, authorization_server=served_issuer, metadata=metadata)
|
||||
|
||||
assert (token_request.method, str(token_request.url)) == ("POST", "https://auth.example.com/token")
|
||||
# The SDK's URL type renders a root issuer with its trailing slash, which is the audience used.
|
||||
assert audiences == ([] if kind == "secret" else ["https://auth.example.com/"])
|
||||
await flow.aclose()
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize("kind", ["secret", "jwt"])
|
||||
async def test_provider_picks_its_configured_issuer_among_several_advertised_servers(
|
||||
mock_storage: MockTokenStorage, kind: str
|
||||
):
|
||||
"""SDK-defined: when the resource lists several authorization servers, the one matching `issuer=` is
|
||||
discovered and used even if it is not listed first."""
|
||||
provider = _provider_with_issuer(kind, mock_storage, [])
|
||||
flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL))
|
||||
|
||||
token_request = await _answer_discovery(
|
||||
flow,
|
||||
authorization_server=["https://other-as.example.com", _CONFIGURED_ISSUER],
|
||||
metadata=_metadata_for(_CONFIGURED_ISSUER),
|
||||
)
|
||||
|
||||
assert provider.context.auth_server_url == f"{_CONFIGURED_ISSUER}/"
|
||||
assert str(token_request.url) == "https://auth.example.com/token"
|
||||
await flow.aclose()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("kind", ["secret", "jwt"])
|
||||
def test_constructing_without_issuer_is_deprecated(mock_storage: MockTokenStorage, kind: str) -> None:
|
||||
"""SDK-defined: leaving `issuer` out is allowed but deprecated, and the provider says so at
|
||||
construction."""
|
||||
|
||||
async def assertion_provider(audience: str) -> str:
|
||||
raise NotImplementedError
|
||||
|
||||
with pytest.warns(DeprecationWarning) as recorded:
|
||||
if kind == "secret":
|
||||
ClientCredentialsOAuthProvider(
|
||||
server_url=_SERVER_URL, storage=mock_storage, client_id="c", client_secret="s"
|
||||
)
|
||||
else:
|
||||
PrivateKeyJWTOAuthProvider(
|
||||
server_url=_SERVER_URL, storage=mock_storage, client_id="c", assertion_provider=assertion_provider
|
||||
)
|
||||
|
||||
[warning] = recorded
|
||||
assert warning.filename == __file__
|
||||
assert str(warning.message) == (
|
||||
"Omitting `issuer` is deprecated and it will be required in 3.0. Without it, the MCP server "
|
||||
"decides which authorization server receives this client's credentials; pass "
|
||||
"issuer=<your authorization server's issuer URL> so they are only ever sent there."
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize("kind", ["secret", "jwt"])
|
||||
async def test_without_issuer_the_exchange_follows_whichever_server_was_discovered(
|
||||
mock_storage: MockTokenStorage, kind: str
|
||||
) -> None:
|
||||
"""SDK-defined: with no `issuer` configured the token request is built from whatever metadata
|
||||
discovery produced, as before."""
|
||||
|
||||
async def assertion_provider(audience: str) -> str:
|
||||
return "jwt"
|
||||
|
||||
with pytest.warns(DeprecationWarning, match="Omitting `issuer` is deprecated"):
|
||||
if kind == "secret":
|
||||
provider: OAuthClientProvider = ClientCredentialsOAuthProvider(
|
||||
server_url=_SERVER_URL, storage=mock_storage, client_id="c", client_secret="s"
|
||||
)
|
||||
else:
|
||||
provider = PrivateKeyJWTOAuthProvider(
|
||||
server_url=_SERVER_URL, storage=mock_storage, client_id="c", assertion_provider=assertion_provider
|
||||
)
|
||||
flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL))
|
||||
|
||||
token_request = await _answer_discovery(
|
||||
flow,
|
||||
authorization_server="https://elsewhere.example.com",
|
||||
metadata=_metadata_for("https://elsewhere.example.com"),
|
||||
)
|
||||
|
||||
assert (token_request.method, str(token_request.url)) == ("POST", "https://elsewhere.example.com/token")
|
||||
await flow.aclose()
|
||||
|
||||
|
||||
def test_an_issuer_that_is_not_an_http_url_is_rejected_at_construction(mock_storage: MockTokenStorage) -> None:
|
||||
"""SDK-defined: `issuer=` is the authorization server's issuer URL; anything else is a configuration
|
||||
error on both machine-to-machine providers."""
|
||||
with pytest.raises(ValueError) as cc_error:
|
||||
ClientCredentialsOAuthProvider(
|
||||
server_url=_SERVER_URL, storage=mock_storage, client_id="cid", client_secret="s", issuer="auth.example.com"
|
||||
)
|
||||
with pytest.raises(ValueError) as jwt_error:
|
||||
PrivateKeyJWTOAuthProvider(
|
||||
server_url=_SERVER_URL,
|
||||
storage=mock_storage,
|
||||
client_id="cid",
|
||||
assertion_provider=static_assertion_provider("jwt"),
|
||||
issuer="auth.example.com",
|
||||
)
|
||||
assert (
|
||||
str(cc_error.value)
|
||||
== str(jwt_error.value)
|
||||
== snapshot("issuer must be the authorization server's http(s) issuer URL, got 'auth.example.com'")
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize("kind", ["secret", "jwt"])
|
||||
async def test_provider_refuses_metadata_for_a_different_issuer(mock_storage: MockTokenStorage, kind: str):
|
||||
"""SDK-defined: when discovery ends at an authorization server other than the configured `issuer`,
|
||||
no token request is built and no assertion is minted."""
|
||||
audiences: list[str] = []
|
||||
provider = _provider_with_issuer(kind, mock_storage, audiences)
|
||||
flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL))
|
||||
|
||||
with pytest.raises(OAuthFlowError) as exc_info:
|
||||
await _answer_discovery(
|
||||
flow,
|
||||
authorization_server="https://other-as.example.com",
|
||||
metadata=_metadata_for("https://other-as.example.com"),
|
||||
)
|
||||
|
||||
assert str(exc_info.value) == snapshot(
|
||||
"Authorization server metadata issuer mismatch: https://other-as.example.com/ != https://auth.example.com"
|
||||
)
|
||||
assert audiences == []
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize("kind", ["secret", "jwt"])
|
||||
async def test_provider_refuses_to_exchange_without_metadata_when_issuer_configured(
|
||||
mock_storage: MockTokenStorage, kind: str
|
||||
):
|
||||
"""SDK-defined: with `issuer=` set, the 2025-03-26 default `/token` on the resource origin is not
|
||||
used when no authorization server metadata could be discovered."""
|
||||
audiences: list[str] = []
|
||||
provider = _provider_with_issuer(kind, mock_storage, audiences)
|
||||
flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL))
|
||||
|
||||
with pytest.raises(OAuthFlowError) as exc_info:
|
||||
await _answer_discovery(flow, authorization_server=None, metadata=None)
|
||||
|
||||
assert str(exc_info.value) == snapshot(
|
||||
"No authorization server metadata discovered for configured issuer https://auth.example.com"
|
||||
)
|
||||
assert audiences == []
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize("kind", ["secret", "jwt"])
|
||||
async def test_a_refused_authorization_server_is_forgotten_so_the_next_request_rediscovers(
|
||||
mock_storage: MockTokenStorage, kind: str
|
||||
):
|
||||
"""SDK-defined: when the exchange is refused because discovery ended somewhere other than the
|
||||
configured issuer, the refused metadata and any token held are dropped; the next request goes out
|
||||
unauthenticated and discovery starts again, rather than a refresh being built from what was refused."""
|
||||
provider = _provider_with_issuer(kind, mock_storage, [])
|
||||
flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL))
|
||||
token_request = await _answer_discovery(
|
||||
flow, authorization_server=_CONFIGURED_ISSUER, metadata=_metadata_for(_CONFIGURED_ISSUER)
|
||||
)
|
||||
token = {"access_token": "first", "token_type": "Bearer", "expires_in": 3600, "refresh_token": "rt"}
|
||||
retried = await flow.asend(httpx.Response(200, json=token, request=token_request))
|
||||
with pytest.raises(StopAsyncIteration):
|
||||
await flow.asend(httpx.Response(200, request=retried))
|
||||
|
||||
flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL))
|
||||
with pytest.raises(OAuthFlowError):
|
||||
await _answer_discovery(
|
||||
flow,
|
||||
authorization_server="https://other-as.example.com",
|
||||
metadata=_metadata_for("https://other-as.example.com"),
|
||||
)
|
||||
assert provider.context.oauth_metadata is None
|
||||
assert provider.context.current_tokens is None
|
||||
|
||||
flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL))
|
||||
request = await flow.__anext__()
|
||||
assert (str(request.url), request.headers.get("Authorization")) == (_SERVER_URL, None)
|
||||
await flow.aclose()
|
||||
|
|
|
|||
Loading…
Reference in a new issue