diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index 436332e4..995f7b3f 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -47,9 +47,9 @@ pass an `MCP::Client::OAuth::Provider` to the transport instead of a static `Aut - 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. -- Request the `offline_access` scope when `client_metadata[:grant_types]` includes `refresh_token` and the authorization server advertises `offline_access` in its metadata - `scopes_supported` (SEP-2207). This is what lets the server issue the `refresh_token` used above. As an SDK-level safeguard, when the authorization server does not advertise - `offline_access` the scope is also stripped from any other source (challenge, PRM, or provider-supplied scope) so a server that does not support it never receives it. +- Request `offline_access` when the client declares the `refresh_token` grant and the authorization server advertises it in + `scopes_supported` (SEP-2207). The PRM scope selector below runs before this augmentation and cannot disable it. + Unsupported `offline_access` is stripped from every scope source. ```ruby require "mcp" @@ -99,7 +99,20 @@ Optional keyword arguments: Omit it when the redirect arrives in a later request, as it does in a web application; see [Authorization in Web Applications](#authorization-in-web-applications). - `pending_authorization_max_age`: Integer seconds a pending authorization stays redeemable, counted from the moment `run!` saves it, when `callback_handler` is omitted. Defaults to 600. -- `scope`: Space-separated scopes to request when the server's `WWW-Authenticate` does not specify one. +- `scope`: Space-separated fallback scopes when neither a challenge nor PRM advertises scopes. +- `scope_selector`: Optional callable for narrowing Protected Resource Metadata (PRM) defaults in the authorization-code flow. + It is invoked only when no nonempty challenged scope was supplied and PRM supplies the default `scopes_supported` list, + before `offline_access` augmentation, request validation, and client registration. It receives a read-only array of PRM tokens + and must return an array containing a subset; custom scopes and `nil` results raise `ArgumentError`. Return `[]` to request + none of those PRM defaults. Challenged scopes, including the step-up union, and provider fallback bypass the selector; + use `authorization_request_validator` to accept or refuse challenged scopes. With no selector, default behavior is unchanged. + This hook does not control `offline_access` augmentation or alter authorization-endpoint query parameters. Returning `[]` + does not guarantee an omitted `scope` parameter: refresh policy can add `offline_access`, and a prefilled endpoint scope + survives when the flow has no scope of its own. The AS can also apply defaults or reject the request + ([RFC 6749 ยง3.3](https://www.rfc-editor.org/rfc/rfc6749#section-3.3)); inspect the granted scopes. + For example, with PRM defaults `mcp:read mcp:write`, `scope_selector: ->(scopes) { scopes & ["mcp:read"] }` narrows the + default request to `mcp:read`; `scope_selector: ->(_scopes) { [] }` requests no PRM defaults. If the AS supports + `offline_access` and the client declares `refresh_token`, either request still includes `offline_access` afterward. - `authorization_request_validator`: Callable invoked with an `MCP::Client::OAuth::AuthorizationRequest` before any authorization request is built. Returning a falsy value abandons the flow with `Flow::AuthorizationRefusedError`. See [Reviewing the authorization request](#reviewing-the-authorization-request). - `http_client_customizer`: Callable invoked with the Faraday connection the SDK builds for the OAuth flow's own requests, after its defaults and before its origin guard. diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index c79a271e..fe808152 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -19,6 +19,9 @@ class Flow METADATA_DIAGNOSTIC_MAX_LENGTH = 128 METADATA_URL_MAX_LENGTH = 2048 + # RFC 6749 scope-token: visible ASCII except space, double quote, and backslash. + SCOPE_TOKEN_FORMAT = /\A[\x21\x23-\x5B\x5D-\x7E]+\z/.freeze + # Token request parameters the flow sets itself. Its values win over a provider's `token_request_params`, # so a provider naming one of these is refused rather than left believing its value was sent. RESERVED_TOKEN_REQUEST_PARAMS = [ @@ -264,7 +267,7 @@ def run!(server_url:, resource_metadata_url: nil, scope: nil) ensure_pkce_supported!(as_metadata) - effective_scope = resolve_scope(scope: scope, prm: prm) + effective_scope = resolve_scope(scope: scope, prm: prm, select_prm_scope: true) effective_scope = normalize_offline_access_scope(effective_scope, as_metadata: as_metadata) # Asked before registering, not after: a refusal must not leave this client registered at an authorization server @@ -857,9 +860,8 @@ def ensure_same_origin!(url, label:, server_url:) # places on MCP servers rather than on clients. # A host that knows which providers its user deals with can apply that knowledge here. # - # The scopes are passed on unchanged whatever the host decides, because the specification requires - # a client to treat the challenged scopes as authoritative for the operation; the choice offered is - # to proceed or to stop, not to quietly ask for less. A provider without the hook proceeds as before. + # Challenged scopes are authoritative for the operation and bypass the PRM scope selector. + # The validator can accept or refuse them, not quietly request fewer scopes. # # Only asked when a new grant is being requested. A refresh is not a new grant, and the host already answered # this question for that authorization server, so `refresh!` enforces `ensure_token_issuer!` instead: @@ -1302,17 +1304,18 @@ def authorization_response_error(error, description) AuthorizationError.new(message, error: error, error_description: description) end - # Per MCP 2025-11-25 Authorization and the TS/Python SDKs, scope resolution - # prefers the `WWW-Authenticate` challenge first, then `scopes_supported` - # from the Protected Resource Metadata, and falls back to a provider-supplied - # scope only if both are absent. The provider-supplied scope must not pre-empt - # a server-advertised one. - def resolve_scope(scope:, prm:) + # MCP scope selection prefers the challenge, then PRM `scopes_supported`, then the provider's fallback. + # Authorization-code clients may narrow only the PRM default, before `offline_access` augmentation. + def resolve_scope(scope:, prm:, select_prm_scope: false) return scope if scope && !scope.empty? # `prm` is nil on the legacy path, where nothing advertises scopes. supported = prm && prm["scopes_supported"] - return supported.join(" ") if supported.is_a?(Array) && !supported.empty? + if supported.is_a?(Array) && !supported.empty? + return select_prm_scopes(supported) if select_prm_scope + + return supported.join(" ") + end return @provider.scope if @provider.scope && !@provider.scope.empty? @@ -1354,6 +1357,23 @@ def server_supports_offline_access?(as_metadata) supported.is_a?(Array) && supported.include?("offline_access") end + # Selects a subset of the PRM default without changing challenges, provider fallback, or refresh policy. + def select_prm_scopes(scopes) + selector = @provider.scope_selector if @provider.respond_to?(:scope_selector) + return scopes.join(" ") unless selector + + candidates = scopes.map { |token| token.dup.freeze }.freeze + selected = selector.call(candidates) + valid = selected.is_a?(Array) && selected.all? do |token| + token.is_a?(String) && SCOPE_TOKEN_FORMAT.match?(token) && candidates.include?(token) + end + unless valid + raise ArgumentError, "scope_selector must return an Array containing only scopes from PRM scopes_supported." + end + + selected.empty? ? nil : selected.join(" ") + end + def wants_refresh_token? metadata = @provider.client_metadata grant_types = metadata[:grant_types] || metadata["grant_types"] diff --git a/lib/mcp/client/oauth/provider.rb b/lib/mcp/client/oauth/provider.rb index ddeff730..98b9b3fb 100644 --- a/lib/mcp/client/oauth/provider.rb +++ b/lib/mcp/client/oauth/provider.rb @@ -37,6 +37,10 @@ module OAuth # `run!` saves it, when `callback_handler` is omitted. Defaults to `DEFAULT_PENDING_AUTHORIZATION_MAX_AGE`. # - `scope` - String of space-separated scopes to request when the server's # `WWW-Authenticate` does not specify one. + # - `scope_selector` - Callable receiving a read-only Array of PRM `scopes_supported` tokens when + # those defaults are selected without a challenged scope. Return an Array containing a subset; + # `[]` requests none of the PRM defaults. Challenged scopes and provider fallback bypass it. + # The existing `offline_access` policy runs afterward; endpoint query parameters are unchanged. # - `storage` - Object responding to `tokens`, `save_tokens(tokens)`, # `client_information`, and `save_client_information(info)`. Defaults to # an `InMemoryStorage`. Persisted `client_information` is stamped with @@ -108,6 +112,7 @@ class PendingAuthorizationStorageError < ArgumentError; end attr_reader :client_metadata, :redirect_uri, :scope, + :scope_selector, :storage, :redirect_handler, :callback_handler, @@ -120,6 +125,7 @@ def initialize( redirect_handler:, callback_handler: nil, scope: nil, + scope_selector: nil, storage: nil, client_id_metadata_document_url: nil, authorization_request_validator: nil, @@ -147,6 +153,10 @@ def initialize( "per the MCP authorization specification and `draft-ietf-oauth-client-id-metadata-document`." end + unless scope_selector.nil? || scope_selector.respond_to?(:call) + raise ArgumentError, "scope_selector must respond to call (got #{scope_selector.class})." + end + http_client_customizer = validated_http_client_customizer(http_client_customizer) unless pending_authorization_max_age.is_a?(Integer) && pending_authorization_max_age.positive? @@ -170,6 +180,7 @@ def initialize( @redirect_handler = redirect_handler @callback_handler = callback_handler @scope = scope + @scope_selector = scope_selector @storage = storage @client_id_metadata_document_url = client_id_metadata_document_url @authorization_request_validator = authorization_request_validator diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index 15cf8dc2..3666512e 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -254,11 +254,10 @@ def call(env) end end - # Runs the full authorization flow and returns the `scope` query parameter - # sent on the authorization request. The caller stubs the AS metadata; - # this helper supplies a provider whose `grant_types` and optional pre-set - # `scope` drive the SEP-2207 offline_access decision. - def capture_authorization_scope(grant_types:, provider_scope: nil) + # Returns the authorization URL's `scope` query parameter from a full flow. + # The caller stubs AS metadata; grant types and selection decide whether + # `offline_access` is added automatically. + def capture_authorization_scope(grant_types:, provider_scope: nil, scope_selector: nil, requested_scope: nil) captured_scope = nil state_holder = {} provider = Provider.new( @@ -276,9 +275,14 @@ def capture_authorization_scope(grant_types:, provider_scope: nil) }, callback_handler: -> { ["test-auth-code", state_holder[:state]] }, scope: provider_scope, + scope_selector: scope_selector, ) - Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + Flow.new(provider: provider).run!( + server_url: @server_url, + resource_metadata_url: @prm_url, + scope: requested_scope, + ) captured_scope end @@ -1123,7 +1127,7 @@ def run_authorization_flow(redirect_uri: "http://localhost:0/callback", client_m # Runs the authorization-code flow with an `authorization_request_validator` that records what it # was handed and answers `approve`. - private def run_flow_with_validator(approve:, recorder: []) + private def run_flow_with_validator(approve:, recorder: [], scope_selector: nil, requested_scope: nil) state_holder = {} provider = Provider.new( client_metadata: { @@ -1142,9 +1146,10 @@ def run_authorization_flow(redirect_uri: "http://localhost:0/callback", client_m recorder << request approve }, + scope_selector: scope_selector, ) - Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url, scope: requested_scope) end def test_run_hands_the_authorization_server_and_scopes_to_the_validator @@ -1169,6 +1174,27 @@ def test_run_hands_the_authorization_server_and_scopes_to_the_validator assert_equal("https://srv.example.com/mcp", request.resource) end + def test_run_validates_the_selected_scopes_before_client_registration + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate( + resource: @server_url, + authorization_servers: [@auth_base], + scopes_supported: ["mcp:read", "admin"], + ), + ) + + recorder = [] + assert_raises(Flow::AuthorizationRefusedError) do + run_flow_with_validator(approve: false, recorder: recorder, scope_selector: ->(_candidates) { [] }) + end + + assert_empty(recorder.first.scopes) + assert_not_requested(:post, "#{@auth_base}/register") + refute_includes(recorder, :redirected) + end + def test_run_refuses_the_flow_and_registers_nothing_when_the_validator_declines recorder = [] @@ -1618,6 +1644,34 @@ def test_run_keeps_an_endpoint_scope_when_the_flow_has_none assert_equal("openid", query.to_h["scope"]) end + def test_run_scope_selector_does_not_remove_prefilled_endpoint_scopes + stub_scope_selector_prm + [ + ["scope=admin&scope=write&audience=api", ["admin", "write"]], + ["sc%6Fpe=admin&audience=api", ["admin"]], + ].each do |endpoint_query, endpoint_scopes| + observed_scopes = nil + query = authorization_url_query_for_endpoint_query( + endpoint_query, + scope_selector: ->(_candidates) { [] }, + validator: ->(request) { + observed_scopes = request.scopes + true + }, + ) + + assert_empty(observed_scopes) + assert_equal(endpoint_scopes, query.filter_map { |name, value| value if name == "scope" }) + assert_equal("api", query.to_h["audience"]) + end + + replaced = authorization_url_query_for_endpoint_query( + "scope=admin&scope=write&audience=api", + scope_selector: ->(_candidates) { ["mcp:read"] }, + ) + assert_equal(["mcp:read"], replaced.filter_map { |name, value| value if name == "scope" }) + end + def test_run_raises_when_prm_resource_is_malformed_uri stub_request(:get, @prm_url).to_return( status: 200, @@ -4157,6 +4211,184 @@ def test_resolve_scope_prefers_prm_scopes_supported_over_provider_scope ) end + private def stub_scope_selector_prm(scopes = ["mcp:read", "mcp:write", "admin"]) + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate( + resource: @server_url, + authorization_servers: [@auth_base], + scopes_supported: scopes, + ), + ) + end + + private def stub_scope_selector_offline_access + stub_request(:get, @as_metadata_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate( + issuer: @auth_base, + authorization_endpoint: "#{@auth_base}/authorize", + token_endpoint: "#{@auth_base}/token", + registration_endpoint: "#{@auth_base}/register", + response_types_supported: ["code"], + grant_types_supported: ["authorization_code", "refresh_token"], + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + scopes_supported: ["mcp:read", "mcp:write", "admin", "offline_access"], + ), + ) + end + + def test_scope_selector_receives_prm_defaults_before_offline_access_augmentation + stub_scope_selector_prm + stub_scope_selector_offline_access + candidates = nil + scope = capture_authorization_scope( + grant_types: ["authorization_code", "refresh_token"], + scope_selector: ->(values) { + candidates = values + [] + }, + ) + + assert_equal(["mcp:read", "mcp:write", "admin"], candidates) + assert_predicate(candidates, :frozen?) + candidates.each { |token| assert_predicate(token, :frozen?) } + assert_equal("offline_access", scope) + + empty_challenge_scope = capture_authorization_scope( + grant_types: ["authorization_code", "refresh_token"], + scope_selector: ->(_values) { [] }, + requested_scope: "", + ) + assert_equal("offline_access", empty_challenge_scope) + + narrowed_scope = capture_authorization_scope( + grant_types: ["authorization_code", "refresh_token"], + scope_selector: ->(values) { values & ["mcp:read"] }, + ) + assert_equal("mcp:read offline_access", narrowed_scope) + end + + def test_scope_selector_empty_subset_requests_no_prm_scopes_without_refresh_grant + stub_scope_selector_prm + stub_scope_selector_offline_access + scope = capture_authorization_scope( + grant_types: ["authorization_code"], + provider_scope: "provider-fallback", + scope_selector: ->(_values) { [] }, + ) + + assert_nil(scope) + end + + def test_scope_selector_cannot_disable_the_unsupported_offline_access_safeguard + stub_scope_selector_prm(["mcp:read", "offline_access"]) + scope = capture_authorization_scope( + grant_types: ["authorization_code", "refresh_token"], + scope_selector: ->(values) { values }, + ) + + assert_equal("mcp:read", scope) + end + + def test_scope_selector_filters_only_prm_defaults + stub_scope_selector_prm + scope = capture_authorization_scope( + grant_types: ["authorization_code"], + provider_scope: "provider-fallback", + scope_selector: ->(values) { values & ["mcp:read"] }, + ) + + assert_equal("mcp:read", scope) + end + + def test_scope_selector_skips_nonempty_requested_scopes_before_normalization + stub_scope_selector_prm + [["mcp:write", "mcp:write"], ["offline_access", nil], [" ", nil]].each do |requested, expected| + scope = capture_authorization_scope( + grant_types: ["authorization_code"], + provider_scope: "provider-fallback", + scope_selector: ->(_values) { flunk("explicit scopes must bypass the selector") }, + requested_scope: requested, + ) + + expected ? assert_equal(expected, scope) : assert_nil(scope) + end + end + + def test_scope_selector_skips_provider_fallback_when_prm_defaults_are_absent_or_empty + [nil, []].each do |scopes| + stub_scope_selector_prm(scopes) + scope = capture_authorization_scope( + grant_types: ["authorization_code"], + provider_scope: "provider-fallback", + scope_selector: ->(_values) { flunk("provider fallback must bypass the selector") }, + ) + + assert_equal("provider-fallback", scope) + end + end + + def test_scope_selector_skips_legacy_fallback_and_non_authorization_code_resolution + provider = Provider.new( + **authorization_code_provider_arguments(->(_url) {}, -> { [nil, nil] }), + scope: "provider-fallback", + scope_selector: ->(_values) { flunk("this scope source must bypass the selector") }, + ) + flow = Flow.new(provider: provider) + + assert_equal("provider-fallback", flow.send(:resolve_scope, scope: nil, prm: nil, select_prm_scope: true)) + assert_equal("mcp:read", flow.send(:resolve_scope, scope: nil, prm: { "scopes_supported" => ["mcp:read"] })) + end + + def test_scope_selector_cannot_mutate_the_prm_candidates + stub_scope_selector_prm + scope = capture_authorization_scope( + grant_types: ["authorization_code"], + scope_selector: ->(values) { + assert_raises(FrozenError) { values << "custom:read" } + assert_raises(FrozenError) { values.first.replace("custom:read") } + values & ["mcp:read"] + }, + ) + + assert_equal("mcp:read", scope) + end + + def test_scope_selector_rejects_invalid_results_and_non_subsets_before_registration + stub_scope_selector_prm + [nil, "mcp:read", ["mcp:read admin"], [""], [123], ["custom:read"], ["offline_access"]].each do |selection| + error = assert_raises(ArgumentError) do + capture_authorization_scope( + grant_types: ["authorization_code"], + scope_selector: ->(_candidates) { selection }, + ) + end + assert_equal("scope_selector must return an Array containing only scopes from PRM scopes_supported.", error.message) + end + + assert_not_requested(:post, "#{@auth_base}/register") + end + + def test_challenged_scopes_reach_the_validator_without_selection + recorder = [] + assert_raises(Flow::AuthorizationRefusedError) do + run_flow_with_validator( + approve: false, + recorder: recorder, + scope_selector: ->(_values) { flunk("challenged scopes must bypass the selector") }, + requested_scope: "mcp:write", + ) + end + + assert_equal(["mcp:write"], recorder.first.scopes) + assert_not_requested(:post, "#{@auth_base}/register") + refute_includes(recorder, :redirected) + end + def test_resolve_scope_falls_back_to_provider_scope_when_prm_omits_scopes_supported captured = nil provider = Provider.new( @@ -4485,7 +4717,7 @@ def test_run_sends_server_url_as_resource_when_prm_omits_it # Serves authorization server metadata whose `authorization_endpoint` carries `endpoint_query`, # runs the authorization-code flow to completion, and returns the query of the URL the browser was # sent to as name/value pairs in order. - def authorization_url_query_for_endpoint_query(endpoint_query) + def authorization_url_query_for_endpoint_query(endpoint_query, scope_selector: nil, validator: nil) stub_request(:get, @as_metadata_url).to_return( status: 200, headers: { "Content-Type" => "application/json" }, @@ -4505,6 +4737,8 @@ def authorization_url_query_for_endpoint_query(endpoint_query) ->(url) { holder[:authorization_url] = url }, -> { ["test-auth-code", URI.decode_www_form(holder[:authorization_url].query).to_h.fetch("state")] }, ), + scope_selector: scope_selector, + authorization_request_validator: validator, ) result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) diff --git a/test/mcp/client/oauth/http_oauth_test.rb b/test/mcp/client/oauth/http_oauth_test.rb index 0a3cd384..2da8a74a 100644 --- a/test/mcp/client/oauth/http_oauth_test.rb +++ b/test/mcp/client/oauth/http_oauth_test.rb @@ -418,7 +418,7 @@ def test_send_request_step_up_unions_existing_scope_with_challenge_scope ) stub_step_up_authorization_server - provider = build_step_up_provider + provider = build_step_up_provider(scope_selector: ->(_scopes) { flunk("step-up scopes must bypass the selector") }) # The provider already holds a token granted for `mcp:read`. provider.save_tokens( "access_token" => "initial-token", @@ -1986,7 +1986,7 @@ def stub_step_up_authorization_server(extra_as_metadata: {}) end end - def build_step_up_provider(grant_types: ["authorization_code"], client_id_metadata_document_url: nil) + def build_step_up_provider(grant_types: ["authorization_code"], client_id_metadata_document_url: nil, scope_selector: nil) state_holder = {} captured_authorization_url = nil provider = Provider.new( @@ -2004,6 +2004,7 @@ def build_step_up_provider(grant_types: ["authorization_code"], client_id_metada }, callback_handler: -> { ["test-auth-code", state_holder[:state]] }, client_id_metadata_document_url: client_id_metadata_document_url, + scope_selector: scope_selector, ) provider.save_tokens("access_token" => "initial-token", "token_type" => "Bearer") diff --git a/test/mcp/client/oauth/provider_test.rb b/test/mcp/client/oauth/provider_test.rb index 312bfdb7..d99857bc 100644 --- a/test/mcp/client/oauth/provider_test.rb +++ b/test/mcp/client/oauth/provider_test.rb @@ -58,6 +58,25 @@ def test_initialize_rejects_a_non_callable_http_client_customizer assert_equal("http_client_customizer must respond to call (got Object).", error.message) end + def test_initialize_accepts_optional_scope_selector + default_provider = Provider.new(**args_for("https://app.example.com/callback")) + selector = ->(_candidates) { [] } + provider = Provider.new(**args_for("https://app.example.com/callback"), scope_selector: selector) + + assert_nil(default_provider.scope_selector) + assert_same(selector, provider.scope_selector) + end + + def test_initialize_rejects_a_non_callable_scope_selector + ["mcp:read", false].each do |selector| + error = assert_raises(ArgumentError) do + Provider.new(**args_for("https://app.example.com/callback"), scope_selector: selector) + end + + assert_equal("scope_selector must respond to call (got #{selector.class}).", error.message) + end + end + def test_initialize_rejects_non_loopback_http_redirect_uri # Communication Security: a non-loopback `http://` redirect URI would # let an attacker steal the authorization code from a network sniffer,