modelcontextprotocol / modelcontextprotocol/python-sdk

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

Abierto
#3,250 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

P1 v1 v2
Lenguaje dominante
Python
Estrellas
24.3k
Forks
4k
Merge medio
1 d 1 h
PR fusionados (30 d)
31

Descripción

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.

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Empieza en src/mcp/client/auth/oauth2.py, en OAuthClientProvider._initialize(); después compara su ruta de carga con set_tokens() e inspecciona los métodos de almacenamiento mencionados en el issue, incluidos get_tokens(), get_client_info() y load_oauth_metadata(). Se considera terminado cuando la expiración de los tokens persistidos se restaura durante la inicialización, los metadatos OAuth disponibles pueden utilizarse durante la renovación y la reproducción existente ya no entra en el bucle de reautenticación.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
python
Área
authentication
Tipo de issue
Error
Dificultad
3/5
Tiempo estimado
1-2 días
Estado de actividad
Tranquilo
Claridad
Bien especificado
Aptitud para principiantes
70/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.