Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
private string? _tokenEndpointAuthMethod;
private ITokenCache _tokenCache;
private AuthorizationServerMetadata? _authServerMetadata;
private string? _lastRequestedScopes;

/// <summary>
/// Initializes a new instance of the <see cref="ClientOAuthProvider"/> class using the specified options.
Expand Down Expand Up @@ -714,16 +715,47 @@ 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<string>(StringComparer.Ordinal);
foreach (var scope in _lastRequestedScopes.Split(' '))
{
if (scope.Length > 0)
{
combined.Add(scope);
}
}
foreach (var scope in newScopes.Split(' '))
{
if (scope.Length > 0)
{
combined.Add(scope);
}
}
newScopes = string.Join(" ", combined);
}

return _configuredScopes;
_lastRequestedScopes = newScopes;
return newScopes;
}

/// <summary>
Expand Down
18 changes: 14 additions & 4 deletions tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>(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.";
}),
]);
Expand Down Expand Up @@ -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<string>(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;
Expand Down Expand Up @@ -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]
Expand Down
Loading