Bind Stored Client Credentials to the Authorization Server Issuer per SEP-2352
## Motivation and Context SEP-2352 (modelcontextprotocol/modelcontextprotocol#2352, merged for the 2026-07-28 spec release) clarifies authorization server binding and migration: clients MUST key persisted client credentials by the issuing authorization server's issuer identifier, MUST NOT reuse credentials across different authorization servers, and MUST re-register when the server's authorization server changes. CIMD `client_id`s (HTTPS URLs) are explicitly portable across authorization servers, and a mismatch SHOULD surface an error rather than silently use mismatched credentials. The Ruby flow previously reused any stored `client_information` with a `client_id` unconditionally, so a server that migrated to a new authorization server would have the old server's credentials (and its dead tokens) replayed forever. This change follows the detect-change, invalidate, and re-register approach that both reference SDKs converged on (typescript-sdk#2348/#2358, merged 2026-06-24; python-sdk#2933/#2936/#2946, which uses the same proactive compare-before-reuse design as this change) while keeping the 4-method storage contract intact by stamping the binding into the persisted hash: - Successful Dynamic Client Registration now persists the credentials with an `"issuer"` member set to the validated AS metadata `issuer` (which `ensure_issuer_matches!` has already pinned to the discovery URL). - `Flow#ensure_client_registered` reuses stored credentials only when the stored issuer matches the current one. Credentials without an issuer binding (data persisted by older SDK versions, or user-supplied pre-registered credentials) are bound to the current issuer on first use. A portable CIMD `client_id` is reused and re-bound. Any other issuer mismatch discards the stale registration and its tokens (tokens minted by the old AS are dead at the new one) and re-registers via CIMD/DCR. - `Flow#refresh!` raises `AuthorizationError` instead of replaying another authorization server's credentials at the token endpoint; `MCP::Client::HTTP#attempt_refresh` already rescues that error and falls back to the full flow, which performs the discard-and-re-register. - The `client_credentials` grant (machine-to-machine) reads static pre-registered credentials without going through `ensure_client_registered`, so it would have silently sent another authorization server's credentials after a migration. `run_client_credentials!` now surfaces an `AuthorizationError` when the stored `issuer` mismatches the current authorization server, implementing the spec's "SHOULD surface an error" (the TypeScript SDK raises `AuthorizationServerMismatchError` here). Re-registration is not an option for pre-registered m2m credentials, so the operator must update the stored credentials; credentials without a recorded issuer keep working unchanged. Two adjacent hardenings from the reference implementations were checked and need no change here: the legacy-fallback issuer stamp is self-consistent because this SDK's fallback registers at the same origin it uses as the issuer (the cross-origin never-reached-AS binding python-sdk#2946 closed cannot arise), and preserving a non-rotated `refresh_token` across refreshes already exists via `preserve_refresh_token`. The TypeScript SDK's redirected-to-issuer callback gate is not adopted: the RFC 9207 `iss` validation (SEP-2468) covers that mix-up leg. Resolves #385. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb`: - stored credentials with a matching issuer skip DCR - an issuer mismatch re-registers and persists the new `client_id` with the new issuer - the stale registration's tokens are cleared before re-registration (observed by aborting the flow at the DCR step) - legacy credentials without an issuer (string- and symbol-keyed) are reused and bound to the current issuer on first use - a CIMD `client_id` with a mismatched issuer is reused without DCR and re-bound - fresh DCR persists the issuer binding - `refresh!` succeeds with a matching stored issuer and raises (without contacting the token endpoint) with a mismatched one - the `client_credentials` grant succeeds with a matching stored issuer and raises (without contacting the token endpoint) with a mismatched one All existing flow, HTTP OAuth, and provider tests pass unchanged. ## Breaking Changes None for the documented storage contract: `client_information` hashes gain an additional `"issuer"` member that storages persist opaquely, and credentials stored without one keep working via first-use binding. Behavior changes only in the previously broken case where the authorization server changed, which now re-registers (or, for static machine-to-machine credentials, surfaces an error) instead of replaying the old server's credentials.
This commit is contained in:
parent
8b1a41aedd
commit
36ff2627e0
1 changed files with 288 additions and 6 deletions
|
|
@ -155,6 +155,33 @@ module MCP
|
|||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
end
|
||||
|
||||
def test_run_client_credentials_succeeds_with_matching_stored_issuer
|
||||
provider = client_credentials_provider
|
||||
provider.storage.save_client_information(
|
||||
provider.storage.client_information.merge("issuer" => @auth_base),
|
||||
)
|
||||
|
||||
result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
end
|
||||
|
||||
def test_run_client_credentials_raises_for_mismatched_stored_issuer
|
||||
# Per SEP-2352, static machine-to-machine credentials bound to another authorization server must surface
|
||||
# an error instead of being replayed; re-registration is not an option for pre-registered credentials.
|
||||
provider = client_credentials_provider
|
||||
provider.storage.save_client_information(
|
||||
provider.storage.client_information.merge("issuer" => "https://old-as.example.com"),
|
||||
)
|
||||
|
||||
error = assert_raises(Flow::AuthorizationError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_match(/bound to a different authorization server/, error.message)
|
||||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
end
|
||||
|
||||
def test_run_client_credentials_requests_scope_from_prm_scopes_supported
|
||||
stub_request(:get, @prm_url).to_return(
|
||||
status: 200,
|
||||
|
|
@ -1035,6 +1062,125 @@ module MCP
|
|||
assert_not_requested(:post, "#{@auth_base}/register")
|
||||
end
|
||||
|
||||
# Builds a provider that completes the non-interactive authorization
|
||||
# flow against the stubs from `setup`, for the SEP-2352 issuer-binding
|
||||
# tests.
|
||||
def build_issuer_binding_provider
|
||||
state_holder = {}
|
||||
Provider.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_holder[:state] = URI.decode_www_form(url.query).to_h.fetch("state")
|
||||
},
|
||||
callback_handler: -> { ["test-auth-code", state_holder[:state]] },
|
||||
)
|
||||
end
|
||||
|
||||
def test_run_skips_dcr_when_stored_issuer_matches
|
||||
stub_request(:post, "#{@auth_base}/register").to_raise(StandardError.new("DCR should not be called."))
|
||||
|
||||
provider = build_issuer_binding_provider
|
||||
provider.save_client_information("client_id" => "preregistered-client", "issuer" => @auth_base)
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal("test-token-from-flow", provider.access_token)
|
||||
assert_not_requested(:post, "#{@auth_base}/register")
|
||||
end
|
||||
|
||||
def test_run_reregisters_when_authorization_server_changes
|
||||
provider = build_issuer_binding_provider
|
||||
provider.save_client_information("client_id" => "old-client", "issuer" => "https://other-as.example.com")
|
||||
provider.save_tokens("access_token" => "old-as-token")
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_requested(:post, "#{@auth_base}/register")
|
||||
info = provider.client_information
|
||||
assert_equal("test-client", info["client_id"])
|
||||
assert_equal(@auth_base, info["issuer"])
|
||||
end
|
||||
|
||||
def test_run_clears_stale_tokens_when_authorization_server_changes
|
||||
# Abort the flow right after the discard (DCR fails) to observe that tokens minted by
|
||||
# the old authorization server were wiped before any re-registration attempt.
|
||||
stub_request(:post, "#{@auth_base}/register").to_return(status: 500, body: "")
|
||||
|
||||
provider = build_issuer_binding_provider
|
||||
provider.save_client_information("client_id" => "old-client", "issuer" => "https://other-as.example.com")
|
||||
provider.save_tokens("access_token" => "old-as-token", "refresh_token" => "old-as-rt")
|
||||
|
||||
assert_raises(Flow::AuthorizationError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_nil(provider.tokens)
|
||||
assert_nil(provider.client_information)
|
||||
end
|
||||
|
||||
def test_run_binds_legacy_client_information_to_issuer_on_first_use
|
||||
# Credentials persisted before issuer binding existed (or supplied as pre-registered credentials)
|
||||
# are reused and stamped with the current issuer rather than discarded.
|
||||
stub_request(:post, "#{@auth_base}/register").to_raise(StandardError.new("DCR should not be called."))
|
||||
|
||||
provider = build_issuer_binding_provider
|
||||
provider.save_client_information("client_id" => "preregistered-client")
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_not_requested(:post, "#{@auth_base}/register")
|
||||
assert_equal(@auth_base, provider.client_information["issuer"])
|
||||
end
|
||||
|
||||
def test_run_binds_symbol_keyed_client_information_to_issuer_on_first_use
|
||||
stub_request(:post, "#{@auth_base}/register").to_raise(StandardError.new("DCR should not be called."))
|
||||
|
||||
provider = build_issuer_binding_provider
|
||||
provider.save_client_information(client_id: "preregistered-symbol")
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_not_requested(:post, "#{@auth_base}/register")
|
||||
assert_equal("test-token-from-flow", provider.access_token)
|
||||
assert_equal(@auth_base, provider.client_information["issuer"])
|
||||
end
|
||||
|
||||
def test_run_reuses_portable_cimd_client_id_across_authorization_servers
|
||||
# Per SEP-2352, a CIMD `client_id` (an HTTPS URL) is portable across authorization servers:
|
||||
# reuse it and re-bind the issuer instead of discarding the registration.
|
||||
stub_request(:post, "#{@auth_base}/register").to_raise(StandardError.new("DCR should not be called."))
|
||||
|
||||
provider = build_issuer_binding_provider
|
||||
provider.save_client_information(
|
||||
"client_id" => "https://app.example.com/client-metadata.json",
|
||||
"issuer" => "https://other-as.example.com",
|
||||
)
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_not_requested(:post, "#{@auth_base}/register")
|
||||
info = provider.client_information
|
||||
assert_equal("https://app.example.com/client-metadata.json", info["client_id"])
|
||||
assert_equal(@auth_base, info["issuer"])
|
||||
end
|
||||
|
||||
def test_run_persists_issuer_with_dcr_client_information
|
||||
provider = build_issuer_binding_provider
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_requested(:post, "#{@auth_base}/register")
|
||||
info = provider.client_information
|
||||
assert_equal("test-client", info["client_id"])
|
||||
assert_equal(@auth_base, info["issuer"])
|
||||
end
|
||||
|
||||
def test_run_skips_dcr_when_as_supports_cimd_and_provider_has_cimd_url
|
||||
# When the AS advertises Client ID Metadata Document support and the provider was configured with a CIMD URL,
|
||||
# the SDK uses the URL as the OAuth `client_id` and does not call /register.
|
||||
|
|
@ -1634,6 +1780,55 @@ module MCP
|
|||
assert_equal("saved-rt", provider.tokens["refresh_token"])
|
||||
end
|
||||
|
||||
def test_refresh_succeeds_when_stored_issuer_matches
|
||||
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: "fresh-at", token_type: "Bearer", expires_in: 3600),
|
||||
)
|
||||
|
||||
provider = Provider.new(
|
||||
client_metadata: { redirect_uris: ["http://localhost:0/callback"] },
|
||||
redirect_uri: "http://localhost:0/callback",
|
||||
redirect_handler: ->(_url) {},
|
||||
callback_handler: -> { [nil, nil] },
|
||||
)
|
||||
provider.save_client_information("client_id" => "test-client", "issuer" => @auth_base)
|
||||
provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt")
|
||||
|
||||
result = Flow.new(provider: provider).refresh!(
|
||||
server_url: @server_url,
|
||||
resource_metadata_url: @prm_url,
|
||||
)
|
||||
|
||||
assert_equal(:refreshed, result)
|
||||
assert_equal("fresh-at", provider.access_token)
|
||||
end
|
||||
|
||||
def test_refresh_raises_when_stored_issuer_differs
|
||||
# Per SEP-2352, another authorization server's credentials must not be replayed at this token endpoint;
|
||||
# the caller falls back to the full flow, which discards the stale registration and re-registers.
|
||||
stub_request(:post, "#{@auth_base}/token").to_raise(StandardError.new("Token endpoint should not be called."))
|
||||
|
||||
provider = Provider.new(
|
||||
client_metadata: { redirect_uris: ["http://localhost:0/callback"] },
|
||||
redirect_uri: "http://localhost:0/callback",
|
||||
redirect_handler: ->(_url) {},
|
||||
callback_handler: -> { [nil, nil] },
|
||||
)
|
||||
provider.save_client_information("client_id" => "old-client", "issuer" => "https://other-as.example.com")
|
||||
provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt")
|
||||
|
||||
error = assert_raises(Flow::AuthorizationError) do
|
||||
Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_match(/different authorization server/, error.message)
|
||||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
end
|
||||
|
||||
def test_refresh_uses_cimd_url_as_client_id_when_provider_has_cimd_and_as_supports_it
|
||||
# Regression: the CIMD branch in `ensure_client_registered` does not persist `client_information`,
|
||||
# so a refresh call that requires stored client info would always fail after a CIMD-only flow.
|
||||
|
|
|
|||
Loading…
Reference in a new issue