Keep interactive OAuth flows alive when the triggering request is canceled

[PederHP]

Fixes #1830

Problem

During the dual-path connect, the server/discover probe's 401 challenge can start an interactive authorization flow via AuthorizationCallbackHandler. When DiscoverProbeTimeout (default 5s) elapses while the user is still completing that flow in a browser, the probe's cancellation aborts the flow. The initialize fallback's challenge then starts a second flow with a fresh state and PKCE verifier — with no way to surface a new authorization URL to the user, who is still completing the first flow. The redirect the user eventually completes carries the first flow's state, and the connect fails with:

Failed to handle unauthorized response with 'Bearer' scheme. The authorization response state did not match the state sent in the authorization request.

The user-approved code is unusable by the second flow regardless (different PKCE verifier), so this failure is unrecoverable. Observed in production against a real OAuth-protected MCP server; full analysis in #1830.

Fix

ClientOAuthProvider now memoizes the in-flight authorization-code flow and detaches it from the triggering request's cancellation token, bounding it by provider disposal instead:

• Only one interactive flow runs at a time; the flow that produced the authorization URL the user is completing is the one that receives the redirect.
• Each challenge handler awaits the shared flow with its own token via Task.WaitAsync, so a canceled request (e.g. the probe) abandons only its wait.
• A later challenge — the initialize fallback here, or any concurrent request — joins the in-flight flow and reuses its result. The existing _tokenAcquisitionLock + cached-token re-check already covers the "token arrived while I waited" case.
• ClientOAuthProvider is now IDisposable; HttpClientTransport.DisposeAsync disposes it, canceling any flow still pending so a parked AuthorizationCallbackHandler observes cancellation.

Docs for DiscoverProbeTimeout and InitializationTimeout now describe the interactive-authorization interplay (notably that the user's login must fit within InitializationTimeout).

Tests

• InteractiveAuthorization_SurvivesCancellationOfTriggeringRequest — deterministic regression test for the general mechanism: connect 1 is canceled while its flow waits on the user; the user then completes the original flow; connect 2 must reuse its outcome with exactly one handler invocation.
• InteractiveAuthorization_SurvivesDiscoverProbeTimeout — end-to-end dual-path scenario: probe 401 → probe canceled by DiscoverProbeTimeout → initialize fallback joins the pending flow.
• DisposingTransport_CancelsDetachedAuthorizationFlow — the detached flow outlives canceled connects but is canceled by transport disposal.

Both regression tests fail against the previous provider behavior and pass with this change; the full OAuth suite (101 tests) and the July 2026 protocol fallback suite (53 tests) pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_011bjA7zRNnUXnh19qSqgpTY

[Copilot]

Pull request overview

Keeps interactive OAuth authorization alive across canceled requests while preserving transport-lifetime cancellation.

Changes:

• Memoizes and shares in-flight authorization flows.
• Cancels detached flows when the HTTP transport is disposed.
• Documents timeout behavior and adds regression tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
ClientOAuthProvider.cs	Shares detached OAuth flows.
HttpClientTransport.cs	Cancels flows during disposal.
McpClientOptions.cs	Documents OAuth timeout behavior.
AuthTests.cs	Tests cancellation and disposal scenarios.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

In-flight flow reuse still has race, step-up, scope-selection, and shared-state concurrency defects.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:531

• A detached flow can finish after the cache re-check at lines 376–381 while this caller is fetching metadata. In that race, this branch discards the successful task and opens a second authorization URL even though the new token has just been cached, recreating the state/PKCE failure this change is meant to prevent. Re-check the cache before replacing a completed flow and reuse a valid token that differs from the one rejected by this request.

        if (flow is null || flow.IsCompleted)
        {
            _inFlightAuthorizationCodeFlow = flow = StartAuthorizationCodeFlow(protectedResourceMetadata, authServerMetadata, _disposeCts.Token);
        }

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:789

• _accumulatedScopes is not the set actually requested in authUrl: ComputeEffectiveScope subsequently adds offline_access and lets ScopeSelector filter or add scopes. Consequently compatibility is tested against raw server scopes rather than the authorization URL. For example, a selector mapping both read and write challenges to the same custom scope causes an unnecessary second interactive flow, while a filtered-out scope can be recorded as covered even though it was omitted. Record and compare the effective scope tokens emitted in the authorization URL.

        // Building the URL folds this challenge's scopes into _accumulatedScopes, so the accumulated set
        // is now exactly the set of scopes this flow asks the authorization server for.
        var authUrl = BuildAuthorizationUrl(protectedResourceMetadata, authServerMetadata, codeChallenge, state);
        _inFlightAuthorizationCodeFlowScopes = new HashSet<string>(_accumulatedScopes, StringComparer.Ordinal);

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:791

• Once the caller cancels its WaitAsync, the semaphore is released but this detached method continues reading mutable provider credentials in CreateTokenRequest and _clientCredentialsAuthorizationServer when storing tokens. A subsequent challenge can acquire the semaphore and rebind or dynamically register against another authorization server before this callback finishes, causing the original code exchange to use/store the later server's credentials. Capture all transaction credentials as locals for the detached flow, or keep later challenges from mutating provider authentication state until it settles.

        return CompleteAuthorizationCodeFlowAsync(protectedResourceMetadata, authServerMetadata, authUrl, state, codeVerifier, cancellationToken);

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Concurrent flow completion and inaccurate scope snapshots can still trigger incorrect retries or duplicate authorization prompts.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:798

• This snapshots _accumulatedScopes, not the scopes actually placed in authUrl. ComputeEffectiveScope can add offline_access, and ScopeSelector can remove challenged scopes or append custom ones, so coverage checks may join a flow that did not request the required scope or start a second prompt even though the pending flow did request it. Snapshot the URL's final scope parameter instead.

        _inFlightAuthorizationCodeFlowScopes = new HashSet<string>(_accumulatedScopes, StringComparer.Ordinal);

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:540

• A detached flow can complete after the earlier cache re-check but before this condition (the intervening metadata/DCR work makes that window substantial). In that case its valid token is discarded and this starts a second authorization flow with new state/PKCE, recreating the failure this change is intended to prevent. Return immediately when a compatible flow was observed pending, and re-check the cache before replacing a flow that completed concurrently.

        if (flow is null || flow.IsCompleted)
        {
            _inFlightAuthorizationCodeFlow = flow = StartAuthorizationCodeFlow(protectedResourceMetadata, authServerMetadata, _disposeCts.Token);
        }

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:426

• There is a completion race between the cache read above and the IsCompleted: false pattern. If the detached step-up stores its token and completes in that interval, this branch is skipped and the code throws the repeated-challenge error even though a new valid token is now available. Observe the task regardless of state: await it if pending, otherwise re-check the cache before rejecting the challenge.

                if (_inFlightAuthorizationCodeFlow is { IsCompleted: false } pendingStepUp &&
                    InFlightAuthorizationCodeFlowCoversChallenge(protectedResourceMetadata))
                {
                    return await pendingStepUp.WaitAsync(cancellationToken).ConfigureAwait(false);

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[PederHP]

Addressed the three findings from the latest Copilot review in 7e390ec:

• Concurrent flow completion (join point, line ~540): the join decision and the repeated-step-up guard now both go through one helper, TryReuseInFlightAuthorizationCodeFlowAsync, which joins the in-flight flow while it is pending and otherwise reuses the token it cached (when the flow ran to completion and the token differs from the one this challenge rejected). A flow that completes during the metadata fetches is therefore reused instead of restarted.
• Cache-read vs IsCompleted race in the repeated-step-up guard (line ~426): same helper, same fix.
• Scope snapshot (line ~798): the flow's recorded scope set is now the effective scope placed in the authorization URL (after offline_access augmentation and any ScopeSelector) rather than _accumulatedScopes; BuildAuthorizationUrl takes the scope as a parameter so it is computed once.

New test InteractiveAuthorization_FlowCompletingDuringChallengeHandling_IsReusedNotRestarted holds the second connect's protected-resource-metadata fetch until the first connect's detached flow has stored its token, with a token cache that discards refresh tokens (the test AS always issues one, and with a refresh token available the old code refreshed instead of prompting). Against 851d59a it fails deterministically with two prompts instead of one; OAuth + July2026 suites pass (151/151).

[Copilot]

🟡 Changes recommended

In-flight flows do not preserve or validate authorization-server context, creating credential races and incorrect flow reuse.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs:2697

• This test can still pass without exercising a flow that survives the probe timeout: if OAuth metadata discovery or dynamic registration takes longer than 500 ms, the probe is canceled before the callback starts, then the fallback starts the sole flow and handlerInvocations is still 1. The raw warmup request only warms the MCP server's 401 pipeline, not those client-side OAuth steps. Record that the callback entered before the unauthenticated initialize request is observed (and use a sufficiently tolerant probe interval) so this regression cannot false-pass on a slow runner.

        await using var client = await McpClient.CreateAsync(
            transport,
            new McpClientOptions { DiscoverProbeTimeout = TimeSpan.FromMilliseconds(500) },
            loggerFactory: LoggerFactory,
            cancellationToken: TestContext.Current.CancellationToken);

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:554

• Reuse is keyed only by scopes, so a concurrent challenge that selects a different authorization server can join this flow and retry with a token minted by the previous issuer. The provider explicitly supports rebinding dynamically registered clients when the selected server changes, so the in-flight descriptor also needs to record the selected authorization server/resource and require that context to match before joining.

        if (_inFlightAuthorizationCodeFlow is not { } flow || !InFlightAuthorizationCodeFlowCoversChallenge(protectedResourceMetadata))
        {

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Flow reuse does not verify authorization-server identity, and the probe-timeout test can pass without exercising its intended path.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs:2690

• This test can still pass without exercising a probe-started flow. If the 500 ms timeout fires before the probe reaches the callback, the initialize middleware signals first, the fallback invokes the only callback, and handlerInvocations == 1 passes through the fallback-only path. Warming the pipeline does not guarantee this ordering on a slow runner. Add an explicit assertion/signal that the first callback entered before initializeFallbackReachedServer completed, and use that to prove the probe—not the fallback—started the shared flow.

        await using var transport = CreateOAuthTransport(async (context, cancellationToken) =>
        {
            Interlocked.Increment(ref handlerInvocations);
            await initializeFallbackReachedServer.Task.WaitAsync(cancellationToken);
            return await HandleAuthorizationUrlAsync(context, cancellationToken);

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:556

• The reuse predicate checks scopes but not the authorization server. With dynamic registration, BindClientCredentialsToAuthorizationServer supports rebinding this provider to a newly selected server; a pending or completed flow from the previous server can therefore be joined when the new challenge happens to require the same scopes. Its token and PKCE registration belong to the old issuer, so the retry fails instead of starting the required flow. Record the selected authorization server (and resource context) with the in-flight flow and require it to match before reuse; otherwise wait for that flow to settle and start a new one.

        if (_inFlightAuthorizationCodeFlow is not { } flow || !InFlightAuthorizationCodeFlowCoversChallenge(protectedResourceMetadata))
        {

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced