From faef74b374bd6ba713bdcdf8be19b5705b0541ac Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Sat, 21 Mar 2026 12:12:50 -0400 Subject: [PATCH] fix: store absolute token expiry to prevent stale expires_in on reload (#3572) --- src/fastmcp/client/auth/oauth.py | 32 +++++++- tests/client/auth/test_oauth_client.py | 101 +++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/src/fastmcp/client/auth/oauth.py b/src/fastmcp/client/auth/oauth.py index a4d1e9c77..64416393a 100644 --- a/src/fastmcp/client/auth/oauth.py +++ b/src/fastmcp/client/auth/oauth.py @@ -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.""" diff --git a/tests/client/auth/test_oauth_client.py b/tests/client/auth/test_oauth_client.py index 20a335e8d..16a5e5e44 100644 --- a/tests/client/auth/test_oauth_client.py +++ b/tests/client/auth/test_oauth_client.py @@ -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