Skip to content

Respect HandleResponse() and SkipHandler() calls in OnResourceMetadataRequest #607

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.

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
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ public async Task<bool> HandleRequestAsync()
return false;
}

await HandleResourceMetadataRequestAsync();
return true;
return await HandleResourceMetadataRequestAsync();
}

/// <summary>
Expand Down Expand Up @@ -78,10 +77,7 @@ private string GetAbsoluteResourceMetadataUri()
return absoluteUri.ToString();
}

/// <summary>
/// Handles the resource metadata request.
/// </summary>
private async Task HandleResourceMetadataRequestAsync()
private async Task<bool> HandleResourceMetadataRequestAsync()
{
var resourceMetadata = Options.ResourceMetadata;

Expand All @@ -93,6 +89,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;
}

Expand All @@ -104,6 +117,7 @@ private async Task HandleResourceMetadataRequestAsync()
}

await Results.Json(resourceMetadata, McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(ProtectedResourceMetadata))).ExecuteAsync(Context);
return true;
}

/// <inheritdoc />
Expand Down
44 changes: 24 additions & 20 deletions src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -238,7 +233,7 @@ private async Task PerformOAuthAuthorizationAsync(
LogOAuthAuthorizationCompleted();
}

private async Task<AuthorizationServerMetadata?> GetAuthServerMetadataAsync(Uri authServerUri, CancellationToken cancellationToken)
private async Task<AuthorizationServerMetadata> GetAuthServerMetadataAsync(Uri authServerUri, CancellationToken cancellationToken)
{
if (!authServerUri.OriginalString.EndsWith("/"))
{
Expand All @@ -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;
Expand All @@ -258,23 +255,36 @@ 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)
{
continue;
}

if (metadata.AuthorizationEndpoint is null)
{
metadata.ResponseTypesSupported ??= ["code"];
metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"];
metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"];
metadata.CodeChallengeMethodsSupported ??= ["S256"];
ThrowFailedToHandleUnauthorizedResponse($"No authorization_endpoint was provided via '{wellKnownEndpoint}'.");
}

return metadata;
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)
{
LogErrorFetchingAuthServerMetadata(ex, path);
}
}

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<TokenContainer> RefreshTokenAsync(string refreshToken, Uri resourceUri, AuthorizationServerMetadata authServerMetadata, CancellationToken cancellationToken)
Expand Down Expand Up @@ -320,12 +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<string, string>
{
["client_id"] = GetClientIdOrThrow(),
Expand Down
81 changes: 81 additions & 0 deletions tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<McpAuthenticationOptions>(
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<McpAuthenticationOptions>(
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<string?> HandleAuthorizationUrlAsync(
Uri authorizationUri,
Uri redirectUri,
Expand Down