Accumulate OAuth scopes across step-up authorization challenges

[Copilot]

Per the updated MCP spec (modelcontextprotocol#2350, aligned with RFC 6750 §3.1), servers should emit only per-operation scopes in 403 insufficient_scope challenges. Clients SHOULD compute the union of previously requested scopes and newly challenged scopes to avoid silently dropping permissions from prior operations.

Previously, GetScopeParameter() returned only the challenged scope, losing any scopes from earlier authorization rounds.

Changes

• ClientOAuthProvider: Added _lastRequestedScopes field; GetScopeParameter() now unions previous and new scopes via a HashSet<string> and persists the result for future accumulation:

  // Before: only the challenge scope was used
  return protectedResourceMetadata.WwwAuthenticateScope; // e.g. "admin:read admin:write"
  
  // After: union with previously requested scopes
  // e.g. "mcp:tools" ∪ "admin:read admin:write" → "mcp:tools admin:read admin:write"

• AuthorizationFlow_UsesScopeFromForbiddenHeader test: Updated the post-step-up assertion from an exact-equality check on the challenge scopes to a containment check verifying the re-authorization request includes both the original scopes and the newly challenged scopes.

• Test middleware/tool assertion: Updated scope checks from HasClaim("scope", exactValue) to split-and-containment logic, since the token now carries the accumulated scope string rather than an exact match.

Original prompt

Context

The MCP spec PR modelcontextprotocol/modelcontextprotocol#2350 (merged 2026-03-28) clarifies that:

1. Servers should only emit per-operation scopes in 403 insufficient_scope challenges (aligned with RFC 6750 §3.1) — they are no longer expected to include previously granted scopes in the challenge.
2. Client-side scope accumulation: Clients SHOULD compute the union of previously requested scopes and newly challenged scopes when re-authorizing, to avoid losing permissions from prior operations.
3. No incremental challenging within a single operation: Servers should emit all required scopes for an operation in a single challenge.

Problem

The current ClientOAuthProvider.GetScopeParameter() method returns only the scope from the current challenge (or metadata/configured scopes), without unioning it with previously requested scopes. This means that when a server follows the spec's "minimum approach" and returns only the per-operation scopes in a 403 challenge, the client will re-authorize with only those new scopes, potentially losing previously granted permissions.

The relevant code is in src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:

private string? GetScopeParameter(ProtectedResourceMetadata protectedResourceMetadata)
{
    if (!string.IsNullOrEmpty(protectedResourceMetadata.WwwAuthenticateScope))
    {
        return protectedResourceMetadata.WwwAuthenticateScope;
    }
    else if (protectedResourceMetadata.ScopesSupported.Count > 0)
    {
        return string.Join(" ", protectedResourceMetadata.ScopesSupported);
    }

    return _configuredScopes;
}

Required Changes

1. Add a field to ClientOAuthProvider to track previously requested scopes (e.g., private string? _lastRequestedScopes;).

2. Update GetScopeParameter() to compute the union of the previously requested scopes and the new scopes. When _lastRequestedScopes is not null and new scopes are determined, combine them using a HashSet<string> to deduplicate, then join back into a space-separated string. Store the result back into _lastRequestedScopes for future accumulation.

3. Update the existing test AuthorizationFlow_UsesScopeFromForbiddenHeader in tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs. Currently, after initial auth with "mcp:tools" and a step-up 403 with "admin:read admin:write", the test asserts requestedScope equals "admin:read admin:write". With scope accumulation, the re-authorization should now request the union: the assertion should verify that the scope contains both the original scopes AND the new scopes (e.g., "mcp:tools admin:read admin:write" — though order may vary since we're using a HashSet).

4. Consider adding a new test that specifically validates scope accumulation behavior — e.g., initial auth gets scope "read", then a 403 challenges with "write", and verify the re-auth request includes both "read write".

Notes

• This is a SHOULD-level requirement in the spec, but it's important for interoperability with servers that use the "minimum approach" for scope challenges.
• The GetScopeParameter method is also called from PerformDynamicClientRegistrationAsync for DCR — scope accumulation should apply there too since it flows through the same method.
• No new public API surface is needed — this is an internal behavioral change.
• Be sure to follow the repo's coding conventions (see .github/copilot-instructions.md).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Anything we need to do to react to this PR in the C# SDK?

Assistant:

Let me look at the PR to understand what changes were made to the MCP specification, and then check the current state of the C# SDK's authorization implementation.

Assistant: Now let me look at the actual diff of the spec PR and the current scope accumulation logic in the C# SDK more closely.

Assistant: Now let me look at what the spec PR actually changed to understand the specific requirements:

Assistant: Now I have a clear picture. Let me analyze the spec change and the current C# SDK implementation.

No immediate code changes are required, but there's a gap worth tracking. Here's the analysis:

What the spec PR changed

PR #2350 clarifies three things in the authorization spec:

1. Servers should only emit per-operation scopes in 403 insufficient_scope challenges (aligned with RFC 6750 §3.1) — they're no longer expected to include previously granted scopes.
2. Client-side scope accumulation: Clients SHOULD compute the union of previously requested scopes and newly challenged scopes when re-authorizing, to avoid losing permissions from prior operations.
3. **No incremental challenging within...

This pull request was created from Copilot chat.

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

[stephentoub]

@copilot fix the build/test failures. Also re-evaluate the SEP and confirm these are all the changes and only the changes necessary to address it.

[Copilot]

@copilot fix the build/test failures. Also re-evaluate the SEP and confirm these are all the changes and only the changes necessary to address it.

Fixed the build failure in afc57dd — string.Split(char, StringSplitOptions) isn't available on netstandard2.0. Changed to Split(' ') with manual empty-entry filtering.

Re-evaluated against the spec (SEP-2350 diff): The spec requires clients to "compute the union of previously requested scopes and newly challenged scopes when re-authorizing." The changes are:

1. ClientOAuthProvider.cs: Added _lastRequestedScopes field + union logic in GetScopeParameter() — this is the only behavioral change needed. Both call sites (BuildAuthorizationUrl and PerformDynamicClientRegistrationAsync) benefit automatically.
2. AuthTests.cs: Updated AuthorizationFlow_UsesScopeFromForbiddenHeader test — the server-simulating middleware and assertion needed to change from exact-match to containment checks since the token now carries accumulated scopes (e.g., "mcp:tools admin:read admin:write" instead of just "admin:read admin:write").

No other changes are necessary — no new public API, no other code paths affected.