From cc163e1de767bcf0f9cf1fd2dec247f75e9ef35e Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 3 Jul 2025 16:44:21 -0700 Subject: [PATCH 1/4] Handle unsuccessful OnResourceMetadataRequest events --- .../McpAuthenticationHandler.cs | 23 +++++- .../AuthEventTests.cs | 81 +++++++++++++++++++ 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs b/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs index f8c6f41c..b5dfd6d6 100644 --- a/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs +++ b/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs @@ -78,10 +78,7 @@ private string GetAbsoluteResourceMetadataUri() return absoluteUri.ToString(); } - /// - /// Handles the resource metadata request. - /// - private async Task HandleResourceMetadataRequestAsync() + private async Task HandleResourceMetadataRequestAsync() { var resourceMetadata = Options.ResourceMetadata; @@ -93,6 +90,23 @@ private async Task HandleResourceMetadataRequestAsync() }; await Options.Events.OnResourceMetadataRequest(context); + + if (context.Result is not null) + { + if (context.Result.Handled) + { + return true; + } + else if (context.Result.Skipped) + { + return false; + } + else if (context.Result.Failure is not null) + { + throw new AuthenticationFailureException("An error occurred from the OnResourceMetadataRequest event.", context.Result.Failure); + } + } + resourceMetadata = context.ResourceMetadata; } @@ -104,6 +118,7 @@ private async Task HandleResourceMetadataRequestAsync() } await Results.Json(resourceMetadata, McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(ProtectedResourceMetadata))).ExecuteAsync(Context); + return true; } /// diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs index 6a48c21d..b5cd78a8 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs @@ -289,6 +289,87 @@ public async Task ResourceMetadataEndpoint_ThrowsException_WhenNoMetadataProvide Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); } + [Fact] + public async Task ResourceMetadataEndpoint_HandlesResponse_WhenHandleResponseCalled() + { + Builder.Services.AddMcpServer().WithHttpTransport(); + + // Override the configuration to test HandleResponse behavior + Builder.Services.Configure( + McpAuthenticationDefaults.AuthenticationScheme, + options => + { + options.ResourceMetadata = null; + options.Events.OnResourceMetadataRequest = async context => + { + // Call HandleResponse() to discontinue processing and return to client + context.HandleResponse(); + await Task.CompletedTask; + }; + } + ); + + await using var app = Builder.Build(); + + app.MapMcp().RequireAuthorization(); + + await app.StartAsync(TestContext.Current.CancellationToken); + + // Make a direct request to the resource metadata endpoint + using var response = await HttpClient.GetAsync( + "/.well-known/oauth-protected-resource", + TestContext.Current.CancellationToken + ); + + // The request should be handled by the event handler without returning metadata + // Since HandleResponse() was called, the handler should have taken responsibility + // for generating the response, which in this case means an empty response + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + // The response should be empty since the event handler called HandleResponse() + // but didn't write any content to the response + var content = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + Assert.Empty(content); + } + + [Fact] + public async Task ResourceMetadataEndpoint_SkipsHandler_WhenSkipHandlerCalled() + { + Builder.Services.AddMcpServer().WithHttpTransport(); + + // Override the configuration to test SkipHandler behavior + Builder.Services.Configure( + McpAuthenticationDefaults.AuthenticationScheme, + options => + { + options.ResourceMetadata = null; + options.Events.OnResourceMetadataRequest = async context => + { + // Call SkipHandler() to discontinue processing in the current handler + context.SkipHandler(); + await Task.CompletedTask; + }; + } + ); + + await using var app = Builder.Build(); + + app.MapMcp().RequireAuthorization(); + + await app.StartAsync(TestContext.Current.CancellationToken); + + // Make a direct request to the resource metadata endpoint + using var response = await HttpClient.GetAsync( + "/.well-known/oauth-protected-resource", + TestContext.Current.CancellationToken + ); + + // When SkipHandler() is called, the authentication handler should skip processing + // and let other handlers in the pipeline handle the request. Since there are no + // other handlers configured for this endpoint, this should result in a 404 + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + private async Task HandleAuthorizationUrlAsync( Uri authorizationUri, Uri redirectUri, From edfa7f23fa8842d199a68c0c5ca42e0369af5ae0 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 9 Jul 2025 16:19:45 -0700 Subject: [PATCH 2/4] Fix Skipped handling --- .../Authentication/McpAuthenticationHandler.cs | 3 +-- tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs b/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs index b5dfd6d6..46b8e898 100644 --- a/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs +++ b/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs @@ -43,8 +43,7 @@ public async Task HandleRequestAsync() return false; } - await HandleResourceMetadataRequestAsync(); - return true; + return await HandleResourceMetadataRequestAsync(); } /// diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs index b5cd78a8..c993edd4 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs @@ -325,7 +325,7 @@ public async Task ResourceMetadataEndpoint_HandlesResponse_WhenHandleResponseCal // Since HandleResponse() was called, the handler should have taken responsibility // for generating the response, which in this case means an empty response Assert.Equal(HttpStatusCode.OK, response.StatusCode); - + // The response should be empty since the event handler called HandleResponse() // but didn't write any content to the response var content = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); From f459ca623a71fd7e8282855153126c1f2f58c378 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 10 Jul 2025 10:34:10 -0700 Subject: [PATCH 3/4] Avoid null dereferencing when authorization_endpoint is missing --- .../Authentication/ClientOAuthProvider.cs | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 96356028..9abd4909 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -212,11 +212,6 @@ private async Task PerformOAuthAuthorizationAsync( // Get auth server metadata var authServerMetadata = await GetAuthServerMetadataAsync(selectedAuthServer, cancellationToken).ConfigureAwait(false); - if (authServerMetadata is null) - { - ThrowFailedToHandleUnauthorizedResponse($"Failed to retrieve metadata for authorization server: '{selectedAuthServer}'"); - } - // Store auth server metadata for future refresh operations _authServerMetadata = authServerMetadata; @@ -238,7 +233,7 @@ private async Task PerformOAuthAuthorizationAsync( LogOAuthAuthorizationCompleted(); } - private async Task GetAuthServerMetadataAsync(Uri authServerUri, CancellationToken cancellationToken) + private async Task GetAuthServerMetadataAsync(Uri authServerUri, CancellationToken cancellationToken) { if (!authServerUri.OriginalString.EndsWith("/")) { @@ -249,7 +244,9 @@ private async Task PerformOAuthAuthorizationAsync( { try { - var response = await _httpClient.GetAsync(new Uri(authServerUri, path), cancellationToken).ConfigureAwait(false); + var wellKnownEndpoint = new Uri(authServerUri, path); + + var response = await _httpClient.GetAsync(wellKnownEndpoint, cancellationToken).ConfigureAwait(false); if (!response.IsSuccessStatusCode) { continue; @@ -258,15 +255,28 @@ private async Task PerformOAuthAuthorizationAsync( using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); var metadata = await JsonSerializer.DeserializeAsync(stream, McpJsonUtilities.JsonContext.Default.AuthorizationServerMetadata, cancellationToken).ConfigureAwait(false); - if (metadata != null) + if (metadata is null) { - metadata.ResponseTypesSupported ??= ["code"]; - metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"]; - metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"]; - metadata.CodeChallengeMethodsSupported ??= ["S256"]; + continue; + } - return metadata; + if (metadata.AuthorizationEndpoint is null) + { + ThrowFailedToHandleUnauthorizedResponse($"No authorization_endpoint was provided via '{wellKnownEndpoint}'."); } + + if (metadata.AuthorizationEndpoint.Scheme != Uri.UriSchemeHttp && + metadata.AuthorizationEndpoint.Scheme != Uri.UriSchemeHttps) + { + ThrowFailedToHandleUnauthorizedResponse($"AuthorizationEndpoint must use HTTP or HTTPS. '{metadata.AuthorizationEndpoint}' does not meet this requirement."); + } + + metadata.ResponseTypesSupported ??= ["code"]; + metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"]; + metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"]; + metadata.CodeChallengeMethodsSupported ??= ["S256"]; + + return metadata; } catch (Exception ex) { @@ -274,7 +284,7 @@ private async Task PerformOAuthAuthorizationAsync( } } - return null; + throw new McpException($"Failed to find .well-known/openid-configuration or .well-known/oauth-authorization-server metadata for authorization server: '{authServerUri}'"); } private async Task RefreshTokenAsync(string refreshToken, Uri resourceUri, AuthorizationServerMetadata authServerMetadata, CancellationToken cancellationToken) @@ -320,11 +330,6 @@ private Uri BuildAuthorizationUrl( AuthorizationServerMetadata authServerMetadata, string codeChallenge) { - if (authServerMetadata.AuthorizationEndpoint.Scheme != Uri.UriSchemeHttp && - authServerMetadata.AuthorizationEndpoint.Scheme != Uri.UriSchemeHttps) - { - throw new ArgumentException("AuthorizationEndpoint must use HTTP or HTTPS.", nameof(authServerMetadata)); - } var queryParamsDictionary = new Dictionary { From 719cd1cbb31d48bb564ac882917d90cbea648d17 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 10 Jul 2025 10:36:08 -0700 Subject: [PATCH 4/4] Delete extra newline --- .../Authentication/ClientOAuthProvider.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 9abd4909..686d749f 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -330,7 +330,6 @@ private Uri BuildAuthorizationUrl( AuthorizationServerMetadata authServerMetadata, string codeChallenge) { - var queryParamsDictionary = new Dictionary { ["client_id"] = GetClientIdOrThrow(),