Implementation for SEP-2350 Client-side scope accumulation in step-up authorization - #1591
Conversation
…zation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Implements SEP-2350 step-up authorization behavior in the C# SDK OAuth client flow by accumulating requested scopes across re-authorization attempts, and updates tests/docs to validate and describe the new semantics.
Changes:
- Updated
ClientOAuthProviderto union previously requested scopes with newly challenged scopes (and added locking for thread-safety). - Updated OAuth step-up tests to validate scope accumulation across one and multiple 403
insufficient_scopechallenges. - Added internal documentation updates (PRM metadata semantics) and a new “run conformance from branch” skill guide.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs |
Expands step-up OAuth tests to validate accumulated scopes across multiple authorization challenges. |
src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs |
Updates internal PRM documentation to describe SEP-2350 scope accumulation expectations. |
src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs |
Implements scope accumulation (union) and adds locking around mutable scope state. |
.github/skills/run-conformance-from-branch/SKILL.md |
Adds workflow documentation for running conformance tests against a GitHub branch build. |
Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/4331950d-e33b-4bdd-b88a-eb6c8f0f6988 Co-authored-by: mikekistler <85643503+mikekistler@users.noreply.github.com>
… no new scope Per SEP-2350, a step-up authorization should be attempted at least once, but a repeated insufficient_scope 403 that introduces no scope beyond those already requested cannot make progress by re-running interactive authorization. Track whether a step-up has been attempted and treat a subsequent no-new-scope challenge as a permanent failure instead of prompting the user again for the same resource and operation. Also return the current operation scopes as a list of tokens so the new-scope comparison and accumulation avoid joining and re-splitting the scopes_supported metadata. Add tests covering the permanent-failure path and the guarantee that the first step-up is always allowed even when the challenge reuses an already-requested scope.
The accumulator holds the union of all scopes requested so far, including the current operation's scopes after they are merged, so _previouslyRequestedScopes read slightly off. Rename it to _accumulatedScopes to match what it actually holds. Internal, no behavioral change.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
tarekgh
left a comment
There was a problem hiding this comment.
I pushed some changes, LGTM otherwise. Please have a look at the changes I pushed just in case you see something. Also, technically the included skill.md file is not part of this PR but no harm if you want to keep or you choose to add it in its own PR.
|
Added AI-generated by GitHub Copilot at the maintainer's request. |
Motivation and Context
Implements SEP-2350 client-side scope accumulation for step-up authorization in the C# SDK OAuth client flow.
Fixes #1547
What changed
Runtime behavior
Thread-safety fix
Documentation/comments
Tests
Updated and added OAuth tests in ModelContextProtocol.AspNetCore.Tests:
All OAuth tests pass across net8.0/net9.0/net10.0 in this branch.
How Has This Been Tested?
Ran auth/scope-step-up against modelcontextprotocol/conformance main (branch install).
Breaking Changes
None.
Types of changes
Checklist
Additional context