Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/mcp/client/auth/extensions/client_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
17 changes: 17 additions & 0 deletions src/mcp/client/auth/oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
7 changes: 7 additions & 0 deletions src/mcp/shared/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ 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

Expand Down
25 changes: 25 additions & 0 deletions tests/client/auth/extensions/test_client_credentials.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import time
import urllib.parse

import jwt
Expand Down Expand Up @@ -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."""
Expand Down
Loading