Skip to content

Commit f53e0d2

Browse files
authored
Respect HandleResponse() and SkipHandler() calls in OnResourceMetadataRequest (#607)
1 parent 3156818 commit f53e0d2

File tree

3 files changed

+125
-26
lines changed

3 files changed

+125
-26
lines changed

src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ public async Task<bool> HandleRequestAsync()
4343
return false;
4444
}
4545

46-
await HandleResourceMetadataRequestAsync();
47-
return true;
46+
return await HandleResourceMetadataRequestAsync();
4847
}
4948

5049
/// <summary>
@@ -78,10 +77,7 @@ private string GetAbsoluteResourceMetadataUri()
7877
return absoluteUri.ToString();
7978
}
8079

81-
/// <summary>
82-
/// Handles the resource metadata request.
83-
/// </summary>
84-
private async Task HandleResourceMetadataRequestAsync()
80+
private async Task<bool> HandleResourceMetadataRequestAsync()
8581
{
8682
var resourceMetadata = Options.ResourceMetadata;
8783

@@ -93,6 +89,23 @@ private async Task HandleResourceMetadataRequestAsync()
9389
};
9490

9591
await Options.Events.OnResourceMetadataRequest(context);
92+
93+
if (context.Result is not null)
94+
{
95+
if (context.Result.Handled)
96+
{
97+
return true;
98+
}
99+
else if (context.Result.Skipped)
100+
{
101+
return false;
102+
}
103+
else if (context.Result.Failure is not null)
104+
{
105+
throw new AuthenticationFailureException("An error occurred from the OnResourceMetadataRequest event.", context.Result.Failure);
106+
}
107+
}
108+
96109
resourceMetadata = context.ResourceMetadata;
97110
}
98111

@@ -104,6 +117,7 @@ private async Task HandleResourceMetadataRequestAsync()
104117
}
105118

106119
await Results.Json(resourceMetadata, McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(ProtectedResourceMetadata))).ExecuteAsync(Context);
120+
return true;
107121
}
108122

109123
/// <inheritdoc />

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,6 @@ private async Task PerformOAuthAuthorizationAsync(
212212
// Get auth server metadata
213213
var authServerMetadata = await GetAuthServerMetadataAsync(selectedAuthServer, cancellationToken).ConfigureAwait(false);
214214

215-
if (authServerMetadata is null)
216-
{
217-
ThrowFailedToHandleUnauthorizedResponse($"Failed to retrieve metadata for authorization server: '{selectedAuthServer}'");
218-
}
219-
220215
// Store auth server metadata for future refresh operations
221216
_authServerMetadata = authServerMetadata;
222217

@@ -238,7 +233,7 @@ private async Task PerformOAuthAuthorizationAsync(
238233
LogOAuthAuthorizationCompleted();
239234
}
240235

241-
private async Task<AuthorizationServerMetadata?> GetAuthServerMetadataAsync(Uri authServerUri, CancellationToken cancellationToken)
236+
private async Task<AuthorizationServerMetadata> GetAuthServerMetadataAsync(Uri authServerUri, CancellationToken cancellationToken)
242237
{
243238
if (!authServerUri.OriginalString.EndsWith("/"))
244239
{
@@ -249,7 +244,9 @@ private async Task PerformOAuthAuthorizationAsync(
249244
{
250245
try
251246
{
252-
var response = await _httpClient.GetAsync(new Uri(authServerUri, path), cancellationToken).ConfigureAwait(false);
247+
var wellKnownEndpoint = new Uri(authServerUri, path);
248+
249+
var response = await _httpClient.GetAsync(wellKnownEndpoint, cancellationToken).ConfigureAwait(false);
253250
if (!response.IsSuccessStatusCode)
254251
{
255252
continue;
@@ -258,23 +255,36 @@ private async Task PerformOAuthAuthorizationAsync(
258255
using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false);
259256
var metadata = await JsonSerializer.DeserializeAsync(stream, McpJsonUtilities.JsonContext.Default.AuthorizationServerMetadata, cancellationToken).ConfigureAwait(false);
260257

261-
if (metadata != null)
258+
if (metadata is null)
259+
{
260+
continue;
261+
}
262+
263+
if (metadata.AuthorizationEndpoint is null)
262264
{
263-
metadata.ResponseTypesSupported ??= ["code"];
264-
metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"];
265-
metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"];
266-
metadata.CodeChallengeMethodsSupported ??= ["S256"];
265+
ThrowFailedToHandleUnauthorizedResponse($"No authorization_endpoint was provided via '{wellKnownEndpoint}'.");
266+
}
267267

268-
return metadata;
268+
if (metadata.AuthorizationEndpoint.Scheme != Uri.UriSchemeHttp &&
269+
metadata.AuthorizationEndpoint.Scheme != Uri.UriSchemeHttps)
270+
{
271+
ThrowFailedToHandleUnauthorizedResponse($"AuthorizationEndpoint must use HTTP or HTTPS. '{metadata.AuthorizationEndpoint}' does not meet this requirement.");
269272
}
273+
274+
metadata.ResponseTypesSupported ??= ["code"];
275+
metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"];
276+
metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"];
277+
metadata.CodeChallengeMethodsSupported ??= ["S256"];
278+
279+
return metadata;
270280
}
271281
catch (Exception ex)
272282
{
273283
LogErrorFetchingAuthServerMetadata(ex, path);
274284
}
275285
}
276286

277-
return null;
287+
throw new McpException($"Failed to find .well-known/openid-configuration or .well-known/oauth-authorization-server metadata for authorization server: '{authServerUri}'");
278288
}
279289

280290
private async Task<TokenContainer> RefreshTokenAsync(string refreshToken, Uri resourceUri, AuthorizationServerMetadata authServerMetadata, CancellationToken cancellationToken)
@@ -320,12 +330,6 @@ private Uri BuildAuthorizationUrl(
320330
AuthorizationServerMetadata authServerMetadata,
321331
string codeChallenge)
322332
{
323-
if (authServerMetadata.AuthorizationEndpoint.Scheme != Uri.UriSchemeHttp &&
324-
authServerMetadata.AuthorizationEndpoint.Scheme != Uri.UriSchemeHttps)
325-
{
326-
throw new ArgumentException("AuthorizationEndpoint must use HTTP or HTTPS.", nameof(authServerMetadata));
327-
}
328-
329333
var queryParamsDictionary = new Dictionary<string, string>
330334
{
331335
["client_id"] = GetClientIdOrThrow(),

tests/ModelContextProtocol.AspNetCore.Tests/AuthEventTests.cs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,87 @@ public async Task ResourceMetadataEndpoint_ThrowsException_WhenNoMetadataProvide
289289
Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
290290
}
291291

292+
[Fact]
293+
public async Task ResourceMetadataEndpoint_HandlesResponse_WhenHandleResponseCalled()
294+
{
295+
Builder.Services.AddMcpServer().WithHttpTransport();
296+
297+
// Override the configuration to test HandleResponse behavior
298+
Builder.Services.Configure<McpAuthenticationOptions>(
299+
McpAuthenticationDefaults.AuthenticationScheme,
300+
options =>
301+
{
302+
options.ResourceMetadata = null;
303+
options.Events.OnResourceMetadataRequest = async context =>
304+
{
305+
// Call HandleResponse() to discontinue processing and return to client
306+
context.HandleResponse();
307+
await Task.CompletedTask;
308+
};
309+
}
310+
);
311+
312+
await using var app = Builder.Build();
313+
314+
app.MapMcp().RequireAuthorization();
315+
316+
await app.StartAsync(TestContext.Current.CancellationToken);
317+
318+
// Make a direct request to the resource metadata endpoint
319+
using var response = await HttpClient.GetAsync(
320+
"/.well-known/oauth-protected-resource",
321+
TestContext.Current.CancellationToken
322+
);
323+
324+
// The request should be handled by the event handler without returning metadata
325+
// Since HandleResponse() was called, the handler should have taken responsibility
326+
// for generating the response, which in this case means an empty response
327+
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
328+
329+
// The response should be empty since the event handler called HandleResponse()
330+
// but didn't write any content to the response
331+
var content = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken);
332+
Assert.Empty(content);
333+
}
334+
335+
[Fact]
336+
public async Task ResourceMetadataEndpoint_SkipsHandler_WhenSkipHandlerCalled()
337+
{
338+
Builder.Services.AddMcpServer().WithHttpTransport();
339+
340+
// Override the configuration to test SkipHandler behavior
341+
Builder.Services.Configure<McpAuthenticationOptions>(
342+
McpAuthenticationDefaults.AuthenticationScheme,
343+
options =>
344+
{
345+
options.ResourceMetadata = null;
346+
options.Events.OnResourceMetadataRequest = async context =>
347+
{
348+
// Call SkipHandler() to discontinue processing in the current handler
349+
context.SkipHandler();
350+
await Task.CompletedTask;
351+
};
352+
}
353+
);
354+
355+
await using var app = Builder.Build();
356+
357+
app.MapMcp().RequireAuthorization();
358+
359+
await app.StartAsync(TestContext.Current.CancellationToken);
360+
361+
// Make a direct request to the resource metadata endpoint
362+
using var response = await HttpClient.GetAsync(
363+
"/.well-known/oauth-protected-resource",
364+
TestContext.Current.CancellationToken
365+
);
366+
367+
// When SkipHandler() is called, the authentication handler should skip processing
368+
// and let other handlers in the pipeline handle the request. Since there are no
369+
// other handlers configured for this endpoint, this should result in a 404
370+
Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
371+
}
372+
292373
private async Task<string?> HandleAuthorizationUrlAsync(
293374
Uri authorizationUri,
294375
Uri redirectUri,

0 commit comments

Comments
 (0)