Skip to content

Commit cb6c959

Browse files
Copilothalter73
andcommitted
Trim trailing slashes from derived resource URIs and improve tests
Changes per MCP spec recommendation: - Remove trailing slash variants from TestOAuthServer.ValidResources (kept only no-slash versions) - Make ValidResources public static to allow test customization - Update McpAuthenticationHandler to trim trailing slashes when deriving resource URIs - Update ProtectedResourceMetadata.Clone to trim trailing slashes from derived URIs - Improve ResourceMetadata_DoesNotAddTrailingSlash test to actually authenticate (not just fetch metadata) - Add ResourceMetadata_PreservesExplicitTrailingSlash test to verify explicitly configured slashes work Per MCP spec: implementations SHOULD use URIs without trailing slashes unless semantically significant. Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
1 parent 45bdb39 commit cb6c959

4 files changed

Lines changed: 69 additions & 31 deletions

File tree

src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ private async Task<bool> HandleResourceMetadataRequestAsync(Uri? derivedResource
165165
throw new InvalidOperationException("ResourceMetadata has not been configured. Please set McpAuthenticationOptions.ResourceMetadata or ensure context.ResourceMetadata is set inside McpAuthenticationOptions.Events.OnResourceMetadataRequest.");
166166
}
167167

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

170170
if (resourceMetadata.Resource is null)
171171
{

src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public ProtectedResourceMetadata Clone(Uri? derivedResourceUri = null)
207207
{
208208
return new ProtectedResourceMetadata
209209
{
210-
Resource = Resource ?? derivedResourceUri?.ToString(),
210+
Resource = Resource ?? derivedResourceUri?.ToString().TrimEnd('/'),
211211
AuthorizationServers = [.. AuthorizationServers],
212212
BearerMethodsSupported = [.. BearerMethodsSupported],
213213
ScopesSupported = [.. ScopesSupported],

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

Lines changed: 64 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -755,38 +755,77 @@ await McpClient.CreateAsync(
755755
[Fact]
756756
public async Task ResourceMetadata_DoesNotAddTrailingSlash()
757757
{
758-
// This test verifies that using string for Resource property avoids URI normalization
759-
// that would add a trailing slash to URIs without paths
760-
const string resourceWithoutTrailingSlash = "http://localhost:5000";
758+
// This test verifies that automatically derived resource URIs don't have trailing slashes
759+
// and that the client doesn't add them during authentication
761760

762-
Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options =>
761+
// Don't explicitly set Resource - let it be derived from the request
762+
await using var app = await StartMcpServerAsync();
763+
764+
// Authenticate with the client - this will derive the resource URI from the server URL
765+
await using var transport = new HttpClientTransport(new()
763766
{
764-
options.ResourceMetadata = new ProtectedResourceMetadata
767+
Endpoint = new(McpServerUrl),
768+
OAuth = new()
765769
{
766-
Resource = resourceWithoutTrailingSlash,
767-
AuthorizationServers = { OAuthServerUrl },
768-
ScopesSupported = ["mcp:tools"],
769-
};
770-
});
770+
ClientId = "demo-client",
771+
ClientSecret = "demo-secret",
772+
RedirectUri = new Uri("http://localhost:1179/callback"),
773+
AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync,
774+
},
775+
}, HttpClient, LoggerFactory);
771776

772-
await using var app = await StartMcpServerAsync();
777+
// This should succeed - the client should not add a trailing slash
778+
// If the client incorrectly added a trailing slash, ValidResources would reject it
779+
await using var client = await McpClient.CreateAsync(
780+
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
781+
}
773782

774-
// Make a direct request to the resource metadata endpoint
775-
using var response = await HttpClient.GetAsync(
776-
"/.well-known/oauth-protected-resource",
777-
TestContext.Current.CancellationToken
778-
);
783+
[Fact]
784+
public async Task ResourceMetadata_PreservesExplicitTrailingSlash()
785+
{
786+
// This test verifies that explicitly configured trailing slashes are preserved
787+
const string resourceWithTrailingSlash = "http://localhost:5000/";
788+
789+
// Explicitly configure ValidResources to accept the trailing slash version
790+
var originalValidResources = ModelContextProtocol.TestOAuthServer.Program.ValidResources;
791+
try
792+
{
793+
ModelContextProtocol.TestOAuthServer.Program.ValidResources = [resourceWithTrailingSlash, "http://localhost:5000/mcp"];
794+
795+
Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options =>
796+
{
797+
options.ResourceMetadata = new ProtectedResourceMetadata
798+
{
799+
Resource = resourceWithTrailingSlash,
800+
AuthorizationServers = { OAuthServerUrl },
801+
ScopesSupported = ["mcp:tools"],
802+
};
803+
});
779804

780-
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
805+
await using var app = await StartMcpServerAsync();
781806

782-
var metadata = await response.Content.ReadFromJsonAsync<ProtectedResourceMetadata>(
783-
McpJsonUtilities.DefaultOptions,
784-
TestContext.Current.CancellationToken
785-
);
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);
786819

787-
Assert.NotNull(metadata);
788-
789-
// Verify that the Resource property does NOT have a trailing slash added
790-
Assert.Equal(resourceWithoutTrailingSlash, metadata.Resource);
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
826+
{
827+
// Restore original ValidResources
828+
ModelContextProtocol.TestOAuthServer.Program.ValidResources = originalValidResources;
829+
}
791830
}
792831
}

tests/ModelContextProtocol.TestOAuthServer/Program.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,11 @@ public sealed class Program
1616
private static readonly string _clientMetadataDocumentUrl = $"{_url}/client-metadata/cimd-client.json";
1717

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

2726
private readonly ConcurrentDictionary<string, AuthorizationCodeInfo> _authCodes = new();

0 commit comments

Comments
 (0)