Skip to content

Commit 72e24d8

Browse files
Copilotstephentoub
andauthored
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>
1 parent 44fcdf0 commit 72e24d8

2 files changed

Lines changed: 39 additions & 7 deletions

File tree

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
4747
private string? _tokenEndpointAuthMethod;
4848
private ITokenCache _tokenCache;
4949
private AuthorizationServerMetadata? _authServerMetadata;
50+
private string? _lastRequestedScopes;
5051

5152
/// <summary>
5253
/// Initializes a new instance of the <see cref="ClientOAuthProvider"/> class using the specified options.
@@ -714,16 +715,37 @@ private async Task PerformDynamicClientRegistrationAsync(
714715

715716
private string? GetScopeParameter(ProtectedResourceMetadata protectedResourceMetadata)
716717
{
718+
string? newScopes;
719+
717720
if (!string.IsNullOrEmpty(protectedResourceMetadata.WwwAuthenticateScope))
718721
{
719-
return protectedResourceMetadata.WwwAuthenticateScope;
722+
newScopes = protectedResourceMetadata.WwwAuthenticateScope;
720723
}
721724
else if (protectedResourceMetadata.ScopesSupported.Count > 0)
722725
{
723-
return string.Join(" ", protectedResourceMetadata.ScopesSupported);
726+
newScopes = string.Join(" ", protectedResourceMetadata.ScopesSupported);
727+
}
728+
else
729+
{
730+
newScopes = _configuredScopes;
731+
}
732+
733+
// Union with previously requested scopes to avoid losing permissions during step-up
734+
// authorization. Per the MCP spec (aligned with RFC 6750 §3.1), servers should emit only
735+
// per-operation scopes in 403 insufficient_scope challenges, so clients SHOULD accumulate
736+
// all previously requested scopes to prevent losing previously granted permissions.
737+
if (_lastRequestedScopes is not null && newScopes is not null)
738+
{
739+
var combined = new HashSet<string>(_lastRequestedScopes.Split(' '), StringComparer.Ordinal);
740+
foreach (var scope in newScopes.Split(' '))
741+
{
742+
combined.Add(scope);
743+
}
744+
newScopes = string.Join(" ", combined);
724745
}
725746

726-
return _configuredScopes;
747+
_lastRequestedScopes = newScopes;
748+
return newScopes;
727749
}
728750

729751
/// <summary>

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,10 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader()
475475
{
476476
// Tool now just checks if user has the required scopes
477477
// If they don't, it shouldn't get here due to middleware
478-
Assert.True(user.HasClaim("scope", adminScopes), "User should have admin scopes when tool executes");
478+
// The token scope may include accumulated scopes, so check for containment
479+
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");
479482
return "Admin tool executed.";
480483
}),
481484
]);
@@ -510,9 +513,11 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader()
510513

511514
if (toolCallParams?.Name == "admin-tool")
512515
{
513-
// Check if user has required scopes
516+
// Check if user has all required scopes (token may include accumulated scopes)
514517
var user = context.User;
515-
if (!user.HasClaim("scope", adminScopes))
518+
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)))
516521
{
517522
// User lacks required scopes, return 403 before MapMcp processes the request
518523
context.Response.StatusCode = StatusCodes.Status403Forbidden;
@@ -554,7 +559,12 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader()
554559
var adminResult = await client.CallToolAsync("admin-tool", cancellationToken: TestContext.Current.CancellationToken);
555560
Assert.Equal("Admin tool executed.", adminResult.Content[0].ToString());
556561

557-
Assert.Equal(adminScopes, requestedScope);
562+
// After the 403 insufficient_scope challenge with adminScopes, the client should re-authorize
563+
// with the union of previously requested scopes ("mcp:tools") and the newly challenged scopes.
564+
var accumulatedScopes = requestedScope!.Split(' ');
565+
Assert.Contains("mcp:tools", accumulatedScopes);
566+
Assert.Contains("admin:read", accumulatedScopes);
567+
Assert.Contains("admin:write", accumulatedScopes);
558568
}
559569

560570
[Fact]

0 commit comments

Comments
 (0)