From 7018afc4ef4bb7950bec438624a0a0864713684d Mon Sep 17 00:00:00 2001 From: dhruvkej9 Date: Tue, 4 Aug 2026 15:46:26 +0000 Subject: [PATCH 1/2] fix: restore OAuth token expiry across process restarts --- .../auth/extensions/client_credentials.py | 2 ++ src/mcp/client/auth/oauth2.py | 17 +++++++++++++ src/mcp/shared/auth.py | 1 + .../extensions/test_client_credentials.py | 25 +++++++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/src/mcp/client/auth/extensions/client_credentials.py b/src/mcp/client/auth/extensions/client_credentials.py index 29197bb504..921dd71458 100644 --- a/src/mcp/client/auth/extensions/client_credentials.py +++ b/src/mcp/client/auth/extensions/client_credentials.py @@ -78,6 +78,7 @@ async def _initialize(self) -> None: """Load stored tokens and set pre-configured client_info.""" self.context.current_tokens = await self.context.storage.get_tokens() self.context.client_info = self._fixed_client_info + self.context.restore_token_expiry() self._initialized = True async def _perform_authorization(self) -> httpx2.Request: @@ -292,6 +293,7 @@ async def _initialize(self) -> None: """Load stored tokens and set pre-configured client_info.""" self.context.current_tokens = await self.context.storage.get_tokens() self.context.client_info = self._fixed_client_info + self.context.restore_token_expiry() self._initialized = True async def _perform_authorization(self) -> httpx2.Request: diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 7dc62b52b9..7f55d9b743 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -180,6 +180,19 @@ def update_token_expiry(self, token: OAuthToken) -> None: """Update token expiry time using shared util function.""" self.token_expiry_time = calculate_token_expiry(token.expires_in) + def restore_token_expiry(self) -> None: + """Restore ``token_expiry_time`` from the persisted absolute expiry. + + ``_initialize`` reloads ``current_tokens`` from storage, but the stored + ``OAuthToken`` only carries the relative ``expires_in``, so the absolute + expiry must be persisted separately (``expires_at``) and restored here. + Without it, ``is_token_valid()`` treats an already-expired access token + as valid on a fresh process and sends a stale Bearer, wasting a 401 + round-trip before re-authentication. + """ + if self.current_tokens and self.current_tokens.expires_at is not None: + self.token_expiry_time = self.current_tokens.expires_at + def is_token_valid(self) -> bool: """Check if current token is valid.""" return bool( @@ -484,6 +497,8 @@ async def _handle_token_response(self, response: httpx2.Response) -> None: # Store tokens in context self.context.current_tokens = token_response self.context.update_token_expiry(token_response) + # Persist the absolute expiry so it survives a process restart + token_response.expires_at = self.context.token_expiry_time await self.context.storage.set_tokens(token_response) async def _refresh_token(self) -> httpx2.Request: @@ -539,6 +554,7 @@ async def _handle_refresh_response(self, response: httpx2.Response) -> bool: self.context.current_tokens = token_response self.context.update_token_expiry(token_response) + token_response.expires_at = self.context.token_expiry_time await self.context.storage.set_tokens(token_response) return True @@ -551,6 +567,7 @@ async def _initialize(self) -> None: """Load stored tokens and client info.""" self.context.current_tokens = await self.context.storage.get_tokens() self.context.client_info = await self.context.storage.get_client_info() + self.context.restore_token_expiry() self._initialized = True def _add_auth_header(self, request: httpx2.Request) -> None: diff --git a/src/mcp/shared/auth.py b/src/mcp/shared/auth.py index 881379d381..cd10be2fb6 100644 --- a/src/mcp/shared/auth.py +++ b/src/mcp/shared/auth.py @@ -29,6 +29,7 @@ class OAuthToken(BaseModel): access_token: str token_type: Literal["Bearer"] = "Bearer" expires_in: int | None = None + expires_at: float | None = None scope: str | None = None refresh_token: str | None = None diff --git a/tests/client/auth/extensions/test_client_credentials.py b/tests/client/auth/extensions/test_client_credentials.py index 16336f8002..3bcf3bd9a9 100644 --- a/tests/client/auth/extensions/test_client_credentials.py +++ b/tests/client/auth/extensions/test_client_credentials.py @@ -1,3 +1,4 @@ +import time import urllib.parse import jwt @@ -94,6 +95,30 @@ async def test_init_with_client_secret_post(self, mock_storage: MockTokenStorage assert provider.context.client_info is not None assert provider.context.client_info.token_endpoint_auth_method == "client_secret_post" + @pytest.mark.anyio + async def test_init_restores_expired_token_expiry(self, mock_storage: MockTokenStorage): + """_initialize must restore token_expiry_time from the persisted expires_at. + + Regression for the stale-Bearer bug: without restoring the absolute + expiry, an already-expired access token looks valid after a restart and + a 401 round-trip is wasted before re-auth. + """ + mock_storage._tokens = OAuthToken( + access_token="expired-token", + expires_at=time.time() - 10, # already expired + ) + provider = ClientCredentialsOAuthProvider( + server_url="https://api.example.com", + storage=mock_storage, + client_id="test-client-id", + client_secret="test-client-secret", + ) + + await provider._initialize() + + assert provider.context.token_expiry_time is not None + assert not provider.context.is_token_valid() + @pytest.mark.anyio async def test_exchange_token_client_credentials(self, mock_storage: MockTokenStorage): """Test token exchange request building.""" From f5b4d8952e03696a7aa8c4817fed60f64ee63d82 Mon Sep 17 00:00:00 2001 From: Dhruv Kejriwal Date: Tue, 4 Aug 2026 16:36:15 +0000 Subject: [PATCH 2/2] docs(auth): document expires_at rationale from mcp2cli reproduction --- src/mcp/shared/auth.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/mcp/shared/auth.py b/src/mcp/shared/auth.py index cd10be2fb6..2fa7dc6dd8 100644 --- a/src/mcp/shared/auth.py +++ b/src/mcp/shared/auth.py @@ -29,6 +29,12 @@ class OAuthToken(BaseModel): access_token: str token_type: Literal["Bearer"] = "Bearer" expires_in: int | None = None + # Absolute unix timestamp when the access token expires. The spec's + # `expires_in` is relative, so a persisted token alone can't tell a fresh + # process whether it's already stale — that caused mcp2cli issues #50/#57 + # (a stale Bearer sent, then a wasted 401 round-trip before re-auth). + # Persisting the absolute expiry and restoring it on _initialize fixes the + # whole class. Backwards compatible: None means "unknown, re-auth once". expires_at: float | None = None scope: str | None = None refresh_token: str | None = None