Repository navigation
fix: preserve canonical OAuth resource identifiers during refresh - #1967
Merged
Merged
Conversation
Reuse the validated protected-resource identifier for silent token refresh and retain it with authorization-server metadata after cold-start refreshes. Reset both together when client credentials move to another authorization server. Preserve exact resource spelling in identity assertion grants and use the discovered canonical resource in the conformance enterprise flow. Cover deterministic access-token expiry, trailing slashes, legacy omission, cold starts, and authorization-server migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep authorization-server metadata and the canonical resource paired at the original post-registration readiness point. Retain the same pair on successful challenge-driven refresh before its early return so cold-start refresh still enables future silent refreshes. Cover retrying the same provider after registration fails with expired, unbound legacy cached tokens. This previously attempted silent refresh without a client ID instead of retrying registration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the changes preserving canonical OAuth resource identifiers.
No open issues 🎉
What changed in this PR
This PR preserves canonical OAuth resource identifiers across token refresh and identity-assertion flows in the C# SDK, avoiding changes caused by reconstructing identifiers from transport URLs.
Changes:
- Retain the validated resource alongside authorization-server metadata for silent refresh, including cold starts and server migration.
- Preserve exact resource spelling in identity-assertion grants and use the discovered resource in the enterprise conformance flow.
- Add regression coverage for resource formatting, legacy omission, and retries after failed registration.
| File | Description |
|---|---|
| tests/ModelContextProtocol.Tests/IdentityAssertionGrantTests.cs | Verify exact resource spelling, trailing slashes, and percent-encoding. |
| tests/ModelContextProtocol.ConformanceClient/ConformanceOAuthHelpers.cs | Pass the discovered canonical resource to enterprise token acquisition. |
| tests/ModelContextProtocol.AspNetCore.Tests/OAuth/TokenCacheTests.cs | Cover canonical refresh resources, cold starts, registration retries, and issuer migration. |
| src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs | Document exact resource identifier preservation. |
| src/ModelContextProtocol.Core/Authentication/IdentityAssertionGrantProvider.cs | Use Uri.OriginalString for the resource parameter. |
| src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs | Retain and clear canonical resource state alongside authorization-server metadata. |
Review effort: Balanced
tarekgh
approved these changes
Oct 28, 2026
mauri0686
added a commit
to mauri0686/csharp-sdk
that referenced
this pull request
Oct 30, 2026
Resolve the conflict with modelcontextprotocol#1967 by keeping both changes: the issuer-based audience from this branch and resourceUrl.OriginalString from main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ProtectedResourceMetadata.Resourcestring for silent token refresh instead of reconstructing it from the outgoing request URI. Preserve authority-only identifiers, meaningful trailing slashes, and resource omission in the existing legacy flow.Uri.OriginalStringin identity-assertion grants and pass the discovered canonical resource, rather than the transport endpoint, through the conformance enterprise flow.Authorization requests, authorization-code exchanges, and challenge-triggered refreshes already used the metadata resource verbatim. This does not change metadata-match rules, issuer validation, scopes, token-cache contracts, or introduce a retry that omits
resource.Regression coverage
Deterministically expire cached access tokens, without delays, and assert the exact refresh resource, successful refresh, no extra challenge, and no additional interactive authorization or code exchange. Coverage includes bare-host resources without an added slash,
/mcpendpoints with an authority-only resource, explicit root/path trailing slashes, legacy absent-resource flows, cold-start cached-token refresh, and authorization-server migration.Also reject the first dynamic-registration request with expired, unbound legacy cached tokens and retry the same provider. The retry must register and authorize successfully rather than attempt refresh without a client ID. Identity-assertion coverage checks root/path spelling, trailing slashes, and percent-encoding.
The resource regressions were demonstrated failing before the production fix. The failed-registration retry regression was demonstrated failing with the earlier readiness-state assignment and passing with the revised ordering.
Validation
dotnet build --no-restore --nologo --verbosity quiet— succeeded with zero warnings and errors.dotnet test --no-build --no-restore --nologo --filter "FullyQualifiedName~.OAuth.|FullyQualifiedName~IdentityAssertionGrantTests|FullyQualifiedName~ClientOAuthProviderApplicationTypeTests" --verbosity quiet— all 487 related cases passed: 129 OAuth cases on each of .NET 10, .NET 9, and .NET 8, plus 25 core authentication cases on each of those frameworks and .NET Framework 4.7.2.Note
This pull request's code and description were generated with GitHub Copilot.