Skip to content

Commit e213557

Browse files
Copilotstephentoub
andauthored
Use StringSplitOptions.RemoveEmptyEntries in scope splitting
Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/cf7499bb-4f2e-430c-82c1-e160d5c12b25 Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent 72e24d8 commit e213557

2 files changed

Lines changed: 6 additions & 6 deletions

File tree

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -736,8 +736,8 @@ private async Task PerformDynamicClientRegistrationAsync(
736736
// all previously requested scopes to prevent losing previously granted permissions.
737737
if (_lastRequestedScopes is not null && newScopes is not null)
738738
{
739-
var combined = new HashSet<string>(_lastRequestedScopes.Split(' '), StringComparer.Ordinal);
740-
foreach (var scope in newScopes.Split(' '))
739+
var combined = new HashSet<string>(_lastRequestedScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.Ordinal);
740+
foreach (var scope in newScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries))
741741
{
742742
combined.Add(scope);
743743
}

tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,8 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader()
477477
// If they don't, it shouldn't get here due to middleware
478478
// The token scope may include accumulated scopes, so check for containment
479479
var scopeClaim = user.FindFirst("scope")?.Value ?? "";
480-
var userScopes = new HashSet<string>(scopeClaim.Split(' '), StringComparer.Ordinal);
481-
Assert.True(adminScopes.Split(' ').All(s => userScopes.Contains(s)), "User should have admin scopes when tool executes");
480+
var userScopes = new HashSet<string>(scopeClaim.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.Ordinal);
481+
Assert.True(adminScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries).All(s => userScopes.Contains(s)), "User should have admin scopes when tool executes");
482482
return "Admin tool executed.";
483483
}),
484484
]);
@@ -516,8 +516,8 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader()
516516
// Check if user has all required scopes (token may include accumulated scopes)
517517
var user = context.User;
518518
var scopeClaim = user.FindFirst("scope")?.Value ?? "";
519-
var userScopes = new HashSet<string>(scopeClaim.Split(' '), StringComparer.Ordinal);
520-
if (!adminScopes.Split(' ').All(s => userScopes.Contains(s)))
519+
var userScopes = new HashSet<string>(scopeClaim.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.Ordinal);
520+
if (!adminScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries).All(s => userScopes.Contains(s)))
521521
{
522522
// User lacks required scopes, return 403 before MapMcp processes the request
523523
context.Response.StatusCode = StatusCodes.Status403Forbidden;

0 commit comments

Comments
 (0)