Conversation
## Motivation and Context `Flow#refresh!` reads `client_id_metadata_document_url` from the provider unconditionally, once to decide whether a provider without stored `client_information` may still refresh and once to build the CIMD `client_id`. Only `Provider` defines that reader: `CrossAppAccessProvider` and `ClientCredentialsProvider` raise `NoMethodError` once a stored `refresh_token` is found. `MCP::Client::HTTP` calls `refresh!` on any `401` while a `refresh_token` is stored, so a `jwt-bearer` client whose authorization server issued one fails with `NoMethodError` on the first expired token instead of refreshing. The error is not an `AuthorizationError`, so the transport does not fall back to the grant either. RFC 7521 Section 4.1 makes a refresh token unusual for an assertion grant, not forbidden, and one in storage must not crash the client. The flow now reads the URL through a duck-typed helper, like `authorization_flow` and `client_assertion`, so a provider without the reader has no CIMD URL and refreshes with its stored `client_information`: the `refresh_token` grant is sent with the stored client authentication, and no new ID-JAG assertion is obtained. The authorization-code registration path reads the URL through the same helper. The docs now say that a refresh token issued anyway is used on the next `401`, and that the assertion provider is not called again on refresh. A refresh token stored without the `issuer` the SDK records is refused for these two providers before any request: unlike `Provider`, they have no tokens that predate the record, and a refresh asks no validator, so such a token would be presented to whatever authorization server discovery names. The refusal is an `AuthorizationError`, which sends the transport back through the grant, and the grant asks. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb` refresh through a `CrossAppAccessProvider` and a `ClientCredentialsProvider` holding a `refresh_token`, asserting the `refresh_token` grant, the Basic credentials, and that the assertion provider is not called. A transport test in `test/mcp/client/oauth/http_oauth_test.rb` sends a `401` to a `CrossAppAccessProvider` with a stored `refresh_token` and expects the refreshed bearer token on the retry. All three raise `NoMethodError` against the previous library. Further tests pin the issuer stamp on the refreshed tokens, the refusal of an authorization server that did not issue them for a `CrossAppAccessProvider`, a clean `AuthorizationError` for a provider with neither the reader nor stored `client_information`, and registration through DCR for a provider without the reader. A refresh token without a recorded issuer is refused for both providers before any request is sent. ## Breaking Changes None.
koic
force-pushed
the
let_providers_without_a_cimd_url_refresh
branch
from
September 18, 2026 02:19
e19d7f0 to
f51ccfb
Compare
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.
Motivation and Context
Flow#refresh!readsclient_id_metadata_document_urlfrom the provider unconditionally, once to decide whether a provider without storedclient_informationmay still refresh and once to build the CIMDclient_id. OnlyProviderdefines that reader:CrossAppAccessProviderandClientCredentialsProviderraiseNoMethodErroronce a storedrefresh_tokenis found.MCP::Client::HTTPcallsrefresh!on any401while arefresh_tokenis stored, so ajwt-bearerclient whose authorization server issued one fails withNoMethodErroron the first expired token instead of refreshing. The error is not anAuthorizationError, so the transport does not fall back to the grant either. RFC 7521 Section 4.1 makes a refresh token unusual for an assertion grant, not forbidden, and one in storage must not crash the client.The flow now reads the URL through a duck-typed helper, like
authorization_flowandclient_assertion, so a provider without the reader has no CIMD URL and refreshes with its storedclient_information: therefresh_tokengrant is sent with the stored client authentication, and no new ID-JAG assertion is obtained. The authorization-code registration path reads the URL through the same helper. The docs now say that a refresh token issued anyway is used on the next401, and that the assertion provider is not called again on refresh.A refresh token stored without the
issuerthe SDK records is refused for these two providers before any request: unlikeProvider, they have no tokens that predate the record, and a refresh asks no validator, so such a token would be presented to whatever authorization server discovery names. The refusal is anAuthorizationError, which sends the transport back through the grant, and the grant asks.How Has This Been Tested?
New tests in
test/mcp/client/oauth/flow_test.rbrefresh through aCrossAppAccessProviderand aClientCredentialsProviderholding arefresh_token, asserting therefresh_tokengrant, the Basic credentials, and that the assertion provider is not called. A transport test intest/mcp/client/oauth/http_oauth_test.rbsends a401to aCrossAppAccessProviderwith a storedrefresh_tokenand expects the refreshed bearer token on the retry. All three raiseNoMethodErroragainst the previous library. Further tests pin the issuer stamp on the refreshed tokens, the refusal of an authorization server that did not issue them for aCrossAppAccessProvider, a cleanAuthorizationErrorfor a provider with neither the reader nor storedclient_information, and registration through DCR for a provider without the reader. A refresh token without a recorded issuer is refused for both providers before any request is sent.Breaking Changes
None.
Types of changes
Checklist