Fix: Relax resource URI validation to accept base URL (#1517)
Co-authored-by: Jérémy Ballan <jballan@chapsvision.com> Co-authored-by: Tarek Mahmoud Sayed <10833894+tarekgh@users.noreply.github.com> Co-authored-by: Tarek Mahmoud Sayed <tarekms@ntdev.microsoft.com>
This commit is contained in:
parent
a87518cf44
commit
168c37179d
2 changed files with 122 additions and 16 deletions
|
|
@ -774,15 +774,19 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Verifies that the resource URI in the metadata exactly matches the original request URL as required by the RFC.
|
/// Verifies that the resource URI in the metadata matches the original request URL.
|
||||||
/// Per RFC: The resource value must be identical to the URL that the client used to make the request to the resource server.
|
/// Accepts either an exact match with the full request URL, or a match with the base URL
|
||||||
|
/// (authority only, path discarded) as allowed by the MCP spec, which derives the authorization
|
||||||
|
/// base URL by discarding the path component from the MCP server URL.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
/// <param name="protectedResourceMetadata">The metadata to verify.</param>
|
/// <param name="protectedResourceMetadata">The metadata to verify.</param>
|
||||||
/// <param name="resourceLocation">
|
/// <param name="resourceLocation">
|
||||||
/// The original URL the client used to make the request to the resource server or the root Uri for the resource server
|
/// The original URL the client used to make the request to the resource server or the root Uri for the resource server
|
||||||
/// if the metadata was automatically requested from the root well-known location.
|
/// if the metadata was automatically requested from the root well-known location.
|
||||||
/// </param>
|
/// </param>
|
||||||
/// <returns>True if the resource URI exactly matches the original request URL, otherwise false.</returns>
|
/// <returns>
|
||||||
|
/// True if the resource URI exactly matches the original request URL or its authority-level base URL, otherwise false.
|
||||||
|
/// </returns>
|
||||||
private static bool VerifyResourceMatch(ProtectedResourceMetadata protectedResourceMetadata, Uri resourceLocation)
|
private static bool VerifyResourceMatch(ProtectedResourceMetadata protectedResourceMetadata, Uri resourceLocation)
|
||||||
{
|
{
|
||||||
if (protectedResourceMetadata.Resource is null)
|
if (protectedResourceMetadata.Resource is null)
|
||||||
|
|
@ -790,14 +794,22 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Per RFC: The resource value must be identical to the URL that the client used
|
|
||||||
// to make the request to the resource server. Compare entire URIs, not just the host.
|
|
||||||
|
|
||||||
// Normalize the URIs to ensure consistent comparison
|
// Normalize the URIs to ensure consistent comparison
|
||||||
string normalizedMetadataResource = NormalizeUri(protectedResourceMetadata.Resource);
|
string normalizedMetadataResource = NormalizeUri(protectedResourceMetadata.Resource);
|
||||||
string normalizedResourceLocation = NormalizeUri(resourceLocation);
|
string normalizedResourceLocation = NormalizeUri(resourceLocation);
|
||||||
|
|
||||||
return string.Equals(normalizedMetadataResource, normalizedResourceLocation, StringComparison.OrdinalIgnoreCase);
|
// Accept exact match with the full MCP endpoint URI
|
||||||
|
if (string.Equals(normalizedMetadataResource, normalizedResourceLocation, StringComparison.OrdinalIgnoreCase))
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Per the MCP spec's "Canonical Server URI" section, both the path-specific URI (e.g. https://mcp.example.com/mcp)
|
||||||
|
// and the authority-only URI (e.g. https://mcp.example.com) are valid canonical URIs for identifying an MCP server.
|
||||||
|
// Accept a match with the base URL (authority only, path discarded) to support servers that use the less specific form.
|
||||||
|
|
||||||
|
string normalizedBaseUrl = NormalizeUri(new Uri(resourceLocation.GetLeftPart(UriPartial.Authority)));
|
||||||
|
return string.Equals(normalizedMetadataResource, normalizedBaseUrl, StringComparison.OrdinalIgnoreCase);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|
@ -916,7 +928,8 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
|
||||||
// https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#protected-resource-metadata-discovery-requirements
|
// https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#protected-resource-metadata-discovery-requirements
|
||||||
metadata.WwwAuthenticateScope = wwwAuthenticateScope;
|
metadata.WwwAuthenticateScope = wwwAuthenticateScope;
|
||||||
|
|
||||||
// Per RFC: The resource value must be identical to the URL that the client used to make the request to the resource server
|
// Validate that the resource URI in metadata corresponds to the server we're connecting to.
|
||||||
|
// VerifyResourceMatch accepts both an exact URI match and an authority-level (base URL) match per the MCP spec.
|
||||||
LogValidatingResourceMetadata(resourceUri);
|
LogValidatingResourceMetadata(resourceUri);
|
||||||
|
|
||||||
if (!isLegacyFallback && !VerifyResourceMatch(metadata, resourceUri))
|
if (!isLegacyFallback && !VerifyResourceMatch(metadata, resourceUri))
|
||||||
|
|
|
||||||
|
|
@ -768,7 +768,7 @@ public class AuthTests : OAuthTestBase
|
||||||
//
|
//
|
||||||
// https://datatracker.ietf.org/doc/html/rfc9728/#section-3.3
|
// https://datatracker.ietf.org/doc/html/rfc9728/#section-3.3
|
||||||
//
|
//
|
||||||
// CannotAuthenticate_WhenWwwAuthenticateResourceMetadataIsRootPath validates we won't fall back to root in this case.
|
// CanAuthenticate_WhenWwwAuthenticateResourceMetadataIsRootPath validates that a root-level resource is accepted in this case.
|
||||||
// CanAuthenticate_WithResourceMetadataPathFallbacks validates we will fall back to root when resource_metadata is missing.
|
// CanAuthenticate_WithResourceMetadataPathFallbacks validates we will fall back to root when resource_metadata is missing.
|
||||||
Builder.Services.Configure<AuthenticationOptions>(options => options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme);
|
Builder.Services.Configure<AuthenticationOptions>(options => options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme);
|
||||||
Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options =>
|
Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options =>
|
||||||
|
|
@ -807,8 +807,14 @@ public class AuthTests : OAuthTestBase
|
||||||
Assert.Contains("does not match", ex.Message);
|
Assert.Contains("does not match", ex.Message);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies that OAuth authentication succeeds when the protected resource metadata URI
|
||||||
|
/// matches the root server URL, even when the actual MCP endpoint is at a subpath.
|
||||||
|
/// This tests the flexible URI matching behavior where the resource URI can be less specific
|
||||||
|
/// than the actual endpoint being accessed.
|
||||||
|
/// </summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task CannotAuthenticate_WhenWwwAuthenticateResourceMetadataIsRootPath()
|
public async Task CanAuthenticate_WhenWwwAuthenticateResourceMetadataIsRootPath()
|
||||||
{
|
{
|
||||||
const string requestedResourcePath = "/mcp/tools";
|
const string requestedResourcePath = "/mcp/tools";
|
||||||
|
|
||||||
|
|
@ -839,12 +845,99 @@ public class AuthTests : OAuthTestBase
|
||||||
},
|
},
|
||||||
}, HttpClient, LoggerFactory);
|
}, HttpClient, LoggerFactory);
|
||||||
|
|
||||||
var ex = await Assert.ThrowsAsync<McpException>(async () =>
|
await using var client = await McpClient.CreateAsync(
|
||||||
{
|
|
||||||
await McpClient.CreateAsync(
|
|
||||||
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
|
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies that OAuth authentication fails when the protected resource metadata URI
|
||||||
|
/// does not match the requested MCP server endpoint. This ensures that clients cannot
|
||||||
|
/// use OAuth tokens intended for one server to access a different server.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public async Task CannotAuthenticate_WhenResourceMetadataUriDoesNotMatch()
|
||||||
|
{
|
||||||
|
const string requestedResourcePath = "/mcp/tools";
|
||||||
|
const string differentResourceUri = "http://different-server.example.com";
|
||||||
|
|
||||||
|
Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options =>
|
||||||
|
{
|
||||||
|
options.ResourceMetadata = new ProtectedResourceMetadata
|
||||||
|
{
|
||||||
|
Resource = differentResourceUri,
|
||||||
|
AuthorizationServers = { OAuthServerUrl },
|
||||||
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
|
await using var app = Builder.Build();
|
||||||
|
|
||||||
|
app.MapMcp(requestedResourcePath).RequireAuthorization();
|
||||||
|
|
||||||
|
await app.StartAsync(TestContext.Current.CancellationToken);
|
||||||
|
|
||||||
|
await using var transport = new HttpClientTransport(new()
|
||||||
|
{
|
||||||
|
Endpoint = new Uri($"{McpServerUrl}{requestedResourcePath}"),
|
||||||
|
OAuth = new()
|
||||||
|
{
|
||||||
|
ClientId = "demo-client",
|
||||||
|
ClientSecret = "demo-secret",
|
||||||
|
RedirectUri = new Uri("http://localhost:1179/callback"),
|
||||||
|
AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync,
|
||||||
|
},
|
||||||
|
}, HttpClient, LoggerFactory);
|
||||||
|
|
||||||
|
// This should fail because the resource URI doesn't match
|
||||||
|
var ex = await Assert.ThrowsAsync<McpException>(() => McpClient.CreateAsync(
|
||||||
|
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));
|
||||||
|
|
||||||
|
Assert.Contains("does not match", ex.Message);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies that OAuth authentication fails when the protected resource metadata URI is an
|
||||||
|
/// unrelated path on the same host as the requested endpoint (e.g. resource=.../service-a vs
|
||||||
|
/// endpoint .../service-b). This ensures the authority-level fallback only accepts an exact match
|
||||||
|
/// or an authority-only resource, and not arbitrary sibling paths on the same host.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public async Task CannotAuthenticate_WhenResourceMetadataResourceIsDifferentPathOnSameAuthority()
|
||||||
|
{
|
||||||
|
const string requestedResourcePath = "/service-b";
|
||||||
|
const string differentResourcePath = "/service-a";
|
||||||
|
|
||||||
|
Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options =>
|
||||||
|
{
|
||||||
|
options.ResourceMetadata = new ProtectedResourceMetadata
|
||||||
|
{
|
||||||
|
Resource = $"{McpServerUrl}{differentResourcePath}",
|
||||||
|
AuthorizationServers = { OAuthServerUrl },
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
await using var app = Builder.Build();
|
||||||
|
|
||||||
|
app.MapMcp(requestedResourcePath).RequireAuthorization();
|
||||||
|
|
||||||
|
await app.StartAsync(TestContext.Current.CancellationToken);
|
||||||
|
|
||||||
|
await using var transport = new HttpClientTransport(new()
|
||||||
|
{
|
||||||
|
Endpoint = new Uri($"{McpServerUrl}{requestedResourcePath}"),
|
||||||
|
OAuth = new()
|
||||||
|
{
|
||||||
|
ClientId = "demo-client",
|
||||||
|
ClientSecret = "demo-secret",
|
||||||
|
RedirectUri = new Uri("http://localhost:1179/callback"),
|
||||||
|
AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync,
|
||||||
|
},
|
||||||
|
}, HttpClient, LoggerFactory);
|
||||||
|
|
||||||
|
// This should fail because the resource URI is a different path on the same host,
|
||||||
|
// which is neither an exact match nor the authority-only base URL.
|
||||||
|
var ex = await Assert.ThrowsAsync<McpException>(() => McpClient.CreateAsync(
|
||||||
|
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));
|
||||||
|
|
||||||
Assert.Contains("does not match", ex.Message);
|
Assert.Contains("does not match", ex.Message);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue