Skip to content

Critical Token Refresh Bugs - Prevents Proactive Refresh #1318

Description

@Norcim133

Initial Checks

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:

  1. During init it is set to None
  2. During full re-auth after receiving a 401 (in call to _handle_token_response)
  3. When a token is deemed invalid in is_token_valid (at start of async_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.

defis_token_valid(self) ->bool:
"""Check if current token is valid."""returnbool(
self.current_tokensandself.current_tokens.access_tokenand (notself.token_expiry_timeortime.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.

defupdate_token_expiry(self, token: OAuthToken) ->None:
"""Update token expiry time."""iftoken.expires_in: # <============================ 0 evaluates to Falseself.token_expiry_time=time.time() +token.expires_inelse:
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:

asyncdef_initialize(self) ->None:
"""Load stored tokens and client info."""self.context.current_tokens=awaitself.context.storage.get_tokens()
self.context.client_info=awaitself.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.

defupdate_token_expiry(self, token: OAuthToken) ->None:
"""Update token expiry time."""ifhasattr(token, 'expires_in') andtoken.expires_inisnotNone: # <================ Fixself.token_expiry_time=time.time() +token.expires_inelse:
self.token_expiry_time=None

Also, the validity check should only work when self.token_expiry_time=None explicitly instead of when 'Falsey':

defis_token_valid(self) ->bool:
"""Check if current token is valid."""returnbool(
self.current_tokensandself.current_tokens.access_tokenand (self.token_expiry_timeisNoneortime.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=awaitself.context.storage.get_tokens()
self.context.client_info=awaitself.context.storage.get_client_info()
# Fix: Update token expiry if tokens loaded from storageifself.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

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1Significant bug affecting many users, highly requested featureauthIssues and PRs related to Authentication / OAuthready for workEnough information for someone to start working on

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions