modelcontextprotocol / modelcontextprotocol/python-sdk

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

未关闭
#3,250 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

P1 v1 v2
主要语言
Python
星标
24.3k
派生
4k
平均合并
1 天 1 小时
30 天内合并 PR
31

描述

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.

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

从 src/mcp/client/auth/oauth2.py 中的 OAuthClientProvider._initialize() 开始,然后将其加载路径与 set_tokens() 进行比较,并检查 issue 中提到的存储方法,包括 get_tokens()、get_client_info() 和 load_oauth_metadata()。完成的标准是:初始化时恢复已持久化 token 的过期时间,可在刷新过程中使用可用的 OAuth 元数据,并且现有复现不再进入重新认证循环。

由索引模型根据 Issue 内容生成。

评估

技术栈
python
领域
authentication
Issue 类型
缺陷
难度
3/5
预计耗时
1-2 天
活跃度
冷清
描述清晰度
描述清楚
新手友好度
70/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。