modelcontextprotocol / modelcontextprotocol/python-sdk
Critical Token Refresh Bugs - Prevents Proactive Refresh
Chưa có ai nhận issue này.
- Ngôn ngữ chính
- Python
- Star
- 24.3k
- Fork
- 4k
- Merge trung bình
- 1 ngày 1 giờ
- Pull request đã merge (30 ngày)
- 31
Mô tả
Initial Checks
- I confirm that I'm using the latest version of MCP Python SDK
- I confirm that I searched for my issue in https://github.com/modelcontextprotocol/python-sdk/issues before opening this issue
Description
tldr: Circular logic in mcp/client/auth.py creates two critical issues that prevent proactive token refresh; this would cause frequent re-auth for all users but it creates a third, worse issue for Clients that use TokenStorage (especially noticeable in a multi-user setup)
Background
In mcp/client/auth.py there are only 3 places where OAuthContext.token_expiry_time is set:
- During init it is set to None
- During full re-auth after receiving a 401 (in call to
_handle_token_response) - When a token is deemed invalid in
is_token_valid(at start ofasync_auth_flow)
Issue 1 - Token validity check treats None as Valid
The ultimate problem is that when token_expiry_time is None, a token will pass validity checks, per the below.
def is_token_valid(self) -> bool:
"""Check if current token is valid."""
return bool(
self.current_tokens
and self.current_tokens.access_token
and (not self.token_expiry_time or time.time() <= self.token_expiry_time)
# ^^^^^^^^^^^^^^^^^^ None makes it perpetually valid so never needs pro-active refresh
)
This is probably intended behavior to enable None to represent 'perpetual tokens'.
It sets up 2 fatal problems.
Issue 2 - Expired tokens get converted to 'perpetual tokens'
In update_token_expiry an expired token with a token.expires time of 0 is 'Falsey' which sets token_expiry_time to None.
def update_token_expiry(self, token: OAuthToken) -> None:
"""Update token expiry time."""
if token.expires_in: # <============================ 0 evaluates to False
self.token_expiry_time = time.time() + token.expires_in
else:
self.token_expiry_time = None # <============================ Makes it perpetually valid
Per Issue 1, when token_expiry_time is None the token is always valid (i.e. perpetual).
Undesirable Result - Only full re-auth can refresh a 'perpetual token' that goes stale
Because the token is now perpetual, it never gets proactively refreshed so it always requires a 401 and full re-auth to refresh.
Issue 3 - Clients with storage always start with token_expiry_time set to None
The OAuth init does not check token expiry:
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._initialized = True
This is often fine for Clients that persist.
For newly or frequently instantiated Clients that use Token Storage, token_expiry_time is None by default.
Due to Issue 1 that None value cause the token to become perpetual.
Due to Issue 2 the perpetual token is never checked for refresh.
As a result, tokens from storage (which are probably stale) are never checked prior to their first call. So they always trigger a 401 and revert to full re-auth.
Proposed Fixes
Token update should check if token has an expires_in attribute and for an explicit None. This preserves perpetual tokens while treating expired tokens appropriately.
def update_token_expiry(self, token: OAuthToken) -> None:
"""Update token expiry time."""
if hasattr(token, 'expires_in') and token.expires_in is not None: # <================ Fix
self.token_expiry_time = time.time() + token.expires_in
else:
self.token_expiry_time = None
Also, the validity check should only work when self.token_expiry_time=None explicitly instead of when 'Falsey':
def is_token_valid(self) -> bool:
"""Check if current token is valid."""
return bool(
self.current_tokens
and self.current_tokens.access_token
and (self.token_expiry_time is None or time.time() <= self.token_expiry_time) # <============== Fix
#^^^^^^^^^^^^^^^^^^^^^^^ None can remain signal for perpetual token
)
Finally, OAuthContext._initialize should call update_token_expiry so stored tokens aren't treated as perpetual by default:
async _initialize(self):
"""Initialize and properly set token expiry from stored tokens."""
self.context.current_tokens = await self.context.storage.get_tokens()
self.context.client_info = await self.context.storage.get_client_info()
# Fix: Update token expiry if tokens loaded from storage
if self.context.current_tokens:
self.context.update_token_expiry(self.context.current_tokens)
self._initialized = True
Example Code
Python & MCP Python SDK
I'm on 1.12.4 but the code is still the same for 1.13.1
Hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Hướng nghiên cứu
Bắt đầu trong mcp/client/auth.py bằng cách đọc OAuthContext.update_token_expiry, is_token_valid và OAuthContext._initialize, sau đó kiểm tra phần coverage hiện có cho việc xác thực và lưu trữ token. Được xem là hoàn tất khi các token đã lưu và đã hết hạn được xử lý chính xác mà không làm mất hành vi perpetual-token, việc refresh chủ động hoạt động và các hồi quy được bao phủ.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Đánh giá
- Công nghệ
- python
- Lĩnh vực
- authentication, backend
- Loại issue
- Lỗi
- Độ khó
- 4/5
- Thời gian dự kiến
- 3-5 ngày
- Mức độ hoạt động
- Ít trao đổi
- Độ rõ ràng
- Khá rõ ràng
- Mức phù hợp với người mới
- 48/100