Skip to content

Let providers without a CIMD URL reader refresh their tokens - #559

Merged
koic merged 1 commit into
modelcontextprotocol:mainfrom
koic:let_providers_without_a_cimd_url_refresh
Sep 18, 2026
Merged

koic merged 1 commit into
modelcontextprotocol:mainfrom
koic:let_providers_without_a_cimd_url_refresh

Conversation

@koic

@koic koic commented Sep 17, 2026

Copy link
Copy Markdown
Member

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.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

## 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
koic force-pushed the let_providers_without_a_cimd_url_refresh branch from e19d7f0 to f51ccfb Compare September 18, 2026 02:19
@koic
koic merged commit e1105c7 into modelcontextprotocol:main Sep 18, 2026
11 checks passed
@koic
koic deleted the let_providers_without_a_cimd_url_refresh branch September 18, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant