mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-09 07:09:11 +02:00
Handle expired OAuth client registrations (#4520)
This commit is contained in:
parent
18b5ab5852
commit
981a69d839
2 changed files with 120 additions and 3 deletions
|
|
@ -51,6 +51,10 @@ class ClientNotFoundError(Exception):
|
|||
"""Raised when OAuth client credentials are not found on the server."""
|
||||
|
||||
|
||||
class ExpiredClientRegistrationError(Exception):
|
||||
"""Raised when dynamic registration returns an expired client secret."""
|
||||
|
||||
|
||||
async def check_if_auth_required(
|
||||
mcp_url: str, httpx_kwargs: dict[str, Any] | None = None
|
||||
) -> bool:
|
||||
|
|
@ -166,6 +170,11 @@ class TokenStorageAdapter(TokenStorage):
|
|||
|
||||
if client_info.client_secret_expires_at:
|
||||
ttl = client_info.client_secret_expires_at - int(time.time())
|
||||
if ttl <= 0:
|
||||
await self._storage_client_info.delete(
|
||||
key=self._get_client_info_cache_key()
|
||||
)
|
||||
return
|
||||
|
||||
await self._storage_client_info.put(
|
||||
key=self._get_client_info_cache_key(),
|
||||
|
|
@ -340,6 +349,20 @@ class OAuth(OAuthClientProvider):
|
|||
else:
|
||||
self.context.update_token_expiry(self.context.current_tokens)
|
||||
|
||||
async def _perform_authorization(self) -> httpx.Request:
|
||||
"""Reject expired registrations before attempting authorization."""
|
||||
client_info = self.context.client_info
|
||||
if (
|
||||
client_info is not None
|
||||
and client_info.client_secret is not None
|
||||
and client_info.client_secret_expires_at
|
||||
and client_info.client_secret_expires_at <= int(time.time())
|
||||
):
|
||||
raise ExpiredClientRegistrationError(
|
||||
"OAuth dynamic registration returned an expired client secret"
|
||||
)
|
||||
return await super()._perform_authorization()
|
||||
|
||||
async def redirect_handler(self, authorization_url: str) -> None:
|
||||
"""Open browser for authorization, with pre-flight check for invalid client."""
|
||||
# Pre-flight check to detect invalid client_id before opening browser
|
||||
|
|
@ -429,7 +452,7 @@ class OAuth(OAuthClientProvider):
|
|||
except StopAsyncIteration:
|
||||
break
|
||||
|
||||
except ClientNotFoundError:
|
||||
except (ClientNotFoundError, ExpiredClientRegistrationError) as exc:
|
||||
# Static credentials are fixed — retrying won't help. Surface the
|
||||
# error so the user can correct their client_id / client_secret.
|
||||
if self._static_client_info is not None:
|
||||
|
|
@ -437,10 +460,10 @@ class OAuth(OAuthClientProvider):
|
|||
"OAuth server rejected the static client credentials. "
|
||||
"Verify that the client_id (and client_secret, if provided) "
|
||||
"are correct and that the client is registered with the server."
|
||||
) from None
|
||||
) from exc
|
||||
|
||||
logger.debug(
|
||||
"OAuth client not found on server, clearing cache and retrying..."
|
||||
"OAuth client registration is invalid, clearing cache and retrying..."
|
||||
)
|
||||
# Clear cached state and retry once
|
||||
self._initialized = False
|
||||
|
|
|
|||
|
|
@ -5,13 +5,17 @@ from urllib.parse import urlparse
|
|||
|
||||
import httpx2
|
||||
import pytest
|
||||
from key_value.aio.stores.memory import MemoryStore
|
||||
from mcp import MCPError
|
||||
from mcp.shared.auth import OAuthClientInformationFull
|
||||
from mcp_types import TextResourceContents
|
||||
from pydantic import AnyUrl
|
||||
|
||||
import fastmcp.client.auth.oauth as oauth_module
|
||||
import fastmcp.utilities.http as http_module
|
||||
from fastmcp.client import Client
|
||||
from fastmcp.client.auth import OAuth
|
||||
from fastmcp.client.auth.oauth import TokenStorageAdapter
|
||||
from fastmcp.client.transports import StreamableHttpTransport
|
||||
from fastmcp.server.auth.auth import ClientRegistrationOptions
|
||||
from fastmcp.server.auth.providers.in_memory import InMemoryOAuthProvider
|
||||
|
|
@ -45,6 +49,23 @@ def fastmcp_server(issuer_url: str):
|
|||
return server
|
||||
|
||||
|
||||
class ExpiredFirstRegistrationProvider(InMemoryOAuthProvider):
|
||||
def __init__(self, base_url: str):
|
||||
super().__init__(
|
||||
base_url=base_url,
|
||||
client_registration_options=ClientRegistrationOptions(enabled=True),
|
||||
)
|
||||
self.registration_count = 0
|
||||
|
||||
async def register_client(self, client_info: OAuthClientInformationFull) -> None:
|
||||
self.registration_count += 1
|
||||
if self.registration_count == 1:
|
||||
client_info.client_secret = "expired-secret"
|
||||
client_info.client_secret_expires_at = int(time.time()) - 1
|
||||
client_info.token_endpoint_auth_method = "client_secret_post"
|
||||
await super().register_client(client_info)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def streamable_http_server():
|
||||
"""Start OAuth-enabled server."""
|
||||
|
|
@ -139,6 +160,23 @@ async def test_oauth_server_metadata_discovery(streamable_http_server: str):
|
|||
assert metadata["token_endpoint"].startswith(server_base_url)
|
||||
|
||||
|
||||
async def test_expired_dynamic_registration_is_retried():
|
||||
port = find_available_port()
|
||||
base_url = f"http://127.0.0.1:{port}"
|
||||
provider = ExpiredFirstRegistrationProvider(base_url)
|
||||
server = FastMCP("TestServer", auth=provider)
|
||||
|
||||
async with run_server_async(server, port=port, transport="http") as url:
|
||||
client = Client(
|
||||
transport=StreamableHttpTransport(url),
|
||||
auth=HeadlessOAuth(mcp_url=url),
|
||||
)
|
||||
async with client:
|
||||
assert await client.ping()
|
||||
|
||||
assert provider.registration_count == 2
|
||||
|
||||
|
||||
class TestOAuthClientUrlHandling:
|
||||
"""Tests for OAuth client URL handling (issue #2573)."""
|
||||
|
||||
|
|
@ -559,3 +597,59 @@ class TestTokenStorageTTL:
|
|||
|
||||
await adapter.clear()
|
||||
assert await adapter.get_token_expiry() is None
|
||||
|
||||
|
||||
class TestClientInfoStorageTTL:
|
||||
async def test_expired_client_info_removes_stale_registration(self):
|
||||
storage = MemoryStore()
|
||||
adapter = TokenStorageAdapter(
|
||||
async_key_value=storage, server_url="https://test"
|
||||
)
|
||||
current = OAuthClientInformationFull(
|
||||
client_id="current-client",
|
||||
client_secret="current-secret",
|
||||
client_secret_expires_at=0,
|
||||
redirect_uris=[AnyUrl("http://localhost/callback")],
|
||||
)
|
||||
await adapter.set_client_info(current)
|
||||
assert await adapter.get_client_info() == current
|
||||
|
||||
expired = current.model_copy(
|
||||
update={
|
||||
"client_id": "expired-client",
|
||||
"client_secret_expires_at": int(time.time()) - 1,
|
||||
}
|
||||
)
|
||||
await adapter.set_client_info(expired)
|
||||
|
||||
assert await adapter.get_client_info() is None
|
||||
|
||||
async def test_never_expiring_client_info_is_stored(self):
|
||||
adapter = TokenStorageAdapter(
|
||||
async_key_value=MemoryStore(), server_url="https://test"
|
||||
)
|
||||
client_info = OAuthClientInformationFull(
|
||||
client_id="never-expiring-client",
|
||||
client_secret="secret",
|
||||
client_secret_expires_at=0,
|
||||
redirect_uris=[AnyUrl("http://localhost/callback")],
|
||||
)
|
||||
|
||||
await adapter.set_client_info(client_info)
|
||||
|
||||
assert await adapter.get_client_info() == client_info
|
||||
|
||||
async def test_future_expiring_client_info_is_stored(self):
|
||||
adapter = TokenStorageAdapter(
|
||||
async_key_value=MemoryStore(), server_url="https://test"
|
||||
)
|
||||
client_info = OAuthClientInformationFull(
|
||||
client_id="future-expiring-client",
|
||||
client_secret="secret",
|
||||
client_secret_expires_at=int(time.time()) + 60,
|
||||
redirect_uris=[AnyUrl("http://localhost/callback")],
|
||||
)
|
||||
|
||||
await adapter.set_client_info(client_info)
|
||||
|
||||
assert await adapter.get_client_info() == client_info
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue