From 44fcdf032dab004595f7d242c79f3d42405bfa12 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:30:51 +0000 Subject: [PATCH 1/4] Initial plan From 72e24d8630bbe035b2e5ef5d562aa942759e340e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:36:28 +0000 Subject: [PATCH 2/4] Add client-side scope accumulation in GetScopeParameter 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> --- .../Authentication/ClientOAuthProvider.cs | 28 +++++++++++++++++-- .../OAuth/AuthTests.cs | 18 +++++++++--- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index ecef8e15e..f6931c130 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -47,6 +47,7 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient private string? _tokenEndpointAuthMethod; private ITokenCache _tokenCache; private AuthorizationServerMetadata? _authServerMetadata; + private string? _lastRequestedScopes; /// /// Initializes a new instance of the class using the specified options. @@ -714,16 +715,37 @@ private async Task PerformDynamicClientRegistrationAsync( private string? GetScopeParameter(ProtectedResourceMetadata protectedResourceMetadata) { + string? newScopes; + if (!string.IsNullOrEmpty(protectedResourceMetadata.WwwAuthenticateScope)) { - return protectedResourceMetadata.WwwAuthenticateScope; + newScopes = protectedResourceMetadata.WwwAuthenticateScope; } else if (protectedResourceMetadata.ScopesSupported.Count > 0) { - return string.Join(" ", protectedResourceMetadata.ScopesSupported); + newScopes = string.Join(" ", protectedResourceMetadata.ScopesSupported); + } + else + { + newScopes = _configuredScopes; + } + + // Union with previously requested scopes to avoid losing permissions during step-up + // authorization. Per the MCP spec (aligned with RFC 6750 ยง3.1), servers should emit only + // per-operation scopes in 403 insufficient_scope challenges, so clients SHOULD accumulate + // all previously requested scopes to prevent losing previously granted permissions. + if (_lastRequestedScopes is not null && newScopes is not null) + { + var combined = new HashSet(_lastRequestedScopes.Split(' '), StringComparer.Ordinal); + foreach (var scope in newScopes.Split(' ')) + { + combined.Add(scope); + } + newScopes = string.Join(" ", combined); } - return _configuredScopes; + _lastRequestedScopes = newScopes; + return newScopes; } /// diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index c4979fb10..ca6fda13d 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -475,7 +475,10 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader() { // Tool now just checks if user has the required scopes // If they don't, it shouldn't get here due to middleware - Assert.True(user.HasClaim("scope", adminScopes), "User should have admin scopes when tool executes"); + // The token scope may include accumulated scopes, so check for containment + var scopeClaim = user.FindFirst("scope")?.Value ?? ""; + var userScopes = new HashSet(scopeClaim.Split(' '), StringComparer.Ordinal); + Assert.True(adminScopes.Split(' ').All(s => userScopes.Contains(s)), "User should have admin scopes when tool executes"); return "Admin tool executed."; }), ]); @@ -510,9 +513,11 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader() if (toolCallParams?.Name == "admin-tool") { - // Check if user has required scopes + // Check if user has all required scopes (token may include accumulated scopes) var user = context.User; - if (!user.HasClaim("scope", adminScopes)) + var scopeClaim = user.FindFirst("scope")?.Value ?? ""; + var userScopes = new HashSet(scopeClaim.Split(' '), StringComparer.Ordinal); + if (!adminScopes.Split(' ').All(s => userScopes.Contains(s))) { // User lacks required scopes, return 403 before MapMcp processes the request context.Response.StatusCode = StatusCodes.Status403Forbidden; @@ -554,7 +559,12 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader() var adminResult = await client.CallToolAsync("admin-tool", cancellationToken: TestContext.Current.CancellationToken); Assert.Equal("Admin tool executed.", adminResult.Content[0].ToString()); - Assert.Equal(adminScopes, requestedScope); + // After the 403 insufficient_scope challenge with adminScopes, the client should re-authorize + // with the union of previously requested scopes ("mcp:tools") and the newly challenged scopes. + var accumulatedScopes = requestedScope!.Split(' '); + Assert.Contains("mcp:tools", accumulatedScopes); + Assert.Contains("admin:read", accumulatedScopes); + Assert.Contains("admin:write", accumulatedScopes); } [Fact] From e21355747972fecc644e975d9d2a6cb11c13f0eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:47:59 +0000 Subject: [PATCH 3/4] 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> --- .../Authentication/ClientOAuthProvider.cs | 4 ++-- .../OAuth/AuthTests.cs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index f6931c130..2a0e15007 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -736,8 +736,8 @@ private async Task PerformDynamicClientRegistrationAsync( // all previously requested scopes to prevent losing previously granted permissions. if (_lastRequestedScopes is not null && newScopes is not null) { - var combined = new HashSet(_lastRequestedScopes.Split(' '), StringComparer.Ordinal); - foreach (var scope in newScopes.Split(' ')) + var combined = new HashSet(_lastRequestedScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.Ordinal); + foreach (var scope in newScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries)) { combined.Add(scope); } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index ca6fda13d..4c8cac4fb 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -477,8 +477,8 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader() // If they don't, it shouldn't get here due to middleware // The token scope may include accumulated scopes, so check for containment var scopeClaim = user.FindFirst("scope")?.Value ?? ""; - var userScopes = new HashSet(scopeClaim.Split(' '), StringComparer.Ordinal); - Assert.True(adminScopes.Split(' ').All(s => userScopes.Contains(s)), "User should have admin scopes when tool executes"); + var userScopes = new HashSet(scopeClaim.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.Ordinal); + Assert.True(adminScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries).All(s => userScopes.Contains(s)), "User should have admin scopes when tool executes"); return "Admin tool executed."; }), ]); @@ -516,8 +516,8 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader() // Check if user has all required scopes (token may include accumulated scopes) var user = context.User; var scopeClaim = user.FindFirst("scope")?.Value ?? ""; - var userScopes = new HashSet(scopeClaim.Split(' '), StringComparer.Ordinal); - if (!adminScopes.Split(' ').All(s => userScopes.Contains(s))) + var userScopes = new HashSet(scopeClaim.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.Ordinal); + if (!adminScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries).All(s => userScopes.Contains(s))) { // User lacks required scopes, return 403 before MapMcp processes the request context.Response.StatusCode = StatusCodes.Status403Forbidden; From afc57ddd888f8db24f1d9bef3b168fc9fcc166a2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:55:46 +0000 Subject: [PATCH 4/4] Fix netstandard2.0 build: use Split(char) instead of Split(char, StringSplitOptions) Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/5e730596-6a29-45ba-8610-4bc03424893e Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Authentication/ClientOAuthProvider.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 2a0e15007..af5cda208 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -736,10 +736,20 @@ private async Task PerformDynamicClientRegistrationAsync( // all previously requested scopes to prevent losing previously granted permissions. if (_lastRequestedScopes is not null && newScopes is not null) { - var combined = new HashSet(_lastRequestedScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries), StringComparer.Ordinal); - foreach (var scope in newScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries)) + var combined = new HashSet(StringComparer.Ordinal); + foreach (var scope in _lastRequestedScopes.Split(' ')) { - combined.Add(scope); + if (scope.Length > 0) + { + combined.Add(scope); + } + } + foreach (var scope in newScopes.Split(' ')) + { + if (scope.Length > 0) + { + combined.Add(scope); + } } newScopes = string.Join(" ", combined); }