diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 785e3cc2e..17e449c16 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -17,7 +17,7 @@ namespace ModelContextProtocol.Authentication; /// /// A generic implementation of an OAuth authorization provider. /// -internal sealed partial class ClientOAuthProvider : McpHttpClient +internal sealed partial class ClientOAuthProvider : McpHttpClient, IDisposable { /// /// The Bearer authentication scheme. @@ -73,6 +73,26 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient private readonly HashSet _accumulatedScopes = new(StringComparer.Ordinal); private bool _hasAttemptedStepUp; + // The single in-flight authorization-code flow, if any, and the scopes it requested. Written only + // while holding _tokenAcquisitionLock. The flow is deliberately detached from the cancellation of + // the request whose challenge started it: the user may already be completing the authorization in + // a browser, and canceling one HTTP request — for example a server/discover probe canceled by + // McpClientOptions.DiscoverProbeTimeout during the dual-path connect — must not abort that flow. + // If it did, the next challenge would start a second flow with a fresh state and PKCE verifier + // that the redirect the user eventually completes can never satisfy. Instead, a later challenge + // whose scopes the flow already requested joins it and shares its result, while each caller + // observes its own cancellation via WaitAsync. A challenge that needs a scope the flow did not + // request cannot be satisfied by its token, so it waits for the flow to settle and then runs its + // own step-up; at most one interactive flow is ever presented to the user at a time. The flow + // itself is bounded by the authorization callback handler's own completion and canceled on + // provider disposal. Because it outlives the lock, it carries its own snapshot of the client + // credentials (see ClientCredentials) rather than reading the mutable fields a later challenge may + // rebind for another authorization server while it is pending. + private Task? _inFlightAuthorizationCodeFlow; + private HashSet? _inFlightAuthorizationCodeFlowScopes; + private readonly CancellationTokenSource _disposeCts = new(); + private int _disposed; + /// /// Initializes a new instance of the class using the specified options. /// @@ -191,6 +211,18 @@ public ClientOAuthProvider( }); } + /// + /// Cancels any in-flight detached authorization-code flow (see ). + /// + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) == 0) + { + _disposeCts.Cancel(); + _disposeCts.Dispose(); + } + } + internal override async Task SendAsync(HttpRequestMessage request, JsonRpcMessage? message, CancellationToken cancellationToken) { bool attemptedRefresh = false; @@ -387,6 +419,15 @@ private async Task GetAccessTokenCoreAsync(HttpResponseMessage response, return steppedUpToken.AccessToken; } + // The step-up that already requested these scopes may still be pending (the request + // that started it was canceled while the user was completing it) or may have completed + // since the cache was read above. Either way its outcome is what satisfies this + // challenge, so reuse it rather than reject the challenge as repeated. + if (await TryReuseInFlightAuthorizationCodeFlowAsync(protectedResourceMetadata, usedAccessToken, cancellationToken).ConfigureAwait(false) is { } steppedUpAccessToken) + { + return steppedUpAccessToken; + } + ThrowFailedToHandleUnauthorizedResponse( "A repeated insufficient_scope challenge added no scope beyond those already requested, " + "so step-up authorization cannot satisfy the request."); @@ -480,8 +521,81 @@ private async Task GetAccessTokenCoreAsync(HttpResponseMessage response, // Store auth server metadata for future refresh operations _authServerMetadata = authServerMetadata; - // Perform the OAuth flow - return await InitiateAuthorizationCodeFlowAsync(protectedResourceMetadata, authServerMetadata, cancellationToken).ConfigureAwait(false); + // Perform the OAuth flow, unless the in-flight flow already satisfies this challenge: a caller + // that reaches this point while a previous caller's flow is still pending (that caller's + // request was canceled mid-flow, releasing the lock) joins it instead of starting a competing + // one, and a flow that completed during the metadata work above has cached the token this + // challenge needs. See the _inFlightAuthorizationCodeFlow comment. + if (await TryReuseInFlightAuthorizationCodeFlowAsync(protectedResourceMetadata, usedAccessToken, cancellationToken).ConfigureAwait(false) is { } reusedAccessToken) + { + return reusedAccessToken; + } + + if (_inFlightAuthorizationCodeFlow is { IsCompleted: false } pendingFlow) + { + // The pending flow did not request a scope this challenge needs, so its token cannot + // satisfy it, but two interactive flows must never be presented at once: let it settle + // first. Its outcome, success or failure, is reported to the callers that joined it and + // is irrelevant here (Task.WhenAny never faults). + await Task.WhenAny(pendingFlow).WaitAsync(cancellationToken).ConfigureAwait(false); + } + + var flow = _inFlightAuthorizationCodeFlow = StartAuthorizationCodeFlow(protectedResourceMetadata, authServerMetadata, _disposeCts.Token); + return await flow.WaitAsync(cancellationToken).ConfigureAwait(false); + } + + /// + /// Satisfies the current challenge from the in-flight authorization-code flow when that flow requested + /// every scope the challenge needs: joins the flow while it is pending, or reuses the token it cached + /// if it completed after the caller last consulted the cache. Returns when there + /// is no such flow or nothing usable came of it, in which case the caller runs a flow of its own. + /// + private async Task TryReuseInFlightAuthorizationCodeFlowAsync(ProtectedResourceMetadata protectedResourceMetadata, string? usedAccessToken, CancellationToken cancellationToken) + { + if (_inFlightAuthorizationCodeFlow is not { } flow || !InFlightAuthorizationCodeFlowCoversChallenge(protectedResourceMetadata)) + { + return null; + } + + if (!flow.IsCompleted) + { + return await flow.WaitAsync(cancellationToken).ConfigureAwait(false); + } + + // A flow that ran to completion stored its token, and a token other than the one this challenge + // rejected is worth retrying with. A flow that faulted or was canceled stored nothing, and a + // long-completed flow's token is the rejected one itself. + if (flow.Status == TaskStatus.RanToCompletion && + await _tokenCache.GetTokensAsync(cancellationToken).ConfigureAwait(false) is { IsExpired: false } cached && + !string.Equals(cached.AccessToken, usedAccessToken, StringComparison.Ordinal)) + { + return cached.AccessToken; + } + + return null; + } + + /// + /// Returns whether the in-flight authorization-code flow requested every scope the current challenge + /// requires, so that joining it can satisfy the challenge. A challenge that names no concrete scope is + /// satisfied by whatever token the flow yields. + /// + private bool InFlightAuthorizationCodeFlowCoversChallenge(ProtectedResourceMetadata protectedResourceMetadata) + { + if (_inFlightAuthorizationCodeFlowScopes is not { } requestedScopes) + { + return false; + } + + foreach (var scope in GetCurrentOperationScopes(protectedResourceMetadata)) + { + if (!requestedScopes.Contains(scope)) + { + return false; + } + } + + return true; } private void ApplyClientIdMetadataDocument(Uri metadataUri) @@ -673,6 +787,8 @@ private static IEnumerable GetWellKnownAuthorizationServerMetadataUris(Uri private async Task RefreshTokensAsync(string refreshToken, string? resourceUri, AuthorizationServerMetadata authServerMetadata, CancellationToken cancellationToken) { + var credentials = CaptureClientCredentials(); + Dictionary formFields = new() { ["grant_type"] = "refresh_token", @@ -684,7 +800,7 @@ private static IEnumerable GetWellKnownAuthorizationServerMetadataUris(Uri formFields["resource"] = resourceUri; } - using var request = CreateTokenRequest(authServerMetadata.TokenEndpoint, formFields); + using var request = CreateTokenRequest(authServerMetadata.TokenEndpoint, formFields, credentials); using var httpResponse = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false); @@ -693,22 +809,46 @@ private static IEnumerable GetWellKnownAuthorizationServerMetadataUris(Uri return null; } - var tokens = await HandleSuccessfulTokenResponseAsync(httpResponse, cancellationToken).ConfigureAwait(false); + var tokens = await HandleSuccessfulTokenResponseAsync(httpResponse, credentials, cancellationToken).ConfigureAwait(false); LogOAuthTokenRefreshCompleted(); return tokens.AccessToken; } - private async Task InitiateAuthorizationCodeFlowAsync( + /// + /// Starts an authorization-code flow for the current challenge and records the scopes it requests in + /// . Callers must hold _tokenAcquisitionLock. + /// + private Task StartAuthorizationCodeFlow( ProtectedResourceMetadata protectedResourceMetadata, AuthorizationServerMetadata authServerMetadata, CancellationToken cancellationToken) { + // The flow outlives this lock scope, so it works from a snapshot of the client credentials + // rather than the mutable fields; see the _inFlightAuthorizationCodeFlow comment. + var credentials = CaptureClientCredentials(); var codeVerifier = GenerateRandomBase64UrlValue(); var codeChallenge = GenerateCodeChallenge(codeVerifier); var state = GenerateRandomBase64UrlValue(); - var authUrl = BuildAuthorizationUrl(protectedResourceMetadata, authServerMetadata, codeChallenge, state); + // Record the scopes this flow actually asks the authorization server for: the effective scope + // placed in the URL, which offline_access augmentation and any ScopeSelector can make differ + // from _accumulatedScopes. + var scope = ComputeEffectiveScope(protectedResourceMetadata, authServerMetadata); + var authUrl = BuildAuthorizationUrl(protectedResourceMetadata, authServerMetadata, credentials.ClientId, codeChallenge, state, scope); + _inFlightAuthorizationCodeFlowScopes = new HashSet(scope is null ? [] : SplitScopes(scope), StringComparer.Ordinal); + + return CompleteAuthorizationCodeFlowAsync(protectedResourceMetadata, authServerMetadata, credentials, authUrl, state, codeVerifier, cancellationToken); + } + private async Task CompleteAuthorizationCodeFlowAsync( + ProtectedResourceMetadata protectedResourceMetadata, + AuthorizationServerMetadata authServerMetadata, + ClientCredentials credentials, + Uri authUrl, + string state, + string codeVerifier, + CancellationToken cancellationToken) + { var authResult = await _authorizationCallbackHandler( new AuthorizationCallbackContext { @@ -740,6 +880,7 @@ private async Task InitiateAuthorizationCodeFlowAsync( return await ExchangeCodeForTokenAsync( protectedResourceMetadata, authServerMetadata, + credentials, authResult.Code!, codeVerifier, cancellationToken).ConfigureAwait(false); @@ -748,14 +889,16 @@ private async Task InitiateAuthorizationCodeFlowAsync( private Uri BuildAuthorizationUrl( ProtectedResourceMetadata protectedResourceMetadata, AuthorizationServerMetadata authServerMetadata, + string clientId, string codeChallenge, - string state) + string state, + string? scope) { var resourceUri = GetResourceUri(protectedResourceMetadata); var queryParamsDictionary = new Dictionary { - ["client_id"] = GetClientIdOrThrow(), + ["client_id"] = clientId, ["redirect_uri"] = _redirectUri.ToString(), ["response_type"] = "code", ["code_challenge"] = codeChallenge, @@ -768,7 +911,6 @@ private Uri BuildAuthorizationUrl( queryParamsDictionary["resource"] = resourceUri; } - var scope = ComputeEffectiveScope(protectedResourceMetadata, authServerMetadata); if (!string.IsNullOrEmpty(scope)) { queryParamsDictionary["scope"] = scope!; @@ -797,6 +939,7 @@ private Uri BuildAuthorizationUrl( private async Task ExchangeCodeForTokenAsync( ProtectedResourceMetadata protectedResourceMetadata, AuthorizationServerMetadata authServerMetadata, + ClientCredentials credentials, string authorizationCode, string codeVerifier, CancellationToken cancellationToken) @@ -816,33 +959,45 @@ private async Task ExchangeCodeForTokenAsync( formFields["resource"] = resourceUri; } - using var request = CreateTokenRequest(authServerMetadata.TokenEndpoint, formFields); + using var request = CreateTokenRequest(authServerMetadata.TokenEndpoint, formFields, credentials); using var httpResponse = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false); await httpResponse.EnsureSuccessStatusCodeWithResponseBodyAsync(cancellationToken).ConfigureAwait(false); - var tokens = await HandleSuccessfulTokenResponseAsync(httpResponse, cancellationToken).ConfigureAwait(false); + var tokens = await HandleSuccessfulTokenResponseAsync(httpResponse, credentials, cancellationToken).ConfigureAwait(false); LogOAuthAuthorizationCompleted(); return tokens.AccessToken; } + /// + /// The client registration a token request is made with and persisted alongside the tokens it yields. + /// Captured under _tokenAcquisitionLock via so that work + /// which outlives the lock, such as a detached authorization-code flow, is unaffected by a later + /// challenge rebinding the provider's mutable credential fields to another authorization server. + /// + private sealed record ClientCredentials(string ClientId, string? ClientSecret, string? TokenEndpointAuthMethod, string? AuthorizationServer); + + /// Snapshots the current client registration. Callers must hold _tokenAcquisitionLock. + private ClientCredentials CaptureClientCredentials() => + new(GetClientIdOrThrow(), _clientSecret, _tokenEndpointAuthMethod, _clientCredentialsAuthorizationServer); + /// /// Creates an HTTP request to the token endpoint, applying the appropriate authentication - /// method based on . + /// method based on . /// - private HttpRequestMessage CreateTokenRequest(Uri tokenEndpoint, Dictionary formFields) + private HttpRequestMessage CreateTokenRequest(Uri tokenEndpoint, Dictionary formFields, ClientCredentials credentials) { HttpRequestMessage request = new(HttpMethod.Post, tokenEndpoint); - var clientId = GetClientIdOrThrow(); - if (string.Equals(_tokenEndpointAuthMethod, "client_secret_basic", StringComparison.Ordinal)) + var clientId = credentials.ClientId; + if (string.Equals(credentials.TokenEndpointAuthMethod, "client_secret_basic", StringComparison.Ordinal)) { // Per RFC 6749 §2.3.1: send client_id:client_secret as HTTP Basic auth. request.Headers.Authorization = new( "Basic", - Convert.ToBase64String(Encoding.UTF8.GetBytes($"{Uri.EscapeDataString(clientId)}:{Uri.EscapeDataString(_clientSecret ?? string.Empty)}"))); + Convert.ToBase64String(Encoding.UTF8.GetBytes($"{Uri.EscapeDataString(clientId)}:{Uri.EscapeDataString(credentials.ClientSecret ?? string.Empty)}"))); } - else if (string.Equals(_tokenEndpointAuthMethod, "none", StringComparison.Ordinal)) + else if (string.Equals(credentials.TokenEndpointAuthMethod, "none", StringComparison.Ordinal)) { // Public client: include client_id in the body but no secret. formFields["client_id"] = clientId; @@ -851,14 +1006,14 @@ private HttpRequestMessage CreateTokenRequest(Uri tokenEndpoint, Dictionary HandleSuccessfulTokenResponseAsync(HttpResponseMessage response, CancellationToken cancellationToken) + private async Task HandleSuccessfulTokenResponseAsync(HttpResponseMessage response, ClientCredentials credentials, CancellationToken cancellationToken) { using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); var tokenResponse = await JsonSerializer.DeserializeAsync(stream, McpJsonUtilities.JsonContext.Default.TokenResponse, cancellationToken).ConfigureAwait(false); @@ -883,10 +1038,10 @@ private async Task HandleSuccessfulTokenResponseAsync(HttpRespon ObtainedAt = DateTimeOffset.UtcNow, // Persist the client registration alongside the tokens so a durable cache can use the // refresh token after a process restart without re-running dynamic client registration. - ClientId = _clientId, - ClientSecret = _clientSecret, - TokenEndpointAuthMethod = _tokenEndpointAuthMethod, - AuthorizationServer = _clientCredentialsAuthorizationServer, + ClientId = credentials.ClientId, + ClientSecret = credentials.ClientSecret, + TokenEndpointAuthMethod = credentials.TokenEndpointAuthMethod, + AuthorizationServer = credentials.AuthorizationServer, }; await _tokenCache.StoreTokensAsync(tokens, cancellationToken).ConfigureAwait(false); diff --git a/src/ModelContextProtocol.Core/Client/HttpClientTransport.cs b/src/ModelContextProtocol.Core/Client/HttpClientTransport.cs index 14044d2d7..a61026809 100644 --- a/src/ModelContextProtocol.Core/Client/HttpClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/HttpClientTransport.cs @@ -105,6 +105,8 @@ private async Task ConnectSseTransportAsync(CancellationToken cancel /// public ValueTask DisposeAsync() { + // Cancels any authorization-code flow still running detached from a canceled request. + (_mcpHttpClient as IDisposable)?.Dispose(); _ownedHttpClient?.Dispose(); return default; } diff --git a/src/ModelContextProtocol.Core/Client/McpClientOptions.cs b/src/ModelContextProtocol.Core/Client/McpClientOptions.cs index 61a0613df..3f5f6cb7d 100644 --- a/src/ModelContextProtocol.Core/Client/McpClientOptions.cs +++ b/src/ModelContextProtocol.Core/Client/McpClientOptions.cs @@ -89,6 +89,16 @@ public sealed class McpClientOptions /// Setting an appropriate timeout prevents the client from hanging indefinitely when /// connecting to unresponsive servers. /// + /// + /// When the transport authenticates via OAuth with an interactive + /// , this timeout also bounds + /// how long the connect attempt waits for the user to complete the browser-based authorization, so + /// increase this value to cover the time a person needs to complete the login, not just the network + /// round-trips. Reaching it fails the connect attempt but does not cancel the authorization flow + /// itself: the flow keeps running so that a compatible later challenge on the same transport (one + /// whose scopes the flow requested) reuses its outcome rather than prompting the user again, and only + /// disposing the transport cancels it. + /// /// public TimeSpan InitializationTimeout { get; set; } = TimeSpan.FromSeconds(60); @@ -121,6 +131,15 @@ public sealed class McpClientOptions /// greater than or equal to , the probe is effectively bounded by /// alone. /// + /// + /// A server that requires OAuth answers the probe with a 401 challenge, which can start an + /// interactive authorization via . + /// If this timeout then elapses while the user is still authorizing, only the probe request is + /// canceled: the authorization flow keeps running, and the challenge raised by the + /// initialize fallback joins that same flow and reuses its token instead of starting a + /// second flow the user never sees. The connect attempt overall remains bounded by + /// , and disposing the transport cancels the flow. + /// /// /// /// The value is not positive and is not . diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index 693c77943..073be8b39 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -2507,6 +2507,32 @@ public async Task AuthorizationFlow_ScopeSelector_ReturningEmpty_OmitsScopeParam Assert.False(scopePresent); } + /// + /// An in-memory token cache that signals when tokens are first stored and can model an authorization + /// server that issues no refresh token by discarding it. + /// + private sealed class SignalingTokenCache(bool discardRefreshTokens = false) : ITokenCache + { + private TokenContainer? _tokens; + + public TaskCompletionSource TokensStored { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public ValueTask StoreTokensAsync(TokenContainer tokens, CancellationToken cancellationToken) + { + if (discardRefreshTokens) + { + tokens.RefreshToken = null; + } + + Volatile.Write(ref _tokens, tokens); + TokensStored.TrySetResult(); + return default; + } + + public ValueTask GetTokensAsync(CancellationToken cancellationToken) => + new(Volatile.Read(ref _tokens)); + } + private HttpClientTransport CreateOAuthTransport( Func>? authorizationCallbackHandler = null) => @@ -2549,4 +2575,517 @@ public async Task DynamicClientRegistration_ScopeSelector_AppliesToDcrScope() Assert.Equal("mcp:tools", TestOAuthServer.LastRegistrationScope); } + + [Fact] + public async Task InteractiveAuthorization_SurvivesCancellationOfTriggeringRequest() + { + // A challenge raised while a previous challenge's interactive flow is still pending must + // join that flow rather than start a second one: the user is already completing the first + // flow's authorization URL in a browser, and a second flow's state and PKCE verifier could + // never match the redirect the user eventually completes. Canceling the request whose + // challenge started the flow must therefore not cancel the flow itself. + await using var app = await StartMcpServerAsync(); + + var handlerInvocations = 0; + var handlerEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var completeAuthorization = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await using var transport = CreateOAuthTransport(async (context, cancellationToken) => + { + Interlocked.Increment(ref handlerInvocations); + handlerEntered.TrySetResult(); + + // Hold the flow open, like a user mid-login. Before the fix, canceling the first + // connect canceled this wait via cancellationToken, and the second connect re-invoked + // the handler for a fresh flow. + await completeAuthorization.Task.WaitAsync(cancellationToken); + return await HandleAuthorizationUrlAsync(context, cancellationToken); + }); + + var clientOptions = new McpClientOptions { ProtocolVersion = "2025-06-18" }; + + using var firstConnectCts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var firstConnect = McpClient.CreateAsync( + transport, clientOptions, loggerFactory: LoggerFactory, cancellationToken: firstConnectCts.Token); + + await handlerEntered.Task.WaitAsync(TestContext.Current.CancellationToken); + firstConnectCts.Cancel(); + await Assert.ThrowsAnyAsync(() => firstConnect); + + // The user now completes the original flow's authorization. The flow must still be alive + // to receive it, and the next connect must reuse its outcome (via the in-flight flow or + // the token it caches) instead of starting a second flow. + completeAuthorization.TrySetResult(); + + await using var client = await McpClient.CreateAsync( + transport, clientOptions, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, handlerInvocations); + } + + [Fact] + public async Task InteractiveAuthorization_SurvivesDiscoverProbeTimeout() + { + // End-to-end version of the dual-path connect scenario: the server/discover probe draws the + // 401 that starts the interactive flow, DiscoverProbeTimeout cancels the probe while the + // flow waits on the user, and the challenge raised by the initialize fallback must reuse the + // pending flow instead of starting a second one the user never sees. + // + // The user finishes the browser login only once the initialize fallback has reached the + // server unauthenticated, which can only happen after the probe was canceled. The flow the + // probe started is therefore still pending when the fallback is challenged, and the fallback + // must reuse it (by joining it, or by finding the token it caches) rather than start its own. + var initializeFallbackReachedServer = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await using var app = await StartMcpServerAsync(configureMiddleware: app => + { + // Registered ahead of the authentication and authorization middleware (added explicitly + // below instead of being auto-inserted at the front of the pipeline), which would + // otherwise challenge the unauthenticated request before it reached this observer. + app.Use(async (context, next) => + { + if (context.Request.Method == HttpMethods.Post && + context.Request.Path == "/" && + context.Request.Headers.Authorization.Count == 0) + { + context.Request.EnableBuffering(); + + var message = await JsonSerializer.DeserializeAsync( + context.Request.Body, + McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(JsonRpcMessage)), + context.RequestAborted) as JsonRpcMessage; + + context.Request.Body.Position = 0; + + if (message is JsonRpcRequest { Method: "initialize" }) + { + initializeFallbackReachedServer.TrySetResult(); + } + } + + await next(context); + }); + + app.UseAuthentication(); + app.UseAuthorization(); + }); + + // Warm the server pipeline (JIT, auth handlers) so the probe's challenge reaches the handler + // well within the probe timeout; otherwise the probe would time out before any flow starts and + // the fallback would simply run the only flow, passing without exercising the scenario. + using (var warmup = await HttpClient.PostAsync( + McpServerUrl, + new StringContent("""{"jsonrpc":"2.0","id":1,"method":"ping"}""", System.Text.Encoding.UTF8, "application/json"), + TestContext.Current.CancellationToken)) + { + Assert.Equal(HttpStatusCode.Unauthorized, warmup.StatusCode); + } + + var handlerInvocations = 0; + + await using var transport = CreateOAuthTransport(async (context, cancellationToken) => + { + Interlocked.Increment(ref handlerInvocations); + await initializeFallbackReachedServer.Task.WaitAsync(cancellationToken); + return await HandleAuthorizationUrlAsync(context, cancellationToken); + }); + + await using var client = await McpClient.CreateAsync( + transport, + new McpClientOptions { DiscoverProbeTimeout = TimeSpan.FromMilliseconds(500) }, + loggerFactory: LoggerFactory, + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, handlerInvocations); + } + + [Fact] + public async Task InteractiveAuthorization_PendingFlowIsNotReusedForChallengeRequiringMoreScopes() + { + // A pending flow may only be reused by a challenge whose scopes it requested. Here a canceled + // read-tool call leaves its "files:read" step-up pending while the user is still logging in; + // a write-tool call challenged for "files:write" must not reuse that flow, since its token + // could never satisfy the write. It must instead let the pending flow settle and then run its + // own step-up for the accumulated scopes, so the user still sees only one prompt at a time. + Builder.Services.AddMcpServer() + .WithTools([ + McpServerTool.Create([McpServerTool(Name = "read-tool")] + (ClaimsPrincipal user) => + { + return "Read tool executed."; + }), + McpServerTool.Create([McpServerTool(Name = "write-tool")] + (ClaimsPrincipal user) => + { + return "Write tool executed."; + }), + ]); + + var writeChallengeRaised = 0; + var writeChallengeBeingHandled = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await using var app = await StartMcpServerAsync(configureMiddleware: app => + { + // Fetching the protected resource metadata is the client's first step in handling a + // challenge, so the first fetch after the write challenge means it is being handled. The + // authentication handler serves that document itself, so this observer must sit ahead of + // the authentication middleware (added explicitly below rather than auto-inserted at the + // front of the pipeline), while the challenge middleware below it needs the authenticated user. + app.Use(async (context, next) => + { + if (context.Request.Path.StartsWithSegments("/.well-known/oauth-protected-resource") && Volatile.Read(ref writeChallengeRaised) == 1) + { + writeChallengeBeingHandled.TrySetResult(); + } + + await next(context); + }); + + app.UseAuthentication(); + app.UseAuthorization(); + + app.Use(async (context, next) => + { + if (context.Request.Method == HttpMethods.Post && context.Request.Path == "/") + { + context.Request.EnableBuffering(); + + var message = await JsonSerializer.DeserializeAsync( + context.Request.Body, + McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(JsonRpcMessage)), + context.RequestAborted) as JsonRpcMessage; + + context.Request.Body.Position = 0; + + if (message is JsonRpcRequest request && request.Method == "tools/call") + { + var toolCallParams = JsonSerializer.Deserialize( + request.Params, + McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(CallToolRequestParams))) as CallToolRequestParams; + + var scopeClaim = context.User.FindFirst("scope")?.Value ?? ""; + var scopeSet = new HashSet(scopeClaim.Split(' ')); + + var missingScope = toolCallParams?.Name switch + { + "read-tool" when !scopeSet.Contains("files:read") => "files:read", + "write-tool" when !scopeSet.Contains("files:write") => "files:write", + _ => null, + }; + + if (missingScope is not null) + { + if (missingScope == "files:write") + { + // Set before the response goes out so the observer above sees the flag + // when the client's metadata fetch arrives. + Volatile.Write(ref writeChallengeRaised, 1); + } + + context.Response.StatusCode = StatusCodes.Status403Forbidden; + context.Response.Headers.WWWAuthenticate = $"Bearer error=\"insufficient_scope\", resource_metadata=\"{McpServerUrl}/.well-known/oauth-protected-resource\", scope=\"{missingScope}\""; + await context.Response.StartAsync(context.RequestAborted); + await context.Response.Body.FlushAsync(context.RequestAborted); + return; + } + } + } + + await next(context); + }); + }); + + List requestedScopes = []; + var readStepUpEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var completeReadStepUp = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await using var transport = CreateOAuthTransport(async (context, cancellationToken) => + { + int invocation; + lock (requestedScopes) + { + requestedScopes.Add(QueryHelpers.ParseQuery(context.AuthorizationUri.Query)["scope"].ToString()); + invocation = requestedScopes.Count; + } + + if (invocation == 2) + { + // The read step-up: hold it open, like a user mid-login. + readStepUpEntered.TrySetResult(); + await completeReadStepUp.Task.WaitAsync(cancellationToken); + } + + return await HandleAuthorizationUrlAsync(context, cancellationToken); + }); + + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + // The read-tool call is canceled while its step-up waits on the user, leaving that flow pending. + using var readCts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var readCall = client.CallToolAsync("read-tool", cancellationToken: readCts.Token).AsTask(); + await readStepUpEntered.Task.WaitAsync(TestContext.Current.CancellationToken); + readCts.Cancel(); + await Assert.ThrowsAnyAsync(() => readCall); + + // The write-tool call is challenged for "files:write" and starts handling that challenge while + // the read step-up is still pending; only then does the user complete the read step-up. + var writeCall = client.CallToolAsync("write-tool", cancellationToken: TestContext.Current.CancellationToken).AsTask(); + await writeChallengeBeingHandled.Task.WaitAsync(TestContext.Current.CancellationToken); + completeReadStepUp.TrySetResult(); + + var writeResult = await writeCall; + Assert.Equal("Write tool executed.", writeResult.Content[0].ToString()); + + // Three prompts in total: the initial connect, the read step-up, and a separate write step-up + // that carries the accumulated scopes instead of reusing the read step-up's token. + Assert.Equal(["mcp:tools", "files:read mcp:tools", "files:read files:write mcp:tools"], requestedScopes); + + // The stepped-up token now covers the read tool as well, with no further prompt. + var readResult = await client.CallToolAsync("read-tool", cancellationToken: TestContext.Current.CancellationToken); + Assert.Equal("Read tool executed.", readResult.Content[0].ToString()); + Assert.Equal(3, requestedScopes.Count); + } + + [Fact] + public async Task InteractiveAuthorization_RepeatedChallengeForSameScopes_JoinsPendingStepUp() + { + // A step-up abandoned by its caller (canceled while the user is mid-login) is still pending. + // Another request challenged for the same scopes must join that flow instead of being rejected + // as an unproductive repeated step-up: the pending flow is exactly what will satisfy it. + Builder.Services.AddMcpServer() + .WithTools([ + McpServerTool.Create([McpServerTool(Name = "read-tool")] + (ClaimsPrincipal user) => + { + return "Read tool executed."; + }), + ]); + + var readChallengesRaised = 0; + var secondChallengeBeingHandled = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await using var app = await StartMcpServerAsync(configureMiddleware: app => + { + // Fetching the protected resource metadata is the client's first step in handling a + // challenge. The authentication handler serves that document itself, so this observer must + // sit ahead of the authentication middleware (added explicitly below rather than + // auto-inserted at the front of the pipeline), while the challenge middleware below it + // needs the authenticated user. + app.Use(async (context, next) => + { + if (context.Request.Path.StartsWithSegments("/.well-known/oauth-protected-resource") && Volatile.Read(ref readChallengesRaised) == 2) + { + secondChallengeBeingHandled.TrySetResult(); + } + + await next(context); + }); + + app.UseAuthentication(); + app.UseAuthorization(); + + app.Use(async (context, next) => + { + if (context.Request.Method == HttpMethods.Post && context.Request.Path == "/") + { + context.Request.EnableBuffering(); + + var message = await JsonSerializer.DeserializeAsync( + context.Request.Body, + McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(JsonRpcMessage)), + context.RequestAborted) as JsonRpcMessage; + + context.Request.Body.Position = 0; + + if (message is JsonRpcRequest request && request.Method == "tools/call") + { + var toolCallParams = JsonSerializer.Deserialize( + request.Params, + McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(CallToolRequestParams))) as CallToolRequestParams; + + var scopeClaim = context.User.FindFirst("scope")?.Value ?? ""; + var scopeSet = new HashSet(scopeClaim.Split(' ')); + + if (toolCallParams?.Name == "read-tool" && !scopeSet.Contains("files:read")) + { + // Counted before the response goes out so the observer above sees it when + // the client's metadata fetch arrives. + Interlocked.Increment(ref readChallengesRaised); + + context.Response.StatusCode = StatusCodes.Status403Forbidden; + context.Response.Headers.WWWAuthenticate = $"Bearer error=\"insufficient_scope\", resource_metadata=\"{McpServerUrl}/.well-known/oauth-protected-resource\", scope=\"files:read\""; + await context.Response.StartAsync(context.RequestAborted); + await context.Response.Body.FlushAsync(context.RequestAborted); + return; + } + } + } + + await next(context); + }); + }); + + List requestedScopes = []; + var stepUpEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var completeStepUp = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await using var transport = CreateOAuthTransport(async (context, cancellationToken) => + { + int invocation; + lock (requestedScopes) + { + requestedScopes.Add(QueryHelpers.ParseQuery(context.AuthorizationUri.Query)["scope"].ToString()); + invocation = requestedScopes.Count; + } + + if (invocation == 2) + { + // The step-up: hold it open, like a user mid-login. + stepUpEntered.TrySetResult(); + await completeStepUp.Task.WaitAsync(cancellationToken); + } + + return await HandleAuthorizationUrlAsync(context, cancellationToken); + }); + + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + // The first read-tool call is canceled while its step-up waits on the user, leaving it pending. + using var firstCallCts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var firstCall = client.CallToolAsync("read-tool", cancellationToken: firstCallCts.Token).AsTask(); + await stepUpEntered.Task.WaitAsync(TestContext.Current.CancellationToken); + firstCallCts.Cancel(); + await Assert.ThrowsAnyAsync(() => firstCall); + + // The second call draws the same challenge and starts handling it while the step-up is still + // pending; only then does the user complete the step-up. + var secondCall = client.CallToolAsync("read-tool", cancellationToken: TestContext.Current.CancellationToken).AsTask(); + await secondChallengeBeingHandled.Task.WaitAsync(TestContext.Current.CancellationToken); + completeStepUp.TrySetResult(); + + var result = await secondCall; + Assert.Equal("Read tool executed.", result.Content[0].ToString()); + + // Two prompts in total: the initial connect and the single step-up both calls shared. + Assert.Equal(["mcp:tools", "files:read mcp:tools"], requestedScopes); + } + + [Fact] + public async Task InteractiveAuthorization_FlowCompletingDuringChallengeHandling_IsReusedNotRestarted() + { + // A detached flow can complete while a later challenge is already being handled, after that + // challenge re-checked the token cache on acquiring the lock but before it decides whether to + // join the flow. Its token must then be reused; starting a second flow would prompt the user + // again for a token that is already cached. With no refresh token available (the test + // authorization server always issues one, so the cache discards it), the cached access token + // is the only alternative to a second prompt. + var tokenCache = new SignalingTokenCache(discardRefreshTokens: true); + var handlerInvocations = 0; + var handlerEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var completeAuthorization = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var secondConnectStarted = 0; + + await using var app = await StartMcpServerAsync(configureMiddleware: app => + { + // Fetching the protected resource metadata is the client's first step in handling a + // challenge. The authentication handler serves that document itself, so this observer must + // sit ahead of the authentication middleware (added explicitly below rather than + // auto-inserted at the front of the pipeline). Once the second connect's challenge fetches + // it, the user completes the first connect's flow, and the response is held back until that + // flow has stored its token, so the challenge finds the flow completed when it decides. + app.Use(async (context, next) => + { + if (context.Request.Path.StartsWithSegments("/.well-known/oauth-protected-resource") && Volatile.Read(ref secondConnectStarted) == 1) + { + completeAuthorization.TrySetResult(); + await tokenCache.TokensStored.Task.WaitAsync(TestConstants.DefaultTimeout, context.RequestAborted); + } + + await next(context); + }); + + app.UseAuthentication(); + app.UseAuthorization(); + }); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + TokenCache = tokenCache, + AuthorizationCallbackHandler = async (context, cancellationToken) => + { + Interlocked.Increment(ref handlerInvocations); + handlerEntered.TrySetResult(); + await completeAuthorization.Task.WaitAsync(cancellationToken); + return await HandleAuthorizationUrlAsync(context, cancellationToken); + }, + }, + }, HttpClient, LoggerFactory); + + var clientOptions = new McpClientOptions { ProtocolVersion = "2025-06-18" }; + + using var firstConnectCts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var firstConnect = McpClient.CreateAsync( + transport, clientOptions, loggerFactory: LoggerFactory, cancellationToken: firstConnectCts.Token); + + await handlerEntered.Task.WaitAsync(TestContext.Current.CancellationToken); + firstConnectCts.Cancel(); + await Assert.ThrowsAnyAsync(() => firstConnect); + + Volatile.Write(ref secondConnectStarted, 1); + await using var client = await McpClient.CreateAsync( + transport, clientOptions, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, handlerInvocations); + } + + [Fact] + public async Task DisposingTransport_CancelsDetachedAuthorizationFlow() + { + await using var app = await StartMcpServerAsync(); + + var handlerCanceled = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var transport = CreateOAuthTransport(async (context, cancellationToken) => + { + try + { + // Park the flow past the entire connect attempt, as if the user never finishes + // the browser login. + await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + } + catch (OperationCanceledException) + { + handlerCanceled.TrySetResult(); + throw; + } + + return null; + }); + + await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, + new McpClientOptions + { + DiscoverProbeTimeout = TimeSpan.FromMilliseconds(100), + InitializationTimeout = TimeSpan.FromSeconds(1), + }, + loggerFactory: LoggerFactory, + cancellationToken: TestContext.Current.CancellationToken)); + + // The flow is detached from the canceled connect requests; only disposing the transport + // cancels it. + Assert.False(handlerCanceled.Task.IsCompleted); + + await transport.DisposeAsync(); + + await handlerCanceled.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken); + } }