From 75c6c24a92a7a16d0011d94f89e63d0f381116da Mon Sep 17 00:00:00 2001 From: Ates Goral Date: Wed, 7 Oct 2026 11:45:57 -0400 Subject: [PATCH] feat: add OAuth scope selector for authorization-code clients --- docs/_client/authorization.md | 35 ++- lib/mcp/client/oauth/flow.rb | 89 ++++-- lib/mcp/client/oauth/provider.rb | 11 + test/mcp/client/oauth/flow_test.rb | 378 ++++++++++++++++++++++- test/mcp/client/oauth/http_oauth_test.rb | 5 +- test/mcp/client/oauth/provider_test.rb | 19 ++ 6 files changed, 498 insertions(+), 39 deletions(-) diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index 436332e4..64006523 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 resolved challenge, PRM, and provider scopes. ```ruby require "mcp" @@ -99,7 +99,22 @@ 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. + Malformed PRM tokens raise `Flow::AuthorizationError` before the callback. It 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 and is included in the validator's scope list. Repeated endpoint `scope` + parameters that would survive are rejected before registration. The authorization server can also apply defaults or reject + the request ([RFC 6749 Section 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 authorization server 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. @@ -436,8 +451,8 @@ provider = MCP::Client::OAuth::Provider.new( ) ``` -The argument is an `MCP::Client::OAuth::AuthorizationRequest` carrying `authorization_server` (the selected issuer), `scopes` (an Array, empty when neither -the challenge nor the metadata named any), `server_url`, and `resource`. It is one object rather than keyword arguments so that later revisions of the specification +The argument is an `MCP::Client::OAuth::AuthorizationRequest` carrying `authorization_server` (the selected issuer), `scopes` (an Array of explicit requested scope tokens), +`server_url`, and `resource`. It is one object rather than keyword arguments so that later revisions of the specification can add to it without changing the shape you wrote. Only the named readers are the contract. The positional access a `Struct` also happens to provide (`request[0]`, `to_a`, `each`) is not, and can break when the representation changes. @@ -450,7 +465,10 @@ Compare the issuer as a whole string, the way the SDK compares it everywhere els and a legacy authorization server whose metadata never named an issuer arrives as `nil`, which an exact comparison refuses instead of raising. The provider is only half of the decision. The MCP server chose the scopes too, so a request naming a provider you allow can still ask for more than that server has -any business asking for. `scopes` rides on the request so that a host with a policy per server can apply it: +any business asking for. `scopes` rides on the request so that a host with a policy per server can apply it. On the authorization-code flow, +this includes any single prefilled endpoint scope that survives when the flow has no scope of its own, including after a selector returns `[]`. +The query names are decoded the same way as the URL builder; repeated `scope` parameters that would survive are rejected before registration. +The following policy therefore checks the explicit scopes that the authorization URL will send: ```ruby ALLOWED_SCOPES = { "https://api.example.com/mcp" => ["mcp:read", "mcp:write"] } @@ -463,6 +481,9 @@ provider = MCP::Client::OAuth::Provider.new( ) ``` +In this example, an empty `request.scopes` approves a request with no explicit scope tokens. This is not a ceiling on what the authorization server may grant: +it can still apply defaults, so inspect the granted token scopes before treating a connection as unscoped. + A host with a user to ask can put the decision to them instead. The request carries what such a prompt has to name: the provider, the scopes, and the server that asked for them. It runs on all three grants, after that server's metadata has been fetched (which is where the validated issuer comes from) and before any registration, credential, diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index c79a271e..ddf81648 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,12 +267,14 @@ 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) + endpoint_uri, endpoint_params = authorization_endpoint_parameters(as_metadata: as_metadata) + request_scope = authorization_request_scope(scope: effective_scope, endpoint_params: endpoint_params) # Asked before registering, not after: a refusal must not leave this client registered at an authorization server - # the embedding application has just rejected. - authorize_request!(as_metadata: as_metadata, scope: effective_scope, server_url: server_url, resource: resource) + # the embedding application has just rejected. Use the scopes that will actually reach the browser URL. + authorize_request!(as_metadata: as_metadata, scope: request_scope, server_url: server_url, resource: resource) client_info = ensure_client_registered(as_metadata: as_metadata) @@ -277,7 +282,8 @@ def run!(server_url:, resource_metadata_url: nil, scope: nil) state = SecureRandom.urlsafe_base64(32) authorization_url = build_authorization_url( - as_metadata: as_metadata, + endpoint_uri: endpoint_uri, + endpoint_params: endpoint_params, client_id: client_info_required_value(client_info, "client_id"), scope: effective_scope, state: state, @@ -850,16 +856,16 @@ def ensure_same_origin!(url, label:, server_url:) # Hands the embedding application the authorization server and the scopes that are about to be requested, # and abandons the flow when it refuses them. # - # Both values are chosen by the MCP server: it names its own authorization server in Protected - # Resource Metadata and states the scopes in `scopes_supported` or the `WWW-Authenticate` challenge. + # The MCP server names its authorization server in Protected Resource Metadata and states scopes + # in `scopes_supported` or the `WWW-Authenticate` challenge. Authorization-code policy also sees + # any prefilled endpoint scope that will survive URL assembly. # Neither the specification nor any MCP SDK binds that choice to the server's own identity, # and validating that a token was issued for the intended audience is a responsibility the specification # 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 +1308,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? @@ -1334,7 +1341,8 @@ def resolve_scope(scope:, prm:) # the authorization request even though the AS will not honour it. Stripping here keeps the SDK's # own request consistent with the AS's advertisement. # - # Returns `nil` when the result is empty so `build_authorization_url` omits the `scope` parameter entirely. + # Returns `nil` when empty so the URL builder adds no flow-owned scope parameter; a prefilled scope + # can still survive and is checked by the authorization-request validator. # https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2207 def normalize_offline_access_scope(scope, as_metadata:) scopes = scope.to_s.split @@ -1354,6 +1362,28 @@ 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 + + unless scopes.all? { |token| token.is_a?(String) && SCOPE_TOKEN_FORMAT.match?(token) } + raise AuthorizationError, "Protected Resource Metadata `scopes_supported` contains invalid OAuth scope tokens." + end + + candidates = scopes.map { |token| token.dup.freeze }.freeze + candidate_index = candidates.to_h { |token| [token, true] } + selected = selector.call(candidates) + valid = selected.is_a?(Array) && selected.all? do |token| + token.is_a?(String) && SCOPE_TOKEN_FORMAT.match?(token) && candidate_index.key?(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"] @@ -1398,7 +1428,7 @@ def provider_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:) + def authorization_endpoint_parameters(as_metadata:) authorization_endpoint = as_metadata["authorization_endpoint"] unless authorization_endpoint raise AuthorizationError, @@ -1412,6 +1442,23 @@ def build_authorization_url(as_metadata:, client_id:, scope:, state:, code_chall "Authorization server metadata `authorization_endpoint` is not a valid URI: #{e.message}." end + [uri, URI.decode_www_form(uri.query.to_s)] + end + + # A flow scope replaces every endpoint scope; otherwise a single prefilled value survives. + # Decode once before policy approval and reuse those parameters when building the URL. + def authorization_request_scope(scope:, endpoint_params:) + return scope if scope + + endpoint_scopes = endpoint_params.filter_map { |name, value| value if name == "scope" } + if endpoint_scopes.length > 1 + raise AuthorizationError, "Authorization endpoint contains repeated `scope` parameters." + end + + endpoint_scopes.first + end + + def build_authorization_url(endpoint_uri:, endpoint_params:, client_id:, scope:, state:, code_challenge:, resource:) # A parameter the flow sets replaces any of the same name the endpoint URL already carries. # RFC 6749 Section 3.1 forbids sending a parameter twice, and which of two values a server would honor is # its own choice; on the legacy path the endpoint URL is served by the MCP server, whose query must not speak @@ -1433,10 +1480,10 @@ def build_authorization_url(as_metadata:, client_id:, scope:, state:, code_chall own_params << ["resource", resource] if resource dropped_names = own_params.map(&:first) + ["request", "request_uri"] - params = URI.decode_www_form(uri.query.to_s).reject { |name, _value| dropped_names.include?(name) } - uri.query = URI.encode_www_form(params + own_params) + params = endpoint_params.reject { |name, _value| dropped_names.include?(name) } + endpoint_uri.query = URI.encode_www_form(params + own_params) - uri + endpoint_uri end def exchange_authorization_code(as_metadata:, client_info:, code:, code_verifier:, resource:, redirect_uri: @provider.redirect_uri) 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..cb78282d 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 authorization server 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,111 @@ 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_preserves_and_reports_prefilled_endpoint_scope + stub_scope_selector_prm(["mcp:read", "mcp:write"]) + ["scope=admin&audience=api", "sc%6Fpe=admin&audience=api"].each do |endpoint_query| + observed_scopes = nil + query = authorization_url_query_for_endpoint_query( + endpoint_query, + scope_selector: ->(_candidates) { [] }, + validator: ->(request) { + observed_scopes = request.scopes + true + }, + ) + + assert_equal(["admin"], observed_scopes) + assert_equal(["admin"], query.filter_map { |name, value| value if name == "scope" }) + assert_equal("api", query.to_h["audience"]) + end + end + + def test_run_validator_refuses_prefilled_scope_after_empty_prm_selection + stub_scope_selector_prm(["mcp:read", "mcp:write"]) + ["scope=admin", "sc%6Fpe=admin"].each do |endpoint_query| + observed_scopes = nil + assert_raises(Flow::AuthorizationRefusedError) do + authorization_url_query_for_endpoint_query( + endpoint_query, + scope_selector: ->(_candidates) { [] }, + validator: ->(request) { + observed_scopes = request.scopes + request.scopes.all? { |scope| ["mcp:read", "mcp:write"].include?(scope) } + }, + ) + end + + assert_equal(["admin"], observed_scopes) + end + assert_not_requested(:post, "#{@auth_base}/register") + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_validator_refuses_prefilled_scope_without_prm_defaults + ["scope=admin", "sc%6Fpe=admin"].each do |endpoint_query| + observed_scopes = nil + assert_raises(Flow::AuthorizationRefusedError) do + authorization_url_query_for_endpoint_query( + endpoint_query, + validator: ->(request) { + observed_scopes = request.scopes + request.scopes.all? { |scope| ["mcp:read", "mcp:write"].include?(scope) } + }, + ) + end + + assert_equal(["admin"], observed_scopes) + end + assert_not_requested(:post, "#{@auth_base}/register") + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_flow_scope_replaces_repeated_endpoint_scopes_before_validation + stub_scope_selector_prm(["mcp:read", "mcp:write"]) + [nil, ->(scopes) { scopes & ["mcp:read"] }].each do |selector| + observed_scopes = nil + query = authorization_url_query_for_endpoint_query( + "scope=admin&sc%6Fpe=write&audience=api", + scope_selector: selector, + validator: ->(request) { + observed_scopes = request.scopes + true + }, + ) + + expected = selector ? ["mcp:read"] : ["mcp:read", "mcp:write"] + assert_equal(expected, observed_scopes) + assert_equal([expected.join(" ")], query.filter_map { |name, value| value if name == "scope" }) + assert_equal("api", query.to_h["audience"]) + end + end + + def test_run_rejects_repeated_surviving_scope_before_validation_and_registration + stub_scope_selector_prm(["mcp:read", "mcp:write"]) + ["scope=admin&scope=write", "scope=admin&sc%6Fpe=write", "scope=&scope="].each do |endpoint_query| + error = assert_raises(Flow::AuthorizationError) do + authorization_url_query_for_endpoint_query( + endpoint_query, + scope_selector: ->(_candidates) { [] }, + validator: ->(_request) { flunk("ambiguous scope must fail before validation") }, + ) + end + assert_equal("Authorization endpoint contains repeated `scope` parameters.", error.message) + end + assert_not_requested(:post, "#{@auth_base}/register") + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_rejects_repeated_scope_without_prm_defaults_or_validator + error = assert_raises(Flow::AuthorizationError) do + authorization_url_query_for_endpoint_query("scope=admin&sc%6Fpe=write") + end + + assert_equal("Authorization endpoint contains repeated `scope` parameters.", error.message) + assert_not_requested(:post, "#{@auth_base}/register") + assert_not_requested(:post, "#{@auth_base}/token") + end + def test_run_raises_when_prm_resource_is_malformed_uri stub_request(:get, @prm_url).to_return( status: 200, @@ -4157,6 +4288,233 @@ 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_large_subset_does_not_scan_the_candidate_array + scopes = Array.new(4096) { |index| "scope#{index}" } + provider = Provider.new( + **authorization_code_provider_arguments(->(_url) {}, -> { [nil, nil] }), + scope_selector: ->(values) { values }, + ) + flow = Flow.new(provider: provider) + array_scans = 0 + trace = TracePoint.new(:c_call) do |event| + array_scans += 1 if event.defined_class == Array && event.method_id == :include? + end + selected_scope = nil + trace.enable do + selected_scope = flow.send(:resolve_scope, scope: nil, prm: { "scopes_supported" => scopes }, select_prm_scope: true) + end + + assert_equal(scopes.join(" "), selected_scope) + assert_equal(0, array_scans) + 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_malformed_prm_candidates_before_callback + ["mcp read", "lesen:\u00E4", "mcp:\"read\"", "", 123, nil].each do |malformed| + stub_scope_selector_prm(["mcp:read", malformed]) + selector_called = false + error = assert_raises(Flow::AuthorizationError) do + capture_authorization_scope( + grant_types: ["authorization_code"], + scope_selector: ->(values) { + selector_called = true + values + }, + ) + end + + assert_equal("Protected Resource Metadata `scopes_supported` contains invalid OAuth scope tokens.", error.message) + refute(selector_called) + end + assert_not_requested(:post, "#{@auth_base}/register") + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_no_selector_keeps_existing_prm_resolution_without_new_token_validation + scopes = ["mcp:read", "mcp read", 123, nil] + provider = Provider.new(**authorization_code_provider_arguments(->(_url) {}, -> { [nil, nil] })) + flow = Flow.new(provider: provider) + + assert_equal(scopes.join(" "), flow.send(:resolve_scope, scope: nil, prm: { "scopes_supported" => scopes }, select_prm_scope: true)) + 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 +4843,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 +4863,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,