Skip to content

bug: concurrent MCP OAuth refreshes race, and any refresh failure deletes valid credentials (forced re-auth) #29048

Description

@aniruddhaadak80

What happened?

Two related defects in MCPOAuthTokenStorage-backed refresh flow of MCPOAuthProvider:

  1. No concurrency control around token refresh. Two MCP tool calls hitting an expired access token concurrently both read the same refresh token and both POST to the token endpoint. Servers that rotate refresh tokens (standard per RFC 6749 §10.4 / OAuth BCP) invalidate the old refresh token on first use — the loser gets invalid_grant, enters the catch, and calls deleteCredentials(serverName), deleting the fresh valid credentials just saved by the winner. Result: forced full browser re-authentication despite a successful refresh.
  2. Any error deletes stored credentials. The same catch wipes credentials for every failure — including transient network errors on the refresh POST. An offline blip at expiry time permanently logs the user out even though the refresh token was perfectly valid.

Affected code

packages/core/src/mcp/oauth-provider.ts:574-617 (getValidToken):

const newTokenResponse = await this.refreshAccessToken(
  { ...config, clientId }, token.refreshToken, ...);
// ...
await this.tokenStorage.saveToken(serverName, newToken, ...);
return newToken.accessToken;
} catch (error) {
  coreEvents.emitFeedback('error', 'Failed to refresh auth token.', error);
  // Remove invalid token
  await this.tokenStorage.deleteCredentials(serverName);   // ANY error -> wipe
}

There is no mutex / in-flight-promise dedup / re-read-after-refresh anywhere; the underlying storage is plain read-modify-write per operation.

How can this be reproduced?

  1. Connect an MCP server that rotates refresh tokens; let the access token expire.
  2. Fire two tool calls concurrently.
  3. Observe two refresh POSTs; the second fails invalid_grant; stored credentials are deleted; next call demands interactive login.

(Or simpler: kill network connectivity during a single refresh → credentials wiped.)

What did you expect to happen?

  • Single-flight refresh (share one in-flight promise across callers).
  • On refresh failure, distinguish invalid_grant (credentials truly dead → delete) from transient errors (keep credentials, surface retryable error).

Impact

Forced re-authentication loops for MCP servers; worst case repeated browser popups in automation contexts.


Found by source audit on current main (commit 5411f113c); platform-independent. No open issue/PR covering this was found (searched: MCP OAuth refresh token deleted).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/securityIssues related to securitystatus/need-triageIssues that need to be triaged by the triage automation.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions