Skip to content

list reports auth required on expired access_token even when refresh_token is valid #166

@chrisabad

Description

@chrisabad

Summary

mcporter list reports auth required for HTTP OAuth servers whenever the cached access_token has expired, even though a valid refresh_token is sitting in ~/.mcporter/credentials.json. Re-running mcporter auth <server> then silently refreshes (no browser, no user input) — proving the refresh_token was always good. The user-visible symptom: list shows false alarms multiple times per day for any server with a short access-token TTL (Notion + Vercel are both 1h).

This is adjacent to but different from #137. #137 was about the cached token not being injected at all (config missing auth: "oauth"). That's fixed in 0.10.0+. This issue is about the cache being injected but not refreshed, so once the access_token TTL passes, list goes red until the user runs auth to manually trigger refresh.

Reproducer

  1. Configure an OAuth HTTP MCP server whose access_tokens are short-lived (e.g. Notion, Vercel — both 1h).
  2. mcporter auth <name> → tokens cached, both access_token and refresh_token populated.
  3. Wait > 1h.
  4. mcporter list → server reports auth required — run 'mcporter auth <name>', even though the refresh_token is still valid.
  5. mcporter auth <name> → completes silently in <1s without opening a browser. Tokens refreshed.
  6. mcporter list → server reports healthy again.

The fact that step 5 doesn't open a browser proves the underlying refresh_token was valid the entire time; the SDK just never tried it from the list path.

Diagnosis

In src/runtime/transport.ts, applyCachedOAuthHeaderIfAvailable calls readCachedAccessToken, which is a thin reader:

export async function readCachedAccessToken(
  definition: ServerDefinition,
  logger?: Logger
): Promise<string | undefined> {
  const persistence = await buildOAuthPersistence(definition, logger);
  const tokens = await persistence.readTokens();
  if (tokens && typeof tokens.access_token === 'string' && tokens.access_token.trim().length > 0) {
    return tokens.access_token;
  }
  return undefined;
}

No expiry check, no refresh attempt. The expired access_token gets injected as a Bearer header, the server returns 401, and mcporter classifies it as auth required. List runs with autoAuthorize: false + maxOAuthAttempts: 0, so the SDK's auth() flow (which would try refresh_token) is never invoked.

Why the obvious fix is dangerous

Naively flipping autoAuthorize: false → true on mcporter list would let the SDK auth() flow run, which DOES try refresh first. But the SDK's refresh path silently falls through to redirectToAuthorization if refresh fails for any reason:

// from @modelcontextprotocol/sdk client/auth.js:
if (tokens?.refresh_token) {
  try { /* refresh */ }
  catch (error) {
    if (!(error instanceof OAuthError) || error instanceof ServerError) {
      // Silent fallthrough — refresh failed, no error surface
    } else { throw error; }
  }
}
// Falls through to redirectToAuthorization → browser

Tested this empirically: with autoAuthorize: true on list, one of two test servers had its refresh fail (transient server-side issue), the SDK fell through to redirect, the 30s list timeout aborted the half-completed flow, and the cached tokens for that server were wiped from credentials.json. The user had to run a full interactive re-auth to recover.

So list needs a "silent refresh only, never redirect" mode, not a blanket autoAuthorize: true.

Proposed fix

In applyCachedOAuthHeaderIfAvailable (or a sibling function in the same code path):

  1. Read both access_token and expires_in / expiresAt from persistence.
  2. If access_token is past expiry (or within a small skew window, e.g. 60s) AND a refresh_token is present, attempt a silent refresh (direct call to the token endpoint with grant_type: refresh_token, no browser, no SDK auth() flow).
  3. On refresh success, persist new tokens, inject the new access_token.
  4. On refresh failure (any error), do NOT clear cached tokens, do NOT open browser. Just return the original (expired) token; let list surface "auth required" to the user.

This is fully non-interactive and fully safe — no data loss path, no surprise browser flows. It only matters for HTTP OAuth servers and only when allowCachedAuth: true.

Environment

  • mcporter 0.7.3 (also reproduces on 0.10.2 per code review of readCachedAccessToken)
  • Servers with 1h access_token TTL: Notion, Vercel
  • macOS 14, Node 22.22.0

Happy to send a PR if the proposed fix shape is acceptable. Wanted to file the issue first since this is a behavior change in the OAuth path and might warrant discussion.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions