-
Notifications
You must be signed in to change notification settings - Fork 627
Add protected resource metadata response inspect & modify support #1262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -853,4 +853,79 @@ public async Task ResourceMetadata_PreservesExplicitTrailingSlash() | |||||||||||||||||||||||||
| await using var client = await McpClient.CreateAsync( | ||||||||||||||||||||||||||
| transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| [Fact] | ||||||||||||||||||||||||||
| public async Task ResourceMetadata_ProtectedResourceMetadataDelegate_Raised() | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| await using var app = await StartMcpServerAsync(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ProtectedResourceMetadata? capturedMetadata = null; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| 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, | ||||||||||||||||||||||||||
| ProtectedResourceMetadataResponseDelegate = metadata => | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| Assert.NotNull(metadata); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return capturedMetadata = metadata; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| }, HttpClient, LoggerFactory); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| await using var client = await McpClient.CreateAsync( | ||||||||||||||||||||||||||
| transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Assert.NotNull(capturedMetadata); | ||||||||||||||||||||||||||
| Assert.Contains(capturedMetadata.Resource, TestOAuthServer.ValidResources); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| [Fact] | ||||||||||||||||||||||||||
| public async Task ResourceMetadata_ProtectedResourceMetadataDelegate_CanModify() | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| // Create a scenario where the protected resource is broadcasting an incorrect resource indicator. | ||||||||||||||||||||||||||
| const string resourceIncorrect = "http://localhost:5001/not-the-right-resource"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Builder.Services.Configure<McpAuthenticationOptions>(McpAuthenticationDefaults.AuthenticationScheme, options => | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| options.ResourceMetadata = new ProtectedResourceMetadata | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| Resource = resourceIncorrect, | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be nice to have an additional test to verify this works when the server returns a 404 like Atlassian rather than an invalid PRM. I think this should work. We could add a bool property to the TestOAuthServer, that if set, will cause it to add middleware before
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or, rather than adding new middleware, it'd probably be easier to set a 404 in a custom csharp-sdk/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs Lines 539 to 550 in b8ae4fe
|
||||||||||||||||||||||||||
| AuthorizationServers = [OAuthServerUrl], | ||||||||||||||||||||||||||
| ScopesSupported = ["mcp:tools"], | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| await using var app = await StartMcpServerAsync(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| 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, | ||||||||||||||||||||||||||
| ProtectedResourceMetadataResponseDelegate = metadata => | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| Assert.NotNull(metadata); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Modify PRM and provide the right resource indicator. | ||||||||||||||||||||||||||
| metadata.Resource = McpServerUrl; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return metadata; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| }, HttpClient, LoggerFactory); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| await using var client = await McpClient.CreateAsync( | ||||||||||||||||||||||||||
| transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if "Filter" is a better suffix than "Delegate" since it's a little more specific.