Skip to content

Commit 040a1d2

Browse files
Copilothalter73
Copilot
and
halter73
committed
Change Clone to take string and make ValidResources non-static
Per feedback: - Changed ProtectedResourceMetadata.Clone() to take string? instead of Uri? parameter - Changed ValidResources from static to instance property (no need for try/finally) - Updated HandleDefaultResourceMetadataRequestAsync to build resource string directly without intermediate Uri - Trim trailing slashes from path when building derived resource string - Added manual HTTP metadata checks to both tests to verify PRM document contents - Removed try/finally from ResourceMetadata_PreservesExplicitTrailingSlash (not needed with instance property) Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
1 parent cb6c959 commit 040a1d2

4 files changed

Lines changed: 86 additions & 50 deletions

File tree

‎src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,28 @@ private async Task<bool> HandleDefaultResourceMetadataRequestAsync()
5656
return false;
5757
}
5858

59-
var deriveResourceUriBuilder = new UriBuilder(Request.Scheme, Request.Host.Host)
59+
// Build the derived resource string directly without trailing slash
60+
var scheme = Request.Scheme;
61+
var host = Request.Host.Host;
62+
var port = Request.Host.Port;
63+
var path = $"{Request.PathBase}{resourceSuffix}".TrimEnd('/');
64+
65+
string derivedResource;
66+
if (port.HasValue && !IsDefaultPort(scheme, port.Value))
6067
{
61-
Path = $"{Request.PathBase}{resourceSuffix}",
62-
};
63-
64-
if (Request.Host.Port is not null)
68+
derivedResource = $"{scheme}://{host}:{port.Value}{path}";
69+
}
70+
else
6571
{
66-
deriveResourceUriBuilder.Port = Request.Host.Port.Value;
72+
derivedResource = $"{scheme}://{host}{path}";
6773
}
6874

69-
return await HandleResourceMetadataRequestAsync(deriveResourceUriBuilder.Uri);
75+
return await HandleResourceMetadataRequestAsync(derivedResource);
76+
}
77+
78+
private static bool IsDefaultPort(string scheme, int port)
79+
{
80+
return (scheme == "http" && port == 80) || (scheme == "https" && port == 443);
7081
}
7182

7283
/// <summary>
@@ -128,9 +139,9 @@ private static string GetConfiguredResourceMetadataPath(Uri resourceMetadataUri)
128139
return path.StartsWith('/') ? path : $"/{path}";
129140
}
130141

131-
private async Task<bool> HandleResourceMetadataRequestAsync(Uri? derivedResourceUri = null)
142+
private async Task<bool> HandleResourceMetadataRequestAsync(string? derivedResource = null)
132143
{
133-
var resourceMetadata = Options.ResourceMetadata?.Clone(derivedResourceUri);
144+
var resourceMetadata = Options.ResourceMetadata?.Clone(derivedResource);
134145

135146
if (Options.Events.OnResourceMetadataRequest is not null)
136147
{
@@ -165,7 +176,7 @@ private async Task<bool> HandleResourceMetadataRequestAsync(Uri? derivedResource
165176
throw new InvalidOperationException("ResourceMetadata has not been configured. Please set McpAuthenticationOptions.ResourceMetadata or ensure context.ResourceMetadata is set inside McpAuthenticationOptions.Events.OnResourceMetadataRequest.");
166177
}
167178

168-
resourceMetadata.Resource ??= derivedResourceUri?.ToString().TrimEnd('/');
179+
resourceMetadata.Resource ??= derivedResource;
169180

170181
if (resourceMetadata.Resource is null)
171182
{

‎src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,13 @@ public sealed class ProtectedResourceMetadata
201201
/// <summary>
202202
/// Creates a deep copy of this <see cref="ProtectedResourceMetadata"/> instance, optionally overriding the Resource property.
203203
/// </summary>
204-
/// <param name="derivedResourceUri">Optional URI to use for the Resource property if the original Resource is null.</param>
204+
/// <param name="derivedResource">Optional resource URI string to use for the Resource property if the original Resource is null.</param>
205205
/// <returns>A new instance of <see cref="ProtectedResourceMetadata"/> with cloned values.</returns>
206-
public ProtectedResourceMetadata Clone(Uri? derivedResourceUri = null)
206+
public ProtectedResourceMetadata Clone(string? derivedResource = null)
207207
{
208208
return new ProtectedResourceMetadata
209209
{
210-
Resource = Resource ?? derivedResourceUri?.ToString().TrimEnd('/'),
210+
Resource = Resource ?? derivedResource,
211211
AuthorizationServers = [.. AuthorizationServers],
212212
BearerMethodsSupported = [.. BearerMethodsSupported],
213213
ScopesSupported = [.. ScopesSupported],

‎tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs‎

Lines changed: 61 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,24 @@ public async Task ResourceMetadata_DoesNotAddTrailingSlash()
761761
// Don't explicitly set Resource - let it be derived from the request
762762
await using var app = await StartMcpServerAsync();
763763

764-
// Authenticate with the client - this will derive the resource URI from the server URL
764+
// First, manually check the PRM document doesn't contain a trailing slash
765+
using var metadataResponse = await HttpClient.GetAsync(
766+
"/.well-known/oauth-protected-resource",
767+
TestContext.Current.CancellationToken
768+
);
769+
770+
Assert.Equal(HttpStatusCode.OK, metadataResponse.StatusCode);
771+
772+
var metadata = await metadataResponse.Content.ReadFromJsonAsync<ProtectedResourceMetadata>(
773+
McpJsonUtilities.DefaultOptions,
774+
TestContext.Current.CancellationToken
775+
);
776+
777+
Assert.NotNull(metadata);
778+
Assert.Equal("http://localhost:5000", metadata.Resource);
779+
Assert.DoesNotMatch(@"/$", metadata.Resource); // No trailing slash
780+
781+
// Then authenticate with the client - this will use the derived resource URI
765782
await using var transport = new HttpClientTransport(new()
766783
{
767784
Endpoint = new(McpServerUrl),
@@ -786,46 +803,54 @@ public async Task ResourceMetadata_PreservesExplicitTrailingSlash()
786803
// This test verifies that explicitly configured trailing slashes are preserved
787804
const string resourceWithTrailingSlash = "http://localhost:5000/";
788805

789-
// Explicitly configure ValidResources to accept the trailing slash version
790-
var originalValidResources = ModelContextProtocol.TestOAuthServer.Program.ValidResources;
791-
try
806+
// Configure ValidResources to accept the trailing slash version for this test
807+
TestOAuthServer.ValidResources = [resourceWithTrailingSlash, "http://localhost:5000/mcp"];
808+
809+
Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options =>
792810
{
793-
ModelContextProtocol.TestOAuthServer.Program.ValidResources = [resourceWithTrailingSlash, "http://localhost:5000/mcp"];
794-
795-
Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options =>
811+
options.ResourceMetadata = new ProtectedResourceMetadata
796812
{
797-
options.ResourceMetadata = new ProtectedResourceMetadata
798-
{
799-
Resource = resourceWithTrailingSlash,
800-
AuthorizationServers = { OAuthServerUrl },
801-
ScopesSupported = ["mcp:tools"],
802-
};
803-
});
813+
Resource = resourceWithTrailingSlash,
814+
AuthorizationServers = { OAuthServerUrl },
815+
ScopesSupported = ["mcp:tools"],
816+
};
817+
});
804818

805-
await using var app = await StartMcpServerAsync();
819+
await using var app = await StartMcpServerAsync();
806820

807-
// Authenticate with the client
808-
await using var transport = new HttpClientTransport(new()
809-
{
810-
Endpoint = new(McpServerUrl),
811-
OAuth = new()
812-
{
813-
ClientId = "demo-client",
814-
ClientSecret = "demo-secret",
815-
RedirectUri = new Uri("http://localhost:1179/callback"),
816-
AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync,
817-
},
818-
}, HttpClient, LoggerFactory);
821+
// First, manually check the PRM document contains the trailing slash
822+
using var metadataResponse = await HttpClient.GetAsync(
823+
"/.well-known/oauth-protected-resource",
824+
TestContext.Current.CancellationToken
825+
);
819826

820-
// This should succeed with the explicitly configured trailing slash
821-
// If the client incorrectly trimmed the slash, ValidResources would reject it
822-
await using var client = await McpClient.CreateAsync(
823-
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
824-
}
825-
finally
827+
Assert.Equal(HttpStatusCode.OK, metadataResponse.StatusCode);
828+
829+
var metadata = await metadataResponse.Content.ReadFromJsonAsync<ProtectedResourceMetadata>(
830+
McpJsonUtilities.DefaultOptions,
831+
TestContext.Current.CancellationToken
832+
);
833+
834+
Assert.NotNull(metadata);
835+
Assert.Equal(resourceWithTrailingSlash, metadata.Resource);
836+
Assert.Matches(@"/$", metadata.Resource); // Has trailing slash
837+
838+
// Then authenticate with the client
839+
await using var transport = new HttpClientTransport(new()
826840
{
827-
// Restore original ValidResources
828-
ModelContextProtocol.TestOAuthServer.Program.ValidResources = originalValidResources;
829-
}
841+
Endpoint = new(McpServerUrl),
842+
OAuth = new()
843+
{
844+
ClientId = "demo-client",
845+
ClientSecret = "demo-secret",
846+
RedirectUri = new Uri("http://localhost:1179/callback"),
847+
AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync,
848+
},
849+
}, HttpClient, LoggerFactory);
850+
851+
// This should succeed with the explicitly configured trailing slash
852+
// If the client incorrectly trimmed the slash, ValidResources would reject it
853+
await using var client = await McpClient.CreateAsync(
854+
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
830855
}
831856
}

‎tests/ModelContextProtocol.TestOAuthServer/Program.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public sealed class Program
1717

1818
// Port 5000 is used by tests and port 7071 is used by the ProtectedMcpServer sample
1919
// Per MCP spec, URIs should not have trailing slashes unless semantically significant
20-
public static string[] ValidResources { get; set; } = [
20+
public string[] ValidResources { get; set; } = [
2121
"http://localhost:5000",
2222
"http://localhost:5000/mcp",
2323
"http://localhost:7071"

0 commit comments

Comments
 (0)