Reauthorize tools changed by call-tool filters (#1737)
Copilot-Session: 5feb0906-73fc-4e1a-8cf2-c9a99c588487
This commit is contained in:
parent
be55536a91
commit
c7e4b820fe
5 changed files with 177 additions and 15 deletions
|
|
@ -20,6 +20,7 @@ internal sealed class AuthorizationFilterSetup(
|
||||||
public void Configure(McpServerOptions options)
|
public void Configure(McpServerOptions options)
|
||||||
{
|
{
|
||||||
ConfigureListToolsFilter(options);
|
ConfigureListToolsFilter(options);
|
||||||
|
ConfigureOrdinaryCallToolFilter(options);
|
||||||
|
|
||||||
ConfigureListResourcesFilter(options);
|
ConfigureListResourcesFilter(options);
|
||||||
ConfigureListResourceTemplatesFilter(options);
|
ConfigureListResourceTemplatesFilter(options);
|
||||||
|
|
@ -85,6 +86,22 @@ internal sealed class AuthorizationFilterSetup(
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void ConfigureOrdinaryCallToolFilter(McpServerOptions options)
|
||||||
|
{
|
||||||
|
options.Filters.Request.CallToolFilters.Add(next => async (context, cancellationToken) =>
|
||||||
|
{
|
||||||
|
var authResult = await GetAuthorizationResultAsync(context.User, context.MatchedPrimitive, context.Services, context);
|
||||||
|
if (!authResult.Succeeded)
|
||||||
|
{
|
||||||
|
throw new McpProtocolException("Access forbidden: This tool requires authorization.", McpErrorCode.InvalidRequest);
|
||||||
|
}
|
||||||
|
|
||||||
|
context.Items[AuthorizationFilterInvokedKey] = true;
|
||||||
|
|
||||||
|
return await next(context, cancellationToken);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
private void ConfigureCallToolFilter(McpServerOptions options)
|
private void ConfigureCallToolFilter(McpServerOptions options)
|
||||||
{
|
{
|
||||||
#pragma warning disable MCPEXP002 // Authorization must run in the alternate-result pipeline before task dispatch.
|
#pragma warning disable MCPEXP002 // Authorization must run in the alternate-result pipeline before task dispatch.
|
||||||
|
|
|
||||||
|
|
@ -58,6 +58,9 @@ public static class HttpMcpServerBuilderExtensions
|
||||||
/// authorization attributes such as <see cref="AuthorizeAttribute"/>
|
/// authorization attributes such as <see cref="AuthorizeAttribute"/>
|
||||||
/// and <see cref="AllowAnonymousAttribute"/>. Tool authorization runs in the alternate-result pipeline before
|
/// and <see cref="AllowAnonymousAttribute"/>. Tool authorization runs in the alternate-result pipeline before
|
||||||
/// the Tasks extension dispatches background execution, so an unauthorized tool call does not create a task.
|
/// the Tasks extension dispatches background execution, so an unauthorized tool call does not create a task.
|
||||||
|
/// Each call to this method also adds an ordinary call-tool authorization checkpoint at that point in the filter
|
||||||
|
/// pipeline. Call this method again after any call-tool filter that changes the matched tool or user to authorize
|
||||||
|
/// the replacement using the updated context.
|
||||||
/// </remarks>
|
/// </remarks>
|
||||||
public static IMcpServerBuilder AddAuthorizationFilters(this IMcpServerBuilder builder)
|
public static IMcpServerBuilder AddAuthorizationFilters(this IMcpServerBuilder builder)
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -23,7 +23,6 @@ public static class McpTasksBuilderExtensions
|
||||||
/// Tasks are implemented as an alternate-result call-tool filter. Alternate-result filters registered before
|
/// Tasks are implemented as an alternate-result call-tool filter. Alternate-result filters registered before
|
||||||
/// the Tasks filter run before task creation. Filters registered after it, along with all ordinary call-tool
|
/// the Tasks filter run before task creation. Filters registered after it, along with all ordinary call-tool
|
||||||
/// filters, run in the background before the tool.
|
/// filters, run in the background before the tool.
|
||||||
/// Register Tasks before configuring ordinary call-tool filters.
|
|
||||||
/// </remarks>
|
/// </remarks>
|
||||||
/// <param name="builder">The server builder.</param>
|
/// <param name="builder">The server builder.</param>
|
||||||
/// <param name="store">The task store.</param>
|
/// <param name="store">The task store.</param>
|
||||||
|
|
@ -120,13 +119,6 @@ public static class McpTasksBuilderExtensions
|
||||||
Handler = HandleCancelTask,
|
Handler = HandleCancelTask,
|
||||||
});
|
});
|
||||||
|
|
||||||
if (options.Filters.Request.CallToolFilters.Count > 0)
|
|
||||||
{
|
|
||||||
throw new InvalidOperationException(
|
|
||||||
$"{nameof(WithTasks)} must be configured before ordinary call-tool filters because " +
|
|
||||||
"the Tasks filter must execute outside the ordinary call-tool pipeline.");
|
|
||||||
}
|
|
||||||
|
|
||||||
// Use a filter rather than a handler so it wraps around Core's tool dispatch.
|
// Use a filter rather than a handler so it wraps around Core's tool dispatch.
|
||||||
// This ensures it intercepts tool calls BEFORE the tool is invoked, allowing
|
// This ensures it intercepts tool calls BEFORE the tool is invoked, allowing
|
||||||
// it to spawn background execution and return the task alternate immediately.
|
// it to spawn background execution and return the task alternate immediately.
|
||||||
|
|
|
||||||
|
|
@ -93,6 +93,91 @@ public class AuthorizeAttributeTests(ITestOutputHelper testOutputHelper) : Kestr
|
||||||
Assert.Equal("Authorized: test", content.Text);
|
Assert.Equal("Authorized: test", content.Text);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Authorize_Tool_ReauthorizesPrimitiveChangedByInterveningFilter()
|
||||||
|
{
|
||||||
|
await using var app = await StartServerWithAuth(builder =>
|
||||||
|
{
|
||||||
|
builder.WithTools<AuthorizationTestTools>();
|
||||||
|
builder.Services.Configure<McpServerOptions>(options =>
|
||||||
|
{
|
||||||
|
if (options.ToolCollection is null ||
|
||||||
|
!options.ToolCollection.TryGetPrimitive("authorized_tool", out var authorizedTool))
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException("The replacement tool was not registered.");
|
||||||
|
}
|
||||||
|
|
||||||
|
options.Filters.Request.CallToolFilters.Add(next => async (context, cancellationToken) =>
|
||||||
|
{
|
||||||
|
context.MatchedPrimitive = authorizedTool;
|
||||||
|
return await next(context, cancellationToken);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
builder.AddAuthorizationFilters();
|
||||||
|
});
|
||||||
|
|
||||||
|
var client = await ConnectAsync();
|
||||||
|
|
||||||
|
var exception = await Assert.ThrowsAsync<McpProtocolException>(async () =>
|
||||||
|
await client.CallToolAsync(
|
||||||
|
"anonymous_tool",
|
||||||
|
new Dictionary<string, object?> { ["message"] = "test" },
|
||||||
|
cancellationToken: TestContext.Current.CancellationToken));
|
||||||
|
|
||||||
|
Assert.Equal("Request failed (remote): Access forbidden: This tool requires authorization.", exception.Message);
|
||||||
|
Assert.Equal(McpErrorCode.InvalidRequest, exception.ErrorCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Authorize_Tool_ReauthorizesAfterEachInterveningFilter()
|
||||||
|
{
|
||||||
|
await using var app = await StartServerWithAuth(builder =>
|
||||||
|
{
|
||||||
|
builder.WithTools<AuthorizationTestTools>();
|
||||||
|
builder.Services.Configure<McpServerOptions>(options =>
|
||||||
|
{
|
||||||
|
if (options.ToolCollection is null ||
|
||||||
|
!options.ToolCollection.TryGetPrimitive("authorized_tool", out var authorizedTool))
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException("The first replacement tool was not registered.");
|
||||||
|
}
|
||||||
|
|
||||||
|
options.Filters.Request.CallToolFilters.Add(next => async (context, cancellationToken) =>
|
||||||
|
{
|
||||||
|
context.MatchedPrimitive = authorizedTool;
|
||||||
|
return await next(context, cancellationToken);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
builder.AddAuthorizationFilters();
|
||||||
|
builder.Services.Configure<McpServerOptions>(options =>
|
||||||
|
{
|
||||||
|
if (options.ToolCollection is null ||
|
||||||
|
!options.ToolCollection.TryGetPrimitive("admin_tool", out var adminTool))
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException("The second replacement tool was not registered.");
|
||||||
|
}
|
||||||
|
|
||||||
|
options.Filters.Request.CallToolFilters.Add(next => async (context, cancellationToken) =>
|
||||||
|
{
|
||||||
|
context.MatchedPrimitive = adminTool;
|
||||||
|
return await next(context, cancellationToken);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
builder.AddAuthorizationFilters();
|
||||||
|
}, "TestUser");
|
||||||
|
|
||||||
|
var client = await ConnectAsync();
|
||||||
|
|
||||||
|
var exception = await Assert.ThrowsAsync<McpProtocolException>(async () =>
|
||||||
|
await client.CallToolAsync(
|
||||||
|
"anonymous_tool",
|
||||||
|
new Dictionary<string, object?> { ["message"] = "test" },
|
||||||
|
cancellationToken: TestContext.Current.CancellationToken));
|
||||||
|
|
||||||
|
Assert.Equal("Request failed (remote): Access forbidden: This tool requires authorization.", exception.Message);
|
||||||
|
Assert.Equal(McpErrorCode.InvalidRequest, exception.ErrorCode);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task AuthorizeWithRoles_Tool_RequiresAdminRole()
|
public async Task AuthorizeWithRoles_Tool_RequiresAdminRole()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,6 @@
|
||||||
using Microsoft.AspNetCore.Builder;
|
using Microsoft.AspNetCore.Builder;
|
||||||
using Microsoft.AspNetCore.Authorization;
|
using Microsoft.AspNetCore.Authorization;
|
||||||
using Microsoft.Extensions.DependencyInjection;
|
using Microsoft.Extensions.DependencyInjection;
|
||||||
using Microsoft.Extensions.Options;
|
|
||||||
using ModelContextProtocol.AspNetCore.Tests.Utils;
|
using ModelContextProtocol.AspNetCore.Tests.Utils;
|
||||||
using ModelContextProtocol.Client;
|
using ModelContextProtocol.Client;
|
||||||
using ModelContextProtocol.Extensions.Tasks;
|
using ModelContextProtocol.Extensions.Tasks;
|
||||||
|
|
@ -44,23 +43,41 @@ public class HttpTaskIntegrationTests(ITestOutputHelper testOutputHelper) : Kest
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task WithTasks_AfterOrdinaryFilter_ThrowsActionableError()
|
public async Task WithTasks_AfterOrdinaryFilter_RunsFilter()
|
||||||
{
|
{
|
||||||
|
var filterInvocationCount = 0;
|
||||||
Builder.Services
|
Builder.Services
|
||||||
.AddMcpServer(options =>
|
.AddMcpServer(options =>
|
||||||
{
|
{
|
||||||
options.Filters.Request.CallToolFilters.Add(next => next);
|
options.Filters.Request.CallToolFilters.Add(next => async (context, cancellationToken) =>
|
||||||
|
{
|
||||||
|
Interlocked.Increment(ref filterInvocationCount);
|
||||||
|
return await next(context, cancellationToken);
|
||||||
|
});
|
||||||
})
|
})
|
||||||
.WithHttpTransport()
|
.WithHttpTransport()
|
||||||
.WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 })
|
.WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 })
|
||||||
.WithTools<TestTools>();
|
.WithTools<TestTools>();
|
||||||
|
|
||||||
await using var app = Builder.Build();
|
await using var app = Builder.Build();
|
||||||
|
app.MapMcp();
|
||||||
|
await app.StartAsync(TestContext.Current.CancellationToken);
|
||||||
|
|
||||||
var exception = Assert.Throws<InvalidOperationException>(
|
await using var transport = new HttpClientTransport(
|
||||||
() => app.Services.GetRequiredService<IOptions<McpServerOptions>>().Value);
|
new HttpClientTransportOptions { Endpoint = new("http://localhost:5000") },
|
||||||
Assert.Contains(nameof(McpTasksBuilderExtensions.WithTasks), exception.Message);
|
HttpClient,
|
||||||
Assert.Contains("before ordinary call-tool filters", exception.Message);
|
LoggerFactory);
|
||||||
|
await using var client = await McpClient.CreateAsync(
|
||||||
|
transport,
|
||||||
|
loggerFactory: LoggerFactory,
|
||||||
|
cancellationToken: TestContext.Current.CancellationToken);
|
||||||
|
|
||||||
|
var result = await client.CallToolWithPollingAsync(
|
||||||
|
new CallToolRequestParams { Name = "test" },
|
||||||
|
cancellationToken: TestContext.Current.CancellationToken);
|
||||||
|
|
||||||
|
Assert.Equal("Hello World!", Assert.IsType<TextContentBlock>(Assert.Single(result.Content)).Text);
|
||||||
|
Assert.Equal(1, filterInvocationCount);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Theory]
|
[Theory]
|
||||||
|
|
@ -165,6 +182,54 @@ public class HttpTaskIntegrationTests(ITestOutputHelper testOutputHelper) : Kest
|
||||||
Times.Never);
|
Times.Never);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task WithTasks_ReauthorizesToolChangedByOrdinaryFilter()
|
||||||
|
{
|
||||||
|
var serverBuilder = Builder.Services
|
||||||
|
.AddMcpServer()
|
||||||
|
.WithHttpTransport()
|
||||||
|
.WithTasks(new InMemoryMcpTaskStore { DefaultPollIntervalMs = 10 })
|
||||||
|
.WithTools<TestTools>()
|
||||||
|
.AddAuthorizationFilters();
|
||||||
|
|
||||||
|
serverBuilder.Services.Configure<McpServerOptions>(options =>
|
||||||
|
{
|
||||||
|
if (options.ToolCollection is null ||
|
||||||
|
!options.ToolCollection.TryGetPrimitive("authorized-test", out var authorizedTool))
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException("The replacement tool was not registered.");
|
||||||
|
}
|
||||||
|
|
||||||
|
options.Filters.Request.CallToolFilters.Add(next => async (context, cancellationToken) =>
|
||||||
|
{
|
||||||
|
context.MatchedPrimitive = authorizedTool;
|
||||||
|
return await next(context, cancellationToken);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
serverBuilder.AddAuthorizationFilters();
|
||||||
|
Builder.Services.AddAuthorization();
|
||||||
|
|
||||||
|
await using var app = Builder.Build();
|
||||||
|
app.MapMcp();
|
||||||
|
await app.StartAsync(TestContext.Current.CancellationToken);
|
||||||
|
|
||||||
|
await using var transport = new HttpClientTransport(
|
||||||
|
new HttpClientTransportOptions { Endpoint = new("http://localhost:5000") },
|
||||||
|
HttpClient,
|
||||||
|
LoggerFactory);
|
||||||
|
await using var client = await McpClient.CreateAsync(
|
||||||
|
transport,
|
||||||
|
loggerFactory: LoggerFactory,
|
||||||
|
cancellationToken: TestContext.Current.CancellationToken);
|
||||||
|
|
||||||
|
var exception = await Assert.ThrowsAsync<McpException>(() =>
|
||||||
|
client.CallToolWithPollingAsync(
|
||||||
|
new CallToolRequestParams { Name = "test" },
|
||||||
|
cancellationToken: TestContext.Current.CancellationToken).AsTask());
|
||||||
|
|
||||||
|
Assert.Contains("Access forbidden: This tool requires authorization.", exception.Message);
|
||||||
|
}
|
||||||
|
|
||||||
[McpServerToolType]
|
[McpServerToolType]
|
||||||
private sealed class TestTools
|
private sealed class TestTools
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue