Skip to content

Commit d3186fd

Browse files
committed
Address PR feedback
- Update filters.md to use DI and logging - Update filters.md Mention that uncaught McpExceptions get turned into JSON-RPC errors - Added newlines to McpServer between blocks - Remove TODO from AuthorizationFilterSetup now that an issue has been filed
1 parent 263313e commit d3186fd

4 files changed

Lines changed: 30 additions & 18 deletions

File tree

docs/concepts/filters.md

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,15 @@ services.AddMcpServer()
3838
})
3939
.AddListToolsFilter(next => async (context, cancellationToken) =>
4040
{
41+
var logger = context.Services?.GetService<ILogger<Program>>();
42+
4143
// Pre-processing logic
42-
Console.WriteLine("Before handler execution");
44+
logger?.LogInformation("Before handler execution");
4345

4446
var result = await next(context, cancellationToken);
4547

4648
// Post-processing logic
47-
Console.WriteLine("After handler execution");
49+
logger?.LogInformation("After handler execution");
4850
return result;
4951
});
5052
```
@@ -67,9 +69,11 @@ Execution flow: `filter1 -> filter2 -> filter3 -> baseHandler -> filter3 -> filt
6769
```csharp
6870
.AddListToolsFilter(next => async (context, cancellationToken) =>
6971
{
70-
Console.WriteLine($"Processing request from {context.Meta.ProgressToken}");
72+
var logger = context.Services?.GetService<ILogger<Program>>();
73+
74+
logger?.LogInformation($"Processing request from {context.Meta.ProgressToken}");
7175
var result = await next(context, cancellationToken);
72-
Console.WriteLine($"Returning {result.Tools?.Count ?? 0} tools");
76+
logger?.LogInformation($"Returning {result.Tools?.Count ?? 0} tools");
7377
return result;
7478
});
7579
```
@@ -97,10 +101,12 @@ Execution flow: `filter1 -> filter2 -> filter3 -> baseHandler -> filter3 -> filt
97101
```csharp
98102
.AddListToolsFilter(next => async (context, cancellationToken) =>
99103
{
104+
var logger = context.Services?.GetService<ILogger<Program>>();
105+
100106
var stopwatch = Stopwatch.StartNew();
101107
var result = await next(context, cancellationToken);
102108
stopwatch.Stop();
103-
Console.WriteLine($"Handler took {stopwatch.ElapsedMilliseconds}ms");
109+
logger?.LogInformation($"Handler took {stopwatch.ElapsedMilliseconds}ms");
104110
return result;
105111
});
106112
```
@@ -109,9 +115,13 @@ Execution flow: `filter1 -> filter2 -> filter3 -> baseHandler -> filter3 -> filt
109115
```csharp
110116
.AddListResourcesFilter(next => async (context, cancellationToken) =>
111117
{
118+
var cache = context.Services!.GetRequiredService<IMemoryCache>();
119+
112120
var cacheKey = $"resources:{context.Params.Cursor}";
113121
if (cache.TryGetValue(cacheKey, out var cached))
114-
return cached;
122+
{
123+
return (ListResourcesResult)cached;
124+
}
115125

116126
var result = await next(context, cancellationToken);
117127
cache.Set(cacheKey, result, TimeSpan.FromMinutes(5));
@@ -207,11 +217,7 @@ The authorization filters work differently for list operations versus individual
207217
For list operations, the filters automatically remove unauthorized items from the results. Users only see tools, prompts, or resources they have permission to access.
208218

209219
#### Individual Operations (CallTool, GetPrompt, ReadResource)
210-
For individual operations, the filters return authorization errors when access is denied:
211-
212-
- **Tools**: Returns a `CallToolResult` with `IsError = true` and an error message
213-
- **Prompts**: Throws an `McpException` with "Access forbidden" message
214-
- **Resources**: Throws an `McpException` with "Access forbidden" message
220+
For individual operations, the filters throw an `McpException` with "Access forbidden" message. These get turned into JSON-RPC errors if uncaught by middleware.
215221

216222
### Filter Execution Order and Authorization
217223

@@ -232,18 +238,22 @@ services.AddMcpServer()
232238
.WithHttpTransport()
233239
.AddListToolsFilter(next => async (context, cancellationToken) =>
234240
{
241+
var logger = context.Services?.GetService<ILogger<Program>>();
242+
235243
// This filter runs BEFORE authorization - sees all tools
236-
Console.WriteLine("Request for tools list - will see all tools");
244+
logger?.LogInformation("Request for tools list - will see all tools");
237245
var result = await next(context, cancellationToken);
238-
Console.WriteLine($"Returning {result.Tools?.Count ?? 0} tools after authorization");
246+
logger?.LogInformation($"Returning {result.Tools?.Count ?? 0} tools after authorization");
239247
return result;
240248
})
241249
.AddAuthorizationFilters() // Authorization filtering happens here
242250
.AddListToolsFilter(next => async (context, cancellationToken) =>
243251
{
252+
var logger = context.Services?.GetService<ILogger<Program>>();
253+
244254
// This filter runs AFTER authorization - only sees authorized tools
245255
var result = await next(context, cancellationToken);
246-
Console.WriteLine($"Post-auth filter sees {result.Tools?.Count ?? 0} authorized tools");
256+
logger?.LogInformation($"Post-auth filter sees {result.Tools?.Count ?? 0} authorized tools");
247257
return result;
248258
})
249259
.WithTools<WeatherTools>();

src/ModelContextProtocol.AspNetCore/AuthorizationFilterSetup.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@ private async ValueTask<AuthorizationResult> GetAuthorizationResultAsync(
283283
throw new InvalidOperationException($"You must call AddAuthorization() because an authorization related attribute was found on {primitive.Id}");
284284
}
285285

286-
// TODO: Cache policy lookup. We would probably use a singleton (not-static) ConditionalWeakTable<IMcpServerPrimitive, AuthorizationPolicy?>.
287286
var policy = await CombineAsync(policyProvider, primitive.Metadata);
288287
if (policy is null)
289288
{

src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,10 +279,9 @@ internal static string MakeNewSessionId()
279279
// Implementation for reading a JSON-RPC message from the request body
280280
var message = await context.Request.ReadFromJsonAsync(s_messageTypeInfo, context.RequestAborted);
281281

282-
if (context.User?.Identity?.IsAuthenticated ?? false && message is not null)
282+
if (context.User?.Identity?.IsAuthenticated == true && message is not null)
283283
{
284-
// We get weird CS0131 errors only on the Windows build GitHub Action if we use "message?.Context = ..."
285-
message!.Context = new()
284+
message.Context = new()
286285
{
287286
User = context.User,
288287
};

src/ModelContextProtocol.Core/Server/McpServer.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,18 +667,22 @@ private static McpRequestHandler<TParams, TResult> BuildFilterPipeline<TParams,
667667
McpRequestFilter<TParams, TResult>? finalHandler = null)
668668
{
669669
var current = baseHandler;
670+
670671
if (finalHandler is not null)
671672
{
672673
current = finalHandler(current);
673674
}
675+
674676
for (int i = filters.Count - 1; i >= 0; i--)
675677
{
676678
current = filters[i](current);
677679
}
680+
678681
if (initialHandler is not null)
679682
{
680683
current = initialHandler(current);
681684
}
685+
682686
return current;
683687
}
684688

0 commit comments

Comments
 (0)