René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Critical Token Refresh Bugs - Prevents Proactive Refresh","articleBody":"### Initial Checks\n\n- [x] I confirm that I'm using the latest version of MCP Python SDK\n- [x] I confirm that I searched for my issue in https://github.com/modelcontextprotocol/python-sdk/issues before opening this issue\n\n### Description\n\ntldr: 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)\n\n## Background \nIn `mcp/client/auth.py` there are only 3 places where `OAuthContext.token_expiry_time` is set:\n1. During init it is set to None\n2. During full re-auth after receiving a 401 (in call to `_handle_token_response`)\n3. When a token is deemed invalid in `is_token_valid` (at start of `async_auth_flow`) \n\n## Issue 1 - Token validity check treats None as Valid\nThe ultimate problem is that when `token_expiry_time` is None, a token will pass validity checks, per the below. \n\n```python\ndef is_token_valid(self) -\u003e bool:\n    \"\"\"Check if current token is valid.\"\"\"\n    return bool(\n        self.current_tokens\n        and self.current_tokens.access_token\n        and (not self.token_expiry_time or time.time() \u003c= self.token_expiry_time)\n#                  ^^^^^^^^^^^^^^^^^^ None makes it perpetually valid so never needs pro-active refresh\n    )\n```\nThis is probably intended behavior to enable None to represent 'perpetual tokens'.\n\nIt sets up 2 fatal problems.\n\n## Issue 2 - Expired tokens get converted to 'perpetual tokens'\nIn `update_token_expiry` an expired token with a `token.expires` time of `0` is 'Falsey' which sets `token_expiry_time` to None.\n```python\ndef update_token_expiry(self, token: OAuthToken) -\u003e None:\n    \"\"\"Update token expiry time.\"\"\"\n    if token.expires_in:     # \u003c============================ 0 evaluates to False\n        self.token_expiry_time = time.time() + token.expires_in\n    else:\n        self.token_expiry_time = None  # \u003c============================ Makes it perpetually valid\n```\n\nPer **Issue 1**, when `token_expiry_time` is None the token is always valid (i.e. perpetual).\n\n## Undesirable Result - Only full re-auth can refresh a 'perpetual token' that goes stale\nBecause the token is now perpetual, it never gets proactively refreshed so it always requires a 401 and full re-auth to refresh.\n\n## Issue 3 - Clients with storage always start with `token_expiry_time` set to None\nThe OAuth init does not check token expiry:\n```python\nasync def _initialize(self) -\u003e None:\n    \"\"\"Load stored tokens and client info.\"\"\"\n    self.context.current_tokens = await self.context.storage.get_tokens()\n    self.context.client_info = await self.context.storage.get_client_info()\n    self._initialized = True\n```\nThis is often fine for Clients that persist.\n\nFor newly or frequently instantiated Clients that use Token Storage, `token_expiry_time` is None by default.\n\nDue to **Issue 1** that None value cause the token to become perpetual.\n\nDue to **Issue 2** the perpetual token is never checked for refresh.\n\nAs 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.\n\n## Proposed Fixes\nToken update should check if token has an expires_in attribute and for an explicit None. This preserves perpetual tokens while treating expired tokens appropriately.\n\n```python\n  def update_token_expiry(self, token: OAuthToken) -\u003e None:\n      \"\"\"Update token expiry time.\"\"\"\n      if hasattr(token, 'expires_in') and token.expires_in is not None: # \u003c================ Fix\n          self.token_expiry_time = time.time() + token.expires_in\n      else:\n          self.token_expiry_time = None\n```\n\nAlso, the validity check should only work when `self.token_expiry_time=None` explicitly instead of when 'Falsey':\n\n```python\ndef is_token_valid(self) -\u003e bool:\n    \"\"\"Check if current token is valid.\"\"\"\n    return bool(\n        self.current_tokens\n        and self.current_tokens.access_token\n        and (self.token_expiry_time is None or time.time() \u003c= self.token_expiry_time) # \u003c============== Fix\n               #^^^^^^^^^^^^^^^^^^^^^^^ None can remain signal for perpetual token\n    )\n```\n\nFinally, OAuthContext._initialize should call update_token_expiry so stored tokens aren't treated as perpetual by default:\n\n```python\nasync _initialize(self):\n    \"\"\"Initialize and properly set token expiry from stored tokens.\"\"\"\n    self.context.current_tokens = await self.context.storage.get_tokens()\n    self.context.client_info = await self.context.storage.get_client_info()\n    \n    # Fix: Update token expiry if tokens loaded from storage\n    if self.context.current_tokens:\n        self.context.update_token_expiry(self.context.current_tokens)\n    self._initialized = True\n```\n\n### Example Code\n\n```Python\n\n```\n\n### Python \u0026 MCP Python SDK\n\n```Text\nI'm on 1.12.4 but the code is still the same for 1.13.1\n```","author":{"url":"https://github.com/Norcim133","@type":"Person","name":"Norcim133"},"datePublished":"2025-08-28T00:16:24.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":9},"url":"https://github.com/1318/python-sdk/issues/1318"}