Skip to content

fix(auth): deduplicate concurrent token refresh requests #1813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Aboudjem wants to merge 2 commits into
base: main
Choose a base branch
from
+386 −0
Open

fix(auth): deduplicate concurrent token refresh requests #1813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
59aa6b1
fix(auth): deduplicate concurrent token refresh requests
Aboudjem Mar 29, 2026
94f69f3
style: fix prettier formatting in auth test
Aboudjem Mar 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions packages/client/src/client/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,26 @@ export async function parseErrorResponse(input: Response | string): Promise<OAut
}
}

/**
* In-flight auth promises keyed by provider, used to deduplicate concurrent
* auth() calls. When multiple requests detect a 401 at the same time, only the
* first caller performs the actual token refresh; subsequent callers await the
* same promise. This prevents rotating refresh tokens from being used more than
* once, which would trigger RFC 6819 §5.2.2.3 replay detection and revoke the
* entire token family.
*/
const pendingAuthRequests = new Map<OAuthClientProvider, Promise<AuthResult>>();

/**
* Orchestrates the full auth flow with a server.
*
* This can be used as a single entry point for all authorization functionality,
* instead of linking together the other lower-level functions in this module.
*
* Concurrent calls for the same provider are deduplicated: only one auth flow
* runs at a time, and all concurrent callers receive the same result. This is
* critical for OAuth servers that use rotating refresh tokens, where concurrent
* refresh requests would invalidate each other.
*/
export async function auth(
provider: OAuthClientProvider,
Expand All @@ -403,6 +418,32 @@ export async function auth(
resourceMetadataUrl?: URL;
fetchFn?: FetchLike;
}
): Promise<AuthResult> {
// If there's already an in-flight auth for this provider, reuse it
const pending = pendingAuthRequests.get(provider);
if (pending) {
return await pending;
}

const authPromise = authWithErrorHandling(provider, options);
pendingAuthRequests.set(provider, authPromise);

try {
return await authPromise;
} finally {
pendingAuthRequests.delete(provider);
}
}
Comment on lines +421 to +436
Copy link
Copy Markdown

@claude claude bot Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The deduplication guard at auth.ts:423-426 keys only on the provider instance and ignores options.authorizationCode, so a concurrent call from finishAuth(code) will silently discard the one-time-use authorization code and return the result of an already in-flight token refresh. Since authorization codes expire in ~60 seconds and are never retried, this permanently breaks the OAuth flow with no error. Fix: skip deduplication when options.authorizationCode is defined, since code exchanges are semantically distinct from token refreshes and must never be coalesced.

Extended reasoning...

The Bug

The new auth() wrapper introduced in this PR coalesces concurrent calls by checking pendingAuthRequests.get(provider) — but the map key is solely the provider object; the options argument (including authorizationCode) is completely ignored. This means two fundamentally different operations — a token refresh (no authorizationCode) and an authorization code exchange (authorizationCode set) — are treated as duplicates and coalesced into one result.

Affected Code Path

The deduplication block at auth.ts:421-426:

const pending = pendingAuthRequests.get(provider);
if (pending) {
    return await pending;   // options.authorizationCode is silently thrown away
}

Both streamableHttp.ts:437 and sse.ts:228 call auth() with an authorizationCode via finishAuth(). These callers rely on auth() actually exchanging the code; they do not retry on failure.

Step-by-step Proof

  1. StreamableHttpClientTransport.connect() hits a 401, calls auth(provider, { serverUrl }) (no authorizationCode).
  2. That call reaches authInternal, which performs async discovery/refresh HTTP requests, suspending at multiple await points.
  3. While suspended, the user completes the OAuth redirect; the app calls transport.finishAuth('code123').
  4. finishAuth calls auth(provider, { serverUrl, authorizationCode: 'code123' }) (streamableHttp.ts:437).
  5. pendingAuthRequests.get(provider) returns the in-flight refresh promise.
  6. The guard immediately returns await pending — authorizationCode: 'code123' is never seen by authInternal.
  7. The authorization code expires (typically within 60 seconds) without being exchanged.
  8. finishAuth receives whatever the refresh returned (likely REDIRECT since no tokens were available), causing StreamableHttpClientTransport to throw UnauthorizedError('Failed to authorize').
  9. The OAuth flow is permanently broken; the user must restart from the beginning.

Why Existing Code Does Not Prevent It

The guard is a pure identity check on provider. There is no inspection of options. The PR's intent was to deduplicate concurrent refreshes (two simultaneous calls triggered by parallel 401 responses with identical semantics). It inadvertently also deduplicates semantically distinct operations.

Impact

This affects any application using StreamableHttpClientTransport or SSEClientTransport where a connection attempt triggers a token refresh in parallel with the user completing an OAuth redirect. The timing window is the duration of network I/O inside authInternal (discovery + refresh token request), which can easily be 100-500 ms — more than enough for an OAuth callback to arrive, especially in apps with automatic reconnection or fast retry.

Fix

const pending = pendingAuthRequests.get(provider);
if (pending && !options.authorizationCode) {  // bypass dedup for code exchanges
    return await pending;
}

Authorization code exchanges are one-time operations and must never be coalesced with refresh flows. This single-line change preserves the beneficial deduplication for concurrent refreshes while ensuring code exchanges always run independently.


async function authWithErrorHandling(
provider: OAuthClientProvider,
options: {
serverUrl: string | URL;
authorizationCode?: string;
scope?: string;
resourceMetadataUrl?: URL;
fetchFn?: FetchLike;
}
): Promise<AuthResult> {
try {
return await authInternal(provider, options);
Expand Down
Loading
Loading