diff --git a/samples/ProtectedMcpServer/Program.cs b/samples/ProtectedMcpServer/Program.cs index 83b4f6da4..97f8456a2 100644 --- a/samples/ProtectedMcpServer/Program.cs +++ b/samples/ProtectedMcpServer/Program.cs @@ -56,8 +56,8 @@ { options.ResourceMetadata = new() { - ResourceDocumentation = new Uri("https://docs.example.com/api/weather"), - AuthorizationServers = { new Uri(inMemoryOAuthServerUrl) }, + ResourceDocumentation = "https://docs.example.com/api/weather", + AuthorizationServers = { inMemoryOAuthServerUrl }, ScopesSupported = ["mcp:tools"], }; }); diff --git a/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs b/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs index 60adfb2b8..b30c7d593 100644 --- a/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs +++ b/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs @@ -56,17 +56,29 @@ private async Task HandleDefaultResourceMetadataRequestAsync() return false; } - var deriveResourceUriBuilder = new UriBuilder(Request.Scheme, Request.Host.Host) + // Build the derived resource string directly without trailing slash + var scheme = Request.Scheme; + var host = Request.Host.Host; + var port = Request.Host.Port; + var path = $"{Request.PathBase}{resourceSuffix}".TrimEnd('/'); + + string derivedResource; + if (port.HasValue && !IsDefaultPort(scheme, port.Value)) { - Path = $"{Request.PathBase}{resourceSuffix}", - }; - - if (Request.Host.Port is not null) + derivedResource = $"{scheme}://{host}:{port.Value}{path}"; + } + else { - deriveResourceUriBuilder.Port = Request.Host.Port.Value; + derivedResource = $"{scheme}://{host}{path}"; } - return await HandleResourceMetadataRequestAsync(deriveResourceUriBuilder.Uri); + return await HandleResourceMetadataRequestAsync(derivedResource); + } + + private static bool IsDefaultPort(string scheme, int port) + { + return (scheme.Equals("http", StringComparison.OrdinalIgnoreCase) && port == 80) || + (scheme.Equals("https", StringComparison.OrdinalIgnoreCase) && port == 443); } /// @@ -128,9 +140,9 @@ private static string GetConfiguredResourceMetadataPath(Uri resourceMetadataUri) return path.StartsWith('/') ? path : $"/{path}"; } - private async Task HandleResourceMetadataRequestAsync(Uri? derivedResourceUri = null) + private async Task HandleResourceMetadataRequestAsync(string? derivedResource = null) { - var resourceMetadata = Options.ResourceMetadata?.Clone(derivedResourceUri); + var resourceMetadata = Options.ResourceMetadata?.Clone(derivedResource); if (Options.Events.OnResourceMetadataRequest is not null) { @@ -165,7 +177,7 @@ private async Task HandleResourceMetadataRequestAsync(Uri? derivedResource throw new InvalidOperationException("ResourceMetadata has not been configured. Please set McpAuthenticationOptions.ResourceMetadata or ensure context.ResourceMetadata is set inside McpAuthenticationOptions.Events.OnResourceMetadataRequest."); } - resourceMetadata.Resource ??= derivedResourceUri; + resourceMetadata.Resource ??= derivedResource; if (resourceMetadata.Resource is null) { diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 2b0d34527..5b6aa8618 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -154,7 +154,7 @@ internal override async Task SendAsync(HttpRequestMessage r // Try to refresh the access token if it is invalid and we have a refresh token. if (_authServerMetadata is not null && tokens?.RefreshToken is { Length: > 0 } refreshToken) { - var accessToken = await RefreshTokensAsync(refreshToken, resourceUri, _authServerMetadata, cancellationToken).ConfigureAwait(false); + var accessToken = await RefreshTokensAsync(refreshToken, resourceUri.ToString(), _authServerMetadata, cancellationToken).ConfigureAwait(false); return (accessToken, true); } @@ -243,15 +243,26 @@ private async Task GetAccessTokenAsync(HttpResponseMessage response, boo ThrowFailedToHandleUnauthorizedResponse("No authorization servers found in authentication challenge"); } + // Convert string URIs to Uri objects for the selector + List authServerUris = []; + foreach (var serverUriString in availableAuthorizationServers) + { + if (!Uri.TryCreate(serverUriString, UriKind.Absolute, out var serverUri)) + { + ThrowFailedToHandleUnauthorizedResponse($"Invalid authorization server URI: '{serverUriString}'. Available servers: {string.Join(", ", availableAuthorizationServers)}"); + } + authServerUris.Add(serverUri); + } + // Select authorization server using configured strategy - var selectedAuthServer = _authServerSelector(availableAuthorizationServers); + var selectedAuthServer = _authServerSelector(authServerUris); if (selectedAuthServer is null) { ThrowFailedToHandleUnauthorizedResponse($"Authorization server selection returned null. Available servers: {string.Join(", ", availableAuthorizationServers)}"); } - if (!availableAuthorizationServers.Contains(selectedAuthServer)) + if (!authServerUris.Contains(selectedAuthServer)) { ThrowFailedToHandleUnauthorizedResponse($"Authorization server selector returned a server not in the available list: {selectedAuthServer}. Available servers: {string.Join(", ", availableAuthorizationServers)}"); } @@ -387,13 +398,13 @@ private static IEnumerable GetWellKnownAuthorizationServerMetadataUris(Uri } } - private async Task RefreshTokensAsync(string refreshToken, Uri resourceUri, AuthorizationServerMetadata authServerMetadata, CancellationToken cancellationToken) + private async Task RefreshTokensAsync(string refreshToken, string resourceUri, AuthorizationServerMetadata authServerMetadata, CancellationToken cancellationToken) { Dictionary formFields = new() { ["grant_type"] = "refresh_token", ["refresh_token"] = refreshToken, - ["resource"] = resourceUri.ToString(), + ["resource"] = resourceUri, }; using var request = CreateTokenRequest(authServerMetadata.TokenEndpoint, formFields); @@ -443,7 +454,7 @@ private Uri BuildAuthorizationUrl( ["response_type"] = "code", ["code_challenge"] = codeChallenge, ["code_challenge_method"] = "S256", - ["resource"] = resourceUri.ToString(), + ["resource"] = resourceUri, }; var scope = GetScopeParameter(protectedResourceMetadata); @@ -487,7 +498,7 @@ private async Task ExchangeCodeForTokenAsync( ["code"] = authorizationCode, ["redirect_uri"] = _redirectUri.ToString(), ["code_verifier"] = codeVerifier, - ["resource"] = resourceUri.ToString(), + ["resource"] = resourceUri, }; using var request = CreateTokenRequest(authServerMetadata.TokenEndpoint, formFields); @@ -659,7 +670,7 @@ private async Task PerformDynamicClientRegistrationAsync( } } - private static Uri GetRequiredResourceUri(ProtectedResourceMetadata protectedResourceMetadata) + private static string GetRequiredResourceUri(ProtectedResourceMetadata protectedResourceMetadata) { if (protectedResourceMetadata.Resource is null) { @@ -732,6 +743,27 @@ private static string NormalizeUri(Uri uri) return builder.ToString(); } + /// + /// Normalizes a URI string for consistent comparison. + /// + /// The URI string to normalize. + /// + /// A normalized string representation of the URI. If the string is a valid absolute URI, + /// it is parsed and normalized (scheme, host, port, and path without trailing slash). + /// If the string is not a valid absolute URI, only the trailing slash is removed. + /// + private static string NormalizeUri(string uriString) + { + // Parse the string as a URI to normalize it + if (!Uri.TryCreate(uriString, UriKind.Absolute, out var uri)) + { + // If it's not a valid URI, return the string with trailing slash removed + return uriString.TrimEnd('/'); + } + + return NormalizeUri(uri); + } + /// /// Responds to a 401 challenge by parsing the WWW-Authenticate header, fetching the resource metadata, /// verifying the resource match, and returning the metadata if valid. diff --git a/src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs b/src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs index 6bdb5de4e..2e3027da7 100644 --- a/src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs +++ b/src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; namespace ModelContextProtocol.Authentication; @@ -20,7 +21,8 @@ public sealed class ProtectedResourceMetadata /// Resource must be explicitly set. Automatic inference only works with the default endpoint pattern. /// [JsonPropertyName("resource")] - public Uri? Resource { get; set; } + [StringSyntax(StringSyntaxAttribute.Uri)] + public string? Resource { get; set; } /// /// Gets or sets the list of authorization server URIs. @@ -33,7 +35,7 @@ public sealed class ProtectedResourceMetadata /// OPTIONAL. /// [JsonPropertyName("authorization_servers")] - public List AuthorizationServers { get; set; } = []; + public List AuthorizationServers { get; set; } = []; /// /// Gets or sets the supported bearer token methods. @@ -69,7 +71,8 @@ public sealed class ProtectedResourceMetadata /// that the resource server uses to sign resource responses. This URL MUST use the HTTPS scheme. /// [JsonPropertyName("jwks_uri")] - public Uri? JwksUri { get; set; } + [StringSyntax(StringSyntaxAttribute.Uri)] + public string? JwksUri { get; set; } /// /// Gets or sets the list of the JWS signing algorithms supported by the protected resource for signing resource responses. @@ -105,7 +108,8 @@ public sealed class ProtectedResourceMetadata /// OPTIONAL. /// [JsonPropertyName("resource_documentation")] - public Uri? ResourceDocumentation { get; set; } + [StringSyntax(StringSyntaxAttribute.Uri)] + public string? ResourceDocumentation { get; set; } /// /// Gets or sets the URL of a page containing human-readable information about the protected resource's requirements. @@ -117,7 +121,8 @@ public sealed class ProtectedResourceMetadata /// OPTIONAL. /// [JsonPropertyName("resource_policy_uri")] - public Uri? ResourcePolicyUri { get; set; } + [StringSyntax(StringSyntaxAttribute.Uri)] + public string? ResourcePolicyUri { get; set; } /// /// Gets or sets the URL of a page containing human-readable information about the protected resource's terms of service. @@ -126,7 +131,8 @@ public sealed class ProtectedResourceMetadata /// OPTIONAL. The value of this field MAY be internationalized. /// [JsonPropertyName("resource_tos_uri")] - public Uri? ResourceTosUri { get; set; } + [StringSyntax(StringSyntaxAttribute.Uri)] + public string? ResourceTosUri { get; set; } /// /// Gets or sets a value indicating whether there is protected resource support for mutual-TLS client certificate-bound access tokens. @@ -195,13 +201,13 @@ public sealed class ProtectedResourceMetadata /// /// Creates a deep copy of this instance, optionally overriding the Resource property. /// - /// Optional URI to use for the Resource property if the original Resource is null. + /// Optional resource URI string to use for the Resource property if the original Resource is null. /// A new instance of with cloned values. - public ProtectedResourceMetadata Clone(Uri? derivedResourceUri = null) + public ProtectedResourceMetadata Clone(string? derivedResource = null) { return new ProtectedResourceMetadata { - Resource = Resource ?? derivedResourceUri, + Resource = Resource ?? derivedResource, AuthorizationServers = [.. AuthorizationServers], BearerMethodsSupported = [.. BearerMethodsSupported], ScopesSupported = [.. ScopesSupported], diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthEventTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthEventTests.cs index 001aa82f0..7aafd312e 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthEventTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthEventTests.cs @@ -25,8 +25,8 @@ public AuthEventTests(ITestOutputHelper outputHelper) // Dynamically provide the resource metadata context.ResourceMetadata = new ProtectedResourceMetadata { - Resource = new Uri(McpServerUrl), - AuthorizationServers = { new Uri(OAuthServerUrl) }, + Resource = McpServerUrl, + AuthorizationServers = { OAuthServerUrl }, ScopesSupported = ["mcp:tools"], }; await Task.CompletedTask; @@ -124,8 +124,8 @@ public async Task ResourceMetadataEndpoint_ReturnsCorrectMetadata_FromEvent() ); Assert.NotNull(metadata); - Assert.Equal(new Uri(McpServerUrl), metadata.Resource); - Assert.Contains(new Uri(OAuthServerUrl), metadata.AuthorizationServers); + Assert.Equal(McpServerUrl, metadata.Resource); + Assert.Contains(OAuthServerUrl, metadata.AuthorizationServers); Assert.Contains("mcp:tools", metadata.ScopesSupported); } @@ -140,8 +140,8 @@ public async Task ResourceMetadataEndpoint_CanModifyExistingMetadata_InEvent() // Set initial metadata options.ResourceMetadata = new ProtectedResourceMetadata { - Resource = new Uri(McpServerUrl), - AuthorizationServers = { new Uri(OAuthServerUrl) }, + Resource = McpServerUrl, + AuthorizationServers = { OAuthServerUrl }, ScopesSupported = ["mcp:basic"], }; @@ -175,8 +175,8 @@ public async Task ResourceMetadataEndpoint_CanModifyExistingMetadata_InEvent() ); Assert.NotNull(metadata); - Assert.Equal(new Uri(McpServerUrl), metadata.Resource); - Assert.Contains(new Uri(OAuthServerUrl), metadata.AuthorizationServers); + Assert.Equal(McpServerUrl, metadata.Resource); + Assert.Contains(OAuthServerUrl, metadata.AuthorizationServers); Assert.Contains("mcp:basic", metadata.ScopesSupported); Assert.Contains("mcp:tools", metadata.ScopesSupported); Assert.Equal("Dynamic Test Resource", metadata.ResourceName); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index be831d523..d40078304 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -9,6 +9,7 @@ using ModelContextProtocol.Client; using ModelContextProtocol.Server; using System.Net; +using System.Net.Http.Json; using System.Security.Claims; using Xunit.Sdk; @@ -506,7 +507,7 @@ public async Task AuthorizationFails_WhenResourceMetadataPortDiffers() { Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => { - options.ResourceMetadata!.Resource = new Uri("http://localhost:5999"); + options.ResourceMetadata!.Resource = "http://localhost:5999"; }); await using var app = await StartMcpServerAsync(); @@ -532,7 +533,7 @@ public async Task CanAuthenticate_WithAuthorizationServerPathInsertionMetadata() { Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => { - options.ResourceMetadata!.AuthorizationServers = [new Uri($"{OAuthServerUrl}/tenant1")]; + options.ResourceMetadata!.AuthorizationServers = [$"{OAuthServerUrl}/tenant1"]; }); await using var app = await StartMcpServerAsync(); @@ -565,7 +566,7 @@ public async Task CanAuthenticate_WithAuthorizationServerPathFallbacks() Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => { - options.ResourceMetadata!.AuthorizationServers = [new Uri($"{OAuthServerUrl}{issuerPath}")]; + options.ResourceMetadata!.AuthorizationServers = [$"{OAuthServerUrl}{issuerPath}"]; }); await using var app = await StartMcpServerAsync(); @@ -606,8 +607,8 @@ public async Task CanAuthenticate_WithResourceMetadataPathFallbacks() var metadata = new ProtectedResourceMetadata { - Resource = new Uri($"{McpServerUrl}{resourcePath}"), - AuthorizationServers = { new Uri(OAuthServerUrl) }, + Resource = $"{McpServerUrl}{resourcePath}", + AuthorizationServers = { OAuthServerUrl }, }; app.Use(async (context, next) => @@ -678,8 +679,8 @@ public async Task CannotAuthenticate_WhenResourceMetadataResourceIsNonRootParent { options.ResourceMetadata = new ProtectedResourceMetadata { - Resource = new Uri($"{McpServerUrl}{configuredResourcePath}"), - AuthorizationServers = { new Uri(OAuthServerUrl) }, + Resource = $"{McpServerUrl}{configuredResourcePath}", + AuthorizationServers = { OAuthServerUrl }, }; }); @@ -719,8 +720,8 @@ public async Task CannotAuthenticate_WhenWwwAuthenticateResourceMetadataIsRootPa { options.ResourceMetadata = new ProtectedResourceMetadata { - Resource = new Uri($"{McpServerUrl}"), - AuthorizationServers = { new Uri(OAuthServerUrl) }, + Resource = McpServerUrl, + AuthorizationServers = { OAuthServerUrl }, }; }); @@ -750,4 +751,106 @@ await McpClient.CreateAsync( Assert.Contains("does not match", ex.Message); } + + [Fact] + public async Task ResourceMetadata_DoesNotAddTrailingSlash() + { + // This test verifies that automatically derived resource URIs don't have trailing slashes + // and that the client doesn't add them during authentication + + // Don't explicitly set Resource - let it be derived from the request + await using var app = await StartMcpServerAsync(); + + // First, manually check the PRM document doesn't contain a trailing slash + using var metadataResponse = await HttpClient.GetAsync( + "/.well-known/oauth-protected-resource", + TestContext.Current.CancellationToken + ); + + Assert.Equal(HttpStatusCode.OK, metadataResponse.StatusCode); + + var metadata = await metadataResponse.Content.ReadFromJsonAsync( + McpJsonUtilities.DefaultOptions, + TestContext.Current.CancellationToken + ); + + Assert.NotNull(metadata); + Assert.Equal("http://localhost:5000", metadata.Resource); + Assert.DoesNotMatch(@"/$", metadata.Resource); // No trailing slash + + // Then authenticate with the client - this will use the derived resource URI + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + // This should succeed - the client should not add a trailing slash + // If the client incorrectly added a trailing slash, ValidResources would reject it + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + } + + [Fact] + public async Task ResourceMetadata_PreservesExplicitTrailingSlash() + { + // This test verifies that explicitly configured trailing slashes are preserved + const string resourceWithTrailingSlash = "http://localhost:5000/"; + + // Configure ValidResources to accept the trailing slash version for this test + TestOAuthServer.ValidResources = [resourceWithTrailingSlash, "http://localhost:5000/mcp"]; + + Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => + { + options.ResourceMetadata = new ProtectedResourceMetadata + { + Resource = resourceWithTrailingSlash, + AuthorizationServers = { OAuthServerUrl }, + ScopesSupported = ["mcp:tools"], + }; + }); + + await using var app = await StartMcpServerAsync(); + + // First, manually check the PRM document contains the trailing slash + using var metadataResponse = await HttpClient.GetAsync( + "/.well-known/oauth-protected-resource", + TestContext.Current.CancellationToken + ); + + Assert.Equal(HttpStatusCode.OK, metadataResponse.StatusCode); + + var metadata = await metadataResponse.Content.ReadFromJsonAsync( + McpJsonUtilities.DefaultOptions, + TestContext.Current.CancellationToken + ); + + Assert.NotNull(metadata); + Assert.Equal(resourceWithTrailingSlash, metadata.Resource); + Assert.Matches(@"/$", metadata.Resource); // Has trailing slash + + // Then authenticate with the client + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + // This should succeed with the explicitly configured trailing slash + // If the client incorrectly trimmed the slash, ValidResources would reject it + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + } } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/McpAuthenticationHandlerTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/McpAuthenticationHandlerTests.cs index 01cb84507..c4659e56d 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/McpAuthenticationHandlerTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/McpAuthenticationHandlerTests.cs @@ -23,7 +23,7 @@ public async Task Challenge_WithRelativeResourceMetadataUri_SetsAbsoluteUrl() await using var app = await StartAuthenticationServerAsync(options => { options.ResourceMetadataUri = new Uri(metadataPath, UriKind.Relative); - options.ResourceMetadata!.Resource = new Uri("http://localhost:5000/challenge"); + options.ResourceMetadata!.Resource = "http://localhost:5000/challenge"; }); using var challengeResponse = await HttpClient.GetAsync(new Uri("/challenge", UriKind.Relative), HttpCompletionOption.ResponseHeadersRead, TestContext.Current.CancellationToken); @@ -64,7 +64,7 @@ public async Task Challenge_WithAbsoluteResourceMetadataUri_SetsConfiguredUrl() await using var app = await StartAuthenticationServerAsync(options => { options.ResourceMetadataUri = metadataUri; - options.ResourceMetadata!.Resource = new Uri("http://localhost:5000/challenge"); + options.ResourceMetadata!.Resource = "http://localhost:5000/challenge"; }); using var challengeResponse = await HttpClient.GetAsync(new Uri("/challenge", UriKind.Relative), HttpCompletionOption.ResponseHeadersRead, TestContext.Current.CancellationToken); @@ -137,7 +137,7 @@ public async Task MetadataRequest_DefaultEndpoint_SetsResourceFromSuffix() McpJsonUtilities.DefaultOptions, TestContext.Current.CancellationToken); Assert.NotNull(metadata); - Assert.Equal(new Uri("http://localhost:5000/resource/tools"), metadata!.Resource); + Assert.Equal("http://localhost:5000/resource/tools", metadata!.Resource); } [Fact] @@ -153,7 +153,7 @@ public async Task MetadataRequest_DefaultEndpoint_WithPathBase_SetsResourceFromS McpJsonUtilities.DefaultOptions, TestContext.Current.CancellationToken); Assert.NotNull(metadata); - Assert.Equal(new Uri("http://localhost:5000/api/resource/tools"), metadata!.Resource); + Assert.Equal("http://localhost:5000/api/resource/tools", metadata!.Resource); } private async Task StartAuthenticationServerAsync(Action? configureOptions = null, PathString? pathBase = null) @@ -169,7 +169,7 @@ private async Task StartAuthenticationServerAsync(Action _authCodes = new();