Skip to content

auth() silently swallows non-OAuthError exceptions from refreshAuthorization / saveTokens, preventing token persistence #2034

Closed

Description

@jbsky

Describe the bug
In auth() (src/client/auth.ts), the catch block after refreshAuthorization() + saveTokens() silently discards all non-OAuthError exceptions — including I/O errors thrown by saveTokens(). No log, no rethrow. The function falls through to startAuthorization() → redirectToAuthorization(), which in headless/CLI clients is typically a no-op.
Result: the Authorization Server may have rotated the refresh token, but the new tokens are never persisted. The client is stuck with stale (or now-invalid) tokens on disk.

// src/client/auth.ts — the catch block around line 277
catch (error) {
    if (!(error instanceof OAuthError) || error instanceof ServerError) {
        // silently discarded — no log, no rethrow
    }
    else {
        throw error;
    }
}

The condition !(error instanceof OAuthError) || error instanceof ServerError catches:

  • Any non-OAuthError (TypeError, filesystem I/O errors from saveTokens, network errors)
  • ServerError (subclass of OAuthError)
    All are silently discarded.
    To Reproduce
  1. Implement an OAuthClientProvider with a saveTokens() that can throw (e.g., filesystem-backed with file lock contention, or any I/O error)
  2. Configure an OAuth 2.1 AS with rotating refresh tokens enabled (default in Keycloak)
  3. Let the access token expire
  4. auth() is called → refreshAuthorization() succeeds at the AS → saveTokens() throws
  5. The catch block swallows the error
  6. auth() falls through to startAuthorization() → redirectToAuthorization() (no-op in CLI clients)
  7. Old expired tokens remain on disk, new tokens are lost
  8. With rotating refresh tokens, the old refresh token is now invalidated server-side — the client must fully re-authenticate
    Expected behavior
    At minimum: log the error so operators can diagnose failures.
    Ideally: separate refreshAuthorization() failures (where fallthrough to re-auth may be appropriate) from saveTokens() failures (which should always propagate since the AS-side exchange already succeeded):
let newTokens;
try {
    newTokens = await refreshAuthorization(authorizationServerUrl, { ... });
} catch (error) {
    if (!(error instanceof OAuthError) || error instanceof ServerError) {
        // AS-side refresh failure — fall through to re-auth
    } else {
        throw error;
    }
}
if (newTokens) {
    await provider.saveTokens(newTokens);  // let this throw naturally
    return 'AUTHORIZED';
}

Logs
No logs — that's the problem. The catch block produces zero output. Diagnosis required reading SDK source code and correlating Keycloak audit logs (which showed successful token exchanges) with the unchanged mcp-auth.json on the client.
Additional context

Activity

  1. added
    bugSomething isn't working
    on May 8, 2026
  2. added a commit that references this issue on May 27, 2026
    5be12c5
  3. added
    triageQueued for automated analysis — bot will process and remove this label
    on Jul 9, 2026
  4. ghost added
    ready for workEnough information for someone to start working on
    on Jul 9, 2026
  5. ghost added
    authIssues and PRs related to Authentication / OAuth
    on Jul 9, 2026
  6. ghost added
    P2Moderate issues affecting some users, edge cases, potentially valuable feature
    on Jul 9, 2026
  7. ghost added
    fix proposedBot has a verified fix diff in the comment
    on Jul 9, 2026
  8. ghost removed
    triageQueued for automated analysis — bot will process and remove this label
    on Jul 9, 2026
  9. Coooder-Crypto commented on Jul 19, 2026

    @Coooder-Crypto

    Hi! I’d like to work on this. I plan to move token persistence outside the refresh error-handling block, add regression coverage ensuring saveTokens() failures propagate without starting a new authorization flow, and keep the change scoped to the behavior described here. Should the initial PR target main, with a separate v1.x backport?

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

    P2Moderate issues affecting some users, edge cases, potentially valuable featureauthIssues and PRs related to Authentication / OAuthbugSomething isn't workingfix proposedBot has a verified fix diff in the commentready 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