Skip to content

OAuthClientProvider._initialize() breaks transparent refresh: missing update_token_expiry + missing oauth_metadata load #3250

Description

@draesthetic

Two bugs in OAuthClientProvider._initialize() combine to break transparent token refresh

Summary

When a client process restarts (or any time OAuthClientProvider is reconstructed), the SDK fails to transparently refresh expired access_tokens even when a valid refresh_token is on disk and the IdP would happily exchange it. Users are forced through an interactive OAuth re-auth on every process restart — even when the refresh_token is still valid for up to 15 days per the IdP's policy.

This affects every MCP server that issues short-lived access_tokens (~15 min) with longer-lived refresh_tokens — Fold MCP, Notion, GitHub PAT-rotated OAuth, any Hydra-style server, etc. — i.e. the entire modern OAuth ecosystem. The symptom is indistinguishable from the server revoking the refresh_token.

Bug 1: _initialize() doesn't compute token_expiry_time

_initialize() loads current_tokens from storage but never calls context.update_token_expiry(token). So context.token_expiry_time stays None.

Then is_token_valid():

def is_token_valid(self) -> bool:
    return bool(
        self.current_tokens
        and self.current_tokens.access_token
        and (not self.token_expiry_time or time.time() <= self.token_expiry_time)
    )

When token_expiry_time is None, the second clause is not None or … = True, so the function unconditionally returns True regardless of whether the access_token is expired by 1 second or 1 hour.

The refresh-on-expiry guard in async_auth_flow:

if not self.context.is_token_valid() and self.context.can_refresh_token():
    refresh_request = await self._refresh_token()
    …

…never fires. Expired access_tokens are sent on every request, the server returns 401, and the user lands in the full-re-auth branch (async_auth_flow lines 514+) which forces interactive login.

This same fix is already applied on the write path: set_tokens() (the function called after a successful refresh) does call update_token_expiry(), and there's even a comment in set_tokens referencing "Fix A … OAuthTokens.expiresAt persistence" that describes the pattern. The read path (_initialize) just doesn't do the same thing.

Bug 2: _refresh_token() builds the wrong endpoint URL when oauth_metadata isn't loaded

_refresh_token() picks the token endpoint like this:

if self.context.oauth_metadata and self.context.oauth_metadata.token_endpoint:
    token_url = str(self.context.oauth_metadata.token_endpoint)  # pragma: no cover
else:
    auth_base_url = self.context.get_authorization_base_url(self.context.server_url)
    token_url = urljoin(auth_base_url, "/token")

For a server like https://mcp.fold.money/mcp, the fallback path produces https://mcp.fold.money/token404. The correct endpoint for Hydra-style servers is https://mcp.fold.money/oauth/token.

oauth_metadata is normally populated via server discovery during the 401-handling flow (after a 401). But the refresh-on-expiry path runs before any 401 — it proactively refreshes when the token is expired, with no 401 yet. So oauth_metadata is never populated, and refresh fails silently with 404. The user then sees the 401 → re-auth loop as if the refresh_token itself were invalid.

The fix is for _initialize() to also load oauth_metadata from storage (e.g., via storage.load_oauth_metadata(), which the reference HermesTokenStorage implementation already provides).

Reproduction

Any MCP client using OAuthClientProvider against an IdP with ~15 min access_tokens and a Hydra-style token endpoint.

import asyncio, httpx
from mcp.client.auth.oauth2 import OAuthClientProvider
from mcp.shared.auth import OAuthClientMetadata, OAuthToken, OAuthClientInformationFull

# Suppose these came from persistent storage (Hermes's HermesTokenStorage
# or any conforming storage impl):
client_info = OAuthClientInformationFull.model_validate(client_info_dict)
current_tokens = OAuthToken.model_validate({
    "access_token": "...", "token_type": "Bearer", "expires_in": 900,
    "refresh_token": "...", "scope": "mcp:read",
})

class _Storage:
    async def get_tokens(self): return current_tokens
    async def set_tokens(self, t): current_tokens = t  # in-memory for repro
    async def get_client_info(self): return client_info

provider = OAuthClientProvider(
    server_url="https://mcp.fold.money/mcp",
    client_metadata=OAuthClientMetadata(...),
    storage=_Storage(),
)
await provider._initialize()
print(provider.context.is_token_valid())  # → True, even if access_token is expired
print(provider.context.token_expiry_time) # → None (Bug 1)

# Now suppose we manually trigger the refresh (mimicking async_auth_flow):
import time
# Force expires_in = 0 in the loaded token (the way HermesTokenStorage does it):
# …

# Refresh URL points to /token, not /oauth/token (Bug 2):
req = await provider._refresh_token()
print(req.url)  # → "https://mcp.fold.money/token" (404), not "/oauth/token"

Fix

In src/mcp/client/auth/oauth2.py, modify OAuthClientProvider._initialize():

async def _initialize(self) -> None:
    """Load stored tokens and client info."""
    import asyncio as _asyncio
    self.context.current_tokens = await self.context.storage.get_tokens()
    self.context.client_info = await self.context.storage.get_client_info()
    # Fix bug 1: compute absolute expiry from the loaded token's
    # `expires_in`, mirroring what set_tokens() does on the write path.
    if self.context.current_tokens is not None:
        self.context.update_token_expiry(self.context.current_tokens)
    # Fix bug 2: load oauth_metadata if the storage supports it, so
    # _refresh_token() can find the correct token_endpoint without
    # having to wait for server discovery.
    loader = getattr(self.context.storage, "load_oauth_metadata", None)
    if callable(loader):
        try:
            meta = loader()
            if _asyncio.iscoroutine(meta):
                meta = await meta
            if meta is not None:
                self.context.oauth_metadata = meta  # type: ignore[assignment]
        except Exception:
            pass
    self._initialized = True

The reference storage (HermesTokenStorage in some downstream clients like hermes-agent) already implements load_oauth_metadata() returning OAuthMetadata.model_validate(<contents of {server}.meta.json>). For SDK-provided storage classes that don't yet implement this, the getattr guard makes the second fix a no-op — bug 2 only manifests for downstream storage classes that already populate .meta.json.

Live verification

Patched locally against mcp==1.28.1 on macOS (Hermes agent 0.20.0). 16-minute live repro against https://mcp.fold.money:

  1. Login via OAuth → fresh token, mtime T0.
  2. Wait 15 min past access_token expiry → token on disk is stale, mtime still T0 (SDK never touched file).
  3. Make MCP call with forced-expired access_token + fresh refresh_token.
  4. Without the fix: SDK sends expired token, gets 401, falls through to re-auth (browser prompt).
  5. With the fix: SDK calls https://mcp.fold.money/oauth/token with grant_type=refresh_token, gets HTTP 200 with fresh rotated pair, writes back to disk. MCP call returns real data (verified: get_total_balance → ₹146,656.35 across 4 accounts).

AI disclosure

Drafted with AI assistance (GPT-class model). The bug analysis, code path tracing, fix design, and live verification were all done by a human reviewer who understood every line. The fix itself is 12 lines, two of which mirror the existing set_tokens write-path logic.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions