mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-21 21:14:17 +02:00
fix: store absolute token expiry to prevent stale expires_in on reload (#3572)
This commit is contained in:
parent
196e2b48c5
commit
faef74b374
2 changed files with 132 additions and 1 deletions
|
|
@ -96,9 +96,16 @@ class TokenStorageAdapter(TokenStorage):
|
|||
def _get_client_info_cache_key(self) -> str:
|
||||
return f"{self._server_url}/client_info"
|
||||
|
||||
def _get_token_expiry_cache_key(self) -> str:
|
||||
return f"{self._server_url}/token_expiry"
|
||||
|
||||
async def clear(self) -> None:
|
||||
await self._storage_oauth_token.delete(key=self._get_token_cache_key())
|
||||
await self._storage_client_info.delete(key=self._get_client_info_cache_key())
|
||||
await self._key_value_store.delete(
|
||||
key=self._get_token_expiry_cache_key(),
|
||||
collection="mcp-oauth-token-expiry",
|
||||
)
|
||||
|
||||
@override
|
||||
async def get_tokens(self) -> OAuthToken | None:
|
||||
|
|
@ -114,6 +121,25 @@ class TokenStorageAdapter(TokenStorage):
|
|||
value=tokens,
|
||||
ttl=60 * 60 * 24 * 365, # 1 year
|
||||
)
|
||||
# Store absolute expiry so reloads don't misinterpret the stale
|
||||
# relative expires_in value (#2862).
|
||||
if tokens.expires_in is not None:
|
||||
expires_at = time.time() + int(tokens.expires_in)
|
||||
await self._key_value_store.put(
|
||||
key=self._get_token_expiry_cache_key(),
|
||||
value={"expires_at": expires_at},
|
||||
collection="mcp-oauth-token-expiry",
|
||||
ttl=60 * 60 * 24 * 365,
|
||||
)
|
||||
|
||||
async def get_token_expiry(self) -> float | None:
|
||||
raw = await self._key_value_store.get(
|
||||
key=self._get_token_expiry_cache_key(),
|
||||
collection="mcp-oauth-token-expiry",
|
||||
)
|
||||
if raw is not None:
|
||||
return float(raw["expires_at"])
|
||||
return None
|
||||
|
||||
@override
|
||||
async def get_client_info(self) -> OAuthClientInformationFull | None:
|
||||
|
|
@ -285,7 +311,11 @@ class OAuth(OAuthClientProvider):
|
|||
await self.token_storage_adapter.set_client_info(self._static_client_info)
|
||||
|
||||
if self.context.current_tokens and self.context.current_tokens.expires_in:
|
||||
self.context.update_token_expiry(self.context.current_tokens)
|
||||
stored_expiry = await self.token_storage_adapter.get_token_expiry()
|
||||
if stored_expiry is not None:
|
||||
self.context.token_expiry_time = stored_expiry
|
||||
else:
|
||||
self.context.update_token_expiry(self.context.current_tokens)
|
||||
|
||||
async def redirect_handler(self, authorization_url: str) -> None:
|
||||
"""Open browser for authorization, with pre-flight check for invalid client."""
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import time
|
||||
from unittest.mock import patch
|
||||
from urllib.parse import urlparse
|
||||
|
||||
|
|
@ -356,3 +357,103 @@ class TestTokenStorageTTL:
|
|||
stored = await adapter.get_tokens()
|
||||
assert stored is not None
|
||||
assert stored.refresh_token == "refresh-token-should-survive"
|
||||
|
||||
async def test_set_tokens_stores_absolute_expiry(self):
|
||||
"""set_tokens should persist an absolute expires_at timestamp."""
|
||||
from key_value.aio.stores.memory import MemoryStore
|
||||
from mcp.shared.auth import OAuthToken
|
||||
|
||||
from fastmcp.client.auth.oauth import TokenStorageAdapter
|
||||
|
||||
storage = MemoryStore()
|
||||
adapter = TokenStorageAdapter(
|
||||
async_key_value=storage, server_url="https://test"
|
||||
)
|
||||
|
||||
before = time.time()
|
||||
token = OAuthToken(
|
||||
access_token="a",
|
||||
token_type="Bearer",
|
||||
expires_in=300,
|
||||
refresh_token="r",
|
||||
)
|
||||
await adapter.set_tokens(token)
|
||||
after = time.time()
|
||||
|
||||
expiry = await adapter.get_token_expiry()
|
||||
assert expiry is not None
|
||||
assert before + 300 <= expiry <= after + 300
|
||||
|
||||
async def test_reload_uses_stored_expiry_not_stale_expires_in(self):
|
||||
"""On reload, _initialize should use the stored absolute expiry rather
|
||||
than recomputing from the stale relative expires_in.
|
||||
|
||||
This is the core bug from #2862: a token issued with expires_in=300
|
||||
that's reloaded an hour later should NOT appear valid for another 5
|
||||
minutes.
|
||||
"""
|
||||
from key_value.aio.stores.memory import MemoryStore
|
||||
from mcp.shared.auth import OAuthToken
|
||||
|
||||
from fastmcp.client.auth.oauth import TokenStorageAdapter
|
||||
|
||||
storage = MemoryStore()
|
||||
adapter = TokenStorageAdapter(
|
||||
async_key_value=storage, server_url="https://test"
|
||||
)
|
||||
|
||||
token = OAuthToken(
|
||||
access_token="a",
|
||||
token_type="Bearer",
|
||||
expires_in=300,
|
||||
refresh_token="r",
|
||||
)
|
||||
await adapter.set_tokens(token)
|
||||
|
||||
# Simulate time passing by overwriting the stored expiry to a past time
|
||||
past_expiry = time.time() - 600
|
||||
await storage.put(
|
||||
key="https://test/token_expiry",
|
||||
value={"expires_at": past_expiry},
|
||||
collection="mcp-oauth-token-expiry",
|
||||
)
|
||||
|
||||
reloaded = await adapter.get_token_expiry()
|
||||
assert reloaded is not None
|
||||
assert reloaded == pytest.approx(past_expiry)
|
||||
|
||||
async def test_get_token_expiry_returns_none_when_not_stored(self):
|
||||
"""get_token_expiry returns None for tokens stored before the fix."""
|
||||
from key_value.aio.stores.memory import MemoryStore
|
||||
|
||||
from fastmcp.client.auth.oauth import TokenStorageAdapter
|
||||
|
||||
storage = MemoryStore()
|
||||
adapter = TokenStorageAdapter(
|
||||
async_key_value=storage, server_url="https://test"
|
||||
)
|
||||
assert await adapter.get_token_expiry() is None
|
||||
|
||||
async def test_clear_removes_token_expiry(self):
|
||||
"""clear() should also remove the stored token expiry."""
|
||||
from key_value.aio.stores.memory import MemoryStore
|
||||
from mcp.shared.auth import OAuthToken
|
||||
|
||||
from fastmcp.client.auth.oauth import TokenStorageAdapter
|
||||
|
||||
storage = MemoryStore()
|
||||
adapter = TokenStorageAdapter(
|
||||
async_key_value=storage, server_url="https://test"
|
||||
)
|
||||
|
||||
token = OAuthToken(
|
||||
access_token="a",
|
||||
token_type="Bearer",
|
||||
expires_in=300,
|
||||
refresh_token="r",
|
||||
)
|
||||
await adapter.set_tokens(token)
|
||||
assert await adapter.get_token_expiry() is not None
|
||||
|
||||
await adapter.clear()
|
||||
assert await adapter.get_token_expiry() is None
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue