modelcontextprotocol / modelcontextprotocol/python-sdk

Critical Token Refresh Bugs - Prevents Proactive Refresh

Abierto
#1,318 9 comentarios 6 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

auth P1 ready for work
Lenguaje dominante
Python
Estrellas
24.3k
Forks
4k
Merge medio
1 d 1 h
PR fusionados (30 d)
31

Descripción

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.

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

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Comienza en mcp/client/auth.py leyendo OAuthContext.update_token_expiry, is_token_valid y OAuthContext._initialize; después, inspecciona la cobertura existente de autenticación y almacenamiento de tokens. Se considera terminado cuando los tokens almacenados y caducados se gestionan correctamente sin perder el comportamiento de perpetual-token, la actualización proactiva funciona y las regresiones están cubiertas.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
python
Área
authentication, backend
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Tranquilo
Claridad
Bastante claro
Aptitud para principiantes
48/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.