diff --git a/README.md b/README.md index 31faa245..d379b461 100644 --- a/README.md +++ b/README.md @@ -2029,6 +2029,8 @@ Required keyword arguments to `Provider.new`: - `client_metadata`: Hash sent to the authorization server's Dynamic Client Registration endpoint. Must include `redirect_uris`, `grant_types`, `response_types`, `token_endpoint_auth_method`. `redirect_uri` (below) must appear in this list, otherwise the constructor raises `Provider::UnregisteredRedirectURIError`. + When `application_type` is omitted, the SDK infers `"native"` or `"web"` from `redirect_uris` per SEP-837 before registering (loopback or custom-scheme URIs are native); + 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`. diff --git a/lib/mcp/client/oauth/discovery.rb b/lib/mcp/client/oauth/discovery.rb index d7ab4a9a..d1540b94 100644 --- a/lib/mcp/client/oauth/discovery.rb +++ b/lib/mcp/client/oauth/discovery.rb @@ -237,6 +237,23 @@ def client_id_metadata_document_url?(url) false end + # Infers the OIDC Dynamic Client Registration `application_type` for a client from its `redirect_uris`. + # Per SEP-837, MCP clients MUST specify an appropriate application type during Dynamic Client Registration + # so the authorization server can apply the matching redirect URI policy. + # + # Returns `"native"` when every redirect URI is a native-app URI: a custom non-http(s) scheme (RFC 8252 Section 7.1) + # or an http(s) URI whose host is a loopback address (`localhost`, `127.0.0.0/8`, or `::1`, RFC 8252 Section 7.3). + # Returns `"web"` otherwise, including when `redirect_uris` is nil, empty, or contains an unparseable URI. + # + # - https://github.com/modelcontextprotocol/modelcontextprotocol/pull/837 + # - https://openid.net/specs/openid-connect-registration-1_0.html#ClientMetadata + def infer_application_type(redirect_uris) + uris = Array(redirect_uris) + return "web" if uris.empty? + + uris.all? { |uri| native_redirect_uri?(uri) } ? "native" : "web" + end + # Like `canonicalize_url` but also strips query string, fragment, and # userinfo. This variant is used for identity comparison against # the request URL Faraday actually sends, which differs from the value @@ -345,6 +362,20 @@ def parse_ip_address(candidate) nil end + # A redirect URI counts as native when it uses a custom non-http(s) scheme + # (e.g. `com.example.app:/callback`) or when it is an http(s) URI whose host is + # a loopback address. A URI without a scheme or one that fails to parse is not native. + def native_redirect_uri?(url) + uri = URI.parse(url.to_s) + scheme = uri.scheme&.downcase + return false if scheme.nil? + return loopback_host?(uri.host) if ["http", "https"].include?(scheme) + + true + rescue URI::InvalidURIError + false + end + def base_url(uri) port_part = uri.port && uri.port != uri.default_port ? ":#{uri.port}" : "" "#{uri.scheme}://#{uri.host}#{port_part}" diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index ed76ee59..54ab17ae 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -367,7 +367,7 @@ def ensure_client_registered(as_metadata:) end response = begin - http_post_json(registration_endpoint, @provider.client_metadata) + http_post_json(registration_endpoint, registration_client_metadata) rescue Faraday::Error => e raise AuthorizationError, "Dynamic client registration failed: #{e.class}: #{e.message}." @@ -393,6 +393,20 @@ def ensure_client_registered(as_metadata:) info end + # Returns the client metadata to submit on Dynamic Client Registration. + # Per SEP-837, MCP clients MUST specify an appropriate OIDC `application_type` + # so the authorization server can apply the matching redirect URI policy. + # When the user did not set one explicitly, infer `"native"` vs `"web"` from + # the registered `redirect_uris`; an explicit value always wins. + # https://github.com/modelcontextprotocol/modelcontextprotocol/pull/837 + def registration_client_metadata + metadata = @provider.client_metadata + return metadata if metadata[:application_type] || metadata["application_type"] + + redirect_uris = metadata[:redirect_uris] || metadata["redirect_uris"] + metadata.merge("application_type" => Discovery.infer_application_type(redirect_uris)) + end + # Reads `key` from a `client_information` hash that may use either string or # symbol keys, so users can persist the result of `JSON.parse` *or* a hand-built # `{ client_id:, client_secret: }` and have both work. diff --git a/lib/mcp/client/oauth/provider.rb b/lib/mcp/client/oauth/provider.rb index ce10dad5..4864ae40 100644 --- a/lib/mcp/client/oauth/provider.rb +++ b/lib/mcp/client/oauth/provider.rb @@ -13,6 +13,9 @@ module OAuth # - `client_metadata` - Hash sent to the authorization server's Dynamic Client # Registration endpoint. Must include at minimum `redirect_uris`, # `grant_types`, `response_types`, and `token_endpoint_auth_method`. + # When `application_type` is omitted, the SDK infers `"native"` or `"web"` + # from `redirect_uris` per SEP-837 before registering; an explicit value + # always wins. # - `redirect_uri` - String: the redirect URI used for the authorization # request. Must be one of `redirect_uris` in `client_metadata`. # - `redirect_handler` - Callable invoked with the fully-built authorization diff --git a/test/mcp/client/oauth/discovery_test.rb b/test/mcp/client/oauth/discovery_test.rb index 245e9b78..4b291848 100644 --- a/test/mcp/client/oauth/discovery_test.rb +++ b/test/mcp/client/oauth/discovery_test.rb @@ -158,6 +158,39 @@ def test_authorization_server_metadata_urls_treat_trailing_slash_issuer_as_root ) end + def test_infer_application_type_returns_native_for_loopback_http_redirect_uris + uris = ["http://localhost:0/callback", "http://127.0.0.1:8080/cb", "http://[::1]/cb"] + + assert_equal("native", Discovery.infer_application_type(uris)) + end + + def test_infer_application_type_returns_native_for_custom_scheme_redirect_uris + assert_equal("native", Discovery.infer_application_type(["com.example.app:/oauth/callback"])) + end + + def test_infer_application_type_returns_web_for_https_redirect_uris + assert_equal("web", Discovery.infer_application_type(["https://app.example.com/callback"])) + end + + def test_infer_application_type_returns_web_when_any_redirect_uri_is_not_native + uris = ["http://localhost:0/callback", "https://app.example.com/callback"] + + assert_equal("web", Discovery.infer_application_type(uris)) + end + + def test_infer_application_type_returns_web_for_localhost_lookalike_host + assert_equal("web", Discovery.infer_application_type(["http://localhost.example.com/cb"])) + end + + def test_infer_application_type_returns_web_for_nil_or_empty_redirect_uris + assert_equal("web", Discovery.infer_application_type(nil)) + assert_equal("web", Discovery.infer_application_type([])) + end + + def test_infer_application_type_returns_web_for_unparseable_redirect_uri + assert_equal("web", Discovery.infer_application_type(["http://[invalid"])) + end + def test_canonicalize_url_normalizes_scheme_host_port_and_path assert_equal( "https://srv.example.com/mcp", diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index d36c5e93..5aa2dfe6 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -200,6 +200,62 @@ def test_run_uses_authorization_code_grant_for_default_provider end end + # Runs the full authorization flow with a minimal provider so tests can assert on + # the Dynamic Client Registration request body. The default loopback redirect URI + # exercises SEP-837's `"native"` inference; passing an HTTPS `redirect_uri` exercises + # the `"web"` inference. + def run_authorization_flow(redirect_uri: "http://localhost:0/callback", client_metadata_extra: {}) + state_holder = {} + provider = Provider.new( + client_metadata: { + redirect_uris: [redirect_uri], + grant_types: ["authorization_code"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }.merge(client_metadata_extra), + redirect_uri: redirect_uri, + redirect_handler: ->(url) { state_holder[:state] = URI.decode_www_form(url.query).to_h.fetch("state") }, + callback_handler: -> { ["test-auth-code", state_holder[:state]] }, + ) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + def test_run_registers_native_application_type_for_loopback_redirect_uri + run_authorization_flow + + assert_requested(:post, "#{@auth_base}/register") do |req| + JSON.parse(req.body)["application_type"] == "native" + end + end + + def test_run_registers_web_application_type_for_https_redirect_uri + run_authorization_flow(redirect_uri: "https://app.example.com/callback") + + assert_requested(:post, "#{@auth_base}/register") do |req| + JSON.parse(req.body)["application_type"] == "web" + end + end + + def test_run_does_not_override_explicit_application_type + run_authorization_flow(client_metadata_extra: { application_type: "web" }) + + assert_requested(:post, "#{@auth_base}/register") do |req| + JSON.parse(req.body)["application_type"] == "web" + end + end + + def test_run_does_not_override_explicit_string_keyed_application_type + run_authorization_flow( + redirect_uri: "https://app.example.com/callback", + client_metadata_extra: { "application_type" => "native" }, + ) + + assert_requested(:post, "#{@auth_base}/register") do |req| + JSON.parse(req.body)["application_type"] == "native" + end + end + def test_run_completes_full_authorization_flow captured_authorization_url = nil state_value = nil