Skip to content

Commit fe76c79

Browse files
committed
Address copilot PR feedback
1 parent ab5daed commit fe76c79

4 files changed

Lines changed: 41 additions & 20 deletions

File tree

src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ private string GetAbsoluteResourceMetadataUri()
7676
return resourceMetadataUri.ToString();
7777
}
7878

79-
var seperator = resourceMetadataUri.OriginalString.StartsWith("/") ? "" : "/";
80-
return $"{Request.Scheme}://{Request.Host.ToUriComponent()}{Request.PathBase}{seperator}{resourceMetadataUri.OriginalString}";
79+
var separator = resourceMetadataUri.OriginalString.StartsWith("/") ? "" : "/";
80+
return $"{Request.Scheme}://{Request.Host.ToUriComponent()}{Request.PathBase}{separator}{resourceMetadataUri.OriginalString}";
8181
}
8282

8383
return $"{Request.Scheme}://{Request.Host.ToUriComponent()}{Request.PathBase}{DefaultResourceMetadataPath}{Request.Path}";
@@ -105,17 +105,15 @@ private bool IsConfiguredEndpointRequest(Uri resourceMetadataUri)
105105
if (!string.Equals(Request.Host.Host, resourceMetadataUri.Host, StringComparison.OrdinalIgnoreCase))
106106
{
107107
Logger.LogWarning(
108-
"Resource metadata request host '{RequestHost}' did not match configured host '{ConfiguredHost}'.",
109-
Request.Host.Value,
108+
"Resource metadata request host did not match configured host '{ConfiguredHost}'.",
110109
resourceMetadataUri.Host);
111110
return false;
112111
}
113112

114113
if (!string.Equals(Request.Scheme, resourceMetadataUri.Scheme, StringComparison.OrdinalIgnoreCase))
115114
{
116115
Logger.LogWarning(
117-
"Resource metadata request scheme '{RequestScheme}' did not match configured scheme '{ConfiguredScheme}'.",
118-
Request.Scheme,
116+
"Resource metadata request scheme did not match configured scheme '{ConfiguredScheme}'.",
119117
resourceMetadataUri.Scheme);
120118
return false;
121119
}
@@ -173,6 +171,11 @@ private async Task<bool> HandleResourceMetadataRequestAsync(Uri? derivedResource
173171

174172
resourceMetadata.Resource ??= derivedResourceUri;
175173

174+
if (resourceMetadata.Resource is null)
175+
{
176+
throw new InvalidOperationException("ResourceMetadata.Resource could not be determined. Please set McpAuthenticationOptions.ResourceMetadata.Resource or avoid setting a custom McpAuthenticationOptions.ResourceMetadataUri.");
177+
}
178+
176179
await Results.Json(resourceMetadata, McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(ProtectedResourceMetadata))).ExecuteAsync(Context);
177180
return true;
178181
}

src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ public sealed class ProtectedResourceMetadata
1515
/// The protected resource's resource identifier.
1616
/// </value>
1717
/// <remarks>
18-
/// OPTIONAL. When omitted, the MCP authentication handler infers the resource URI from the incoming request when serving
19-
/// the default <c>/.well-known/oauth-protected-resource</c> endpoint.
18+
/// OPTIONAL. When omitted, the MCP authentication handler infers the resource URI from the incoming request only when serving
19+
/// the default <c>/.well-known/oauth-protected-resource</c> endpoint. If a custom <c>ResourceMetadataUri</c> is configured,
20+
/// <b>Resource</b> must be explicitly set. Automatic inference only works with the default endpoint pattern.
2021
/// </remarks>
2122
[JsonPropertyName("resource")]
2223
public Uri? Resource { get; set; }

tests/ModelContextProtocol.AspNetCore.Tests/MapMcpTests.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using Microsoft.AspNetCore.Builder;
22
using Microsoft.AspNetCore.Http;
33
using Microsoft.Extensions.DependencyInjection;
4-
using Microsoft.Extensions.Logging;
54
using ModelContextProtocol.AspNetCore.Tests.Utils;
65
using ModelContextProtocol.Client;
76
using ModelContextProtocol.Protocol;
@@ -147,7 +146,6 @@ public async Task Sampling_DoesNotCloseStreamPrematurely()
147146
Assert.SkipWhen(Stateless, "Sampling is not supported in stateless mode.");
148147

149148
Builder.Services.AddMcpServer().WithHttpTransport(ConfigureStateless).WithTools<SamplingRegressionTools>();
150-
Builder.Logging.SetMinimumLevel(LogLevel.Debug);
151149

152150
await using var app = Builder.Build();
153151

tests/ModelContextProtocol.AspNetCore.Tests/OAuth/McpAuthenticationHandlerTests.cs

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
using System.Net.Http.Json;
1212
using System.Text.Encodings.Web;
1313

14-
namespace ModelContextProtocol.AspNetCore.Tests.Authentication;
14+
namespace ModelContextProtocol.AspNetCore.Tests.OAuth;
1515

1616
public class McpAuthenticationHandlerTests(ITestOutputHelper outputHelper) : KestrelInMemoryTest(outputHelper)
1717
{
@@ -23,6 +23,7 @@ public async Task Challenge_WithRelativeResourceMetadataUri_SetsAbsoluteUrl()
2323
await using var app = await StartAuthenticationServerAsync(options =>
2424
{
2525
options.ResourceMetadataUri = new Uri(metadataPath, UriKind.Relative);
26+
options.ResourceMetadata!.Resource = new Uri("http://localhost:5000/challenge");
2627
});
2728

2829
using var challengeResponse = await HttpClient.GetAsync(new Uri("/challenge", UriKind.Relative), HttpCompletionOption.ResponseHeadersRead, TestContext.Current.CancellationToken);
@@ -36,14 +37,34 @@ public async Task Challenge_WithRelativeResourceMetadataUri_SetsAbsoluteUrl()
3637
metadataResponse.EnsureSuccessStatusCode();
3738
}
3839

40+
[Fact]
41+
public async Task MetadataRequest_CustomResourceMetadataUriWithoutResource_ThrowsInvalidOperationException()
42+
{
43+
const string metadataPath = "/.well-known/custom-metadata";
44+
45+
await using var app = await StartAuthenticationServerAsync(options =>
46+
{
47+
options.ResourceMetadataUri = new Uri(metadataPath, UriKind.Relative);
48+
});
49+
50+
using var response = await HttpClient.GetAsync(new Uri(metadataPath, UriKind.Relative), TestContext.Current.CancellationToken);
51+
52+
Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
53+
Assert.Contains(MockLoggerProvider.LogMessages, log =>
54+
log.LogLevel == LogLevel.Error &&
55+
log.Exception is InvalidOperationException &&
56+
log.Exception.Message.Contains("ResourceMetadata.Resource could not be determined", StringComparison.Ordinal));
57+
}
58+
3959
[Fact]
4060
public async Task Challenge_WithAbsoluteResourceMetadataUri_SetsConfiguredUrl()
4161
{
42-
var metadataUri = new Uri("http://localhost:5000/.well-known/custom-absolute", UriKind.Absolute);
62+
var metadataUri = new Uri("http://localhost:5000/.well-known/custom-absolute");
4363

4464
await using var app = await StartAuthenticationServerAsync(options =>
4565
{
4666
options.ResourceMetadataUri = metadataUri;
67+
options.ResourceMetadata!.Resource = new Uri("http://localhost:5000/challenge");
4768
});
4869

4970
using var challengeResponse = await HttpClient.GetAsync(new Uri("/challenge", UriKind.Relative), HttpCompletionOption.ResponseHeadersRead, TestContext.Current.CancellationToken);
@@ -60,7 +81,7 @@ public async Task Challenge_WithAbsoluteResourceMetadataUri_SetsConfiguredUrl()
6081
[Fact]
6182
public async Task MetadataRequest_WithHostMismatch_LogsWarning()
6283
{
63-
var metadataUri = new Uri("http://expected-host:5000/.well-known/host-mismatch", UriKind.Absolute);
84+
var metadataUri = new Uri("http://expected-host:5000/.well-known/host-mismatch");
6485

6586
await using var app = await StartAuthenticationServerAsync(options =>
6687
{
@@ -146,7 +167,11 @@ private async Task<WebApplication> StartAuthenticationServerAsync(Action<McpAuth
146167

147168
authenticationBuilder.AddScheme<McpAuthenticationOptions, McpAuthenticationHandler>(McpAuthenticationDefaults.AuthenticationScheme, options =>
148169
{
149-
options.ResourceMetadata = CreateMetadata();
170+
options.ResourceMetadata = new()
171+
{
172+
AuthorizationServers = [new Uri("https://localhost:7029")],
173+
ScopesSupported = ["mcp:tools"],
174+
};
150175
configureOptions?.Invoke(options);
151176
});
152177

@@ -167,12 +192,6 @@ private async Task<WebApplication> StartAuthenticationServerAsync(Action<McpAuth
167192
return app;
168193
}
169194

170-
private static ProtectedResourceMetadata CreateMetadata() => new()
171-
{
172-
AuthorizationServers = [new Uri("https://localhost:7029")],
173-
ScopesSupported = ["mcp:tools"],
174-
};
175-
176195
private sealed class NoopBearerAuthenticationHandler(
177196
IOptionsMonitor<AuthenticationSchemeOptions> options,
178197
ILoggerFactory logger,

0 commit comments

Comments
 (0)