diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cff0c26..61f3b194 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Validate RFC 9207 `iss` authorization response parameters in OAuth client flows per SEP-2468 + ### Deprecated - Annotate Roots, Sampling, and Logging APIs as deprecated per SEP-2577 (#429) diff --git a/README.md b/README.md index b986c262..feba2698 100644 --- a/README.md +++ b/README.md @@ -2075,6 +2075,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. +- Validate the URL-form-decoded RFC 9207 `iss` parameter from authorization responses before exchanging the authorization code. + Any returned `iss` must exactly match the discovered issuer, even when the authorization server has not advertised support. + If the server advertises `authorization_response_iss_parameter_supported: true`, the callback must return the `iss` value. - 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. @@ -2097,7 +2100,10 @@ provider = MCP::Client::OAuth::Provider.new( }, callback_handler: -> { # Capture the redirect (for example, by running a small HTTP listener on - # `redirect_uri`) and return [code, state] from the query string. + # `redirect_uri`) and return URL-form-decoded [code, state, iss] values + # from the query string. + # The `iss` value is optional unless the authorization server advertises + # `authorization_response_iss_parameter_supported: true`. }, ) @@ -2118,7 +2124,8 @@ Required keyword arguments to `Provider.new`: an explicit value always wins. - `redirect_uri`: String. Must use HTTPS or be a loopback URL (`localhost`, `127.0.0.0/8`, `::1`); other values raise `Provider::InsecureRedirectURIError`. - `redirect_handler`: Callable invoked with the fully-built authorization `URI`. Typically opens the user's browser. -- `callback_handler`: Callable that returns `[code, state]` after the user is redirected back to `redirect_uri`. +- `callback_handler`: Callable that returns `[code, state]` or `[code, state, iss]` after the user is redirected back to `redirect_uri`. + Return URL-form-decoded query values, including the RFC 9207 `iss` parameter when present; the SDK rejects mismatched values and rejects missing `iss` when the authorization server advertises support. Optional keyword arguments: diff --git a/conformance/client.rb b/conformance/client.rb index c9f3ec5f..e9e9a807 100644 --- a/conformance/client.rb +++ b/conformance/client.rb @@ -95,7 +95,7 @@ def build_oauth_provider(context, scenario:) callback_handler = -> do query = URI.decode_www_form(callback_holder.fetch(:url).query).to_h - [query["code"], query["state"]] + [query["code"], query["state"], query["iss"]] end MCP::Client::OAuth::Provider.new( diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 7236f712..5c0ba285 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -74,13 +74,15 @@ def run!(server_url:, resource_metadata_url: nil, scope: nil) ) @provider.redirect_handler.call(authorization_url) - code, returned_state = Array(@provider.callback_handler.call) + code, returned_state, returned_iss = Array(@provider.callback_handler.call) raise AuthorizationError, "Authorization callback did not return an authorization code." unless code unless states_match?(returned_state, state) raise AuthorizationError, "OAuth state mismatch (CSRF protection)." end + ensure_authorization_response_issuer_matches!(as_metadata: as_metadata, returned: returned_iss) + tokens = exchange_authorization_code( as_metadata: as_metadata, client_info: client_info, @@ -411,6 +413,26 @@ def ensure_issuer_matches!(expected:, returned:) "(expected #{expected.inspect}, got #{returned.inspect})." end + # SEP-2468 / RFC 9207: bind the authorization response to the AS identity. + # The callback handler owns redirect parsing, so `returned` is expected to + # be the URL-form-decoded `iss` value from the authorization response. + def ensure_authorization_response_issuer_matches!(as_metadata:, returned:) + if as_metadata["authorization_response_iss_parameter_supported"] == true && returned.nil? + raise AuthorizationError, + "Authorization server advertises authorization_response_iss_parameter_supported " \ + "but no `iss` parameter was received." + end + + return if returned.nil? + + expected = as_metadata["issuer"] + return if expected.to_s == returned.to_s + + raise AuthorizationError, + "Authorization response `iss` does not match the expected issuer " \ + "(expected #{expected.inspect}, got #{returned.inspect})." + end + def ensure_client_registered(as_metadata:) existing = @provider.client_information return existing if existing.is_a?(Hash) && client_info_required_value(existing, "client_id") diff --git a/lib/mcp/client/oauth/provider.rb b/lib/mcp/client/oauth/provider.rb index 4864ae40..e05a39dc 100644 --- a/lib/mcp/client/oauth/provider.rb +++ b/lib/mcp/client/oauth/provider.rb @@ -21,8 +21,9 @@ module OAuth # - `redirect_handler` - Callable invoked with the fully-built authorization # URL (a `URI`). Implementations typically open the user's browser. # - `callback_handler` - Callable invoked after `redirect_handler`. Returns - # `[code, state]` where `code` is the authorization code and `state` is - # the `state` parameter received on the redirect URI. + # `[code, state]` or `[code, state, iss]` where `code` is the authorization + # code, `state` is the `state` parameter, and `iss` is the optional, + # URL-form-decoded RFC 9207 issuer parameter received on the redirect URI. # # Optional keyword arguments: # - `scope` - String of space-separated scopes to request when the server's diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index b92b9f95..871f6886 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -76,6 +76,23 @@ def client_credentials_provider(token_endpoint_auth_method: "client_secret_basic ) end + def authorization_response_iss_provider(iss) + 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], iss] }, + ) + 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 @@ -1532,6 +1549,74 @@ def test_run_rejects_when_as_metadata_lacks_issuer assert_match(/issuer/i, error.message) end + def test_run_accepts_matching_authorization_response_iss + 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", + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + authorization_response_iss_parameter_supported: true, + ), + ) + + provider = authorization_response_iss_provider(@auth_base) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_equal("test-token-from-flow", provider.access_token) + end + + def test_run_requires_authorization_response_iss_when_advertised + 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", + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + authorization_response_iss_parameter_supported: true, + ), + ) + + provider = authorization_response_iss_provider(nil) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/iss/i, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_accepts_matching_authorization_response_iss_without_advertisement + provider = authorization_response_iss_provider(@auth_base) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_equal("test-token-from-flow", provider.access_token) + end + + def test_run_rejects_authorization_response_iss_mismatch + provider = authorization_response_iss_provider("https://attacker.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(/iss.*does not match/i, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + def test_run_rejects_non_https_authorization_server # Communication Security: an `http://` authorization server URL (other # than loopback) MUST be refused. Returning a non-HTTPS auth server