From f51ccfbe662f4f0320ceb5974d39eab317589e3d Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 17 Sep 2026 02:39:45 +0900 Subject: [PATCH] Let providers without a CIMD URL reader refresh their tokens ## 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. --- docs/_client/authorization.md | 6 +- lib/mcp/client/oauth/flow.rb | 32 +++++- test/mcp/client/oauth/flow_test.rb | 132 +++++++++++++++++++++++ test/mcp/client/oauth/http_oauth_test.rb | 62 +++++++++++ 4 files changed, 227 insertions(+), 5 deletions(-) diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index b35b5978..bb864582 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -40,6 +40,8 @@ pass an `MCP::Client::OAuth::Provider` to the transport instead of a static `Aut as the authorization base URL, its metadata is fetched from `/.well-known/oauth-authorization-server` without the RFC 8414 issuer byte-match (which the legacy spec predates), and when even that is absent the spec's default endpoints `/authorize`, `/token`, and `/register` at the origin are used with PKCE S256 assumed. - On subsequent 401s with a saved `refresh_token`, exchange it at the token endpoint before falling back to the full interactive flow (RFC 6749 Section 6). + `ClientCredentialsProvider` and `CrossAppAccessProvider` refresh the same way and fall back to their own grant instead; + their refresh also requires the `issuer` the SDK records on the tokens, so tokens stored without it run the grant again. - On a `403 Forbidden` whose `WWW-Authenticate` header carries `error="insufficient_scope"` (OAuth 2.0 step-up, RFC 6750 Section 3.1 and the MCP scope-selection-strategy), run a fresh authorization request for the union of the currently granted scope and the scope named in the challenge, then retry the failed request once. The refresh path is bypassed because refreshing would re-issue the same scope set the server just rejected. A `403` without that challenge is surfaced unchanged. @@ -192,7 +194,8 @@ and redaction policy before persisting them or displaying them to users. For a confidential machine-to-machine client (no user, no browser redirect), use `MCP::Client::OAuth::ClientCredentialsProvider` instead of `Provider`. The transport discovers the authorization server the same way, then exchanges the OAuth 2.1 `client_credentials` grant (RFC 6749 Section 4.4) at -the token endpoint. There is no authorization request, PKCE, or `offline_access`, because the grant does not issue a refresh token. +the token endpoint. There is no authorization request, PKCE, or `offline_access`, because the grant is not expected to issue a refresh token (RFC 6749 Section 4.4.3); +a refresh token the authorization server issues anyway is used on the next `401`. ```ruby provider = MCP::Client::OAuth::ClientCredentialsProvider.new( @@ -224,6 +227,7 @@ Keyword arguments: For enterprise MCP deployments where an identity provider (IdP) governs authorization (SEP-990), use `MCP::Client::OAuth::CrossAppAccessProvider` instead of `Provider`. The client exchanges an IdP-issued ID token for an Identity Assertion Authorization Grant (ID-JAG) at the IdP via RFC 8693 token exchange, then presents the ID-JAG to the MCP authorization server with the RFC 7523 `jwt-bearer` grant, authenticating with `client_secret_basic`. There is no authorization request, PKCE, DCR, or `offline_access`. +A refresh token the authorization server issues is exchanged on the next `401` with the stored client secret, without calling `assertion_provider` again. Mirrors `CrossAppAccessProvider` and `requestJwtAuthorizationGrant` in the TypeScript SDK. `MCP::Client::OAuth::IDJAGTokenExchange.request` performs the RFC 8693 exchange at the IdP token endpoint. Wrap it in a callable so the same provider can plug into diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 1093eba7..c34d6171 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -182,7 +182,7 @@ def run!(server_url:, resource_metadata_url: nil, scope: nil) # Runs the OAuth 2.1 `client_credentials` grant (machine-to-machine, no user interaction) and persists # the resulting token. Shares the same discovery and security checks as `run!`; the only difference is # the grant exchanged at the token endpoint. There is no PKCE, redirect, or authorization request, - # and no `offline_access` augmentation because the grant does not issue a refresh token (OAuth 2.1 Section 4.3.3). + # and no `offline_access` augmentation because the grant is not expected to issue a refresh token (OAuth 2.1 Section 4.3.3). # The pre-registered `client_id` / `client_secret` come from the provider's stored `client_information`. # https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization def run_client_credentials!(as_metadata:, prm:, resource:, scope:, server_url:) @@ -275,19 +275,22 @@ def run_jwt_bearer!(as_metadata:, prm:, resource:, scope:, server_url:) # checks before talking to it. # # Returns `:refreshed` on success. Raises `AuthorizationError` when the provider has no refresh token, no client information, + # when a `client_credentials` or `jwt-bearer` provider's tokens record no issuer, # or when the token endpoint refuses the refresh request. # https://www.rfc-editor.org/rfc/rfc6749#section-6 def refresh!(server_url:, resource_metadata_url: nil) refresh_token = read_token("refresh_token") raise AuthorizationError, "Cannot refresh: no refresh_token in provider storage." unless refresh_token + ensure_refresh_token_issuer_recorded! + stored_client_info = @provider.client_information have_stored_client_info = stored_client_info.is_a?(Hash) && client_info_required_value(stored_client_info, "client_id") # A CIMD-configured provider stores no `client_information` on purpose # (the CIMD URL is re-resolved against the live AS metadata on every flow). # Allow refresh to proceed in that case so the `refresh_token` obtained via the CIMD flow remains usable. - have_cimd_url = !@provider.client_id_metadata_document_url.nil? + have_cimd_url = !provider_client_id_metadata_document_url.nil? unless have_stored_client_info || have_cimd_url raise AuthorizationError, "Cannot refresh: no client_information in provider storage." @@ -323,7 +326,7 @@ def refresh!(server_url:, resource_metadata_url: nil) ensure_refreshable_client_information!(stored_client_info, as_metadata: as_metadata) stored_client_info elsif as_metadata["client_id_metadata_document_supported"] == true - { "client_id" => @provider.client_id_metadata_document_url } + { "client_id" => provider_client_id_metadata_document_url } else raise AuthorizationError, "Cannot refresh: provider has a CIMD URL but the authorization server no longer advertises " \ @@ -684,7 +687,7 @@ def ensure_client_registered(as_metadata:) # (or the operator may rotate the CIMD URL), and a stale `client_information` entry would otherwise # keep sending the old CIMD URL forever. Re-evaluating on every flow re-reads the current AS metadata # and the current `provider.client_id_metadata_document_url`. - cimd_url = @provider.client_id_metadata_document_url + cimd_url = provider_client_id_metadata_document_url if cimd_url && as_metadata["client_id_metadata_document_supported"] == true return { "client_id" => cimd_url } end @@ -814,6 +817,18 @@ def ensure_token_issuer!(as_metadata:) MESSAGE end + # `Provider` tolerates tokens stored before the issuer was recorded (see `ensure_token_issuer!`). + # The `client_credentials` and `jwt-bearer` providers have no such tokens, since their refresh is new, + # and a refresh asks no validator, so a token without an `issuer` would be presented to whatever + # authorization server discovery names now. Refusing it sends the transport back through the grant, + # which does ask. + def ensure_refresh_token_issuer_recorded! + return if provider_authorization_flow == :authorization_code + return unless read_token("issuer").nil? + + raise AuthorizationError, "Cannot refresh: the stored tokens record no issuer; re-authorization is required." + end + def ensure_refreshable_client_information!(client_info, as_metadata:) stored_issuer = client_info_required_value(client_info, "issuer") return if stored_issuer.nil? @@ -1020,6 +1035,15 @@ def provider_token_request_params params end + # The Client ID Metadata Document URL, when the provider has one. Only `Provider` exposes the reader + # (CIMD replaces Dynamic Client Registration on the authorization-code flow), while `refresh!` serves + # every provider that holds a `refresh_token`, so the read is duck-typed like `authorization_flow`. + def provider_client_id_metadata_document_url + return unless @provider.respond_to?(:client_id_metadata_document_url) + + @provider.client_id_metadata_document_url + end + def build_authorization_url(as_metadata:, client_id:, scope:, state:, code_challenge:, resource:) authorization_endpoint = as_metadata["authorization_endpoint"] unless authorization_endpoint diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index c463fdc9..799ecd0d 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -557,6 +557,138 @@ def test_run_jwt_bearer_raises_when_assertion_provider_returns_nothing assert_not_requested(:post, "#{@auth_base}/token") end + def test_refresh_uses_the_stored_credentials_of_a_cross_app_access_provider + # RFC 7521 Section 4.1 makes a refresh token unusual for an assertion grant, not forbidden; + # when the authorization server issued one, it is exchanged with the stored client secret + # and no new ID-JAG assertion is minted. + assertion_calls = 0 + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { + assertion_calls += 1 + "id-jag-assertion" + }, + ) + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + result = Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:refreshed, result) + assert_equal("test-token-from-flow", provider.access_token) + assert_equal(@auth_base, provider.tokens["issuer"]) + assert_equal(0, assertion_calls) + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "refresh_token" && + form["refresh_token"] == "saved-rt" && + !form.key?("assertion") && + req.headers["Authorization"] == "Basic " + Base64.strict_encode64("xaa-client:xaa-secret") + end + end + + def test_refresh_uses_the_stored_credentials_of_a_client_credentials_provider + # RFC 6749 Section 4.4.3 says a refresh token SHOULD NOT be issued for this grant, + # so one in storage is unusual, but it must be exchanged rather than crash the refresh. + provider = client_credentials_provider + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + result = Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:refreshed, result) + assert_equal(@auth_base, provider.tokens["issuer"]) + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "refresh_token" && + form["refresh_token"] == "saved-rt" && + req.headers["Authorization"] == "Basic " + Base64.strict_encode64("cc-client:cc-secret") + end + end + + def test_refresh_refuses_an_authorization_server_that_did_not_issue_a_cross_app_access_token + # The token-level issuer binding is what protects these providers: their stored `client_information` carries no `issuer`, + # so `ensure_refreshable_client_information!` has nothing to compare. + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag-assertion" }, + ) + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => "https://old-as.example.com") + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/issued by a different authorization server/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + + def test_refresh_refuses_tokens_without_a_recorded_issuer_for_the_client_credentials_and_jwt_bearer_providers + # These providers had no refresh before their tokens recorded the issuer, so nothing older needs + # tolerating; without the record a refresh would go to whatever server discovery names, unasked. + cross_app = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag-assertion" }, + ) + [cross_app, client_credentials_provider].each do |provider| + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt") + + error = assert_raises(Flow::AuthorizationError, "should refuse #{provider.class}") do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/record no issuer/, error.message) + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + + assert_not_requested(:get, @prm_url) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_refresh_raises_cleanly_for_a_provider_with_neither_client_information_nor_a_cimd_url + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag-assertion" }, + ) + provider.storage.save_client_information(nil) + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/no client_information/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_registers_through_dcr_for_a_provider_without_a_cimd_url_reader + # Registration reads the CIMD URL through the same duck-typed helper as refresh, + # so a provider that lacks the reader goes to Dynamic Client Registration. + provider_class = Class.new(Provider) { undef_method :client_id_metadata_document_url } + state_value = nil + provider = provider_class.new( + client_metadata: { + redirect_uris: ["http://localhost:0/callback"], + grant_types: ["authorization_code"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }, + redirect_uri: "http://localhost:0/callback", + redirect_handler: ->(url) { state_value = URI.decode_www_form(url.query).to_h.fetch("state") }, + callback_handler: -> { ["test-auth-code", state_value] }, + ) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_requested(:post, "#{@auth_base}/register") + end + def test_run_uses_authorization_code_grant_for_default_provider # A standard `Provider` declares `authorization_flow == :authorization_code`, # so `Flow` runs the interactive grant regardless of what `client_metadata[:grant_types]` happens to list. diff --git a/test/mcp/client/oauth/http_oauth_test.rb b/test/mcp/client/oauth/http_oauth_test.rb index 5b1be133..ce026c92 100644 --- a/test/mcp/client/oauth/http_oauth_test.rb +++ b/test/mcp/client/oauth/http_oauth_test.rb @@ -627,6 +627,68 @@ def test_send_request_refreshes_when_refresh_token_is_available assert_equal("refreshed-token", provider.access_token) end + def test_send_request_refreshes_for_a_cross_app_access_provider_holding_a_refresh_token + # Refresh serves every provider, not only the authorization-code one that has a CIMD URL reader. + stub_request(:post, @mcp_url).with { |req| + req.headers["Authorization"] != "Bearer refreshed-token" + }.to_return( + status: 401, + headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") }, + body: "", + ) + + stub_request(:post, @mcp_url).with( + headers: { "Authorization" => "Bearer refreshed-token" } + ).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(jsonrpc: "2.0", id: "1", result: { ok: true }), + ) + + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + + stub_request(:get, "#{@auth_base}/.well-known/oauth-authorization-server").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate( + issuer: @auth_base, + token_endpoint: "#{@auth_base}/token", + grant_types_supported: ["urn:ietf:params:oauth:grant-type:jwt-bearer", "refresh_token"], + token_endpoint_auth_methods_supported: ["client_secret_basic"], + ), + ) + + stub_request(:post, "#{@auth_base}/token").with( + body: hash_including("grant_type" => "refresh_token", "refresh_token" => "saved-rt") + ).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(access_token: "refreshed-token", token_type: "Bearer", expires_in: 3600), + ) + + assertion_calls = 0 + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { + assertion_calls += 1 + "id-jag-assertion" + }, + ) + provider.save_tokens("access_token" => "stale-token", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + transport = HTTP.new(url: @mcp_url, oauth: provider) + response = transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" }) + + assert_equal({ "ok" => true }, response["result"]) + assert_equal("refreshed-token", provider.access_token) + assert_equal(0, assertion_calls) + end + def test_send_request_surfaces_a_bad_token_request_params_hook_instead_of_reauthorizing # A provider whose `token_request_params` names a reserved key is misconfigured, not unauthorized: # the refresh attempt must raise rather than fall through to the interactive flow,