Skip to content

Commit 4531320

Browse files
halter73jeffhandley
authored andcommitted
Validate user on Streamable HTTP session DELETE (#1604)
1 parent 55646c3 commit 4531320

3 files changed

Lines changed: 88 additions & 2 deletions

File tree

.github/workflows/ci-code-coverage.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
pattern: testresults-*
2525

2626
- name: Combine coverage reports
27-
uses: danielpalme/ReportGenerator-GitHub-Action@5.5.5
27+
uses: danielpalme/ReportGenerator-GitHub-Action@7ae927204961589fcb0b0be245c51fbbc87cbca2 # 5.5.5
2828
with:
2929
reports: "**/*.cobertura.xml"
3030
targetdir: "${{ github.workspace }}/report"

src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,20 @@ public async Task HandleDeleteRequestAsync(HttpContext context)
209209
}
210210

211211
var sessionId = context.Request.Headers[McpSessionIdHeaderName].ToString();
212-
if (sessionManager.TryRemove(sessionId, out var session))
212+
if (string.IsNullOrEmpty(sessionId) || !sessionManager.TryGetValue(sessionId, out var session))
213+
{
214+
return;
215+
}
216+
217+
if (!session.HasSameUserId(context.User))
218+
{
219+
await WriteJsonRpcErrorAsync(context,
220+
"Forbidden: The currently authenticated user does not match the user who initiated the session.",
221+
StatusCodes.Status403Forbidden);
222+
return;
223+
}
224+
225+
if (sessionManager.TryRemove(sessionId, out session))
213226
{
214227
await session.DisposeAsync();
215228
}

tests/ModelContextProtocol.AspNetCore.Tests/MapMcpStreamableHttpTests.cs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using ModelContextProtocol.Tests.Utils;
99
using System.Collections.Concurrent;
1010
using System.Net;
11+
using System.Security.Claims;
1112
using System.Threading;
1213
using System.Threading.Tasks;
1314

@@ -868,4 +869,76 @@ public async Task EndpointFilter_CanReadSessionId_BeforeAndAfterHandler()
868869
});
869870
}
870871
}
872+
873+
[Fact]
874+
public async Task DeleteRequest_FromDifferentUser_IsRejected_AndSessionSurvives()
875+
{
876+
Assert.SkipWhen(Stateless, "Sessions don't exist in stateless mode.");
877+
878+
Builder.Services.AddMcpServer().WithHttpTransport(ConfigureStateless).WithTools<EchoHttpContextUserTools>();
879+
Builder.Services.AddHttpContextAccessor();
880+
881+
await using var app = Builder.Build();
882+
883+
// Pick the user from a test header so different HttpClient requests can act as different users.
884+
app.Use(next => async context =>
885+
{
886+
var name = context.Request.Headers["X-Test-User"].ToString();
887+
if (!string.IsNullOrEmpty(name))
888+
{
889+
context.User = new ClaimsPrincipal(new ClaimsIdentity(
890+
[new Claim("name", name), new Claim(ClaimTypes.NameIdentifier, name)],
891+
"TestAuthType", "name", "role"));
892+
}
893+
await next(context);
894+
});
895+
896+
app.MapMcp();
897+
await app.StartAsync(TestContext.Current.CancellationToken);
898+
899+
const string initializeRequest = """
900+
{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"test-client","version":"1.0.0"}}}
901+
""";
902+
903+
using var initRequest = new HttpRequestMessage(HttpMethod.Post, "http://localhost:5000/")
904+
{
905+
Content = new StringContent(initializeRequest, System.Text.Encoding.UTF8, "application/json"),
906+
};
907+
initRequest.Headers.Add("X-Test-User", "Alice");
908+
initRequest.Headers.Accept.ParseAdd("application/json");
909+
initRequest.Headers.Accept.ParseAdd("text/event-stream");
910+
911+
using var initResponse = await HttpClient.SendAsync(initRequest, TestContext.Current.CancellationToken);
912+
Assert.True(initResponse.IsSuccessStatusCode);
913+
var sessionId = Assert.Single(initResponse.Headers.GetValues("Mcp-Session-Id"));
914+
915+
// A DELETE from a different authenticated user must not be able to tear down Alice's session.
916+
using var bobDelete = new HttpRequestMessage(HttpMethod.Delete, "http://localhost:5000/");
917+
bobDelete.Headers.Add("X-Test-User", "Bob");
918+
bobDelete.Headers.Add("Mcp-Session-Id", sessionId);
919+
using var bobDeleteResponse = await HttpClient.SendAsync(bobDelete, TestContext.Current.CancellationToken);
920+
Assert.Equal(HttpStatusCode.Forbidden, bobDeleteResponse.StatusCode);
921+
922+
// Alice should still be able to use the session.
923+
const string toolCallRequest = """
924+
{"jsonrpc":"2.0","id":2,"method":"tools/list"}
925+
""";
926+
using var aliceCall = new HttpRequestMessage(HttpMethod.Post, "http://localhost:5000/")
927+
{
928+
Content = new StringContent(toolCallRequest, System.Text.Encoding.UTF8, "application/json"),
929+
};
930+
aliceCall.Headers.Add("X-Test-User", "Alice");
931+
aliceCall.Headers.Add("Mcp-Session-Id", sessionId);
932+
aliceCall.Headers.Accept.ParseAdd("application/json");
933+
aliceCall.Headers.Accept.ParseAdd("text/event-stream");
934+
using var aliceCallResponse = await HttpClient.SendAsync(aliceCall, TestContext.Current.CancellationToken);
935+
Assert.True(aliceCallResponse.IsSuccessStatusCode);
936+
937+
// Alice can still terminate her own session.
938+
using var aliceDelete = new HttpRequestMessage(HttpMethod.Delete, "http://localhost:5000/");
939+
aliceDelete.Headers.Add("X-Test-User", "Alice");
940+
aliceDelete.Headers.Add("Mcp-Session-Id", sessionId);
941+
using var aliceDeleteResponse = await HttpClient.SendAsync(aliceDelete, TestContext.Current.CancellationToken);
942+
Assert.True(aliceDeleteResponse.IsSuccessStatusCode);
943+
}
871944
}

0 commit comments

Comments
 (0)