modelcontextprotocol / modelcontextprotocol/python-sdk
OAuthClientProvider._initialize() breaks transparent refresh: missing update_token_expiry + missing oauth_metadata load
还没有人认领这个 Issue。
- 主要语言
- 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/token — 404. 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:
- Login via OAuth → fresh token, mtime T0.
- Wait 15 min past access_token expiry → token on disk is stale, mtime still T0 (SDK never touched file).
- Make MCP call with forced-expired access_token + fresh refresh_token.
- Without the fix: SDK sends expired token, gets 401, falls through to re-auth (browser prompt).
- With the fix: SDK calls
https://mcp.fold.money/oauth/tokenwithgrant_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.
贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 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