From bfc6e551a3a9b738def565ae90fe2814706265bd Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 17 Aug 2026 03:07:22 +0900 Subject: [PATCH 01/24] Update examples for the 2026-07-28 era ## Motivation and Context The runnable examples drifted from the SDK they ship with: - POST requests whose `Accept` header does not cover both `application/json` and `text/event-stream` are answered 406, so the cURL walkthroughs no longer worked as printed. - `MCP::Client#connect` now defaults to `mode: :auto`, which adopts the sessionless modern lifecycle (2026-07-28) against a dual-era server; the Streamable HTTP client example requires a session and broke against its own server. - POST responses on an established session arrive as an SSE stream, which the hand-rolled HTTP client did not parse. - `notification_tool` claimed to send SSE notifications but only returned a `Tool::Response`. - `http_client.rb` read a resource URI the server never registered and skipped `notifications/initialized`. Handshake examples now offer the latest handshake protocol version (`2025-11-25`) instead of `2024-11-05`, the Streamable HTTP client pins `mode: :legacy` for its session-based demo, `notification_tool` reports real `notifications/progress` events requested through `_meta.progressToken`, and the interactive client streams those events from the POST response so they are visible on screen. The modern lifecycle itself had no runnable example, so a new server and client pair (`modern_http_server.rb`, `modern_http_client.rb`) shows the sessionless flow end to end: `server/discover` capability discovery, the per-request `_meta` envelope with the `Mcp-Method` and `Mcp-Name` headers (stamped by the SDK client automatically), `resultType` stamping, SEP-2549 cache hints, a SEP-2322 multi round-trip `deploy` tool resumed automatically by the client's elicitation handler, and the removal of legacy-only methods such as `ping`. The same server keeps serving the legacy `initialize` flow, routed per request by the `MCP-Protocol-Version` header. `examples/README.md` documents the pair and adds a cURL walkthrough covering the modern headers, the envelope, and the multi round-trip exchange. ## How Has This Been Tested? - `ruby examples/stdio_client.rb` against `stdio_server.rb` - `ruby examples/http_client.rb` against `http_server.rb` - `ruby examples/streamable_http_client.rb` against `streamable_http_server.rb`, including the progress stream - `ruby examples/modern_http_client.rb` against `modern_http_server.rb` (discover, modern adoption, cache hints, `resultType`, the multi round-trip resume, `ping` rejection) - The cURL walkthroughs in the banners and `examples/README.md`, including the `requestState` echo and a legacy `initialize` against the modern example server ## Breaking Changes None. --- examples/README.md | 112 +++++++++++++++++++++--- examples/http_client.rb | 73 ++++++++++++---- examples/http_server.rb | 5 +- examples/modern_http_client.rb | 133 +++++++++++++++++++++++++++++ examples/modern_http_server.rb | 126 +++++++++++++++++++++++++++ examples/rails/README.md | 2 +- examples/streamable_http_client.rb | 62 ++++++++++++-- examples/streamable_http_server.rb | 39 ++++++--- 8 files changed, 501 insertions(+), 51 deletions(-) create mode 100644 examples/modern_http_client.rb create mode 100644 examples/modern_http_server.rb diff --git a/examples/README.md b/examples/README.md index 86931c5..af1828a 100644 --- a/examples/README.md +++ b/examples/README.md @@ -98,7 +98,7 @@ A specialized HTTP server designed to test and demonstrate Server-Sent Events (S **Available Tools:** -- `notification_tool` - Send custom SSE notifications with optional delays +- `notification_tool` - Sends progress notifications over SSE, with optional delays - `echo` - Simple echo tool for basic testing **Usage:** @@ -124,14 +124,15 @@ An interactive client that connects to the SSE stream and provides a menu-driven 1. Start the SSE test server in one terminal: - ```console - $ ruby examples/streamable_http_server.rb - ``` +```console +$ ruby examples/streamable_http_server.rb +``` 2. Run the SSE test client in another terminal: - ```console - $ ruby examples/streamable_http_client.rb - ``` + +```console +$ ruby examples/streamable_http_client.rb +``` The client will: @@ -155,6 +156,34 @@ $ bundle exec puma --port 9292 The MCP endpoint is available at `http://localhost:9292/mcp`. See [`rails/README.md`](rails/README.md) for a full curl-based walkthrough. +### 8. Modern Lifecycle HTTP Server / Client (`modern_http_server.rb`, `modern_http_client.rb`) + +A server and client pair demonstrating the 2026-07-28 modern lifecycle (SEP-2575), which replaces the `initialize` handshake and per-session state with sessionless, self-contained requests. + +**Features:** + +- `server/discover` capability discovery before (or instead of) a handshake +- The per-request `_meta` envelope and `Mcp-Method` / `Mcp-Name` headers, stamped by the SDK automatically +- `resultType` stamping and SEP-2549 cache hints (`ttlMs`, `cacheScope`) on results +- A multi round-trip `deploy` tool (SEP-2322), resumed automatically by the client's elicitation handler +- The removal of legacy-only methods such as `ping` + +**Usage:** + +1. Start the server in one terminal: + +```console +$ ruby examples/modern_http_server.rb +``` + +2. Run the client in another terminal: + +```console +$ ruby examples/modern_http_client.rb +``` + +The same server still accepts the legacy `initialize` flow: the transport routes each request to the legacy or modern lifecycle by its `MCP-Protocol-Version` header. + ### Testing with MCP Inspector [MCP Inspector](https://modelcontextprotocol.io/docs/tools/inspector) is a browser-based tool for testing and debugging MCP servers. @@ -188,24 +217,78 @@ You can also test SSE functionality manually using cURL: ```console SESSION_ID=$(curl -D - -s -o /dev/null http://localhost:9393 \ - --json '{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"curl-test","version":"1.0"}}}' | grep -i "Mcp-Session-Id:" | cut -d' ' -f2- | tr -d '\r') + -H "Accept: application/json, text/event-stream" \ + --json '{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"curl-test","version":"1.0"}}}' | grep -i "Mcp-Session-Id:" | cut -d' ' -f2- | tr -d '\r') ``` -2. Connect to SSE stream (in one terminal): +2. Optionally connect the standalone SSE stream, which carries server-initiated messages that are not tied to a request (in another terminal): ```console curl -i -N -H "Mcp-Session-Id: $SESSION_ID" http://localhost:9393 ``` -3. Trigger notifications (in another terminal): +3. Call the notification tool. The `notifications/progress` events (requested via the `progressToken` in `_meta`) and the final response arrive as SSE events on the POST response itself: ```console -# Send immediate notification curl -i http://localhost:9393 \ + -H "Accept: application/json, text/event-stream" \ -H "Mcp-Session-Id: $SESSION_ID" \ - --json '{"jsonrpc":"2.0","method":"tools/call","id":2,"params":{"name":"notification_tool","arguments":{"message":"Hello from cURL!"}}}' + --json '{"jsonrpc":"2.0","method":"tools/call","id":2,"params":{"name":"notification_tool","arguments":{"message":"Hello from cURL!","delay":0.5},"_meta":{"progressToken":"curl-progress"}}}' ``` +### Testing the modern lifecycle with cURL + +The modern lifecycle (2026-07-28, SEP-2575) is sessionless: there is no `initialize` handshake and no `Mcp-Session-Id`. Start `examples/modern_http_server.rb` and walk it manually: + +1. Probe capabilities with `server/discover`: + +```console +curl -s http://localhost:9494 \ + -H "Accept: application/json, text/event-stream" \ + --json '{"jsonrpc":"2.0","id":0,"method":"server/discover"}' +``` + +2. List tools (the `MCP-Protocol-Version` header selects the era, `Mcp-Method` mirrors the method, and `params._meta` carries the SEP-2575 envelope): + +```console +curl -s http://localhost:9494 \ + -H "Accept: application/json, text/event-stream" \ + -H "MCP-Protocol-Version: 2026-07-28" \ + -H "Mcp-Method: tools/list" \ + --json '{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{"_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28","io.modelcontextprotocol/clientCapabilities":{}}}}' +``` + +3. Call a tool (name-bearing methods additionally mirror the name in the `Mcp-Name` header): + +```console +curl -s http://localhost:9494 \ + -H "Accept: application/json, text/event-stream" \ + -H "MCP-Protocol-Version: 2026-07-28" \ + -H "Mcp-Method: tools/call" \ + -H "Mcp-Name: greet" \ + --json '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"greet","arguments":{"name":"curl"},"_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28","io.modelcontextprotocol/clientCapabilities":{}}}}' +``` + +4. Observe a multi round-trip result (SEP-2322), then resume it by echoing the `requestState` back together with the answer. Declaring the `elicitation` capability in the envelope is required before the server may embed elicitation requests: + +```console +curl -s http://localhost:9494 \ + -H "Accept: application/json, text/event-stream" \ + -H "MCP-Protocol-Version: 2026-07-28" \ + -H "Mcp-Method: tools/call" \ + -H "Mcp-Name: deploy" \ + --json '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"deploy","arguments":{"app":"storefront"},"_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28","io.modelcontextprotocol/clientCapabilities":{"elicitation":{}}}}}' + +curl -s http://localhost:9494 \ + -H "Accept: application/json, text/event-stream" \ + -H "MCP-Protocol-Version: 2026-07-28" \ + -H "Mcp-Method: tools/call" \ + -H "Mcp-Name: deploy" \ + --json '{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"deploy","arguments":{"app":"storefront"},"inputResponses":{"environment":{"action":"accept","content":{"environment":"staging"}}},"requestState":"{\"app\":\"storefront\"}","_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28","io.modelcontextprotocol/clientCapabilities":{"elicitation":{}}}}}' +``` + +The same server still accepts the legacy flow from the previous sections, routed by the `MCP-Protocol-Version` header. + ## Streamable HTTP Transport Details ### Protocol Flow @@ -236,13 +319,15 @@ Initialize a session: ```console curl -i http://localhost:9292 \ - --json '{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}' + -H "Accept: application/json, text/event-stream" \ + --json '{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}' ``` List tools (using the session ID from initialization): ```console curl -i http://localhost:9292 \ + -H "Accept: application/json, text/event-stream" \ -H "Mcp-Session-Id: YOUR_SESSION_ID" \ --json '{"jsonrpc":"2.0","method":"tools/list","id":2}' ``` @@ -251,6 +336,7 @@ Call a tool: ```console curl -i http://localhost:9292 \ + -H "Accept: application/json, text/event-stream" \ -H "Mcp-Session-Id: YOUR_SESSION_ID" \ --json '{"jsonrpc":"2.0","method":"tools/call","id":3,"params":{"name":"example_tool","arguments":{"a":5,"b":3}}}' ``` diff --git a/examples/http_client.rb b/examples/http_client.rb index 4ef95c7..b61b563 100644 --- a/examples/http_client.rb +++ b/examples/http_client.rb @@ -4,7 +4,7 @@ require "net/http" require "json" require "uri" -# Simple HTTP client example for interacting with the MCP HTTP server +# Simple HTTP client example for interacting with the MCP HTTP server. class MCPHTTPClient def initialize(base_url = "http://localhost:9292") @base_url = base_url @@ -30,19 +30,51 @@ class MCPHTTPClient response = http.request(request) - # Store session ID if provided + # Store session ID if provided. if response["Mcp-Session-Id"] @session_id = response["Mcp-Session-Id"] puts "Session ID: #{@session_id}" end - JSON.parse(response.body) + parse_response_body(response) + end + + # In the transport's default SSE mode, POST responses on an established session arrive as + # a Server-Sent Events stream whose `data:` line carries the JSON-RPC response; unwrap it + # before parsing. + def parse_response_body(response) + body = response.body + + if response["Content-Type"]&.start_with?("text/event-stream") + data_lines = body.lines.select { |line| line.start_with?("data:") } + body = data_lines.map { |line| line.sub(/\Adata:\s*/, "") }.join + end + + JSON.parse(body) + end + + def send_notification(method, params = nil) + uri = URI(@base_url) + http = Net::HTTP.new(uri.host, uri.port) + + request = Net::HTTP::Post.new(uri.path.empty? ? "/" : uri.path) + request["Content-Type"] = "application/json" + request["Mcp-Session-Id"] = @session_id if @session_id + + # Notifications carry no `id` and receive no JSON-RPC response body. + request.body = { + jsonrpc: "2.0", + method: method, + params: params, + }.compact.to_json + + http.request(request) end def initialize_session puts "=== Initializing session ===" result = send_request("initialize", { - protocolVersion: "2024-11-05", + protocolVersion: "2025-11-25", capabilities: {}, clientInfo: { name: "example_client", @@ -54,6 +86,14 @@ class MCPHTTPClient result end + def notify_initialized + puts "=== Sending notifications/initialized ===" + response = send_notification("notifications/initialized") + puts "Response status: #{response.code} #{response.message}" + + response + end + def ping puts "=== Sending ping ===" result = send_request("ping") @@ -147,37 +187,40 @@ def main client = MCPHTTPClient.new begin - # Initialize session + # Initialize session. client.initialize_session - # Test ping + # Complete the handshake. + client.notify_initialized + + # Test ping. client.ping - # List available tools + # List available tools. client.list_tools - # Call the example_tool (note: snake_case name) + # Call the example_tool (note: snake_case name). client.call_tool("example_tool", { a: 5, b: 3 }) - # Call the echo tool + # Call the echo tool. client.call_tool("echo", { message: "Hello from client!" }) - # List prompts + # List prompts. client.list_prompts - # Get a prompt (note: snake_case name) + # Get a prompt (note: snake_case name). client.get_prompt("example_prompt", { message: "This is a test message" }) - # List resources + # List resources. client.list_resources - # Read a resource - client.read_resource("test_resource") + # Read a resource (the URI registered by http_server.rb). + client.read_resource("https://test_resource.invalid") rescue => e puts "Error: #{e.message}" puts e.backtrace ensure - # Clean up session + # Clean up session. client.close_session end end diff --git a/examples/http_server.rb b/examples/http_server.rb index 4f3edee..33d8d80 100644 --- a/examples/http_server.rb +++ b/examples/http_server.rb @@ -56,6 +56,7 @@ end # Set up the server server = MCP::Server.new( name: "example_http_server", + version: "1.0.0", tools: [ExampleTool], prompts: [ExamplePrompt], resources: [ @@ -169,7 +170,9 @@ puts <<~MESSAGE Starting MCP HTTP server on http://localhost:9292 Use POST requests to initialize and send JSON-RPC commands Example initialization: - curl -i http://localhost:9292 --json '{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}' + curl -i http://localhost:9292 \\ + -H "Accept: application/json, text/event-stream" \\ + --json '{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}' The server will return a session ID in the Mcp-Session-Id header. Use this session ID for subsequent requests. diff --git a/examples/modern_http_client.rb b/examples/modern_http_client.rb new file mode 100644 index 0000000..ca2cd1f --- /dev/null +++ b/examples/modern_http_client.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +$LOAD_PATH.unshift(File.expand_path("../lib", __dir__)) +require "mcp" +require "json" + +# Client example for the 2026-07-28 modern lifecycle (SEP-2575), which replaces the `initialize` handshake +# and per-session state with sessionless, self-contained requests. +class MCPModernHTTPClient + def initialize(base_url = "http://localhost:9494") + @transport = MCP::Client::HTTP.new(url: base_url) + @client = MCP::Client.new(transport: @transport) + end + + # `server/discover` (SEP-2575) is sessionless capability discovery that works before, or instead of, `connect`. + def discover + puts "=== Discovering server capabilities (server/discover) ===" + result = @client.discover + puts <<~MESSAGE + Supported versions: #{result.supported_versions.inspect} + Server: #{result.server_info} + Cache hints: ttlMs=#{result.ttl_ms}, cacheScope=#{result.cache_scope} (SEP-2549) + Instructions: #{result.instructions} + MESSAGE + + result + end + + # The handler answers `elicitation/create` requests embedded in SEP-2322 `input_required` results; once registered, + # `call_tool` resumes those results automatically. + # Register it before `connect` so the connection never needs a server-to-client route. + def register_elicitation_handler + @client.on_elicitation do |params| + puts " Server asked: #{params["message"]}" + + { action: "accept", content: MCP::Client::Elicitation.apply_defaults(params["requestedSchema"]) } + end + end + + # `mode: :modern` requires the modern lifecycle and fails on legacy-only servers; + # the default `:auto` probes `server/discover` first and falls back to the legacy `initialize` handshake. + # The `elicitation` capability must be declared before the server may embed elicitation requests. + def connect + puts "=== Adopting the modern lifecycle ===" + result = @client.connect( + client_info: { name: "modern-http-client", version: "1.0" }, + capabilities: { elicitation: {} }, + mode: :modern, + ) + puts <<~MESSAGE + Modern connection: #{@transport.modern?} + Protocol version: #{@client.protocol_version} + MESSAGE + + result + end + + # Modern requests are self-contained: the SDK stamps the `_meta` envelope (protocol version, + # client info, capabilities) and the Mcp-Method and Mcp-Name headers on every request automatically. + def list_tools + puts "=== Listing tools ===" + page = @client.list_tools + page.tools.each { |tool| puts " - #{tool.name}: #{tool.description}" } + puts "Cache hints: ttlMs=#{page.ttl_ms}, cacheScope=#{page.cache_scope}" + + page + end + + def call_tool(name, arguments) + puts "=== Calling tool: #{name} ===" + response = @client.call_tool(name: name, arguments: arguments) + puts <<~MESSAGE + resultType: #{response.dig("result", "resultType")} + Response: #{response.dig("result", "content", 0, "text")} + MESSAGE + + response + end + + # The modern lifecycle removed `initialize`, `ping`, `logging/setLevel`, and resource subscriptions (SEP-2575); + # calling one is an error. + def ping + puts "=== Calling removed method: ping ===" + @client.ping + rescue MCP::Client::RequestHandlerError => e + puts "ping was removed by the modern lifecycle (SEP-2575): #{e.message}" + end + + def close + # No session exists on a modern connection; this only clears local state. + @transport.close + end +end + +puts <<~MESSAGE + MCP Modern Lifecycle Client (2026-07-28) + Make sure the server is running (ruby examples/modern_http_server.rb) + #{"=" * 60} +MESSAGE + +client = MCPModernHTTPClient.new + +begin + # Probe the server before adopting a lifecycle. + client.discover + + # Answer the elicitation requests the server embeds in its results. + client.register_elicitation_handler + + # Adopt the modern lifecycle. + client.connect + + # List available tools. + client.list_tools + + # Call a plain single round-trip tool. + client.call_tool("greet", { name: "MCP" }) + + # Call a multi round-trip tool (SEP-2322): the driver fulfills the embedded elicitation via the handler registered above + # and re-issues the call with `inputResponses` plus the echoed `requestState`. Without a handler, `call_tool` raises + # `MCP::Client::InputRequiredError` for manual driving via the `input_responses:` / `request_state:` keywords. + client.call_tool("deploy", { app: "storefront" }) + + # Call a method the modern lifecycle removed. + client.ping +rescue => e + puts <<~MESSAGE + Error: #{e.message} + #{e.backtrace.join("\n")} + MESSAGE +ensure + client.close +end diff --git a/examples/modern_http_server.rb b/examples/modern_http_server.rb new file mode 100644 index 0000000..b21c35d --- /dev/null +++ b/examples/modern_http_server.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +$LOAD_PATH.unshift(File.expand_path("../lib", __dir__)) +require "mcp" +require "rackup" +require "json" +require "logger" + +# Example server for the 2026-07-28 modern lifecycle (SEP-2575). +# +# The modern lifecycle is sessionless: there is no `initialize` handshake and no `Mcp-Session-Id`. +# Clients probe the server with `server/discover` and then send self-contained requests that carry +# a `_meta` envelope (protocol version, client info, capabilities) plus `MCP-Protocol-Version` / `Mcp-Method` headers. +# +# The same server still accepts the legacy `initialize` flow: the transport routes each request to +# the legacy or modern lifecycle by its `MCP-Protocol-Version` header, so no constructor flag is needed. +server = MCP::Server.new( + name: "modern_lifecycle_server", + version: "1.0.0", + instructions: "Example server demonstrating the 2026-07-28 modern lifecycle.", + # SEP-2549 cache hints, advertised in `server/discover` and stamped on cacheable results. + # The defaults are 0 / "private"; non-default values are used here so they are visible + # in the example output. + ttl_ms: 60_000, + cache_scope: "public", +) + +server.define_tool( + name: "greet", + description: "Greets a name; a plain single round-trip tool", + input_schema: { properties: { name: { type: "string" } }, required: ["name"] }, +) do |name:| + MCP::Tool::Response.new([{ type: "text", text: "Hello, #{name}!" }]) +end + +# A multi round-trip tool (SEP-2322). The first round returns an `input_required` result asking +# the client to answer an embedded `elicitation/create` request; the client re-issues the call with +# `inputResponses` plus the echoed opaque `requestState`, and the handler re-runs from the start +# and reads the answer via `server_context`. +# Over a legacy connection the default `input_required_legacy_shim` fulfills the same result through +# a real server-to-client elicitation request instead. +server.define_tool( + name: "deploy", + description: "Deploys an app; asks which environment via a multi round-trip elicitation (SEP-2322)", + input_schema: { properties: { app: { type: "string" } }, required: ["app"] }, +) do |app:, server_context:| + answer = server_context.input_response("environment") + + if answer.nil? + MCP::Server::InputRequiredResult.new( + input_requests: { + "environment" => { + method: "elicitation/create", + params: { + message: "Which environment should #{app} deploy to?", + requestedSchema: { + type: "object", + properties: { + environment: { type: "string", title: "Environment", default: "staging" }, + }, + required: ["environment"], + }, + }, + }, + }, + request_state: JSON.generate(app: app), + ) + else + # Transports parse JSON with `symbolize_names: true`, so answers normally arrive symbol-keyed; + # the string fallback covers custom transports. + action = answer[:action] || answer["action"] + + if action == "accept" + environment = answer.dig(:content, :environment) || answer.dig("content", "environment") + MCP::Tool::Response.new([{ type: "text", text: "Deployed #{app} to #{environment}" }]) + else + MCP::Tool::Response.new([{ type: "text", text: "Deployment of #{app} was cancelled (#{action})" }]) + end + end +end + +transport = MCP::Server::Transports::StreamableHTTPTransport.new(server) + +# `StreamableHTTPTransport` responds to `call(env)`, so it can be used directly as a Rack app. +# See http_server.rb for a CORS setup for browser-based clients. +rack_app = Rack::Builder.new do + use(Rack::CommonLogger, Logger.new($stdout)) + use(Rack::ShowExceptions) + + run(transport) +end + +puts <<~MESSAGE + === MCP Modern Lifecycle Server (2026-07-28) === + + Starting server on http://localhost:9494 + + Run the SDK client against it: + ruby examples/modern_http_client.rb + + Or walk the modern lifecycle with cURL: + + 1. Probe capabilities (sessionless, works without any protocol headers): + curl -s http://localhost:9494 \\ + -H "Accept: application/json, text/event-stream" \\ + --json '{"jsonrpc":"2.0","id":0,"method":"server/discover"}' + + 2. Modern tools/list (the MCP-Protocol-Version header selects the era, the + Mcp-Method header mirrors the method, and params._meta carries the + SEP-2575 envelope): + curl -s http://localhost:9494 \\ + -H "Accept: application/json, text/event-stream" \\ + -H "MCP-Protocol-Version: 2026-07-28" -H "Mcp-Method: tools/list" \\ + --json '{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{"_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28","io.modelcontextprotocol/clientCapabilities":{}}}}' + + 3. Modern tools/call (name-bearing methods additionally mirror the name in + the Mcp-Name header; no Mcp-Session-Id is ever sent): + curl -s http://localhost:9494 \\ + -H "Accept: application/json, text/event-stream" \\ + -H "MCP-Protocol-Version: 2026-07-28" -H "Mcp-Method: tools/call" -H "Mcp-Name: greet" \\ + --json '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"greet","arguments":{"name":"curl"},"_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28","io.modelcontextprotocol/clientCapabilities":{}}}}' + + Press Ctrl+C to stop the server +MESSAGE + +Rackup::Handler.get("puma").run(rack_app, Port: 9494, Host: "localhost") diff --git a/examples/rails/README.md b/examples/rails/README.md index 6a599c3..eaee531 100644 --- a/examples/rails/README.md +++ b/examples/rails/README.md @@ -36,7 +36,7 @@ per the MCP Streamable HTTP transport spec. Responses arrive as SSE `data:` line ```console SESSION_ID=$(curl -s -D - -o /dev/null http://localhost:9292/mcp \ -H "Accept: application/json, text/event-stream" \ - --json '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"curl","version":"1.0"}}}' \ + --json '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"curl","version":"1.0"}}}' \ | grep -i "^mcp-session-id:" | cut -d' ' -f2 | tr -d '\r') ``` diff --git a/examples/streamable_http_client.rb b/examples/streamable_http_client.rb index 79e3f38..ab7aeed 100644 --- a/examples/streamable_http_client.rb +++ b/examples/streamable_http_client.rb @@ -18,8 +18,11 @@ def create_logger logger end -# The SDK does not yet implement the optional GET SSE stream, so this example -# uses MCP::Client for JSON-RPC requests and raw Net::HTTP for the event stream. +# MCP::Client::HTTP opens the optional GET SSE stream itself once a server-request handler +# (e.g. `client.on_elicitation`) is registered. This example instead opens the stream with +# raw `Net::HTTP` to show the SSE wire format. The GET stream carries server-initiated messages +# that are not tied to a client request; request-scoped notifications such as progress arrive on +# the POST's own SSE response stream (see `stream_tool_call` below). def connect_sse(session_id, logger) uri = URI(SERVER_URL) @@ -64,6 +67,42 @@ def print_response(response) end end +# Calls a tool with a raw streaming POST so each SSE event on the response stream is printed as +# it arrives: the `notifications/progress` events requested via `_meta.progressToken`, +# then the final JSON-RPC response. `MCP::Client#call_tool` reads the same stream but consumes +# the progress notifications internally, so the raw request makes them visible here. +def stream_tool_call(session_id, request_id, name:, arguments:, logger:) + uri = URI(SERVER_URL) + + body = { + jsonrpc: "2.0", + id: request_id, + method: "tools/call", + params: { + name: name, + arguments: arguments, + _meta: { progressToken: "streamable-http-client" }, + }, + } + + Net::HTTP.start(uri.host, uri.port) do |http| + request = Net::HTTP::Post.new(uri) + request["Content-Type"] = "application/json" + request["Accept"] = "application/json, text/event-stream" + request["Mcp-Session-Id"] = session_id + request.body = JSON.generate(body) + + http.request(request) do |response| + parser = EventStreamParser::Parser.new + response.read_body do |chunk| + parser.feed(chunk) do |_type, data, _id| + logger.info("SSE event: #{data}") + end + end + end + end +end + def main logger = create_logger @@ -76,11 +115,17 @@ def main http_transport = MCP::Client::HTTP.new(url: SERVER_URL) client = MCP::Client.new(transport: http_transport) sse_thread = nil + raw_request_id = 0 begin puts "=== Initializing session ===" + + # The default `mode: :auto` adopts the sessionless modern lifecycle (2026-07-28) + # when the server supports it. This example demonstrates the legacy session's SSE stream, + # so pin the legacy `initialize` handshake. server_info = client.connect( client_info: { name: "streamable-http-client", version: "1.0" }, + mode: :legacy, ) puts <<~MESSAGE @@ -104,8 +149,6 @@ def main sse_thread = Thread.new { connect_sse(http_transport.session_id, logger) } sleep(1) - # Once the optional SSE stream is active, POST requests may receive only a - # 202 ACK while the actual JSON-RPC response is delivered over SSE. loop do puts <<~MENU.chomp @@ -126,12 +169,15 @@ def main print("Enter delay in seconds (0 for immediate): ") delay = gets.chomp.to_f - puts "=== Calling tool: notification_tool ===" - response = client.call_tool( - tool: notification_tool, + puts "=== Calling tool: notification_tool (raw streaming POST) ===" + raw_request_id += 1 + stream_tool_call( + http_transport.session_id, + "raw-#{raw_request_id}", + name: notification_tool.name, arguments: { message: message, delay: delay }, + logger: logger, ) - print_response(response) else puts "notification_tool not available" end diff --git a/examples/streamable_http_server.rb b/examples/streamable_http_server.rb index a20d28e..f122296 100644 --- a/examples/streamable_http_server.rb +++ b/examples/streamable_http_server.rb @@ -13,14 +13,16 @@ sse_logger.formatter = proc do |severity, datetime, _progname, msg| "[SSE] #{severity} #{datetime.strftime("%H:%M:%S.%L")} - #{msg}\n" end -# Tool that returns a response that will be sent via SSE if a stream is active +# Tool that emits `notifications/progress` events, delivered over the SSE stream. +# The client must request them by sending a `progressToken` in the request `_meta`. class NotificationTool < MCP::Tool tool_name "notification_tool" - description "Returns a notification message that will be sent via SSE if stream is active" + description "Sends the message back as progress notifications via SSE, then returns a summary" input_schema( properties: { message: { type: "string", description: "Message to send via SSE" }, - delay: { type: "number", description: "Delay in seconds before returning (optional)" }, + steps: { type: "number", description: "Number of progress notifications to send (default: 3)" }, + delay: { type: "number", description: "Delay in seconds between notifications (optional)" }, }, required: ["message"], ) @@ -28,14 +30,19 @@ class NotificationTool < MCP::Tool class << self attr_accessor :logger - def call(message:, delay: 0) - sleep(delay) if delay > 0 + def call(message:, server_context:, steps: 3, delay: 0) + steps = steps.to_i.clamp(1, 10) - logger&.info("Returning notification message: #{message}") + steps.times do |i| + sleep(delay) if delay > 0 + + logger&.info("Reporting progress #{i + 1}/#{steps}: #{message}") + server_context.report_progress(i + 1, total: steps, message: message) + end MCP::Tool::Response.new([{ type: "text", - text: "Notification: #{message} (timestamp: #{Time.now.iso8601})", + text: "Sent #{steps} progress notifications for: #{message}", }]) end end @@ -44,6 +51,7 @@ end # Create the server server = MCP::Server.new( name: "sse_test_server", + version: "1.0.0", tools: [NotificationTool], prompts: [], resources: [], @@ -149,29 +157,34 @@ puts <<~MESSAGE Starting server on http://localhost:9393 Available Tools: - 1. NotificationTool - Returns messages that are sent via SSE when stream is active" + 1. notification_tool - Sends the message back as progress notifications via SSE 2. echo - Simple echo tool Testing SSE: 1. Initialize session: curl -i http://localhost:9393 \\ - --json '{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"sse-test","version":"1.0"}}}' + -H "Accept: application/json, text/event-stream" \\ + --json '{"jsonrpc":"2.0","method":"initialize","id":1,"params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"sse-test","version":"1.0"}}}' - 2. Connect SSE stream (use the session ID from step 1):" + 2. Connect SSE stream (use the session ID from step 1): curl -i -N -H "Mcp-Session-Id: YOUR_SESSION_ID" http://localhost:9393 3. In another terminal, test tools (responses will be sent via SSE if stream is active): Echo tool: curl -i http://localhost:9393 -H "Mcp-Session-Id: YOUR_SESSION_ID" \\ + -H "Accept: application/json, text/event-stream" \\ --json '{"jsonrpc":"2.0","method":"tools/call","id":2,"params":{"name":"echo","arguments":{"message":"Hello SSE!"}}}' - Notification tool (with 2 second delay): + Notification tool (progress notifications require a progressToken in _meta): curl -i http://localhost:9393 -H "Mcp-Session-Id: YOUR_SESSION_ID" \\ - --json '{"jsonrpc":"2.0","method":"tools/call","id":3,"params":{"name":"notification_tool","arguments":{"message":"Hello SSE!", "delay": 2}}}' + -H "Accept: application/json, text/event-stream" \\ + --json '{"jsonrpc":"2.0","method":"tools/call","id":3,"params":{"name":"notification_tool","arguments":{"message":"Hello SSE!","delay":1},"_meta":{"progressToken":"curl-progress"}}}' - Note: When an SSE stream is active, tool responses will appear in the SSE stream and the POST request will return 202 Accepted with no body. + Note: Each POST is answered with an SSE stream of its own; request-scoped notifications + (such as the progress events above) appear on it before the final response. + The standalone GET stream carries server-initiated messages that are not tied to a request. Press Ctrl+C to stop the server MESSAGE From 920707cd7edca4aeb17dcaf9cd65ffeae031caa8 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 30 Aug 2026 02:09:00 +0900 Subject: [PATCH 02/24] Group the conformance Rake tasks under a `conformance` namespace ## Motivation and Context The three conformance tasks were named `conformance`, `conformance_list`, and `conformance_server`, spelling out with underscores the grouping that Rake expresses with namespaces. They are now `conformance:test`, `conformance:list`, and `conformance:server`. The bare `conformance` task is kept as an alias for `conformance:test`. ## How Has This Been Tested? By running `rake -T` to confirm the three tasks are grouped under `conformance:` with the alias listed beside them, `rake --dry-run default` to confirm that the default task resolves `conformance:test`, and `rake conformance:list` to exercise a renamed task end to end. ## Breaking Changes None. These are development-only Rake tasks. --- .github/workflows/conformance.yml | 2 +- AGENTS.md | 4 +- Rakefile | 68 +++++++++++++++++-------------- conformance/README.md | 16 ++++---- 4 files changed, 49 insertions(+), 41 deletions(-) diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml index dccb8af..ff178c2 100644 --- a/.github/workflows/conformance.yml +++ b/.github/workflows/conformance.yml @@ -26,4 +26,4 @@ jobs: - uses: actions/setup-node@v7 with: node-version: '24' # Specify the latest Node.js version. - - run: bundle exec rake conformance + - run: bundle exec rake conformance:test diff --git a/AGENTS.md b/AGENTS.md index 5d34462..6635c3b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,8 +15,8 @@ This is the official Ruby SDK for the Model Context Protocol (MCP), implementing - `bundle install` - Install dependencies - `rake test` - Run all tests - `rake rubocop` - Run linter -- `rake` - Run tests and linting (default task) -- `bundle exec rake conformance` - Run the MCP conformance suite (see conformance/README.md) +- `rake` - Run linting, tests, and the conformance suite (default task) +- `bundle exec rake conformance:test` - Run the MCP conformance suite (see conformance/README.md) - `bundle exec rake docs:preview` - Serve the documentation site locally at http://localhost:4000 (PORT to override) - `ruby -I lib -I test test/path/to/specific_test.rb` - Run single test file - `gem build mcp.gemspec` - Build the gem diff --git a/Rakefile b/Rakefile index c795e56..409eef9 100644 --- a/Rakefile +++ b/Rakefile @@ -14,42 +14,50 @@ require "rubocop/rake_task" RuboCop::RakeTask.new -task default: [:rubocop, :test, :conformance] +task default: [:rubocop, :test, "conformance:test"] -desc "Run MCP conformance tests (PORT, SCENARIO, SPEC_VERSION, VERBOSE)" -task :conformance do |t| - next unless npx_available?(t.name) +namespace :conformance do + desc "Run MCP conformance tests (PORT, SCENARIO, SPEC_VERSION, VERBOSE)" + task :test do |t| + next unless npx_available?(t.name) - require_relative "conformance/server_runner" - require_relative "conformance/client_runner" + require_relative "conformance/server_runner" + require_relative "conformance/client_runner" - options = {} - options[:port] = Integer(ENV["PORT"]) if ENV["PORT"] - options[:scenario] = ENV["SCENARIO"] if ENV["SCENARIO"] - options[:spec_version] = ENV["SPEC_VERSION"] if ENV["SPEC_VERSION"] - options[:verbose] = true if ENV["VERBOSE"] + options = {} + options[:port] = Integer(ENV["PORT"]) if ENV["PORT"] + options[:scenario] = ENV["SCENARIO"] if ENV["SCENARIO"] + options[:spec_version] = ENV["SPEC_VERSION"] if ENV["SPEC_VERSION"] + options[:verbose] = true if ENV["VERBOSE"] - Conformance::ServerRunner.new(**options).run - Conformance::ClientRunner.new(**options.reject { |key, _value| key == :port }).run + Conformance::ServerRunner.new(**options).run + Conformance::ClientRunner.new(**options.reject { |key, _value| key == :port }).run + end + + desc "List available conformance scenarios" + task :list do |t| + next unless npx_available?(t.name) + + system("npx", "--yes", "@modelcontextprotocol/conformance", "list", "--server") + system("npx", "--yes", "@modelcontextprotocol/conformance", "list", "--client") + end + + desc "Start the conformance server (PORT)" + task :server do + require_relative "conformance/server" + + options = {} + options[:port] = Integer(ENV["PORT"]) if ENV["PORT"] + + Conformance::Server.new(**options).start + end end -desc "List available conformance scenarios" -task :conformance_list do |t| - next unless npx_available?(t.name) - - system("npx", "--yes", "@modelcontextprotocol/conformance", "list", "--server") - system("npx", "--yes", "@modelcontextprotocol/conformance", "list", "--client") -end - -desc "Start the conformance server (PORT)" -task :conformance_server do - require_relative "conformance/server" - - options = {} - options[:port] = Integer(ENV["PORT"]) if ENV["PORT"] - - Conformance::Server.new(**options).start -end +# The other two former names now fail with a "Did you mean?" suggestion, but a bare `conformance` +# would match the `conformance/` directory: Rake synthesizes a file task for it and exits 0 without +# running anything. This keeps the name pointing at the suite instead of at that silent no-op. +desc "Alias for conformance:test" +task conformance: "conformance:test" namespace :docs do desc "Serve the documentation site locally at http://localhost:4000 (PORT)" diff --git a/conformance/README.md b/conformance/README.md index ea2c917..1e2e660 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -12,7 +12,7 @@ Validates the Ruby SDK's conformance to the MCP specification using [`@modelcont ### Run all scenarios ```bash -bundle exec rake conformance +bundle exec rake conformance:test ``` Runs both server and client conformance tests in sequence. Server conformance starts the @@ -31,20 +31,20 @@ are allowed to fail without affecting the exit code. ```bash # Run a single server scenario -bundle exec rake conformance SCENARIO=ping +bundle exec rake conformance:test SCENARIO=ping # Use a different port with verbose output -bundle exec rake conformance PORT=3000 VERBOSE=1 +bundle exec rake conformance:test PORT=3000 VERBOSE=1 # Start the server on a specific port -bundle exec rake conformance_server PORT=3000 +bundle exec rake conformance:server PORT=3000 ``` ### Start the server and test separately ```bash # Terminal 1: start the server -bundle exec rake conformance_server +bundle exec rake conformance:server # Terminal 2: run all scenarios npx @modelcontextprotocol/conformance@alpha server --url http://localhost:9292/mcp @@ -59,7 +59,7 @@ on a single scenario. Stop the server with Ctrl+C when done. ### List available scenarios ```bash -bundle exec rake conformance_list +bundle exec rake conformance:list ``` Prints all scenario names that can be passed to `SCENARIO`. @@ -76,7 +76,7 @@ repository with the conformance server running: ```bash # Terminal 1 (this repository): start the conformance server -bundle exec rake conformance_server +bundle exec rake conformance:server # Terminal 2 (conformance repository): run the tier audit skill as a slash command in Claude Code /mcp-sdk-tier-audit /path/to/modelcontextprotocol/ruby-sdk http://localhost:9292/mcp @@ -102,4 +102,4 @@ conformance/ Known-failing scenarios are registered in `conformance/expected_failures.yml`. They are allowed to fail without affecting the exit code and are tracked to catch regressions. -These are shown in the output of `bundle exec rake conformance`. +These are shown in the output of `bundle exec rake conformance:test`. From 09d4bf41aa0f2143fdd8a0ef61a7b64a58f9c37b Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 30 Aug 2026 06:41:27 +0900 Subject: [PATCH 03/24] Let the embedding application refuse an authorization request ## Motivation and Context An MCP server chooses the authorization server its users sign in to. It names one in `authorization_servers` in its Protected Resource Metadata, and states the scopes it wants in `scopes_supported` or in the `WWW-Authenticate` challenge. Nothing binds either choice to the server's own identity. No origin rule can settle this, because an authorization server is legitimately a different origin from the resource it protects. Validating that a token was issued for the intended audience is a responsibility the specification places on MCP servers rather than on clients, and there is no portable way for a client to perform it either: access tokens are frequently opaque, and an authorization server is not required to honor the `resource` parameter or to report an audience in the token response. Two things follow from that, and they are separate. The first is a judgement, and it belongs to whoever embeds this SDK, because an application often knows which providers its users deal with. `authorization_request_validator` is handed an `AuthorizationRequest` describing the authorization server, the scopes, the MCP server, and the RFC 8707 resource, and can refuse. It is one object rather than keyword arguments so that later revisions of the specification can add to it without changing the shape implementers wrote. It runs on all three grants, and on the authorization-code grant it runs before dynamic client registration, which is why scope resolution moved above `ensure_client_registered`: a refusal must not leave this client registered at an authorization server the host just rejected. The second is not a judgement and is enforced here rather than asked about. `refresh!` rediscovers the authorization server on every call, so the server named for a session can differ from the one the stored tokens came from. SEP-2352 already binds client credentials to their issuer, but that check passes when the issuer was never recorded and is deliberately skipped for a Client ID Metadata Document URL, which is portable across authorization servers. The issuer that minted the tokens is now recorded with them, and `refresh!` refuses to present them to a different one. The transport answers that refusal by running a full authorization, which brings the new authorization server to the validator above. Tokens stored before this carry no issuer and keep refreshing. The hook decides whether to proceed, not what to ask for. The scopes reach the authorization server unchanged either way, because the specification requires a client to treat the scopes in the challenge as authoritative for the operation, and narrowing them silently would break that. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb` assert what the validator receives, that a refusal raises before any registration request is sent and before the redirect handler runs, and that the `client_credentials` and `jwt-bearer` grants consult it too. Others cover the issuer recorded on authorization, the refusal to refresh against a different one, and that tokens stored without an issuer still refresh. A test in `test/mcp/client/oauth/http_oauth_test.rb` drives the two halves together: a stale issuer makes the refresh fail, the transport reauthorizes, and the validator sees the newly named authorization server while no refresh token is ever posted to it. Each of them fails with its half of this change removed. `bundle exec rake` (tests, RuboCop, and conformance baseline) passes. ## Breaking Changes Refreshing now fails when the authorization server named for a session differs from the one that issued the stored tokens, and the caller reauthorizes instead. Tokens stored before this change carry no recorded issuer and are unaffected until their next authorization. The validator itself is additive: it defaults to `nil`, and a provider that does not offer it authorizes as before. --- docs/_client/authorization.md | 77 +++++++- lib/mcp/client/oauth.rb | 1 + lib/mcp/client/oauth/authorization_request.rb | 27 +++ .../oauth/client_credentials_provider.rb | 4 +- .../client/oauth/cross_app_access_provider.rb | 3 +- lib/mcp/client/oauth/flow.rb | 111 +++++++++-- lib/mcp/client/oauth/provider.rb | 4 +- .../client/oauth/storage_backed_provider.rb | 7 + test/mcp/client/oauth/flow_test.rb | 173 ++++++++++++++++++ test/mcp/client/oauth/http_oauth_test.rb | 88 +++++++++ 10 files changed, 478 insertions(+), 17 deletions(-) create mode 100644 lib/mcp/client/oauth/authorization_request.rb diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index 1c73eeb..a673a19 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -93,10 +93,14 @@ Required keyword arguments to `Provider.new`: Optional keyword arguments: - `scope`: Space-separated scopes to request when the server's `WWW-Authenticate` does not specify one. +- `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). - `storage`: Object responding to `tokens`, `save_tokens(t)`, `client_information`, `save_client_information(info)`. Defaults to `MCP::Client::OAuth::InMemoryStorage`, which keeps credentials in process memory only. Persisted `client_information` is stamped with an `"issuer"` member binding it to the authorization server that issued it (SEP-2352): when the server's authorization server changes, the SDK discards the stale registration and its tokens and re-registers automatically - (portable CIMD `client_id`s are kept). Treat the hash as opaque and persist it as-is. + (portable CIMD `client_id`s are kept). Saved `tokens` carry an `"issuer"` member of their own, recording the authorization server that minted them, which is what + lets a later refresh refuse a server the MCP server has since renamed. Treat both hashes as opaque and persist them as-is; a storage that writes out selected members + instead drops these bindings with no error. - `client_id_metadata_document_url`: URL where you publish a Client ID Metadata Document (`draft-ietf-oauth-client-id-metadata-document` and the MCP authorization specification). When the authorization server advertises `client_id_metadata_document_supported: true`, @@ -182,7 +186,7 @@ Keyword arguments: - `private_key`, `signing_algorithm`: Required with `private_key_jwt` - the key (a PEM string or `OpenSSL::PKey::PKey`, never written to `storage`) signs the client assertion with `"ES256"` or `"RS256"`; `client_secret` must not be set, because the private key is the credential. -- `scope`, `storage`: Optional, same meaning as on `Provider`. +- `scope`, `storage`, `authorization_request_validator`: Optional, same meaning as on `Provider`. ### Cross-App Access (JWT Bearer) Grant @@ -219,7 +223,7 @@ Keyword arguments: - `assertion_provider`: Required. Callable invoked as `call(audience:, resource:)` and returning the ID-JAG assertion. `audience` is the MCP authorization server's validated issuer identifier; `resource` is the canonical MCP server URL (RFC 8707). Passing both through to `IDJAGTokenExchange.request` covers the common case. -- `scope`, `storage`: Optional, same meaning as on `Provider`. +- `scope`, `storage`, `authorization_request_validator`: Optional, same meaning as on `Provider`. ### Communication Security @@ -258,3 +262,70 @@ The SDK also bounds what those endpoints may return. A discovery, dynamic client measured as the body arrives rather than after it has been buffered, so a compressed body that expands past the limit is refused partway through the expansion. Unlike the transport's `max_message_bytes:`, this limit is not configurable: these documents run to kilobytes in normal operation, and a connection supplied through `http_client_factory:` is bounded as well, so there is no way to opt out of it. + +### Reviewing the authorization request + +The checks above constrain where the SDK will send a request. What they cannot decide is whether the authorization server an MCP server names is one you want +your users signing in to. That choice belongs to the MCP server: it publishes `authorization_servers` in its Protected Resource Metadata and states the scopes +it wants in `scopes_supported` or in the `WWW-Authenticate` challenge. An authorization server is legitimately a different origin from the resource it protects, +so no origin rule can settle the question, and validating that a token was issued for the intended audience is a responsibility the specification places on +MCP servers rather than on clients. + +`authorization_request_validator` is where an application that *does* know which providers its users deal with can say so: + +```ruby +ALLOWED_ISSUERS = ["https://login.example.com", "https://accounts.google.com"] + +provider = MCP::Client::OAuth::Provider.new( + # client_metadata:, redirect_uri:, redirect_handler:, and callback_handler: as in the first example. + authorization_request_validator: ->(request) { + ALLOWED_ISSUERS.include?(request.authorization_server) + }, +) +``` + +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 +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. + +`server_url` is the URL the transport was configured with, verbatim; `resource` is the value actually sent as the RFC 8707 `resource`, which is that URL canonicalized +(fragment and userinfo dropped, scheme and host lowercased, a default port removed) or, when the Protected Resource Metadata advertises one, +the canonicalized value from there. That advertised value may name a parent path, so a server at `https://api.example.com/mcp` can legitimately produce +a `resource` of `https://api.example.com`. Match on `server_url` when you mean the server you configured. + +Compare the issuer as a whole string, the way the SDK compares it everywhere else, rather than picking its host out: multi-tenant providers tell tenants apart by path, +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: + +```ruby +ALLOWED_SCOPES = { "https://api.example.com/mcp" => ["mcp:read", "mcp:write"] } + +provider = MCP::Client::OAuth::Provider.new( + # client_metadata:, redirect_uri:, redirect_handler:, and callback_handler: as in the first example. + authorization_request_validator: ->(request) { + ALLOWED_ISSUERS.include?(request.authorization_server) && request.scopes.all? { |scope| ALLOWED_SCOPES.fetch(request.server_url, []).include?(scope) } + }, +) +``` + +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, +or user reaches it: on the authorization-code grant before dynamic client registration and before any browser is opened, +and on the JWT bearer grant before `assertion_provider` is invoked, since obtaining an ID-JAG tells your identity provider which authorization server +the assertion is for. Refusing therefore leaves nothing registered at, and no assertion minted for, the authorization server you rejected. +Returning a falsy value raises `Flow::AuthorizationRefusedError`. It subclasses `Flow::AuthorizationError`, so a rescue written for that still catches it, +while rescuing the narrower class tells a refusal by your own policy apart from a network or metadata failure. + +The hook decides whether to proceed, not what to ask for: the scopes are passed to the authorization server unchanged either way, because the specification +requires a client to treat the scopes in the challenge as authoritative for the operation. Leaving it unset authorizes whatever the server asked for, +which is what every MCP SDK does today. + +It is asked when a new grant is requested, not on every token refresh, which happens unattended and against an authorization server you already answered for. +An authorization server that changes between authorization and refresh is caught instead: the SDK records which one issued the tokens, and `refresh!` refuses to +present a refresh token to a different one, even when the client identity is portable across authorization servers as a Client ID Metadata Document URL is. +The transport answers that refusal by running a full authorization, which brings the new authorization server back here for you to accept or refuse. +Tokens stored before this behavior shipped carry no issuer and keep refreshing; the binding applies from their next authorization. diff --git a/lib/mcp/client/oauth.rb b/lib/mcp/client/oauth.rb index 4fc615b..ecacc88 100644 --- a/lib/mcp/client/oauth.rb +++ b/lib/mcp/client/oauth.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require_relative "oauth/authorization_request" require_relative "oauth/bounded_body" require_relative "oauth/discovery" require_relative "oauth/flow" diff --git a/lib/mcp/client/oauth/authorization_request.rb b/lib/mcp/client/oauth/authorization_request.rb new file mode 100644 index 0000000..57462f5 --- /dev/null +++ b/lib/mcp/client/oauth/authorization_request.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module MCP + class Client + module OAuth + # The decision an `authorization_request_validator` is asked to approve: which authorization server this client is about to send the user to, + # and with which scopes. + # + # Handed over as one object rather than as keyword arguments so that later revisions of the MCP authorization specification can add to it + # without changing the shape every implementer wrote. + # + # Only the named readers are the contract. The positional access `Struct` also provides (`request[0]`, `to_a`, `each`) is not, + # so the representation can change later. + AuthorizationRequest = Struct.new( + # `issuer` of the authorization server selected from the MCP server's Protected Resource Metadata. + :authorization_server, + # The scopes about to be requested, as an Array. Chosen by the MCP server, through the `WWW-Authenticate` challenge or `scopes_supported`. + :scopes, + # The MCP server URL this client was configured with. + :server_url, + # The RFC 8707 `resource` the token is being requested for. + :resource, + keyword_init: true, + ) + end + end +end diff --git a/lib/mcp/client/oauth/client_credentials_provider.rb b/lib/mcp/client/oauth/client_credentials_provider.rb index 688ff17..21276bb 100644 --- a/lib/mcp/client/oauth/client_credentials_provider.rb +++ b/lib/mcp/client/oauth/client_credentials_provider.rb @@ -60,7 +60,8 @@ module MCP private_key: nil, signing_algorithm: nil, scope: nil, - storage: nil + storage: nil, + authorization_request_validator: nil ) if blank?(client_id) raise InvalidCredentialsError, "client_id is required for the client_credentials grant." @@ -103,6 +104,7 @@ module MCP @signing_algorithm = signing_algorithm @scope = scope @storage = storage || InMemoryStorage.new + @authorization_request_validator = authorization_request_validator @storage.save_client_information(client_information) end diff --git a/lib/mcp/client/oauth/cross_app_access_provider.rb b/lib/mcp/client/oauth/cross_app_access_provider.rb index af5b599..67342c0 100644 --- a/lib/mcp/client/oauth/cross_app_access_provider.rb +++ b/lib/mcp/client/oauth/cross_app_access_provider.rb @@ -35,7 +35,7 @@ module MCP attr_reader :scope, :storage - def initialize(client_id:, client_secret:, assertion_provider:, scope: nil, storage: nil) + def initialize(client_id:, client_secret:, assertion_provider:, scope: nil, storage: nil, authorization_request_validator: nil) if blank?(client_id) raise InvalidConfigurationError, "client_id is required for the jwt-bearer grant." end @@ -51,6 +51,7 @@ module MCP @assertion_provider = assertion_provider @scope = scope @storage = storage || InMemoryStorage.new + @authorization_request_validator = authorization_request_validator @storage.save_client_information( "client_id" => client_id, "client_secret" => client_secret, diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 68ccd9c..9a825c0 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -23,6 +23,11 @@ module MCP # should leave the refresh token intact. class InvalidGrantError < AuthorizationError; end + # Raised when `authorization_request_validator` returns a falsy value. Separate from its parent + # so that a caller can tell a refusal by its own policy from a network, discovery, + # or authorization server metadata failure by rescuing a class rather than by matching the message text. + class AuthorizationRefusedError < AuthorizationError; end + def initialize(provider:, http_client_factory: nil) @provider = provider @http_client_factory = http_client_factory || -> { default_http_client } @@ -63,17 +68,22 @@ module MCP case provider_authorization_flow when :client_credentials - return run_client_credentials!(as_metadata: as_metadata, prm: prm, resource: resource, scope: scope) + return run_client_credentials!(as_metadata: as_metadata, prm: prm, resource: resource, scope: scope, server_url: server_url) when :jwt_bearer - return run_jwt_bearer!(as_metadata: as_metadata, prm: prm, resource: resource, scope: scope) + return run_jwt_bearer!(as_metadata: as_metadata, prm: prm, resource: resource, scope: scope, server_url: server_url) end ensure_pkce_supported!(as_metadata) - client_info = ensure_client_registered(as_metadata: as_metadata) - effective_scope = resolve_scope(scope: scope, prm: prm || {}) 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 + # the embedding application has just rejected. + authorize_request!(as_metadata: as_metadata, scope: effective_scope, server_url: server_url, resource: resource) + + client_info = ensure_client_registered(as_metadata: as_metadata) + pkce = PKCE.generate state = SecureRandom.urlsafe_base64(32) @@ -109,7 +119,7 @@ module MCP resource: resource, ) - @provider.save_tokens(tokens) + save_tokens_issued_by(tokens, as_metadata: as_metadata) :authorized end @@ -119,17 +129,19 @@ module MCP # and no `offline_access` augmentation because the grant does not issue a refresh token (OAuth 2.1 Section 4.3.3). # The pre-registered `client_id` / `client_secret` come from the provider's stored `client_information`. # https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization - def run_client_credentials!(as_metadata:, prm:, resource:, scope:) + def run_client_credentials!(as_metadata:, prm:, resource:, scope:, server_url:) client_info = client_credentials_client_info ensure_client_credentials_issuer!(client_info, as_metadata: as_metadata) form = { "grant_type" => "client_credentials" } effective_scope = resolve_scope(scope: scope, prm: prm) + authorize_request!(as_metadata: as_metadata, scope: effective_scope, server_url: server_url, resource: resource) form["scope"] = effective_scope if effective_scope form["resource"] = resource if resource tokens = post_to_token_endpoint(as_metadata: as_metadata, client_info: client_info, form: form) - @provider.save_tokens(tokens) + save_tokens_issued_by(tokens, as_metadata: as_metadata) + :authorized end @@ -173,12 +185,18 @@ module MCP # and security checks as `run!`; like `client_credentials`, there is no PKCE, redirect, or authorization request. # The assertion's audience is the issuer identifier that `ensure_issuer_matches!` validated. # https://github.com/modelcontextprotocol/modelcontextprotocol/issues/990 - def run_jwt_bearer!(as_metadata:, prm:, resource:, scope:) + def run_jwt_bearer!(as_metadata:, prm:, resource:, scope:, server_url:) client_info = @provider.client_information unless client_info.is_a?(Hash) && client_info_required_value(client_info, "client_id") raise AuthorizationError, "Cannot run the jwt-bearer grant: the provider has no stored `client_id`." end + # Asked before the assertion is minted, for the same reason registration waits on the authorization-code grant: + # obtaining an ID-JAG sends the identity provider an audience of this authorization server, which must not happen once + # the host has refused it. + effective_scope = resolve_scope(scope: scope, prm: prm) + authorize_request!(as_metadata: as_metadata, scope: effective_scope, server_url: server_url, resource: resource) + assertion = @provider.jwt_bearer_assertion(audience: as_metadata["issuer"], resource: resource) if assertion.nil? || assertion.to_s.empty? raise AuthorizationError, "The provider's assertion_provider returned no ID-JAG assertion." @@ -188,12 +206,11 @@ module MCP "grant_type" => "urn:ietf:params:oauth:grant-type:jwt-bearer", "assertion" => assertion, } - effective_scope = resolve_scope(scope: scope, prm: prm) form["scope"] = effective_scope if effective_scope form["resource"] = resource if resource tokens = post_to_token_endpoint(as_metadata: as_metadata, client_info: client_info, form: form) - @provider.save_tokens(tokens) + save_tokens_issued_by(tokens, as_metadata: as_metadata) :authorized end @@ -242,6 +259,8 @@ module MCP server_url: server_url, ) + ensure_token_issuer!(as_metadata: as_metadata) + client_info = if have_stored_client_info # Pre-registered / DCR-issued `client_information` always wins: if the user picked an explicit identity, # do not silently swap it for the CIMD URL even when the AS also advertises CIMD support. @@ -262,7 +281,7 @@ module MCP resource: resource, ) - @provider.save_tokens(preserve_refresh_token(new_tokens, refresh_token)) + save_tokens_issued_by(preserve_refresh_token(new_tokens, refresh_token), as_metadata: as_metadata) :refreshed end @@ -499,6 +518,43 @@ module MCP # the authorization server on two loopback ports), and deployments that never leave a corporate network. # Only a server reachable on the public internet is barred from steering the client inward. # https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization + # 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. + # 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. + # + # 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: + # an authorization server that has changed since the tokens were issued sends the flow back through here, + # where the host sees the new one and decides again. + def authorize_request!(as_metadata:, scope:, server_url:, resource:) + return unless @provider.respond_to?(:authorization_request_validator) + return unless (validator = @provider.authorization_request_validator) + + request = AuthorizationRequest.new( + authorization_server: as_metadata["issuer"], + scopes: scope.to_s.split, + server_url: server_url, + resource: resource, + ).freeze + + return if validator.call(request) + + raise AuthorizationRefusedError, <<~MESSAGE + The authorization request was refused by `authorization_request_validator` \ + (authorization server #{request.authorization_server.inspect}, scopes #{request.scopes.inspect}). + MESSAGE + end + def ensure_routable_destination!(url, label:, server_url:) return unless private_network_url?(url) return if private_network_url?(server_url) @@ -669,6 +725,39 @@ module MCP # the error and falls back to the full flow, which discards the stale registration and re-registers. # Credentials without an `"issuer"` binding predate this check and are allowed through; # CIMD `client_id`s are portable. + # Records which authorization server minted these tokens, alongside the tokens themselves. + # SEP-2352 already binds *client credentials* to their issuer; this binds the *tokens*, which that + # SEP leaves unbound. + # Stored in the token hash so it travels through any `storage` a caller supplied, without widening + # the storage interface that custom implementations have to satisfy. + def save_tokens_issued_by(tokens, as_metadata:) + issuer = as_metadata["issuer"] + tokens = tokens.merge("issuer" => issuer) if tokens.is_a?(Hash) && issuer + + @provider.save_tokens(tokens) + end + + # Refuses to present a refresh token to an authorization server other than the one that issued it. + # `refresh!` rediscovers the authorization server from Protected Resource Metadata every time, + # so the server named for a session can differ from the one the stored tokens came from. + # Unlike `ensure_refreshable_client_information!` this holds whatever the client identity is, + # including a Client ID Metadata Document URL, which is portable across authorization servers precisely + # so that re-registration is unnecessary. + # + # Tokens stored before this shipped carry no issuer and are left alone, matching how SEP-2352 treats + # client information without one; the binding takes effect from their next authorization. + # The caller answers a refusal by running the full authorization flow, which is where the embedding application + # is asked about the new authorization server. + def ensure_token_issuer!(as_metadata:) + stored_issuer = read_token("issuer") + return if stored_issuer.nil? || stored_issuer == as_metadata["issuer"] + + raise AuthorizationError, <<~MESSAGE + Cannot refresh: the stored tokens were issued by a different authorization server (stored issuer #{stored_issuer.inspect}, \ + current #{as_metadata["issuer"].inspect}); re-authorization is required. + MESSAGE + end + def ensure_refreshable_client_information!(client_info, as_metadata:) stored_issuer = client_info_required_value(client_info, "issuer") return if stored_issuer.nil? diff --git a/lib/mcp/client/oauth/provider.rb b/lib/mcp/client/oauth/provider.rb index 741c6c5..7d1a0f6 100644 --- a/lib/mcp/client/oauth/provider.rb +++ b/lib/mcp/client/oauth/provider.rb @@ -86,7 +86,8 @@ module MCP callback_handler:, scope: nil, storage: nil, - client_id_metadata_document_url: nil + client_id_metadata_document_url: nil, + authorization_request_validator: nil ) unless Discovery.secure_url?(redirect_uri) raise InsecureRedirectURIError, @@ -115,6 +116,7 @@ module MCP @scope = scope @storage = storage || InMemoryStorage.new @client_id_metadata_document_url = client_id_metadata_document_url + @authorization_request_validator = authorization_request_validator end # Identifies the OAuth flow this provider drives. diff --git a/lib/mcp/client/oauth/storage_backed_provider.rb b/lib/mcp/client/oauth/storage_backed_provider.rb index 499db6b..44d09a1 100644 --- a/lib/mcp/client/oauth/storage_backed_provider.rb +++ b/lib/mcp/client/oauth/storage_backed_provider.rb @@ -14,6 +14,13 @@ module MCP # `save_tokens(tokens)`, `client_information`, and `save_client_information(info)` # (see `InMemoryStorage`). module StorageBackedProvider + # Optional `->(request) { true | false }` hook, called with an `AuthorizationRequest`. + # An MCP server names its own authorization server and the scopes to ask for, so this is where + # an embedding application sees both before the request is made and can decline to proceed. + # `nil` (the default) authorizes whatever the server asked for, which is the behavior every + # MCP SDK has today. + attr_reader :authorization_request_validator + def access_token tokens&.dig("access_token") || tokens&.dig(:access_token) end diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index 511e23f..dc78e03 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -363,6 +363,107 @@ module MCP Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) end + # 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: []) + state_holder = {} + provider = 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) { + recorder << :redirected + state_holder[:state] = URI.decode_www_form(url.query).to_h.fetch("state") + }, + callback_handler: -> { ["test-auth-code", state_holder[:state]] }, + authorization_request_validator: ->(request) { + recorder << request + approve + }, + ) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + def test_run_hands_the_authorization_server_and_scopes_to_the_validator + 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: ["Mail.Read", "Files.ReadWrite.All"], + ), + ) + + recorder = [] + run_flow_with_validator(approve: true, recorder: recorder) + + request = recorder.first + + assert_equal(@auth_base, request.authorization_server) + assert_equal(["Mail.Read", "Files.ReadWrite.All"], request.scopes) + assert_equal(@server_url, request.server_url) + assert_equal("https://srv.example.com/mcp", request.resource) + end + + def test_run_refuses_the_flow_and_registers_nothing_when_the_validator_declines + recorder = [] + + error = assert_raises(Flow::AuthorizationRefusedError) do + run_flow_with_validator(approve: false, recorder: recorder) + end + + assert_match(/refused by `authorization_request_validator`/, error.message) + + # Refused before registration, so nothing of this client is left at the authorization server + # the host just rejected, and no browser was opened. + assert_not_requested(:post, "#{@auth_base}/register") + refute_includes(recorder, :redirected) + end + + def test_run_client_credentials_consults_the_validator + provider = ClientCredentialsProvider.new( + client_id: "cc-client", + client_secret: "cc-secret", + authorization_request_validator: ->(_request) { false }, + ) + + error = assert_raises(Flow::AuthorizationRefusedError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/refused by `authorization_request_validator`/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_jwt_bearer_consults_the_validator + minted = false + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(audience:, resource:) { + minted = true + "id-jag-assertion" + }, + authorization_request_validator: ->(_request) { false }, + ) + + error = assert_raises(Flow::AuthorizationRefusedError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/refused by `authorization_request_validator`/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + + # Refused before the identity provider was asked for an assertion carrying this authorization server as its audience. + refute(minted, "the ID-JAG assertion must not be minted for a refused authorization server") + end + def test_run_registers_native_application_type_for_loopback_redirect_uri run_authorization_flow @@ -2264,6 +2365,64 @@ module MCP assert_equal("saved-rt", provider.tokens["refresh_token"]) end + def test_run_records_the_issuer_that_minted_the_tokens + state_holder = {} + provider = 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]] }, + ) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(@auth_base, provider.tokens["issuer"]) + end + + def test_refresh_refuses_an_authorization_server_that_did_not_issue_the_tokens + # The stored tokens came from a different authorization server, so this refresh token must not reach the one named now. + stub_request(:post, "#{@auth_base}/token").to_raise(StandardError.new("must not reach the token endpoint.")) + + provider = refresh_only_provider + provider.save_tokens( + "access_token" => "stale-at", + "refresh_token" => "saved-rt", + "issuer" => "https://issued-by.example.com", + ) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/issued by a different authorization server/, error.message) + + # Left in place: it is still good at the server that issued it, and the caller answers this by running the full authorization flow. + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + + def test_refresh_proceeds_for_tokens_stored_before_the_issuer_was_recorded + stub_request(:post, "#{@auth_base}/token").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(access_token: "fresh-at", token_type: "Bearer"), + ) + + provider = refresh_only_provider + 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) + + # Recorded on the way out, so the binding takes effect from here on. + assert_equal(@auth_base, provider.tokens["issuer"]) + 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"), @@ -2913,6 +3072,20 @@ module MCP URI.decode_www_form(req.body).to_h["resource"] == "https://srv.example.com/mcp" end end + + private + + def refresh_only_provider + 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") + + provider + end end end end diff --git a/test/mcp/client/oauth/http_oauth_test.rb b/test/mcp/client/oauth/http_oauth_test.rb index 73fcf04..9970971 100644 --- a/test/mcp/client/oauth/http_oauth_test.rb +++ b/test/mcp/client/oauth/http_oauth_test.rb @@ -1433,6 +1433,94 @@ module MCP assert_requested(:post, "#{@auth_base}/token", times: 1) end + def test_send_request_reauthorizes_when_the_authorization_server_did_not_issue_the_stored_tokens + # The refusal in `refresh!` is only half the answer; this is the other half. + # A renamed authorization server makes the refresh fail, the transport falls through to a full authorization, + # and that is where the embedding application is asked about the new one. + stub_request(:post, @mcp_url).with { |req| + req.headers["Authorization"] != "Bearer fresh-at" + }.to_return( + status: 401, + headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") }, + body: "", + ) + stub_request(:post, @mcp_url).with( + headers: { "Authorization" => "Bearer fresh-at" } + ).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(jsonrpc: "2.0", id: "1", result: { ok: true }), + ) + + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: @mcp_url, authorization_servers: [@auth_base]), + ) + stub_request(:get, "#{@auth_base}/.well-known/oauth-authorization-server").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"], + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + ), + ) + stub_request(:post, "#{@auth_base}/register").to_return( + status: 201, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(client_id: "reauthorized-client"), + ) + stub_request(:post, "#{@auth_base}/token").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(access_token: "fresh-at", token_type: "Bearer"), + ) + + seen = [] + state_holder = {} + provider = 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]] }, + authorization_request_validator: ->(request) { + seen << request.authorization_server + true + }, + ) + + # Registered and refreshable, so the refresh path is genuinely reachable and the issuer binding is + # what stops it rather than a missing client identity. + provider.save_client_information("client_id" => "test-client") + provider.save_tokens( + "access_token" => "stale-at", + "refresh_token" => "saved-rt", + "issuer" => "https://issued-by.example.com", + ) + + HTTP.new(url: @mcp_url, oauth: provider).send_request( + request: { jsonrpc: "2.0", id: "1", method: "tools/call", params: {} }, + ) + + # The refresh token was never presented to the authorization server now being named. + assert_not_requested(:post, "#{@auth_base}/token") do |req| + URI.decode_www_form(req.body).to_h["grant_type"] == "refresh_token" + end + assert_equal([@auth_base], seen) + assert_equal(@auth_base, provider.tokens["issuer"]) + end + private # Stubs PRM, AS metadata, /register, and /token so a full authorization From 6f0b85486bd1fd43ee7975939c31605f60fdbfdd Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 1 Sep 2026 01:14:59 +0900 Subject: [PATCH 04/24] [Doc] Mark the deprecated features on the server overview The overview's Key Features and Supported Methods listed `roots`, `sampling`, and `logging/setLevel` with no deprecation marking, while their own pages carry the SEP-2577 warnings, presenting a current-looking feature surface to a reader skimming the overview. The five entries now carry the pages' "deprecated as of 2026-07-28" wording; `elicitation/create` stays unmarked - it is not deprecated, and the modern lifecycle keeps carrying it as requests embedded per the multi round-trip requests pattern. --- docs/_server/index.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/_server/index.md b/docs/_server/index.md index 2872959..5d938f1 100644 --- a/docs/_server/index.md +++ b/docs/_server/index.md @@ -22,8 +22,8 @@ It implements the Model Context Protocol specification, handling model context r - Supports resource registration and retrieval - Supports stdio and Streamable HTTP (including SSE) transports - Supports notifications for list changes (tools, prompts, resources) -- Supports roots (server-to-client filesystem boundary queries) -- Supports sampling (server-to-client LLM completion requests) +- Supports roots (server-to-client filesystem boundary queries; deprecated as of 2026-07-28) +- Supports sampling (server-to-client LLM completion requests; deprecated as of 2026-07-28) - Supports cursor-based pagination for list operations - Supports cancellation of in-flight requests on both server and client (notifications/cancelled) @@ -37,7 +37,7 @@ It implements the Model Context Protocol specification, handling model context r - Multi round-trip `input_required` results (MCP 2026-07-28, SEP-2322): handlers return `MCP::Server::InputRequiredResult` to ask the client for additional input instead of performing a server-initiated request; see [Multi Round-Trip Requests](/server/mrtr/) - `ping` - Simple health check -- `logging/setLevel` - Configures the minimum log level for the server +- `logging/setLevel` - Configures the minimum log level for the server (deprecated as of 2026-07-28) - `tools/list` - Lists all registered tools and their schemas - `tools/call` - Invokes a specific tool with provided arguments - `prompts/list` - Lists all registered prompts and their schemas @@ -48,6 +48,6 @@ It implements the Model Context Protocol specification, handling model context r - `resources/subscribe` - Subscribes to updates for a specific resource - `resources/unsubscribe` - Unsubscribes from updates for a specific resource - `completion/complete` - Returns autocompletion suggestions for prompt arguments and resource URIs -- `roots/list` - Requests filesystem roots from the client (server-to-client) -- `sampling/createMessage` - Requests LLM completion from the client (server-to-client) +- `roots/list` - Requests filesystem roots from the client (server-to-client; deprecated as of 2026-07-28) +- `sampling/createMessage` - Requests LLM completion from the client (server-to-client; deprecated as of 2026-07-28) - `elicitation/create` - Requests user input from the client (server-to-client) From 088f4b99e4fdc9db1bb562be2591fc8425e14a4c Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 3 Sep 2026 16:55:06 +0900 Subject: [PATCH 05/24] Answer server-to-client pings in the client ## Motivation and Context The ping utility says the receiver "MUST respond promptly with an empty response", and a server may treat a missing answer as a stale connection and drop it. Both reference SDKs honor that on the client side (the TypeScript protocol layer installs an automatic pong, and the Python session answers `PingRequest` with an empty result), but this client answered a server ping with Method not found over Streamable HTTP and ignored it entirely over stdio, leaving long-lived legacy sessions vulnerable to a liveness-checking server. Over HTTP, an unhandled `ping` is now answered with the empty result in `dispatch_server_request`. The default deliberately does not live in `@server_request_handlers`: that registry's `any?` opens the GET listening stream on connect, and a built-in pong must not change listener behavior - a server can only ping on a stream that is already open. A handler registered via `on_server_request("ping")` still wins. Over stdio, the read loop now answers a ping frame inline and keeps waiting for its response. Frames are told apart by shape rather than id: one carrying `method` is a request or notification, never the awaited response, so a server-chosen ping id that collides with the awaited one is still answered - and a request frame can no longer be returned to the caller as if it were the response. Between requests nothing reads stdout, so a queued ping is answered when the next request starts reading, which is inherent to the single-threaded stdio client. Other server-to-client requests over stdio stay unsupported as documented. The pong is best effort on both transports, in the ways the reference SDKs imply: a pong that cannot be written over a broken stdin is dropped instead of failing the unrelated request whose response the loop is reading, and a ping whose id is not a String or a Number - a shape the TypeScript and Python schemas reject outright - is not answered with a pong: over stdio it is skipped entirely, and over HTTP it falls through to Method not found, exactly as before this change. The modern lifecycle removes `ping` altogether (SEP-2575), so this applies to handshake-lifecycle connections only. ## How Has This Been Tested? New transport tests: over HTTP, a ping delivered on the SSE stream results in a POSTed empty-result response with the ping's id, and a user-registered `"ping"` handler overrides the default; over stdio, a ping injected while a response is awaited is answered on stdin and the original response still returns. Both fail without the change (Method not found / no answer). Further tests cover the best-effort edges: a pong write failure over a closed stdin leaves the awaited response intact, and a malformed-id ping is skipped over stdio and answered with Method not found over HTTP. `bundle exec rake` (tests, RuboCop, and conformance baseline) passes. ## Breaking Changes None. A server ping that previously received Method not found (or nothing) now receives the empty result the specification requires. --- docs/_client/ping.md | 10 ++ docs/_client/transports.md | 6 + lib/mcp/client/http.rb | 13 ++ lib/mcp/client/stdio.rb | 28 ++++ test/mcp/client/http_test.rb | 125 ++++++++++++++++++ test/mcp/client/stdio_test.rb | 239 ++++++++++++++++++++++++++++++++++ 6 files changed, 421 insertions(+) diff --git a/docs/_client/ping.md b/docs/_client/ping.md index e01e21b..f6889e9 100644 --- a/docs/_client/ping.md +++ b/docs/_client/ping.md @@ -29,6 +29,16 @@ is not a Hash (matching the spec requirement that `result` be an object). Transport-level errors (for example, `MCP::Client::Stdio`'s `read_timeout:` firing) propagate as exceptions raised by the transport layer. +## Answering Server Pings + +On handshake-lifecycle connections a server may ping the client the same way, and the client answers automatically with +the empty result - no handler is needed. Registering `transport.on_server_request("ping")` on `MCP::Client::HTTP` replaces +the automatic answer. +Over stdio, a ping that arrives while a response is awaited is answered inline; between requests it is answered when +the next request starts reading. The answer is best effort: a pong that cannot be written (for example, over a broken pipe) is +dropped rather than failing the request whose response is being read. Independently of pings, consider setting `read_timeout:` +on `MCP::Client::Stdio`, since a server that never answers otherwise holds the read until the process exits. + ## Server Side How servers answer `ping` requests and ping the client themselves is documented on diff --git a/docs/_client/transports.md b/docs/_client/transports.md index 1e1aaac..a8a4764 100644 --- a/docs/_client/transports.md +++ b/docs/_client/transports.md @@ -233,6 +233,12 @@ http_transport = MCP::Client::HTTP.new(url: "https://api.example.com/mcp") do |f end ``` +{: .note } +> Answers to server-to-client requests (a pong, an elicitation result) are POSTed from inside +> the SSE streaming callback of another response, re-entering the connection on the same thread. +> The default Net::HTTP adapter opens a connection per request, which makes this safe; an adapter +> with persistent connections must tolerate that re-entry. + ## Custom Transports If the transport layer you need is not included in the gem, you can build and pass your own instances so long as they conform to the following interface: diff --git a/lib/mcp/client/http.rb b/lib/mcp/client/http.rb index e189c74..1c3fd86 100644 --- a/lib/mcp/client/http.rb +++ b/lib/mcp/client/http.rb @@ -1118,6 +1118,19 @@ module MCP }, } end + elsif message["method"] == MCP::Methods::PING && (message["id"].is_a?(String) || message["id"].is_a?(Numeric)) + # The ping receiver "MUST respond promptly with an empty response" (MCP ping utility), + # so an unhandled ping is answered with the empty result rather than Method not found. + # A JSON-RPC id is a String or a Number; the reference SDKs reject other shapes at + # schema validation, so a ping carrying one falls through to Method not found instead. + # The default lives here instead of in `@server_request_handlers`, whose `any?` opens + # the GET listening stream on connect: a built-in pong must not change listener behavior. + # A handler registered via `on_server_request("ping")` still wins through the branch above. + { + jsonrpc: JsonRpcHandler::Version::V2_0, + id: message["id"], + result: {}, + } else { jsonrpc: JsonRpcHandler::Version::V2_0, diff --git a/lib/mcp/client/stdio.rb b/lib/mcp/client/stdio.rb index fee1a3d..0fbb96e 100644 --- a/lib/mcp/client/stdio.rb +++ b/lib/mcp/client/stdio.rb @@ -436,6 +436,18 @@ module MCP parsed = JSON.parse(line.strip) + # A frame carrying `method` is a request or notification, never the awaited response, + # whatever its id says: JSON-RPC id spaces are per sender, so a server-chosen ping id + # may legitimately collide with the awaited one. Pings are answered; + # other server-to-client requests over stdio stay unsupported as documented, + # and notifications carry no id. + if parsed.is_a?(Hash) && parsed.key?("method") + # A JSON-RPC id is a String or a Number; the reference SDKs reject other shapes at + # schema validation, so a ping carrying one is skipped rather than echoed back. + answer_ping(parsed) if parsed["method"] == MCP::Methods::PING && json_rpc_id?(parsed["id"]) + next + end + # A JSON-RPC message is an object; skip a non-object frame (array or scalar) # the same way as a frame without an id. next unless parsed.is_a?(Hash) && parsed.key?("id") @@ -451,6 +463,22 @@ module MCP ) end + # The ping receiver "MUST respond promptly with an empty response". Answered inline from + # the read loop, so a keepalive cannot stall unanswered while a response is awaited; + # between requests nothing reads stdout, and a queued ping is answered on the next read. + def answer_ping(parsed) + @write_mutex.synchronize do + write_message({ jsonrpc: JsonRpcHandler::Version::V2_0, id: parsed["id"], result: {} }) + end + rescue RequestHandlerError + # Best effort: a pong cannot be delivered over a broken stdin, and the failure must not + # surface as an error of the unrelated request whose response the loop is reading. + end + + def json_rpc_id?(id) + id.is_a?(String) || id.is_a?(Numeric) + end + def ensure_running! return if @wait_thread.alive? diff --git a/test/mcp/client/http_test.rb b/test/mcp/client/http_test.rb index 87cdb55..077db31 100644 --- a/test/mcp/client/http_test.rb +++ b/test/mcp/client/http_test.rb @@ -1303,6 +1303,131 @@ module MCP assert_equal(100, received_params["maxTokens"]) end + def test_send_request_answers_a_server_ping_with_an_empty_result + request = { + jsonrpc: "2.0", + id: "test_id", + method: "tools/call", + params: { name: "some_tool", arguments: {} }, + } + + server_ping = { jsonrpc: "2.0", id: 7, method: "ping" } + tool_result = { jsonrpc: "2.0", id: "test_id", result: { content: [] } } + sse_body = <<~BODY + event: message + data: #{server_ping.to_json} + + event: message + data: #{tool_result.to_json} + + BODY + + stub_request(:post, url).with( + body: request.to_json, + ).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: sse_body, + ) + + pong = { jsonrpc: "2.0", id: 7, result: {} } + pong_stub = stub_request(:post, url).with( + body: pong.to_json, + ).to_return(status: 202, body: "") + + response = client.send_request(request: request) + + assert_equal({ "content" => [] }, response["result"]) + assert_requested(pong_stub) + end + + def test_a_registered_ping_handler_overrides_the_default_pong + request = { + jsonrpc: "2.0", + id: "test_id", + method: "tools/call", + params: { name: "some_tool", arguments: {} }, + } + + server_ping = { jsonrpc: "2.0", id: 8, method: "ping" } + tool_result = { jsonrpc: "2.0", id: "test_id", result: { content: [] } } + sse_body = <<~BODY + event: message + data: #{server_ping.to_json} + + event: message + data: #{tool_result.to_json} + + BODY + + stub_request(:post, url).with( + body: request.to_json, + ).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: sse_body, + ) + + expected_response = { jsonrpc: "2.0", id: 8, result: { custom: "pong" } } + response_stub = stub_request(:post, url).with( + body: expected_response.to_json, + ).to_return(status: 202, body: "") + + client.on_server_request("ping") { { custom: "pong" } } + + response = client.send_request(request: request) + + assert_equal({ "content" => [] }, response["result"]) + assert_requested(response_stub) + end + + def test_a_ping_with_a_malformed_id_is_answered_with_method_not_found + request = { + jsonrpc: "2.0", + id: "test_id", + method: "tools/call", + params: { name: "some_tool", arguments: {} }, + } + + # A Hash id is not a JSON-RPC id; the reference SDKs reject such frames at schema + # validation, so the default pong does not fire and the frame falls through. + server_ping = { jsonrpc: "2.0", id: { x: 1 }, method: "ping" } + tool_result = { jsonrpc: "2.0", id: "test_id", result: { content: [] } } + sse_body = <<~BODY + event: message + data: #{server_ping.to_json} + + event: message + data: #{tool_result.to_json} + + BODY + + stub_request(:post, url).with( + body: request.to_json, + ).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: sse_body, + ) + + expected_error = { + jsonrpc: "2.0", + id: { x: 1 }, + error: { + code: JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, + message: "Method not found: ping", + }, + } + error_stub = stub_request(:post, url).with( + body: expected_error.to_json, + ).to_return(status: 202, body: "") + + response = client.send_request(request: request) + + assert_equal({ "content" => [] }, response["result"]) + assert_requested(error_stub) + end + def test_send_request_answers_unregistered_server_request_with_method_not_found request = { jsonrpc: "2.0", diff --git a/test/mcp/client/stdio_test.rb b/test/mcp/client/stdio_test.rb index 3b16040..08e8d17 100644 --- a/test/mcp/client/stdio_test.rb +++ b/test/mcp/client/stdio_test.rb @@ -85,6 +85,245 @@ module MCP stderr_read.close end + def test_send_request_answers_a_server_ping_inline + stdin_read, stdin_write = IO.pipe + stdout_read, stdout_write = IO.pipe + stderr_read, _ = IO.pipe + + Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread]) + + transport = Stdio.new(command: "ruby", args: ["server.rb"]) + + request = { + jsonrpc: "2.0", + id: "test-id", + method: "tools/list", + } + + pong_line = nil + server_thread = Thread.new do + init_line = stdin_read.gets + init_request = JSON.parse(init_line) + stdout_write.puts(JSON.generate({ + jsonrpc: "2.0", + id: init_request["id"], + result: { + protocolVersion: "2025-11-25", + capabilities: {}, + serverInfo: { name: "test-server", version: "1.0.0" }, + }, + })) + stdout_write.flush + + # Read initialized notification, then the tools/list request. + stdin_read.gets + stdin_read.gets + + # Ping the client while it awaits the response; the receiver "MUST respond promptly". + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: "srv-ping-1", method: "ping" })) + stdout_write.flush + + pong_line = stdin_read.gets + + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: "test-id", result: { tools: [] } })) + stdout_write.flush + end + + transport.connect + response = transport.send_request(request: request) + server_thread.join + + assert_equal("test-id", response["id"]) + pong = JSON.parse(pong_line) + assert_equal("srv-ping-1", pong["id"]) + assert_equal({}, pong["result"]) + refute(pong.key?("error")) + ensure + server_thread.join + stdin_read.close + stdin_write.close + stdout_read.close + stdout_write.close + end + + def test_a_server_ping_reusing_the_awaited_id_is_answered_and_not_returned_as_the_response + stdin_read, stdin_write = IO.pipe + stdout_read, stdout_write = IO.pipe + stderr_read, _ = IO.pipe + + Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread]) + + transport = Stdio.new(command: "ruby", args: ["server.rb"]) + + request = { + jsonrpc: "2.0", + id: "test-id", + method: "tools/list", + } + + pong_line = nil + server_thread = Thread.new do + init_line = stdin_read.gets + init_request = JSON.parse(init_line) + stdout_write.puts(JSON.generate({ + jsonrpc: "2.0", + id: init_request["id"], + result: { + protocolVersion: "2025-11-25", + capabilities: {}, + serverInfo: { name: "test-server", version: "1.0.0" }, + }, + })) + stdout_write.flush + + # Read initialized notification, then the tools/list request. + stdin_read.gets + stdin_read.gets + + # JSON-RPC id spaces are per sender, so a server ping may reuse the id the client + # is awaiting; it must still be answered rather than treated as the response. + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: "test-id", method: "ping" })) + stdout_write.flush + + pong_line = stdin_read.gets + + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: "test-id", result: { tools: [] } })) + stdout_write.flush + end + + transport.connect + response = transport.send_request(request: request) + server_thread.join + + assert_equal("test-id", response["id"]) + assert_empty(response.dig("result", "tools")) + pong = JSON.parse(pong_line) + assert_equal("test-id", pong["id"]) + assert_equal({}, pong["result"]) + ensure + server_thread.join + stdin_read.close + stdin_write.close + stdout_read.close + stdout_write.close + end + + def test_a_pong_that_cannot_be_written_does_not_fail_the_awaited_request + stdin_read, stdin_write = IO.pipe + stdout_read, stdout_write = IO.pipe + stderr_read, _ = IO.pipe + + Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread]) + + transport = Stdio.new(command: "ruby", args: ["server.rb"]) + + request = { + jsonrpc: "2.0", + id: "test-id", + method: "tools/list", + } + + server_thread = Thread.new do + init_line = stdin_read.gets + init_request = JSON.parse(init_line) + stdout_write.puts(JSON.generate({ + jsonrpc: "2.0", + id: init_request["id"], + result: { + protocolVersion: "2025-11-25", + capabilities: {}, + serverInfo: { name: "test-server", version: "1.0.0" }, + }, + })) + stdout_write.flush + + # Read initialized notification, then the tools/list request. + stdin_read.gets + stdin_read.gets + + # Break the write path: the pong cannot be delivered, and that must not fail the request. + stdin_read.close + + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: "srv-ping-1", method: "ping" })) + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: "test-id", result: { tools: [] } })) + stdout_write.flush + end + + transport.connect + response = transport.send_request(request: request) + + assert_equal("test-id", response["id"]) + assert_empty(response.dig("result", "tools")) + ensure + server_thread.join + stdin_read.close unless stdin_read.closed? + stdin_write.close + stdout_read.close + stdout_write.close + end + + def test_a_ping_with_a_malformed_id_is_not_answered + stdin_read, stdin_write = IO.pipe + stdout_read, stdout_write = IO.pipe + stderr_read, _ = IO.pipe + + Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread]) + + transport = Stdio.new(command: "ruby", args: ["server.rb"]) + + request = { + jsonrpc: "2.0", + id: "test-id", + method: "tools/list", + } + + pong_line = nil + server_thread = Thread.new do + init_line = stdin_read.gets + init_request = JSON.parse(init_line) + stdout_write.puts(JSON.generate({ + jsonrpc: "2.0", + id: init_request["id"], + result: { + protocolVersion: "2025-11-25", + capabilities: {}, + serverInfo: { name: "test-server", version: "1.0.0" }, + }, + })) + stdout_write.flush + + # Read initialized notification, then the tools/list request. + stdin_read.gets + stdin_read.gets + + # A Hash id is not a JSON-RPC id; the reference SDKs reject such frames outright. + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: { "x" => 1 }, method: "ping" })) + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: "srv-ping-2", method: "ping" })) + stdout_write.flush + + # The single pong that arrives must answer the well-formed ping, not the malformed one. + pong_line = stdin_read.gets + + stdout_write.puts(JSON.generate({ jsonrpc: "2.0", id: "test-id", result: { tools: [] } })) + stdout_write.flush + end + + transport.connect + response = transport.send_request(request: request) + server_thread.join + + assert_equal("test-id", response["id"]) + pong = JSON.parse(pong_line) + assert_equal("srv-ping-2", pong["id"]) + assert_equal({}, pong["result"]) + ensure + server_thread.join + stdin_read.close + stdin_write.close + stdout_read.close + stdout_write.close + end + def test_send_request_skips_notifications stdin_read, stdin_write = IO.pipe stdout_read, stdout_write = IO.pipe From 09c310341b9421189e0ba0b1c58f805a2ad7c43f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 4 Sep 2026 00:16:39 +0900 Subject: [PATCH 06/24] Fix the flaky thread count assertion in the listen keepalive test ## Motivation and Context "listen keepalive is not started when the interval is nil" fails intermittently on CI. The most recent run failed on Ruby 3.2 alone while 3.1 and head passed, and a re-run of the same commit went green. The assertion samples a process-wide count: ```ruby before = Thread.list.size open_listen_stream(id: "listen-1", notifications: { toolsListChanged: true }) assert_equal before, Thread.list.size, "a nil interval must not spawn a keepalive thread" ``` `Thread.list` covers the whole process, so the count moves for reasons this test has no interest in. The observed failure reported 54 against 51: three threads had *gone away* between the samples, not appeared. This file starts threads in sixteen places, and the SSE stream and reaper threads they leave behind finish whenever they finish, so whether the count holds still depends on the test order and on the scheduler rather than on the behavior under test. The test itself is asking the right question. `setup` builds its transport with `listen_keepalive_interval: nil` precisely so opening a listen stream spawns no timer, and the point is to hold that guarantee. Only the measurement is wrong, so this compares the set of threads instead: a thread that disappears no longer registers, and a thread that appears still does. Minitest runs these tests serially and `open_listen_stream` drives the SSE body inline rather than on a thread, so a keepalive timer is the only thread that can appear across the two samples. No library code changes. ## How Has This Been Tested? The rewritten assertion was checked for sensitivity before being kept: a temporary probe pointed `open_listen_stream` at a transport built with `listen_keepalive_interval: 15` and asserted the set difference was *not* empty. It passed, confirming the assertion still catches a spawned keepalive thread rather than merely never failing. The probe was removed afterwards. The failure did not reproduce locally beforehand, across six seeds of the file on its own and six full-suite runs, which matches a timing-sensitive flake that needs a slower machine. ## Breaking Changes None. --- .../server/transports/streamable_http_transport_test.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/mcp/server/transports/streamable_http_transport_test.rb b/test/mcp/server/transports/streamable_http_transport_test.rb index 36fe9ac..bf032b2 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -6307,10 +6307,13 @@ module MCP end test "listen keepalive is not started when the interval is nil" do - before = Thread.list.size + # The set of threads, not their count: `Thread.list` is process-wide, so a thread another test left running + # that finishes between the two samples moves the count in the direction this assertion does not care about. + # Only a thread that appears is evidence of a keepalive timer. + before = Thread.list open_listen_stream(id: "listen-1", notifications: { toolsListChanged: true }) - assert_equal before, Thread.list.size, "a nil interval must not spawn a keepalive thread" + assert_empty(Thread.list - before, "a nil interval must not spawn a keepalive thread") end test "listen_keepalive_interval rejects a non-positive value" do From 33525d6494ff3b9ffa81a708151a5b0c2e5fb5db Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 5 Sep 2026 02:28:50 +0900 Subject: [PATCH 07/24] Release 1.5.0 This release makes the client answer a server's `ping` with the empty result the specification requires: earlier clients replied with Method not found over Streamable HTTP and stayed silent over stdio, so a server that checks liveness dropped their long-lived sessions. On the OAuth side, the embedding application can refuse an authorization request through `authorization_request_validator`, and stored tokens are refreshed only against the authorization server that issued them. The validator is what makes this a minor release. The refresh check is the one behavior change: when the authorization server named for a session differs from the one that issued the stored tokens, the client reauthorizes instead of refreshing, and tokens stored by earlier releases, which carry no issuer, keep refreshing. --- CHANGELOG.md | 18 ++++++++++++++++++ lib/mcp/version.rb | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81a301c..051f239 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.5.0] - 2026-09-05 + +This release makes the client answer a server's `ping` with the empty result the specification requires. +Earlier clients replied with Method not found over Streamable HTTP and stayed silent over stdio, +so a server that checks liveness dropped their long-lived sessions. On the OAuth side, +the embedding application can refuse an authorization request through `authorization_request_validator`, +and stored tokens are refreshed only against the authorization server that issued them: +when the server named for a session differs, the client reauthorizes instead of refreshing, +and the validator sees the newly named server. Tokens stored by earlier releases carry no issuer and keep refreshing. + +### Added + +- Add `authorization_request_validator` to let the embedding application refuse an authorization request (#539) + +### Fixed + +- Answer server-to-client pings in the client (#541) + ## [1.4.0] - 2026-08-28 This release repairs the `subscriptions/listen` stream. A host that buffers its responses, diff --git a/lib/mcp/version.rb b/lib/mcp/version.rb index 8699187..1bbb8dd 100644 --- a/lib/mcp/version.rb +++ b/lib/mcp/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module MCP - VERSION = "1.4.0" + VERSION = "1.5.0" end From 0da02918877bbef8a243b134ce9caa8b87a4a226 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 6 Sep 2026 03:19:52 +0900 Subject: [PATCH 08/24] [Doc] Explain when the HTTP client needs event_stream_parser ## Motivation and Context The Installation page names faraday as the example of an optional dependency but leaves out event_stream_parser, which the HTTP client transport needs as soon as a server answers with SSE. The Transports page documents both gems, so a reader who starts from Installation learns about the second one only when a streaming response fails to parse. The Transports page also marks event_stream_parser as "optional, required only for SSE responses", which reads as a corner case. Whether a response is JSON or SSE is the server's choice, made for each response, a client must accept both, and this SDK's own server picks SSE by default and always on the modern lifecycle, so a client written for arbitrary servers needs both gems. Both pages now say so, and the Transports page spells out the one setup that can leave the gem out: a server known to answer with JSON alone, such as this SDK's server in JSON response mode serving handshake-lifecycle clients, and no listening stream. The Installation page shows the faraday-only Gemfile first and then the one that adds event_stream_parser, so both cases are visible at a glance. ## How Has This Been Tested? Documentation only; the Gemfile snippets on both pages match. ## Breaking Changes None. --- docs/_client/transports.md | 9 ++++++++- docs/installation.md | 12 +++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/_client/transports.md b/docs/_client/transports.md index a8a4764..a30259c 100644 --- a/docs/_client/transports.md +++ b/docs/_client/transports.md @@ -64,7 +64,14 @@ The stdio transport automatically handles: Use the `MCP::Client::HTTP` transport to interact with MCP servers using simple HTTP requests. -You'll need to add `faraday` as a dependency in order to use the HTTP transport layer. Add `event_stream_parser` as well if the server uses SSE (`text/event-stream`) responses: +You'll need to add `faraday` as a dependency in order to use the HTTP transport layer, and `event_stream_parser` +to read SSE (`text/event-stream`) responses. +Whether a response is JSON or SSE is the server's choice, made for each response, and a client must accept both, +so a client written for arbitrary servers needs both gems. This SDK's own server picks SSE by default and always +does on the modern lifecycle, and the listening stream that `on_elicitation` and `on_sampling` open is SSE as well. +`event_stream_parser` is loaded on the first SSE response, so it can be left out only against a server known to +answer with JSON alone, such as this SDK's server in [JSON response mode](/server/transports/#json-response-mode) +serving handshake-lifecycle clients, and only while no listening stream is opened: ```ruby gem "mcp" diff --git a/docs/installation.md b/docs/installation.md index f93c6e5..91aaee1 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -38,6 +38,16 @@ $ gem install mcp You may need to add additional dependencies depending on which features you wish to access. For example, the HTTP client transport requires the `faraday` gem: ```ruby -gem "mcp" gem "faraday", ">= 2.0" ``` + +Reading SSE (`text/event-stream`) responses needs `event_stream_parser` as well. +Whether a response is JSON or SSE is the server's choice, made for each response, and this SDK's own server picks SSE by default, +so a client written for arbitrary servers needs both gems: + +```ruby +gem "faraday", ">= 2.0" +gem "event_stream_parser", ">= 1.0" +``` + +The [Transports](/client/transports/#http-transport-layer) page describes the one setup where `faraday` alone is enough. From 09c89aef14fbbb5b6f9a05a65c355daf17a6c719 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 8 Sep 2026 17:04:43 +0900 Subject: [PATCH 09/24] Parse HTTP client response bodies without Faraday's JSON middleware ## Motivation and Context json 3.0, released on 2026-09-07, accepts the options of `JSON.parse` as keywords only, and Faraday's json response middleware, through 2.14.3, hands them over as a positional Hash, which Ruby 3 no longer converts. With both installed, every JSON body the client receives fails inside that middleware, and `MCP::Client::HTTP` reports each request as an internal error. Faraday's main branch passes the options as keywords, but no release carries that yet. The client already parses streamed bodies and buffered SSE bodies itself; only a JSON body delivered whole by an adapter without streaming support still went through the middleware. The client now parses that body too and no longer registers the middleware, so every body takes `parse_json_buffer` and a malformed one answers `:parse_error` whichever way it arrived. A JSON middleware added through the Faraday customizer block still works, since a body it has parsed already is used as is. ## How Has This Been Tested? A new test drives a buffered JSON response through a stand-in `JSON.parse` with the json 3.0 signature, options as keywords only, so on Ruby 3 it failed with the middleware registered whatever json version is installed and passes now. The existing buffered-body test keeps covering the plain path. The full suite passes on Ruby 3.0.7 with json 3.0.1 and faraday 2.14.3, where five client tests failed before. ## Breaking Changes None. A malformed JSON body delivered by an adapter without streaming support now raises `RequestHandlerError` with `error_type: :parse_error`, as the streamed path already did, instead of `:internal_error` wrapping a `Faraday::ParsingError`. --- lib/mcp/client/http.rb | 7 ++++--- test/mcp/client/http_test.rb | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/mcp/client/http.rb b/lib/mcp/client/http.rb index 1c3fd86..6969b07 100644 --- a/lib/mcp/client/http.rb +++ b/lib/mcp/client/http.rb @@ -723,7 +723,8 @@ module MCP require_faraday! @client ||= Faraday.new(url) do |faraday| faraday.request(:json) - faraday.response(:json) + # The client parses response bodies itself (`resolve_response_body`), so streamed and buffered responses + # take the same path; Faraday's json response middleware is deliberately left out. faraday.response(:raise_error) faraday.headers["Accept"] = ACCEPT_HEADER @@ -1074,8 +1075,8 @@ module MCP elsif content_type&.include?("application/json") return parse_json_buffer(stream.buffer, method, params) unless stream.buffer.empty? - # Adapters without `on_data` support deliver the body via `response.body`, - # already parsed by the json response middleware. + # Adapters without `on_data` support deliver the body via `response.body`; a JSON middleware + # added by the Faraday customizer may have parsed it already. response.body.is_a?(String) ? parse_json_buffer(response.body, method, params) : response.body else raise RequestHandlerError.new( diff --git a/test/mcp/client/http_test.rb b/test/mcp/client/http_test.rb index 077db31..0b9960a 100644 --- a/test/mcp/client/http_test.rb +++ b/test/mcp/client/http_test.rb @@ -940,6 +940,26 @@ module MCP assert_equal({ "result" => { "tools" => [] } }, response) end + def test_send_request_parses_a_json_body_with_a_parser_taking_keyword_options_only + # json 3.0 accepts parser options as keywords only, and Faraday's json response middleware + # passes them as a positional Hash, which Ruby 3 no longer converts. The stand-in parser has + # the json 3.0 signature, so the body must reach `JSON.parse` through the client's own call. + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("/") do + [200, { "Content-Type" => "application/json" }, { result: { tools: [] } }.to_json] + end + end + client = HTTP.new(url: url) { |faraday| faraday.adapter(:test, stubs) } + parse = JSON.method(:parse) + keyword_options_only_parse = ->(source, **options) { parse.call(source, **options) } + + response = JSON.stub(:parse, keyword_options_only_parse) do + client.send_request(request: { jsonrpc: "2.0", id: "test_id", method: "tools/list" }) + end + + assert_equal({ "result" => { "tools" => [] } }, response) + end + def test_send_request_mirrors_x_mcp_header_params_into_mcp_param_headers # SEP-2243: on a modern connection, `tools/list` teaches the transport the `x-mcp-header` # declarations, and the following `tools/call` mirrors the annotated arguments into From d40d9018d51c2137928d12e480aec3a9848fa0e1 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 8 Sep 2026 17:15:26 +0900 Subject: [PATCH 10/24] Keep json below 3.0 on Ruby 2.7.0 through 2.7.2 in the Gemfile ## Motivation and Context json 3.0 declares Ruby 2.7 as its floor, but its `load_file` forwards arguments after a leading parameter, `def load_file(filespec, ...)`, syntax Ruby 2.7.3 was the first to parse. On Ruby 2.7.0 through 2.7.2, requiring json 3.0.1 is a `SyntaxError`, so `require "mcp"` fails before any test runs, and the CI job pinned to 2.7.0, the minimum the gemspec supports, fails the whole suite at load time. The Gemfile keeps json below 3.0 on those three patch levels only. Later 2.7 releases and Ruby 3 parse the syntax and stay on whatever json the resolver picks. The line carries a FIXME to drop it once a json release includes ruby/json#1072, whichever version that turns out to be; excluding only the two broken releases would silently trust a 3.0.2 that may not carry the fix. ## How Has This Been Tested? `gem "json", "3.0.1"; require "json"` raises the `SyntaxError` on Ruby 2.7.0 and loads on 2.7.3 and 2.7.8. With the pin, a fresh `bundle install` on Ruby 2.7.2 resolves json 2.x and `bundle exec rake test` passes. ## Breaking Changes None. The Gemfile is not part of the gem. --- Gemfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Gemfile b/Gemfile index 0aa50ee..fd061bd 100644 --- a/Gemfile +++ b/Gemfile @@ -19,6 +19,9 @@ gem "activesupport" gem "debug" if RUBY_VERSION >= "3.1" # Avoid i18n 1.15.0, which breaks on Ruby 3.1 (ruby-i18n/i18n#735). gem "i18n", "!= 1.15.0" +# FIXME: Drop this once a json release includes https://github.com/ruby/json/pull/1072. json 3.0.0 and 3.0.1 +# forward arguments after a leading parameter, syntax Ruby 2.7.3 was the first to parse, while allowing Ruby 2.7.0. +gem "json", "< 3" if RUBY_VERSION < "2.7.3" gem "rake", "~> 13.0" gem "sorbet-static-and-runtime" if RUBY_VERSION >= "3.0" gem "yard", "~> 0.9" From 3b44ec4f488f3d38271de0631c773cf1c3d45c88 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 9 Sep 2026 10:43:15 +0900 Subject: [PATCH 11/24] Release 1.5.1 This release keeps the HTTP client working with json 3.0, which accepts the options of `JSON.parse` as keywords only. Faraday's JSON response middleware, through 2.14.3, passes them as a positional Hash that Ruby 3 no longer converts, so every JSON body the client received failed as an internal error. The client now parses response bodies itself, the way it already handled streamed ones, and no longer registers the middleware. The one visible difference: a malformed JSON body delivered whole by an adapter without streaming support now raises `RequestHandlerError` with `error_type: :parse_error`, as the streamed path already did, instead of `:internal_error` wrapping a `Faraday::ParsingError`. --- CHANGELOG.md | 13 +++++++++++++ lib/mcp/version.rb | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 051f239..88ec62e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.5.1] - 2026-09-09 + +This release keeps the HTTP client working with json 3.0. That release accepts the options of `JSON.parse` +as keywords only, and Faraday's JSON response middleware, through 2.14.3, passes them as a positional Hash, +which Ruby 3 no longer converts, so every JSON body the client received failed as an internal error. +The client now parses response bodies itself, the way it already handled streamed ones, and no longer +registers the middleware. A malformed JSON body delivered whole by an adapter without streaming support +now raises `RequestHandlerError` with `error_type: :parse_error`, as the streamed path already did. + +### Fixed + +- Parse HTTP client response bodies without Faraday's JSON middleware, which json 3.0 breaks (#546) + ## [1.5.0] - 2026-09-05 This release makes the client answer a server's `ping` with the empty result the specification requires. diff --git a/lib/mcp/version.rb b/lib/mcp/version.rb index 1bbb8dd..2d0dff8 100644 --- a/lib/mcp/version.rb +++ b/lib/mcp/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module MCP - VERSION = "1.5.0" + VERSION = "1.5.1" end From 4192ea58eae515fce6304e1731f2c2c4cdaa1b39 Mon Sep 17 00:00:00 2001 From: Junyuan Zheng Date: Wed, 9 Sep 2026 12:14:21 -0400 Subject: [PATCH 12/24] fix(client/oauth): omit body client_id when using HTTP Basic --- lib/mcp/client/oauth/flow.rb | 49 ++++++++++++++++-------------- test/mcp/client/oauth/flow_test.rb | 49 +++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 24 deletions(-) diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 9a825c0..5b4c0c1 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -997,9 +997,9 @@ module MCP post_to_token_endpoint(as_metadata: as_metadata, client_info: client_info, form: form) end - # Submits a form-encoded request to the token endpoint, applying - # the client authentication method advertised in `client_information` and - # adding `client_id` (and `client_secret` when not using HTTP Basic). + # Submits a form-encoded token request using the authentication method + # stored in `client_information`. The method determines whether client + # credentials belong in the form body, a Basic header, or a JWT assertion. def post_to_token_endpoint(as_metadata:, client_info:, form:) client_id = client_info_required_value(client_info, "client_id") unless client_id @@ -1010,12 +1010,13 @@ module MCP client_secret = client_info_required_value(client_info, "client_secret") token_endpoint_auth_method = client_info_value(client_info, "token_endpoint_auth_method") - form = if token_endpoint_auth_method == "private_key_jwt" - # RFC 7523 Section 2.2 JWT client assertion for the `private_key_jwt` method of - # the `io.modelcontextprotocol/oauth-client-credentials` extension (SEP-1046). - # The client identity travels in the assertion's `iss`/`sub` claims, so `client_id` is - # omitted from the body per RFC 7521 Section 4.2 (the `client_assertion` conveys the client identity). - # The audience is the issuer identifier that `ensure_issuer_matches!` already byte-validated. + # Apply one client authentication method per request (RFC 6749 Section 2.3). + headers = {} + form = case token_endpoint_auth_method + when "private_key_jwt" + # The assertion identifies the client through its `iss` and `sub` + # claims, so the body needs no separate `client_id` (RFC 7521 Section 4.2). + # Use the issuer already checked by `ensure_issuer_matches!` as the audience. unless @provider.respond_to?(:client_assertion) raise AuthorizationError, "token_endpoint_auth_method is private_key_jwt but the provider does not " \ @@ -1026,22 +1027,24 @@ module MCP "client_assertion_type" => JWTClientAssertion::ASSERTION_TYPE, "client_assertion" => @provider.client_assertion(audience: as_metadata["issuer"]), ) + when "client_secret_post" + # Send the client ID and available secret in the form body. + body = form.merge("client_id" => client_id) + body["client_secret"] = client_secret if client_secret + body else - form.merge("client_id" => client_id) - end - - headers = {} - if client_secret - case token_endpoint_auth_method - when "client_secret_post" - form["client_secret"] = client_secret - when "none" - # Public client; no credential. - else - # RFC 6749 §2.3.1 recommends Basic for confidential clients and - # both Python and TypeScript SDKs default here when - # the authentication method is not explicitly stored. + if client_secret && token_endpoint_auth_method != "none" + # Basic is also the fallback when a secret is present but no method + # is stored. A body `client_id` is optional (RFC 6749 Section 3.2.1); + # omit it because some servers treat it alongside Basic as a second + # authentication method and reject the request with `invalid_request`. headers["Authorization"] = "Basic " + basic_auth_credentials(client_id, client_secret) + form + else + # With `none` or no secret, identify the client using `client_id`. + # This is required for unauthenticated authorization-code exchanges + # (RFC 6749 Section 3.2.1). + form.merge("client_id" => client_id) end end diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index dc78e03..326dbd8 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -158,6 +158,21 @@ module MCP end end + def test_run_client_credentials_with_client_secret_basic_omits_client_id_from_body + # Explicit Basic authentication must keep both credentials in the header + # to avoid servers treating body credentials as a second auth method. + provider = client_credentials_provider(token_endpoint_auth_method: "client_secret_basic") + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + req.headers["Authorization"] == "Basic " + Base64.strict_encode64("cc-client:cc-secret") && + !form.key?("client_id") && + !form.key?("client_secret") + end + end + def test_run_client_credentials_raises_clean_error_when_client_information_missing # The constructor always stores credentials, but if a custom storage loses them # the grant must fail with a domain error rather than a `NoMethodError` from @@ -1860,8 +1875,10 @@ module MCP expected = "Basic " + Base64.strict_encode64("pre-registered-client:pre-registered-secret") assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h req.headers["Authorization"] == expected && - !URI.decode_www_form(req.body).to_h.key?("client_secret") + !form.key?("client_id") && + !form.key?("client_secret") end end @@ -2365,6 +2382,36 @@ module MCP assert_equal("saved-rt", provider.tokens["refresh_token"]) end + def test_refresh_with_client_secret_basic_omits_client_id_from_body + # Stored credentials without an auth method default to Basic. Refresh + # must also omit the body `client_id` when using that fallback. + stub_request(:post, "#{@auth_base}/token") + .with(body: hash_including("grant_type" => "refresh_token")) + .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" => "conf-client", "client_secret" => "conf-secret") + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt") + + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + req.headers["Authorization"] == "Basic " + Base64.strict_encode64("conf-client:conf-secret") && + !form.key?("client_id") && + !form.key?("client_secret") + end + end + def test_run_records_the_issuer_that_minted_the_tokens state_holder = {} provider = Provider.new( From 2f65a6e502e104bddab43d13786b9f3cbd474be7 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 9 Sep 2026 18:36:20 +0900 Subject: [PATCH 13/24] Drop the json pin for Ruby 2.7.0 through 2.7.2 from the Gemfile ## Motivation and Context json 3.0.2 loads on Ruby 2.7.0 through 2.7.2 again: it no longer forwards arguments after a leading parameter, the syntax those patch levels could not parse, so the Gemfile no longer has to keep json below 3.0 there, and the CI job pinned to 2.7.0, the minimum the gemspec supports, resolves json 3.0.2 like every other job. The maintainers intend to raise json's own Ruby floor to 2.7.3 in the release after 3.0.2, so those patch levels stay on 3.0.2 by json's gemspec from then on, and no pin of this Gemfile's own is needed for that either. ## How Has This Been Tested? A fresh `bundle install` on Ruby 2.7.2 resolves json 3.0.2, `require "json"` loads it there, and `bundle exec rake test` passes. Ruby 3.0 and later were never affected by the pin. ## Breaking Changes None. The Gemfile is not part of the gem. --- Gemfile | 3 --- 1 file changed, 3 deletions(-) diff --git a/Gemfile b/Gemfile index fd061bd..0aa50ee 100644 --- a/Gemfile +++ b/Gemfile @@ -19,9 +19,6 @@ gem "activesupport" gem "debug" if RUBY_VERSION >= "3.1" # Avoid i18n 1.15.0, which breaks on Ruby 3.1 (ruby-i18n/i18n#735). gem "i18n", "!= 1.15.0" -# FIXME: Drop this once a json release includes https://github.com/ruby/json/pull/1072. json 3.0.0 and 3.0.1 -# forward arguments after a leading parameter, syntax Ruby 2.7.3 was the first to parse, while allowing Ruby 2.7.0. -gem "json", "< 3" if RUBY_VERSION < "2.7.3" gem "rake", "~> 13.0" gem "sorbet-static-and-runtime" if RUBY_VERSION >= "3.0" gem "yard", "~> 0.9" From 7e507f2963a24709c5c62800bf1d5ccd8cc10736 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 12 Sep 2026 00:02:30 +0900 Subject: [PATCH 14/24] [Doc] Document the client's ping answers and the stdio request limit ## Motivation and Context The client answers server pings automatically, but the docs leave out where that applies and what the stdio transport does with the other server-to-client requests. The client Transports page now lists the automatic ping answer among the things the stdio transport handles, and a note states that `ping` is the only server-to-client request answered there: a wire-level `elicitation/create` or `sampling/createMessage` is ignored over stdio, so a server that sends one waits for an answer that never comes, while the SEP-2322 `input_required` route works on every transport and keeps `on_elicitation` and `on_sampling` handlers working over stdio. Without this note, the only trace of the limitation is a hung tool call. The client Ping page now says that over HTTP a ping reaches the client only on a stream that is already open and that the client never opens one to receive pings, so a server can only ping a client that has a stream open. Registering `on_server_request("ping")` moves to the end of the section as the advanced option it is, together with its side effect of opening the GET listening stream on `connect`. The client overview mentions the automatic answer next to `MCP::Client#ping`, and the server Ping page's Client Side section points at it. ## How Has This Been Tested? Documentation only; the linked headings and pages exist. ## Breaking Changes None. --- docs/_client/index.md | 3 ++- docs/_client/ping.md | 8 ++++++-- docs/_client/transports.md | 8 ++++++++ docs/_server/ping.md | 2 ++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/docs/_client/index.md b/docs/_client/index.md index c919622..2e3077c 100644 --- a/docs/_client/index.md +++ b/docs/_client/index.md @@ -17,7 +17,8 @@ This class supports: - Lifecycle negotiation and connection via `MCP::Client#connect`, adopting the modern lifecycle when the server serves it; see [Lifecycle](/client/lifecycle/) - Server discovery via the `server/discover` method (`MCP::Client#discover`); see [Explicit Discovery](/client/lifecycle/#explicit-discovery) -- Liveness check via the `ping` method (`MCP::Client#ping`) +- Liveness check via the `ping` method (`MCP::Client#ping`), and automatic answers to server pings; + see [Answering Server Pings](/client/ping/#answering-server-pings) - Tool listing via the `tools/list` method (`MCP::Client#tools`) - Tool invocation via the `tools/call` method (`MCP::Client#call_tool`) - Resource listing via the `resources/list` method (`MCP::Client#resources`) diff --git a/docs/_client/ping.md b/docs/_client/ping.md index f6889e9..f546afe 100644 --- a/docs/_client/ping.md +++ b/docs/_client/ping.md @@ -32,13 +32,17 @@ propagate as exceptions raised by the transport layer. ## Answering Server Pings On handshake-lifecycle connections a server may ping the client the same way, and the client answers automatically with -the empty result - no handler is needed. Registering `transport.on_server_request("ping")` on `MCP::Client::HTTP` replaces -the automatic answer. +the empty result - no handler is needed. Over HTTP a ping reaches the client only on a stream that is already open +(the SSE response of an in-flight POST, or the GET listening stream); the client never opens a stream just to receive pings. Over stdio, a ping that arrives while a response is awaited is answered inline; between requests it is answered when the next request starts reading. The answer is best effort: a pong that cannot be written (for example, over a broken pipe) is dropped rather than failing the request whose response is being read. Independently of pings, consider setting `read_timeout:` on `MCP::Client::Stdio`, since a server that never answers otherwise holds the read until the process exits. +To answer pings yourself, register `transport.on_server_request("ping")` on `MCP::Client::HTTP`; it replaces +the automatic answer and, like any handler registered there, opens the standalone GET listening stream on `connect`, +as described on the [Transports](/client/transports/#server-to-client-requests-elicitation) page. + ## Server Side How servers answer `ping` requests and ping the client themselves is documented on diff --git a/docs/_client/transports.md b/docs/_client/transports.md index a30259c..4362666 100644 --- a/docs/_client/transports.md +++ b/docs/_client/transports.md @@ -59,6 +59,14 @@ The stdio transport automatically handles: - Spawning the server process with `Open3.popen3` - MCP protocol initialization handshake (`initialize` request + `notifications/initialized`) - JSON-RPC 2.0 message framing over newline-delimited JSON +- Answering server `ping` requests; see [Answering Server Pings](/client/ping/#answering-server-pings) + +{: .note } +> `ping` is the only server-to-client request answered over stdio. A wire-level `elicitation/create` +> or `sampling/createMessage` is ignored on this transport, so a server that sends one waits for an answer +> that never comes; use Streamable HTTP for those, as described below. The SEP-2322 `input_required` route, +> which is how the modern lifecycle asks for the same input, works on every transport, so `on_elicitation` +> and `on_sampling` handlers still fire over stdio; see [Multi Round-Trip Requests](/client/mrtr/). ## HTTP Transport Layer diff --git a/docs/_server/ping.md b/docs/_server/ping.md index 6272062..d8c6627 100644 --- a/docs/_server/ping.md +++ b/docs/_server/ping.md @@ -48,3 +48,5 @@ Server-to-client requests are bounded by a timeout on the Streamable HTTP transp ## Client Side Pinging the server with `MCP::Client#ping` is documented on the client [Ping](/client/ping/) page. +The Ruby client answers the pings sent with `ServerSession#ping` automatically; +see [Answering Server Pings](/client/ping/#answering-server-pings). From ef3eb7b0b2c41e20a6a956ec0c44eb61a9144544 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 13 Sep 2026 17:10:12 +0900 Subject: [PATCH 15/24] [Doc] State the Listen Stream Delivery Semantics The Subscriptions page documented the acknowledgement-first contract but not what delivery means: the stream carries only changes that happen after the acknowledgement, nothing is queued while no stream is connected, and a dropped stream is gone together with anything it missed. The Python SDK's subscriptions page states the same contract ("It is not a replay log"). Spelling it out gives client implementers the fetch-after-subscribing guidance and makes the acknowledgement-window behavior explicit. --- docs/_server/subscriptions.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/_server/subscriptions.md b/docs/_server/subscriptions.md index 9777d8c..22116aa 100644 --- a/docs/_server/subscriptions.md +++ b/docs/_server/subscriptions.md @@ -49,6 +49,13 @@ The first SSE event on the stream is the acknowledgement: } ``` +## Delivery Semantics + +A stream delivers only the changes that happen after its acknowledgement; it is not a replay log. +Nothing is queued while no stream is connected, and a dropped stream is gone together with anything it missed. +Clients fetch the current state after subscribing and treat each notification as a cue to refetch, +rather than as a payload. + ## Transport Support The stream is served on the Streamable HTTP modern path; stdio answers `-32601`. From 8d1d5be4bad5a6b85b1e6cda8d1c1f7c0d739b68 Mon Sep 17 00:00:00 2001 From: Junyuan Zheng Date: Mon, 14 Sep 2026 09:45:38 -0400 Subject: [PATCH 16/24] fix(client/oauth): preserve token endpoint error diagnostics --- docs/_client/authorization.md | 21 ++++ lib/mcp/client/oauth/flow.rb | 58 ++++++++--- test/mcp/client/oauth/flow_test.rb | 156 ++++++++++++++++++++++++++++- 3 files changed, 218 insertions(+), 17 deletions(-) diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index a673a19..750e0c6 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -159,6 +159,27 @@ provider = MCP::Client::OAuth::Provider.new( ) ``` +### Token Endpoint Errors + +When a token exchange or refresh fails, `MCP::Client::OAuth::Flow::AuthorizationError` includes the HTTP status and +the authorization server's `error` and `error_description` from [RFC 6749 Section 5.2](https://www.rfc-editor.org/rfc/rfc6749#section-5.2). +For example: + +```text +Token endpoint returned status 400. invalid_request: Client must not use multiple authentication methods +``` + +The exception exposes `http_status`, `error`, and `error_description` readers for structured diagnostics. Missing or non-string +diagnostic fields are `nil`; non-JSON responses retain the status-only message. Other authorization failures have `nil` readers. +An `invalid_grant` response still raises `Flow::InvalidGrantError`, a subclass of `Flow::AuthorizationError`, so refresh-token +recovery behavior is unchanged. + +Diagnostic fields are limited to 128 characters for `error` and 512 for `error_description`, including a trailing `...` when +truncated. Characters outside the RFC's printable ASCII set are replaced with spaces, and surrounding whitespace is removed. +The SDK excludes all other response fields, including `error_uri`, and does not include the raw response body in these errors. +Descriptions are provider-controlled text, not guaranteed to be free of sensitive information; apply your application's logging +and redaction policy before persisting them or displaying them to users. + ### Client Credentials Grant For a confidential machine-to-machine client (no user, no browser redirect), use `MCP::Client::OAuth::ClientCredentialsProvider` instead of `Provider`. diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 5b4c0c1..acc0f1c 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -14,7 +14,19 @@ module MCP # `Provider`; this class consumes a Provider plus signal data extracted from # the failing response (resource_metadata URL, scope challenge). class Flow - class AuthorizationError < StandardError; end + TOKEN_ENDPOINT_ERROR_MAX_LENGTH = 128 + TOKEN_ENDPOINT_ERROR_DESCRIPTION_MAX_LENGTH = 512 + + class AuthorizationError < StandardError + attr_reader :http_status, :error, :error_description + + def initialize(message = nil, http_status: nil, error: nil, error_description: nil) + super(message) + @http_status = http_status + @error = error + @error_description = error_description + end + end # Raised specifically when the token endpoint rejects a grant with # `error: "invalid_grant"` (RFC 6749 §5.2). Callers use this to @@ -1062,11 +1074,7 @@ module MCP end if response.status < 200 || response.status >= 300 - if token_endpoint_error_code(response) == "invalid_grant" - raise InvalidGrantError, "Token endpoint rejected the grant: invalid_grant." - end - - raise AuthorizationError, "Token endpoint returned status #{response.status}." + raise token_endpoint_error(response) end parsed = begin @@ -1087,17 +1095,35 @@ module MCP parsed end - # Extracts the `error` code from an RFC 6749 §5.2 error response body - # when one is parseable. Returns nil on any parse failure or when - # the body is not JSON. - def token_endpoint_error_code(response) - body = response_body_string(response).to_s - return if body.empty? + # Surface only RFC 6749 §5.2 diagnostic fields, never the raw response, + # which may contain tokens or other credentials. Classify the original + # code so sanitization cannot turn malformed input into invalid_grant. + def token_endpoint_error(response) + message = "Token endpoint returned status #{response.status}." + error_class = AuthorizationError + parsed = JSON.parse(response_body_string(response)) + parsed = {} unless parsed.is_a?(Hash) - parsed = JSON.parse(body) - parsed["error"] if parsed.is_a?(Hash) - rescue JSON::ParserError - nil + error_class = parsed["error"] == "invalid_grant" ? InvalidGrantError : AuthorizationError + error = token_endpoint_diagnostic(parsed["error"], limit: TOKEN_ENDPOINT_ERROR_MAX_LENGTH) + description = token_endpoint_diagnostic(parsed["error_description"], limit: TOKEN_ENDPOINT_ERROR_DESCRIPTION_MAX_LENGTH) + message += " #{[error, description].compact.join(": ")}" if error || description + + error_class.new(message, http_status: response.status, error: error, error_description: description) + rescue StandardError + # Diagnostics must not mask the endpoint failure or change refresh recovery. + error_class.new("Token endpoint returned status #{response.status}.", http_status: response.status) + end + + def token_endpoint_diagnostic(value, limit:) + return unless value.is_a?(String) + + # RFC 6749 permits printable ASCII except double quotes and backslashes. + # Replace other characters to keep provider text on one log line. + value = value.scrub(" ").gsub(/[^\x20-\x21\x23-\x5B\x5D-\x7E]/, " ").strip + return if value.empty? + + value.length > limit ? "#{value[0, limit - 3]}..." : value end # Per RFC 6749 Section 2.3.1, the `client_id` and `client_secret` MUST be diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index 326dbd8..5daff12 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -2731,7 +2731,161 @@ module MCP provider.save_client_information("client_id" => "test-client") provider.save_tokens("access_token" => "stale-at", "refresh_token" => "revoked-rt") - assert_raises(Flow::InvalidGrantError) do + error = assert_raises(Flow::InvalidGrantError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + assert_equal(400, error.http_status) + assert_equal("invalid_grant", error.error) + assert_equal("refresh token expired", error.error_description) + assert_equal("Token endpoint returned status 400. invalid_grant: refresh token expired", error.message) + end + + def test_token_exchange_preserves_oauth_diagnostics + stub_request(:post, "#{@auth_base}/token").to_return( + status: 400, + body: JSON.generate( + error: "invalid_request", + error_description: "Client must not use multiple authentication methods", + access_token: "do-not-log-access-token", + refresh_token: "do-not-log-refresh-token", + client_secret: "do-not-log-client-secret", + error_uri: "https://auth.example.com/error?secret=do-not-log", + ), + ) + + error = assert_raises(Flow::AuthorizationError) do + capture_authorization_scope(grant_types: ["authorization_code"]) + end + + assert_equal(400, error.http_status) + assert_equal("invalid_request", error.error) + assert_equal("Client must not use multiple authentication methods", error.error_description) + assert_equal( + "Token endpoint returned status 400. invalid_request: Client must not use multiple authentication methods", + error.message, + ) + end + + def test_refresh_preserves_oauth_diagnostics + error = refresh_token_endpoint_error( + JSON.generate(error: "invalid_client", error_description: "Client authentication failed"), + status: 401, + ) + + assert_instance_of(Flow::AuthorizationError, error) + assert_equal(401, error.http_status) + assert_equal("invalid_client", error.error) + assert_equal("Client authentication failed", error.error_description) + assert_equal("Token endpoint returned status 401. invalid_client: Client authentication failed", error.message) + end + + def test_token_endpoint_errors_fall_back_for_malformed_bodies + ["", "secret", "{broken", "null", "[]", '"secret"', "42"].each do |body| + error = refresh_token_endpoint_error(body) + + assert_instance_of(Flow::AuthorizationError, error) + assert_equal(400, error.http_status) + assert_nil(error.error) + assert_nil(error.error_description) + assert_equal("Token endpoint returned status 400.", error.message) + end + end + + def test_token_endpoint_errors_ignore_non_string_and_empty_fields + [nil, 42, true, [], { secret: "hidden" }, "", " \r\n\t"].each do |value| + error = refresh_token_endpoint_error(JSON.generate(error: value, error_description: value)) + + assert_nil(error.error) + assert_nil(error.error_description) + assert_equal("Token endpoint returned status 400.", error.message) + end + end + + def test_token_endpoint_errors_preserve_optional_fields_independently + error = refresh_token_endpoint_error(JSON.generate(error: "provider_extension")) + assert_equal("provider_extension", error.error) + assert_nil(error.error_description) + assert_equal("Token endpoint returned status 400. provider_extension", error.message) + + error = refresh_token_endpoint_error(JSON.generate(error_description: "Details without a code")) + assert_nil(error.error) + assert_equal("Details without a code", error.error_description) + assert_equal("Token endpoint returned status 400. Details without a code", error.message) + end + + def test_token_endpoint_errors_sanitize_without_changing_grant_classification + error = refresh_token_endpoint_error( + JSON.generate(error: "invalid_grant\n", error_description: "expired\r\n\t\e\u0000\"\\\u2028token"), + ) + + assert_instance_of(Flow::AuthorizationError, error) + assert_equal("invalid_grant", error.error) + assert_equal("expired token", error.error_description) + assert_equal("Token endpoint returned status 400. invalid_grant: expired token", error.message) + end + + def test_invalid_grant_with_invalid_utf8_description_preserves_classification + error = refresh_token_endpoint_error( + "{\"error\":\"invalid_grant\",\"error_description\":\"bad \xFF byte\"}".b, + ) + + assert_instance_of(Flow::InvalidGrantError, error) + assert_equal(400, error.http_status) + assert_equal("invalid_grant", error.error) + assert_equal("bad byte", error.error_description) + assert_equal("Token endpoint returned status 400. invalid_grant: bad byte", error.message) + end + + def test_token_endpoint_errors_scrub_invalid_utf8_in_both_fields + error = refresh_token_endpoint_error( + "{\"error\":\"invalid_grant\xFF\",\"error_description\":\"bad \xFF byte\"}".b, + ) + + assert_instance_of(Flow::AuthorizationError, error) + assert_equal(400, error.http_status) + assert_equal("invalid_grant", error.error) + assert_equal("bad byte", error.error_description) + assert_equal("Token endpoint returned status 400. invalid_grant: bad byte", error.message) + end + + def test_token_endpoint_errors_fall_back_when_diagnostic_extraction_raises + Flow.any_instance.stubs(:token_endpoint_diagnostic).raises(ArgumentError, "sensitive provider text") + + { "invalid_grant" => Flow::InvalidGrantError, "invalid_client" => Flow::AuthorizationError }.each do |code, klass| + error = refresh_token_endpoint_error(JSON.generate(error: code, error_description: "details")) + + assert_instance_of(klass, error) + assert_equal(400, error.http_status) + assert_nil(error.error) + assert_nil(error.error_description) + assert_equal("Token endpoint returned status 400.", error.message) + end + end + + def test_token_endpoint_errors_bound_diagnostic_lengths + error = refresh_token_endpoint_error(JSON.generate(error: "e" * 200, error_description: "d" * 1000)) + + assert_equal("#{"e" * 125}...", error.error) + assert_equal("#{"d" * 509}...", error.error_description) + assert_equal("Token endpoint returned status 400. #{error.error}: #{error.error_description}", error.message) + end + + def test_other_authorization_errors_have_no_token_endpoint_diagnostics + error = Flow::AuthorizationError.new("Discovery failed") + + assert_equal("Discovery failed", error.message) + assert_nil(error.http_status) + assert_nil(error.error) + assert_nil(error.error_description) + end + + def refresh_token_endpoint_error(body, status: 400) + stub_request(:post, "#{@auth_base}/token").to_return(status: status, body: body) + provider = ssrf_test_provider + provider.save_client_information("client_id" => "test-client") + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt") + + assert_raises(Flow::AuthorizationError) do Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) end end From 99ef8732feb46e79713e59ee63866bbba9687409 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 15 Sep 2026 12:33:24 +0900 Subject: [PATCH 17/24] Fix a redundant regexp escape in the tool name validation ## Motivation and Context This PR suppresses the following offense of RuboCop. ```console $ bundle exec rubocop Inspecting 163 files (snip) Offenses: lib/mcp/tool.rb:152:48: C: [Correctable] Style/RedundantRegexpEscape: Redundant escape inside regexp literal unless tool_name.match?(/\A[A-Za-z\d_\-\.]+\z/) ^^ 163 files inspected, 1 offense detected, 1 offense autocorrectable ``` ## How Has This Been Tested? `bundle exec rake` passes: RuboCop reports no offenses. ## Breaking Changes None. --- lib/mcp/tool.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mcp/tool.rb b/lib/mcp/tool.rb index 37c5d92..1b76b80 100644 --- a/lib/mcp/tool.rb +++ b/lib/mcp/tool.rb @@ -149,7 +149,7 @@ module MCP raise ArgumentError, "Tool names should be between 1 and 128 characters in length (inclusive)." end - unless tool_name.match?(/\A[A-Za-z\d_\-\.]+\z/) + unless tool_name.match?(/\A[A-Za-z\d_\-.]+\z/) raise ArgumentError, <<~MESSAGE Tool names only allowed characters: uppercase and lowercase ASCII letters (A-Z, a-z), digits (0-9), underscore (_), hyphen (-), and dot (.). MESSAGE From 8ad80dd0b48fc7ad49befaea5e84f5b08e5bf1de Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 17 Sep 2026 00:52:10 +0900 Subject: [PATCH 18/24] Let a provider add parameters to the token requests it makes ## Motivation and Context `ClientCredentialsProvider` builds a fixed `client_credentials` token request: `grant_type`, `scope`, `resource`, and the client authentication that `Flow#post_to_token_endpoint` adds. An authorization server that requires further parameters, such as Auth0's `audience`, cannot be used through `oauth:` at all, and giving up `oauth:` also gives up the bearer header and the 401-driven retry that the transport provides. RFC 6749 Section 8.2 allows extension parameters on the token request, and the TypeScript SDK lets a provider shape its token request through `prepareTokenRequest`. `ClientCredentialsProvider.new` now takes `token_request_params:`, a Hash of String keys and values added to every token request the provider makes, and `Flow#post_to_token_endpoint` reads the same name by duck typing, so a `Provider` or `CrossAppAccessProvider` subclass that defines the method gets the same treatment on its authorization code exchange, refresh, or `jwt-bearer` request. `Provider.new` and `CrossAppAccessProvider.new` take the same keyword, held by `StorageBackedProvider` like `authorization_request_validator`, so every bundled provider refuses a bad value before anything is stored. The parameters go underneath the flow's own, so the SDK's values always win, and a key the SDK sets itself (`Flow::RESERVED_TOKEN_REQUEST_PARAMS`) is refused rather than silently overridden: with `ArgumentError` from the constructor, before any `client_information` is written, and again with `ArgumentError` from the flow, before the token request is sent. Only Strings are accepted because `URI.encode_www_form` encodes other scalars and Arrays in ways the caller did not write, and the provider keeps a frozen copy so a later change to the caller's Hash cannot alter what is sent. A Hash comparing keys by identity is refused too, since two equal keys are two entries there, sent twice by a provider method or silently collapsed into one by the copy. The flow raises `ArgumentError` rather than `AuthorizationError` because `MCP::Client::HTTP` treats a failed refresh as a reason to run the interactive flow, which would then fail the same way after the user signed in. Both refusals raise `Flow::InvalidTokenRequestParamsError`, a subclass of `ArgumentError`, so a caller can tell them from an unknown keyword, and the copy duplicates keys as well as values, since `Hash` copies only keys whose class is exactly `String`. Fixes #554. ## How Has This Been Tested? New tests cover every token request the parameters can ride (the `client_credentials` grant under all three client authentication methods, the authorization code exchange, refresh, and `jwt-bearer`), both refusal boundaries (the constructors of all three providers, and the flow for a provider defining the method), and the transport surfacing a refused hook from a refresh attempt without starting the interactive flow. All of them fail against the previous library. ## Breaking Changes None. The keyword is optional and the hook is opt-in; a provider that does not define `token_request_params` sends the same requests as before. --- docs/_client/authorization.md | 14 +- .../oauth/client_credentials_provider.rb | 18 +- .../client/oauth/cross_app_access_provider.rb | 15 +- lib/mcp/client/oauth/flow.rb | 64 +++++ lib/mcp/client/oauth/provider.rb | 9 +- .../client/oauth/storage_backed_provider.rb | 32 ++- .../oauth/client_credentials_provider_test.rb | 101 ++++++++ .../oauth/cross_app_access_provider_test.rb | 36 +++ test/mcp/client/oauth/flow_test.rb | 243 +++++++++++++++++- test/mcp/client/oauth/http_oauth_test.rb | 51 ++++ test/mcp/client/oauth/provider_test.rb | 24 ++ 11 files changed, 583 insertions(+), 24 deletions(-) diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index 750e0c6..b35b597 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -112,6 +112,14 @@ Optional keyword arguments: served at the URL is a separate JSON artifact from the `client_metadata` keyword above: the DCR `client_metadata` MUST NOT include `client_id`, while the CIMD document MUST include `client_id` set to the document URL, `client_name`, and `redirect_uris` covering `redirect_uri`. +- `token_request_params`: Hash of String keys and values added to every token request the provider makes, + for parameters the authorization server requires beyond the grant itself, such as Auth0's `audience`. + Defaults to `nil`, which adds nothing. The authorization request is not affected. + A key the SDK sets itself (listed in `Flow::RESERVED_TOKEN_REQUEST_PARAMS`), a Hash that compares keys by identity, + or a value that is not a Hash of Strings is refused with `Flow::InvalidTokenRequestParamsError`, a subclass of `ArgumentError`, + when the provider is built, and the accepted Hash is copied and frozen. + Any provider that defines a `token_request_params` method gets the same treatment on every token request it makes; + the flow checks the returned value by the same rules and raises the same error before the token request is sent. {: .warning } > The OAuth 2.0 Dynamic Client Registration Protocol (RFC 7591) is deprecated as a client registration mechanism as of MCP 2026-07-28 in favor of Client ID Metadata Documents, @@ -192,6 +200,7 @@ provider = MCP::Client::OAuth::ClientCredentialsProvider.new( client_secret: ENV.fetch("MCP_CLIENT_SECRET"), # token_endpoint_auth_method: "client_secret_basic" (default), "client_secret_post", or "private_key_jwt" # scope: "mcp:read mcp:write" (optional; used when the server does not advertise scopes) + # token_request_params: { "audience" => "https://api.example.com" } (optional; parameters the authorization server requires) ) transport = MCP::Client::HTTP.new(url: "https://api.example.com/mcp", oauth: provider) @@ -207,7 +216,8 @@ Keyword arguments: - `private_key`, `signing_algorithm`: Required with `private_key_jwt` - the key (a PEM string or `OpenSSL::PKey::PKey`, never written to `storage`) signs the client assertion with `"ES256"` or `"RS256"`; `client_secret` must not be set, because the private key is the credential. -- `scope`, `storage`, `authorization_request_validator`: Optional, same meaning as on `Provider`. +- `scope`, `storage`, `authorization_request_validator`, `token_request_params`: Optional, same meaning as on `Provider`. + Use `token_request_params` for a parameter the authorization server requires on the `client_credentials` grant, such as Auth0's `audience`. ### Cross-App Access (JWT Bearer) Grant @@ -244,7 +254,7 @@ Keyword arguments: - `assertion_provider`: Required. Callable invoked as `call(audience:, resource:)` and returning the ID-JAG assertion. `audience` is the MCP authorization server's validated issuer identifier; `resource` is the canonical MCP server URL (RFC 8707). Passing both through to `IDJAGTokenExchange.request` covers the common case. -- `scope`, `storage`, `authorization_request_validator`: Optional, same meaning as on `Provider`. +- `scope`, `storage`, `authorization_request_validator`, `token_request_params`: Optional, same meaning as on `Provider`. ### Communication Security diff --git a/lib/mcp/client/oauth/client_credentials_provider.rb b/lib/mcp/client/oauth/client_credentials_provider.rb index 21276bb..22ef56a 100644 --- a/lib/mcp/client/oauth/client_credentials_provider.rb +++ b/lib/mcp/client/oauth/client_credentials_provider.rb @@ -37,11 +37,15 @@ module MCP # also take the algorithm as an explicit option). # - `scope` - String of space-separated scopes to request when the server's # `WWW-Authenticate` and the Protected Resource Metadata do not specify one. - # - `storage` - Object responding to `tokens`, `save_tokens(tokens)`, - # `client_information`, and `save_client_information(info)`. Defaults to - # an `InMemoryStorage`. The `client_id` / `client_secret` are written - # into it so the token exchange reads them through the same path as - # a pre-registered authorization-code client. + # - `storage` - Object responding to `tokens`, `save_tokens(tokens)`, `client_information`, + # and `save_client_information(info)`. Defaults to an `InMemoryStorage`. + # The `client_id` / `client_secret` are written into it so the token exchange reads + # them through the same path as a pre-registered authorization-code client. + # - `token_request_params` - Hash of String keys and values added to every + # token request this provider makes, for parameters the authorization + # server requires beyond the grant itself (Auth0's `audience`, for example). + # A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS` raises `Flow::InvalidTokenRequestParamsError`. + # The Hash is copied and frozen. See `StorageBackedProvider#token_request_params`. class ClientCredentialsProvider include StorageBackedProvider @@ -61,7 +65,8 @@ module MCP signing_algorithm: nil, scope: nil, storage: nil, - authorization_request_validator: nil + authorization_request_validator: nil, + token_request_params: nil ) if blank?(client_id) raise InvalidCredentialsError, "client_id is required for the client_credentials grant." @@ -105,6 +110,7 @@ module MCP @scope = scope @storage = storage || InMemoryStorage.new @authorization_request_validator = authorization_request_validator + @token_request_params = frozen_token_request_params(token_request_params) @storage.save_client_information(client_information) end diff --git a/lib/mcp/client/oauth/cross_app_access_provider.rb b/lib/mcp/client/oauth/cross_app_access_provider.rb index 67342c0..422cbb5 100644 --- a/lib/mcp/client/oauth/cross_app_access_provider.rb +++ b/lib/mcp/client/oauth/cross_app_access_provider.rb @@ -25,6 +25,10 @@ module MCP # the Protected Resource Metadata do not specify one. # - `storage` - Object responding to `tokens`, `save_tokens(tokens)`, `client_information`, and `save_client_information(info)`. # Defaults to an `InMemoryStorage`. + # - `token_request_params` - Hash of String keys and values added to the `jwt-bearer` token request, for parameters + # the authorization server requires beyond the grant itself. A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS` raises + # `Flow::InvalidTokenRequestParamsError`. The Hash is copied and frozen. + # See `StorageBackedProvider#token_request_params`. # # https://github.com/modelcontextprotocol/modelcontextprotocol/issues/990 class CrossAppAccessProvider @@ -35,7 +39,15 @@ module MCP attr_reader :scope, :storage - def initialize(client_id:, client_secret:, assertion_provider:, scope: nil, storage: nil, authorization_request_validator: nil) + def initialize( + client_id:, + client_secret:, + assertion_provider:, + scope: nil, + storage: nil, + authorization_request_validator: nil, + token_request_params: nil + ) if blank?(client_id) raise InvalidConfigurationError, "client_id is required for the jwt-bearer grant." end @@ -52,6 +64,7 @@ module MCP @scope = scope @storage = storage || InMemoryStorage.new @authorization_request_validator = authorization_request_validator + @token_request_params = frozen_token_request_params(token_request_params) @storage.save_client_information( "client_id" => client_id, "client_secret" => client_secret, diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index acc0f1c..1093eba 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -17,6 +17,23 @@ module MCP TOKEN_ENDPOINT_ERROR_MAX_LENGTH = 128 TOKEN_ENDPOINT_ERROR_DESCRIPTION_MAX_LENGTH = 512 + # 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 = [ + "grant_type", + "client_id", + "client_secret", + "client_assertion", + "client_assertion_type", + "scope", + "resource", + "code", + "code_verifier", + "redirect_uri", + "refresh_token", + "assertion", + ].freeze + class AuthorizationError < StandardError attr_reader :http_status, :error, :error_description @@ -40,6 +57,33 @@ module MCP # or authorization server metadata failure by rescuing a class rather than by matching the message text. class AuthorizationRefusedError < AuthorizationError; end + # Raised for a `token_request_params` value the SDK refuses: a reserved key, a Hash comparing keys by identity, + # or anything but a Hash of Strings. An `ArgumentError` because the value is a configuration mistake, + # not a failed authorization, and deliberately outside `AuthorizationError`, which `MCP::Client::HTTP` treats on + # a failed refresh as a reason to run the interactive flow. + class InvalidTokenRequestParamsError < ArgumentError; end + + class << self + # Returns why `params` cannot ride a token request as `token_request_params`, or `nil` when it can. + # Shared by the provider constructors and the flow, which both refuse the value with `InvalidTokenRequestParamsError`, + # so the same problem reads the same wherever it surfaces. + def token_request_params_problem(params) + return "must be a Hash (got #{params.class})." unless params.is_a?(Hash) + + # Two equal keys are two entries here, which would be sent twice from a provider method + # or silently collapse into one when the constructors copy the Hash. + return "must not compare keys by identity." if params.compare_by_identity? + + params.each do |key, value| + return "keys must be Strings (got #{key.class})." unless key.is_a?(String) + return "values must be Strings (got #{value.class} for #{key.inspect})." unless value.is_a?(String) + return "must not set #{key.inspect}, which the SDK sets itself." if RESERVED_TOKEN_REQUEST_PARAMS.include?(key) + end + + nil + end + end + def initialize(provider:, http_client_factory: nil) @provider = provider @http_client_factory = http_client_factory || -> { default_http_client } @@ -960,6 +1004,22 @@ module MCP @provider.authorization_flow end + # Parameters the provider adds to every token request it makes (RFC 6749 Section 8.2 leaves room for them; + # Auth0's `audience` is the usual one). Duck-typed like `authorization_flow`, so a provider without the method, + # or one returning `nil`, adds nothing. + # A bad value raises `InvalidTokenRequestParamsError`, as the provider constructors do. + def provider_token_request_params + return {} unless @provider.respond_to?(:token_request_params) + + params = @provider.token_request_params + return {} if params.nil? + + problem = self.class.token_request_params_problem(params) + raise InvalidTokenRequestParamsError, "The provider's token_request_params #{problem}" if problem + + params + end + def build_authorization_url(as_metadata:, client_id:, scope:, state:, code_challenge:, resource:) authorization_endpoint = as_metadata["authorization_endpoint"] unless authorization_endpoint @@ -1012,6 +1072,9 @@ module MCP # Submits a form-encoded token request using the authentication method # stored in `client_information`. The method determines whether client # credentials belong in the form body, a Basic header, or a JWT assertion. + # A provider's `token_request_params` go underneath the flow's own parameters, + # which therefore win, and are refused before the request is sent when they name + # a reserved parameter or are not a Hash of Strings. def post_to_token_endpoint(as_metadata:, client_info:, form:) client_id = client_info_required_value(client_info, "client_id") unless client_id @@ -1021,6 +1084,7 @@ module MCP client_secret = client_info_required_value(client_info, "client_secret") token_endpoint_auth_method = client_info_value(client_info, "token_endpoint_auth_method") + form = provider_token_request_params.merge(form) # Apply one client authentication method per request (RFC 6749 Section 2.3). headers = {} diff --git a/lib/mcp/client/oauth/provider.rb b/lib/mcp/client/oauth/provider.rb index 7d1a0f6..16f8c4e 100644 --- a/lib/mcp/client/oauth/provider.rb +++ b/lib/mcp/client/oauth/provider.rb @@ -48,6 +48,11 @@ module MCP # The document served at the URL is a separate JSON artifact from the `client_metadata` keyword: # DCR `client_metadata` MUST NOT include `client_id`, while the CIMD document MUST include `client_id` set # to the URL, `client_name`, and `redirect_uris` covering `redirect_uri`. + # - `token_request_params` - Hash of String keys and values added to every token request this provider makes + # (the authorization code exchange and refresh), for parameters the authorization server requires beyond + # the grant itself. The authorization request is not affected. A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS` + # raises `Flow::InvalidTokenRequestParamsError`. + # The Hash is copied and frozen. See `StorageBackedProvider#token_request_params`. class Provider include StorageBackedProvider @@ -87,7 +92,8 @@ module MCP scope: nil, storage: nil, client_id_metadata_document_url: nil, - authorization_request_validator: nil + authorization_request_validator: nil, + token_request_params: nil ) unless Discovery.secure_url?(redirect_uri) raise InsecureRedirectURIError, @@ -117,6 +123,7 @@ module MCP @storage = storage || InMemoryStorage.new @client_id_metadata_document_url = client_id_metadata_document_url @authorization_request_validator = authorization_request_validator + @token_request_params = frozen_token_request_params(token_request_params) end # Identifies the OAuth flow this provider drives. diff --git a/lib/mcp/client/oauth/storage_backed_provider.rb b/lib/mcp/client/oauth/storage_backed_provider.rb index 44d09a1..2be8acc 100644 --- a/lib/mcp/client/oauth/storage_backed_provider.rb +++ b/lib/mcp/client/oauth/storage_backed_provider.rb @@ -4,11 +4,11 @@ module MCP class Client module OAuth # Shared token/credential persistence for the OAuth provider classes - # (`Provider` for the authorization-code flow and `ClientCredentialsProvider` - # for the client_credentials flow). The two grants differ in how they authenticate, - # but both read and write the same two pieces of state through a `storage` object: - # the token response and the client information. This module supplies that delegation - # so the `Flow` orchestrator can treat any provider uniformly. + # (`Provider` for the authorization-code flow, `ClientCredentialsProvider` + # for the client_credentials flow, and `CrossAppAccessProvider` for the jwt-bearer flow). + # The grants differ in how they authenticate, but all read and write the same two pieces of state + # through a `storage` object: the token response and the client information. This module supplies + # that delegation so the `Flow` orchestrator can treat any provider uniformly. # # Including classes must set `@storage` to an object responding to `tokens`, # `save_tokens(tokens)`, `client_information`, and `save_client_information(info)` @@ -21,6 +21,13 @@ module MCP # MCP SDK has today. attr_reader :authorization_request_validator + # Optional Hash of String keys and values added to every token request the provider makes, for parameters + # the authorization server requires beyond the grant itself (RFC 6749 Section 8.2 leaves room for them; + # Auth0's `audience` is the usual one). `nil` (the default) adds nothing. + # Set through `frozen_token_request_params`, which refuses a key `Flow` sets itself or a wrongly shaped value + # with `Flow::InvalidTokenRequestParamsError`. + attr_reader :token_request_params + def access_token tokens&.dig("access_token") || tokens&.dig(:access_token) end @@ -44,6 +51,21 @@ module MCP def clear_tokens! @storage.save_tokens(nil) end + + private + + # Constructors call this before writing to `storage`, so a rejected provider leaves it untouched. + # The copy has its own frozen keys and values, so a later change to the caller's Hash, to a key, + # or to a value cannot alter what is sent to the token endpoint. Keys are copied explicitly + # because `Hash` copies and freezes only keys whose class is exactly `String`. + def frozen_token_request_params(params) + return if params.nil? + + problem = Flow.token_request_params_problem(params) + raise Flow::InvalidTokenRequestParamsError, "token_request_params #{problem}" if problem + + params.each_with_object({}) { |(key, value), copy| copy[key.dup.freeze] = value.dup.freeze }.freeze + end end end end diff --git a/test/mcp/client/oauth/client_credentials_provider_test.rb b/test/mcp/client/oauth/client_credentials_provider_test.rb index c112aba..5ff4d20 100644 --- a/test/mcp/client/oauth/client_credentials_provider_test.rb +++ b/test/mcp/client/oauth/client_credentials_provider_test.rb @@ -7,6 +7,23 @@ module MCP class Client module OAuth class ClientCredentialsProviderTest < Minitest::Test + # Every parameter the flow sets on a token request, spelled out rather than read from + # `Flow::RESERVED_TOKEN_REQUEST_PARAMS`, so a key dropped from that constant fails here. + RESERVED_TOKEN_REQUEST_PARAMS = [ + "grant_type", + "client_id", + "client_secret", + "client_assertion", + "client_assertion_type", + "scope", + "resource", + "code", + "code_verifier", + "redirect_uri", + "refresh_token", + "assertion", + ].freeze + def test_initialize_stores_credentials_as_client_information provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret") @@ -160,6 +177,90 @@ module MCP assert_equal("cc-client", claims["sub"]) assert_equal("https://auth.example.com", claims["aud"]) end + + def test_token_request_params_is_nil_by_default + provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret") + + assert_nil(provider.token_request_params) + end + + def test_initialize_keeps_a_frozen_copy_of_token_request_params + params = { "audience" => +"https://api.example.com" } + provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", token_request_params: params) + + params["audience"] << "/changed" + params["organization"] = "org_123" + + assert_equal({ "audience" => "https://api.example.com" }, provider.token_request_params) + assert_predicate(provider.token_request_params, :frozen?) + assert_predicate(provider.token_request_params["audience"], :frozen?) + end + + def test_initialize_rejects_token_request_params_that_the_sdk_sets_itself + RESERVED_TOKEN_REQUEST_PARAMS.each do |key| + error = assert_raises(Flow::InvalidTokenRequestParamsError, "should reject #{key.inspect}") do + ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", token_request_params: { key => "x" }) + end + + assert_includes(error.message, key.inspect) + end + end + + def test_initialize_rejects_token_request_params_that_are_not_a_hash_of_strings + ["audience=x", { audience: "x" }, { "audience" => 1 }, { "audience" => nil }].each do |params| + assert_raises(Flow::InvalidTokenRequestParamsError, "should reject #{params.inspect}") do + ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", token_request_params: params) + end + end + end + + def test_initialize_rejects_token_request_params_that_compare_keys_by_identity + # Two equal keys are two entries in such a Hash and would silently collapse into one when copied. + error = assert_raises(Flow::InvalidTokenRequestParamsError) do + ClientCredentialsProvider.new( + client_id: "cc-client", + client_secret: "cc-secret", + token_request_params: { "audience" => "x" }.compare_by_identity, + ) + end + + assert_match(/identity/, error.message) + end + + def test_initialize_copies_keys_that_hash_would_share_with_the_caller + # `Hash` copies and freezes only keys whose class is exactly `String`; a subclass key would otherwise + # stay shared with the caller, and a later `replace` would turn it into a reserved name. + key = Class.new(String).new("audience") + provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", token_request_params: { key => "x" }) + + key.replace("grant_type") + + assert_equal({ "audience" => "x" }, provider.token_request_params) + refute_same(key, provider.token_request_params.keys.first) + assert_predicate(provider.token_request_params.keys.first, :frozen?) + end + + def test_initialize_keeps_empty_token_request_params_as_a_frozen_empty_hash + provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", token_request_params: {}) + + assert_equal({}, provider.token_request_params) + assert_predicate(provider.token_request_params, :frozen?) + end + + def test_initialize_writes_no_client_information_when_token_request_params_are_rejected + storage = InMemoryStorage.new + + assert_raises(Flow::InvalidTokenRequestParamsError) do + ClientCredentialsProvider.new( + client_id: "cc-client", + client_secret: "cc-secret", + storage: storage, + token_request_params: { "grant_type" => "password" }, + ) + end + + assert_nil(storage.client_information) + end end end end diff --git a/test/mcp/client/oauth/cross_app_access_provider_test.rb b/test/mcp/client/oauth/cross_app_access_provider_test.rb index deb7e4b..1283b62 100644 --- a/test/mcp/client/oauth/cross_app_access_provider_test.rb +++ b/test/mcp/client/oauth/cross_app_access_provider_test.rb @@ -88,6 +88,42 @@ module MCP provider.clear_tokens! assert_nil(provider.tokens) end + + def test_token_request_params_is_nil_by_default + assert_nil(build_provider.token_request_params) + end + + def test_initialize_keeps_a_frozen_copy_of_token_request_params + params = { "audience" => +"https://api.example.com" } + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag" }, + token_request_params: params, + ) + + params["audience"] << "/changed" + + assert_equal({ "audience" => "https://api.example.com" }, provider.token_request_params) + assert_predicate(provider.token_request_params, :frozen?) + end + + def test_initialize_writes_no_client_information_when_token_request_params_are_rejected + storage = InMemoryStorage.new + + error = assert_raises(Flow::InvalidTokenRequestParamsError) do + CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag" }, + storage: storage, + token_request_params: { "assertion" => "x" }, + ) + end + + assert_includes(error.message, '"assertion"') + assert_nil(storage.client_information) + end end end end diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index 5daff12..c463fdc 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -85,14 +85,62 @@ module MCP ) end - def client_credentials_provider(token_endpoint_auth_method: "client_secret_basic") + def client_credentials_provider(token_endpoint_auth_method: "client_secret_basic", token_request_params: nil) ClientCredentialsProvider.new( client_id: "cc-client", client_secret: "cc-secret", token_endpoint_auth_method: token_endpoint_auth_method, + token_request_params: token_request_params, ) end + # An authorization-code provider that drives the code exchange and refresh with the given parameters. + def provider_with_token_request_params(params, redirect_handler: ->(_url) {}, callback_handler: -> { [nil, nil] }) + Provider.new(**authorization_code_provider_arguments(redirect_handler, callback_handler), token_request_params: params) + end + + # Overriding the reader bypasses the constructor's validation, leaving the flow's own check to be exercised. + def provider_returning_token_request_params(params, redirect_handler: ->(_url) {}, callback_handler: -> { [nil, nil] }) + provider_class = Class.new(Provider) do + define_method(:token_request_params) { params } + end + + provider_class.new(**authorization_code_provider_arguments(redirect_handler, callback_handler)) + end + + def authorization_code_provider_arguments(redirect_handler, callback_handler) + { + 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: redirect_handler, + callback_handler: callback_handler, + } + end + + # See `provider_returning_token_request_params`. + def client_credentials_provider_returning(token_request_params:) + provider_class = Class.new(ClientCredentialsProvider) do + define_method(:token_request_params) { token_request_params } + end + + provider_class.new(client_id: "cc-client", client_secret: "cc-secret") + end + + # `OpenSSL::PKey::EC.generate` only exists from the openssl gem 2.2 (Ruby 3.0); + # fall back to the pre-3.0 API on older Rubies. + def generate_es256_key + if OpenSSL::PKey::EC.respond_to?(:generate) + OpenSSL::PKey::EC.generate("prime256v1") + else + OpenSSL::PKey::EC.new("prime256v1").tap(&:generate_key) + 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 @@ -250,17 +298,10 @@ module MCP ), ) - # `OpenSSL::PKey::EC.generate` only exists from the openssl gem 2.2 (Ruby 3.0); - # fall back to the pre-3.0 API on older Rubies. - key = if OpenSSL::PKey::EC.respond_to?(:generate) - OpenSSL::PKey::EC.generate("prime256v1") - else - OpenSSL::PKey::EC.new("prime256v1").tap(&:generate_key) - end provider = ClientCredentialsProvider.new( client_id: "cc-client", token_endpoint_auth_method: "private_key_jwt", - private_key: key, + private_key: generate_es256_key, signing_algorithm: "ES256", ) @@ -285,6 +326,190 @@ module MCP end end + def test_run_client_credentials_sends_token_request_params_with_client_secret_basic + provider = client_credentials_provider(token_request_params: { "audience" => "https://api.example.com" }) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "client_credentials" && + form["resource"] == "https://srv.example.com/mcp" && + form["audience"] == "https://api.example.com" && + !form.key?("client_id") && + req.headers["Authorization"] == "Basic " + Base64.strict_encode64("cc-client:cc-secret") + end + end + + def test_run_client_credentials_sends_token_request_params_returned_by_a_provider_method + provider = client_credentials_provider_returning(token_request_params: { "audience" => "https://api.example.com" }) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_requested(:post, "#{@auth_base}/token") do |req| + URI.decode_www_form(req.body).to_h["audience"] == "https://api.example.com" + end + end + + def test_run_client_credentials_sends_the_same_request_for_empty_token_request_params + provider = client_credentials_provider(token_request_params: {}) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_requested(:post, "#{@auth_base}/token") do |req| + URI.decode_www_form(req.body).map(&:first) == ["grant_type", "resource"] + end + end + + def test_run_client_credentials_sends_token_request_params_with_client_secret_post + provider = client_credentials_provider( + token_endpoint_auth_method: "client_secret_post", + token_request_params: { "audience" => "https://api.example.com" }, + ) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["audience"] == "https://api.example.com" && + form["client_id"] == "cc-client" && + form["client_secret"] == "cc-secret" + end + end + + def test_run_client_credentials_sends_token_request_params_with_private_key_jwt + provider = ClientCredentialsProvider.new( + client_id: "cc-client", + token_endpoint_auth_method: "private_key_jwt", + private_key: generate_es256_key, + signing_algorithm: "ES256", + token_request_params: { "audience" => "https://api.example.com" }, + ) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["audience"] == "https://api.example.com" && + form["client_assertion_type"] == "urn:ietf:params:oauth:client-assertion-type:jwt-bearer" && + !form["client_assertion"].to_s.empty? + end + end + + def test_run_sends_token_request_params_on_the_authorization_code_exchange + state_value = nil + provider = provider_with_token_request_params( + { "audience" => "https://api.example.com" }, + redirect_handler: ->(url) { state_value = URI.decode_www_form(url.query).to_h.fetch("state") }, + callback_handler: -> { ["test-auth-code", state_value] }, + ) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "authorization_code" && + form["code"] == "test-auth-code" && + form["audience"] == "https://api.example.com" + end + end + + def test_refresh_sends_token_request_params + provider = provider_with_token_request_params({ "audience" => "https://api.example.com" }) + provider.save_client_information("client_id" => "test-client") + 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_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "refresh_token" && + form["refresh_token"] == "saved-rt" && + form["audience"] == "https://api.example.com" + end + end + + def test_run_sends_token_request_params_on_the_jwt_bearer_grant + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag-assertion" }, + token_request_params: { "audience" => "https://api.example.com" }, + ) + + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "urn:ietf:params:oauth:grant-type:jwt-bearer" && + form["assertion"] == "id-jag-assertion" && + form["audience"] == "https://api.example.com" + end + end + + def test_run_refuses_token_request_params_that_name_a_reserved_parameter + provider = client_credentials_provider_returning(token_request_params: { "grant_type" => "password" }) + + error = assert_raises(Flow::InvalidTokenRequestParamsError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/"grant_type"/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_refuses_token_request_params_that_are_not_a_hash_of_strings + shapes = ["audience=x", { audience: "x" }, { "audience" => 1 }, { "audience" => nil }, { "audience" => "x" }.compare_by_identity] + shapes.each do |params| + provider = client_credentials_provider_returning(token_request_params: params) + + assert_raises(Flow::InvalidTokenRequestParamsError, "should refuse #{params.inspect}") do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + end + + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_refresh_refuses_token_request_params_that_name_a_reserved_parameter + # Not an `AuthorizationError`: the transport treats a failed refresh as a reason + # to run the interactive flow, which would fail the same way afterwards. + provider = provider_returning_token_request_params({ "refresh_token" => "other" }) + provider.save_client_information("client_id" => "test-client") + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt") + + error = assert_raises(Flow::InvalidTokenRequestParamsError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/"refresh_token"/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_refuses_token_request_params_on_the_authorization_code_exchange + state_value = nil + provider = provider_returning_token_request_params( + { "code" => "other" }, + redirect_handler: ->(url) { state_value = URI.decode_www_form(url.query).to_h.fetch("state") }, + callback_handler: -> { ["test-auth-code", state_value] }, + ) + + error = assert_raises(Flow::InvalidTokenRequestParamsError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/"code"/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + def test_run_uses_jwt_bearer_grant_for_cross_app_access_provider # SEP-990: the ID-JAG assertion (obtained out of band, typically via IdP token exchange) is presented at # the token endpoint with client_secret_basic; no PKCE, redirect, or DCR is involved. diff --git a/test/mcp/client/oauth/http_oauth_test.rb b/test/mcp/client/oauth/http_oauth_test.rb index 9970971..5b1be13 100644 --- a/test/mcp/client/oauth/http_oauth_test.rb +++ b/test/mcp/client/oauth/http_oauth_test.rb @@ -627,6 +627,57 @@ module MCP assert_equal("refreshed-token", provider.access_token) end + def test_send_request_surfaces_a_bad_token_request_params_hook_instead_of_reauthorizing + # A provider whose `token_request_params` names a reserved key is misconfigured, not unauthorized: + # the refresh attempt must raise rather than fall through to the interactive flow, + # which would fail the same way after the user signed in. + stub_request(:post, @mcp_url).to_return( + status: 401, + headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") }, + body: "", + ) + + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + + stub_request(:get, "#{@auth_base}/.well-known/oauth-authorization-server").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", + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + ), + ) + + redirected = false + provider_class = Class.new(Provider) do + define_method(:token_request_params) { { "grant_type" => "password" } } + end + provider = provider_class.new( + client_metadata: { redirect_uris: ["http://localhost:0/callback"] }, + redirect_uri: "http://localhost:0/callback", + redirect_handler: ->(_url) { redirected = true }, + callback_handler: -> { ["code", "state"] }, + ) + provider.save_client_information("client_id" => "test-client") + provider.save_tokens("access_token" => "stale-token", "refresh_token" => "saved-rt") + + transport = HTTP.new(url: @mcp_url, oauth: provider) + + assert_raises(Flow::InvalidTokenRequestParamsError) do + transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" }) + end + refute(redirected) + assert_not_requested(:post, "#{@auth_base}/token") + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + def test_send_request_preserves_refresh_token_when_refresh_hits_a_transient_failure # A 5xx (or any non-`invalid_grant`) from the token endpoint indicates # a transient AS outage, NOT that the refresh token is dead. diff --git a/test/mcp/client/oauth/provider_test.rb b/test/mcp/client/oauth/provider_test.rb index 3db9427..d9fa64e 100644 --- a/test/mcp/client/oauth/provider_test.rb +++ b/test/mcp/client/oauth/provider_test.rb @@ -187,6 +187,30 @@ module MCP assert_equal(:authorization_code, provider.authorization_flow) end + + def test_token_request_params_is_nil_by_default + provider = Provider.new(**args_for("https://app.example.com/callback")) + + assert_nil(provider.token_request_params) + end + + def test_initialize_keeps_a_frozen_copy_of_token_request_params + params = { "audience" => +"https://api.example.com" } + provider = Provider.new(**args_for("https://app.example.com/callback"), token_request_params: params) + + params["audience"] << "/changed" + + assert_equal({ "audience" => "https://api.example.com" }, provider.token_request_params) + assert_predicate(provider.token_request_params, :frozen?) + end + + def test_initialize_rejects_token_request_params_that_the_sdk_sets_itself + error = assert_raises(Flow::InvalidTokenRequestParamsError) do + Provider.new(**args_for("https://app.example.com/callback"), token_request_params: { "code" => "x" }) + end + + assert_includes(error.message, '"code"') + end end end end From 47eb74aa7aea5db27704e815a08a9082cd4b727b Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 17 Sep 2026 12:30:05 +0900 Subject: [PATCH 19/24] Validate the issuer of legacy authorization server metadata ## Motivation and Context On the legacy 2025-03-26 path, taken when a server publishes no Protected Resource Metadata, the authorization server metadata is fetched from the MCP server's origin, which that spec names as the authorization base URL, but its `issuer` was never compared with that origin. RFC 8414 Section 3.3 requires the comparison, the 2025-03-26 spec makes RFC 8414 a MUST for clients, and the TypeScript and Python SDKs both compare on this path, tolerating only a trailing slash. The flow nevertheless used the unverified `issuer` as the authorization server's identity everywhere: tokens were stamped with it, stored client information was matched against it, the `private_key_jwt` and ID-JAG assertions took it as their audience, and `authorization_request_validator` was shown it. A server on that path could therefore claim any issuer. Claiming the one that minted the stored tokens made `refresh!` send the refresh token and the stored client secret to the server's own token endpoint, claiming the one that issued stored client information made the flow reuse those credentials there, and a validator allow-listing the claimed issuer approved the request. The legacy path now applies the same check as the modern one: the document's `issuer` must be the origin, rendered with or without a trailing slash, and any other document is refused before registration, redirect, or token request. A server without a metadata document still gets the spec's default endpoints. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb` serve legacy metadata at the MCP server origin that claims `https://auth.example.com` and check that the flow, a refresh of tokens issued by that server, and a flow holding client information bound to it are all refused before any request, with the validator never asked and the credentials never presented. Metadata naming the origin, with or without a trailing slash and with endpoints under a path prefix, completes the flow, including the RFC 9207 `iss` check. The former test of a path-prefix issuer now expects the refusal. The refusal tests fail against the previous library. ## Breaking Changes A server without Protected Resource Metadata whose authorization server metadata names an issuer other than its origin is refused instead of used. The TypeScript and Python SDKs already refuse such a server, and the 2025-03-26 spec places the metadata at the origin. --- docs/_client/authorization.md | 2 +- lib/mcp/client/oauth/flow.rb | 28 +++++- test/mcp/client/oauth/flow_test.rb | 150 ++++++++++++++++++++++++++++- 3 files changed, 169 insertions(+), 11 deletions(-) diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index 750e0c6..e3c0709 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -37,7 +37,7 @@ pass an `MCP::Client::OAuth::Provider` to the transport instead of a static `Aut - On a `401 Unauthorized`, parse the `WWW-Authenticate` header, discover the authorization server (Protected Resource Metadata + RFC 8414 Authorization Server Metadata), perform Dynamic Client Registration if needed, run the OAuth 2.1 Authorization Code flow with PKCE (S256), and retry the failed request with the acquired token. - Fall back to the legacy 2025-03-26 discovery when the server publishes no Protected Resource Metadata, matching the TypeScript and Python SDKs: the MCP server's origin acts - as the authorization base URL, its metadata is fetched from `/.well-known/oauth-authorization-server` without the RFC 8414 issuer byte-match (which the legacy spec predates), + as the authorization base URL, its metadata is fetched from `/.well-known/oauth-authorization-server` and must name that origin as its `issuer` (RFC 8414 Section 3.3), and when even that is absent the spec's default endpoints `/authorize`, `/token`, and `/register` at the origin are used with PKCE S256 assumed. - On subsequent 401s with a saved `refresh_token`, exchange it at the token endpoint before falling back to the full interactive flow (RFC 6749 Section 6). - 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), diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index acc0f1c..fdbf009 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -361,16 +361,26 @@ module MCP # Fetches and validates the authorization server's RFC 8414 metadata. # - # On the modern path the metadata `issuer` must be byte-identical to the discovery URL (RFC 8414 Section 3.3). - # On the legacy 2025-03-26 path that validation is skipped: the legacy spec predates the requirement, - # and a pre-PRM server may host its OAuth endpoints under a path prefix whose `issuer` legitimately differs from - # the origin the metadata was discovered at (neither the TypeScript nor the Python SDK validates the issuer on this path). + # The metadata `issuer` must be byte-identical to the discovery URL (RFC 8414 Section 3.3) on both paths. + # On the legacy 2025-03-26 path the discovery URL is the MCP server's origin, which that spec names as + # the authorization base URL and which a document may render with a trailing slash; the TypeScript and Python SDKs + # accept the same slash-only difference. A document naming any other issuer is refused: an unverified `issuer` + # would otherwise become the identity tokens and client information are bound to, assertions are minted for, + # and the validator is shown, so a server could claim another authorization server and unlock the credentials + # bound to it. # When even the metadata document is absent, the legacy spec's default endpoints are used. def authorization_server_metadata(authorization_server:, legacy:, server_url:) metadata = if legacy - begin + fetched = begin fetch_authorization_server_metadata(issuer_url: authorization_server) rescue AuthorizationError + nil + end + + if fetched + ensure_legacy_issuer_matches!(expected: authorization_server, returned: fetched["issuer"]) + fetched + else default_legacy_metadata(authorization_server) end else @@ -415,6 +425,14 @@ module MCP fetch_metadata_json(urls, label: "authorization server metadata") end + # The legacy authorization base is an origin, which a document may render as `https://host/`; + # both name the same server, and nothing else does. + def ensure_legacy_issuer_matches!(expected:, returned:) + return if returned == "#{expected}/" + + ensure_issuer_matches!(expected: expected, returned: returned) + end + # Reads `authorization_servers` from a PRM document and returns # the first entry, raising `AuthorizationError` for any of the malformed # shapes a non-compliant server could emit (missing field, non-Array diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index 5daff12..61d7559 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -917,22 +917,22 @@ module MCP stub_request(:get, "https://srv.example.com/.well-known/oauth-protected-resource").to_return(status: 404) end - def test_run_falls_back_to_server_origin_metadata_without_prm - # Legacy 2025-03-26 shape: no PRM, AS metadata served from the MCP server origin, - # OAuth endpoints under a path prefix whose `issuer` differs from the discovery origin. - # The legacy path must not apply the RFC 8414 issuer byte-match (the legacy spec predates it). + # Legacy 2025-03-26 shape: no PRM, AS metadata served from the MCP server origin with the given `issuer` + # and the OAuth endpoints under a path prefix. + def stub_legacy_metadata_with_prefixed_endpoints(issuer:, iss_supported: false) stub_prm_not_found stub_request(:get, "https://srv.example.com/.well-known/oauth-authorization-server").to_return( status: 200, headers: { "Content-Type" => "application/json" }, body: JSON.generate( - issuer: "https://srv.example.com/oauth", + issuer: issuer, authorization_endpoint: "https://srv.example.com/oauth/authorize", token_endpoint: "https://srv.example.com/oauth/token", registration_endpoint: "https://srv.example.com/oauth/register", response_types_supported: ["code"], code_challenge_methods_supported: ["S256"], token_endpoint_auth_methods_supported: ["none"], + authorization_response_iss_parameter_supported: iss_supported, ), ) stub_request(:post, "https://srv.example.com/oauth/register").to_return( @@ -945,6 +945,11 @@ module MCP headers: { "Content-Type" => "application/json" }, body: JSON.generate(access_token: "legacy-token", token_type: "Bearer", expires_in: 3600), ) + end + + def test_run_falls_back_to_server_origin_metadata_without_prm + # The document names the origin, so the RFC 8414 Section 3.3 check passes and its prefixed endpoints are used. + stub_legacy_metadata_with_prefixed_endpoints(issuer: "https://srv.example.com") holder = {} provider = build_legacy_discovery_provider(holder) @@ -953,11 +958,54 @@ module MCP assert_equal(:authorized, result) assert_equal("legacy-token", provider.access_token) + assert_equal("https://srv.example.com", provider.tokens["issuer"]) assert_equal("/oauth/authorize", holder[:authorization_url].path) assert_requested(:post, "https://srv.example.com/oauth/register") assert_requested(:post, "https://srv.example.com/oauth/token") end + def test_run_accepts_legacy_metadata_naming_the_origin_with_a_trailing_slash + # A root issuer rendered as `https://host/` names the same server; the TypeScript and Python SDKs accept it too, + # and the document's spelling is what the RFC 9207 `iss` and the recorded issuer carry. + stub_legacy_metadata_with_prefixed_endpoints(issuer: "https://srv.example.com/", iss_supported: true) + + holder = {} + provider = 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) { holder[:state] = URI.decode_www_form(url.query).to_h.fetch("state") }, + callback_handler: -> { ["test-auth-code", holder[:state], "https://srv.example.com/"] }, + ) + + result = Flow.new(provider: provider).run!(server_url: @server_url) + + assert_equal(:authorized, result) + assert_equal("https://srv.example.com/", provider.tokens["issuer"]) + end + + def test_run_refuses_legacy_metadata_whose_issuer_is_not_the_origin + # RFC 8414 Section 3.3 applies on the legacy path as well: the 2025-03-26 spec places the metadata + # at the origin, so an issuer under a path prefix is a mismatch, as it is for the TypeScript and Python SDKs. + stub_legacy_metadata_with_prefixed_endpoints(issuer: "https://srv.example.com/oauth") + + holder = {} + provider = build_legacy_discovery_provider(holder) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).run!(server_url: @server_url) + end + + assert_match(/`issuer` does not match/, error.message) + assert_nil(holder[:authorization_url]) + assert_not_requested(:post, "https://srv.example.com/oauth/register") + assert_not_requested(:post, "https://srv.example.com/oauth/token") + end + def test_run_falls_back_to_default_endpoints_without_any_metadata # Legacy 2025-03-26 "Fallbacks for Servers without Metadata Discovery": with no PRM and no AS metadata, # the client MUST use /authorize, /token, and /register at the authorization base URL, still sending PKCE S256. @@ -1032,6 +1080,98 @@ module MCP assert_match(/`issuer` does not match/, error.message) end + # Legacy metadata served at the MCP server origin that claims the identity of another authorization server, + # with every endpoint at the origin itself. The claim fails the RFC 8414 check, so none of these endpoints is reached. + def stub_legacy_metadata_claiming(issuer) + stub_prm_not_found + stub_request(:get, "https://srv.example.com/.well-known/oauth-authorization-server").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate( + issuer: issuer, + authorization_endpoint: "https://srv.example.com/authorize", + token_endpoint: "https://srv.example.com/token", + registration_endpoint: "https://srv.example.com/register", + code_challenge_methods_supported: ["S256"], + ), + ) + stub_request(:post, "https://srv.example.com/register").to_return( + status: 201, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(client_id: "legacy-client"), + ) + stub_request(:post, "https://srv.example.com/token").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(access_token: "legacy-token", token_type: "Bearer", expires_in: 3600), + ) + end + + def test_run_refuses_legacy_metadata_claiming_another_authorization_server_before_the_validator + stub_legacy_metadata_claiming(@auth_base) + recorder = [] + provider = 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) { recorder << :redirected }, + callback_handler: -> { ["test-auth-code", "state"] }, + authorization_request_validator: ->(request) { + recorder << request + true + }, + ) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).run!(server_url: @server_url) + end + + assert_match(/`issuer` does not match/, error.message) + assert_empty(recorder) + assert_not_requested(:post, "https://srv.example.com/register") + assert_not_requested(:post, "https://srv.example.com/token") + end + + def test_refresh_refuses_legacy_metadata_claiming_the_issuer_that_minted_the_tokens + # The tokens came from https://auth.example.com; a PRM-less server claiming that issuer at its own origin + # must not receive them, so the refresh token stays for the server that issued it. + stub_legacy_metadata_claiming(@auth_base) + provider = build_legacy_discovery_provider({}) + provider.save_client_information("client_id" => "conf-client", "client_secret" => "conf-secret") + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).refresh!(server_url: @server_url) + end + + assert_match(/`issuer` does not match/, error.message) + assert_not_requested(:post, "https://srv.example.com/token") + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + + def test_run_does_not_present_client_information_bound_elsewhere_when_legacy_metadata_claims_that_issuer + # Client information issued by https://auth.example.com is bound to it (SEP-2352); a PRM-less server claiming + # that issuer is refused before the credentials could be presented anywhere. + stub_legacy_metadata_claiming(@auth_base) + holder = {} + provider = build_legacy_discovery_provider(holder) + provider.save_client_information("client_id" => "conf-client", "client_secret" => "conf-secret", "issuer" => @auth_base) + + assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).run!(server_url: @server_url) + end + + assert_nil(holder[:authorization_url]) + assert_not_requested(:post, "https://srv.example.com/register") + assert_not_requested(:post, "https://srv.example.com/token") + assert_equal("conf-secret", provider.client_information["client_secret"]) + assert_equal(@auth_base, provider.client_information["issuer"]) + end + def test_run_raises_when_prm_authorization_servers_is_not_an_array # `authorization_servers` MUST be an Array per RFC 9728. A misbehaving # PRM that returns a String would otherwise reach `.first` and raise From 29e8d3f2b91259f526c66329dff08a697d2fcf6f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 17 Sep 2026 19:05:51 +0900 Subject: [PATCH 20/24] Let the client_credentials and jwt-bearer grants run without Protected Resource Metadata ## Motivation and Context When a server publishes no Protected Resource Metadata, discovery takes the legacy 2025-03-26 path and `prm` is `nil`. The authorization-code branch of `run!` allowed for that by passing `prm || {}` to `resolve_scope`, but `run_client_credentials!` and `run_jwt_bearer!` passed `prm` as is, and `resolve_scope` then called `prm["scopes_supported"]` on `nil`. Without a non-empty challenge scope, a `ClientCredentialsProvider` or `CrossAppAccessProvider` pointed at such a server therefore failed with `NoMethodError` after discovery and before the token request or the ID-JAG callback, instead of running its grant against the legacy authorization base the way `Provider` does. `resolve_scope` now treats a missing PRM as advertising no scopes, so the challenge scope or the provider's own scope applies, and the authorization-code branch passes `prm` unchanged like the other two. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb` run the `client_credentials` grant against a PRM-less server with metadata at the origin, checking the provider's scope, the `resource`, and the recorded issuer, and run the `jwt-bearer` grant against a server without any metadata, checking the default token endpoint, the assertion audience, the `resource`, and the recorded issuer. Both raise `NoMethodError` against the previous library. ## Breaking Changes None. --- lib/mcp/client/oauth/flow.rb | 5 ++- test/mcp/client/oauth/flow_test.rb | 69 ++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index acc0f1c..f99b6f1 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -87,7 +87,7 @@ module MCP ensure_pkce_supported!(as_metadata) - effective_scope = resolve_scope(scope: scope, prm: prm || {}) + effective_scope = resolve_scope(scope: scope, prm: prm) 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 @@ -898,7 +898,8 @@ module MCP def resolve_scope(scope:, prm:) return scope if scope && !scope.empty? - supported = prm["scopes_supported"] + # `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? return @provider.scope if @provider.scope && !@provider.scope.empty? diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index 5daff12..a91006d 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -991,6 +991,75 @@ module MCP end end + def test_run_client_credentials_without_prm_uses_the_legacy_authorization_base + # A PRM-less server takes the legacy path for this grant too: metadata at the origin, no advertised scopes, + # so the provider's own scope is used and the tokens are bound to the origin. + stub_prm_not_found + stub_request(:get, "https://srv.example.com/.well-known/oauth-authorization-server").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate( + issuer: "https://srv.example.com", + token_endpoint: "https://srv.example.com/token", + token_endpoint_auth_methods_supported: ["client_secret_basic"], + ), + ) + stub_request(:post, "https://srv.example.com/token").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(access_token: "legacy-token", token_type: "Bearer", expires_in: 3600), + ) + provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", scope: "mcp:read") + + result = Flow.new(provider: provider).run!(server_url: @server_url) + + assert_equal(:authorized, result) + assert_equal("legacy-token", provider.access_token) + assert_equal("https://srv.example.com", provider.tokens["issuer"]) + assert_requested(:post, "https://srv.example.com/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "client_credentials" && + form["scope"] == "mcp:read" && + form["resource"] == "https://srv.example.com/mcp" + end + end + + def test_run_jwt_bearer_without_any_metadata_uses_the_legacy_default_endpoints + stub_prm_not_found + stub_request(:get, "https://srv.example.com/.well-known/oauth-authorization-server").to_return(status: 404) + stub_request(:get, "https://srv.example.com/.well-known/openid-configuration").to_return(status: 404) + stub_request(:post, "https://srv.example.com/token").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(access_token: "legacy-token", token_type: "Bearer", expires_in: 3600), + ) + received = nil + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(audience:, resource:) { + received = { audience: audience, resource: resource } + "id-jag-assertion" + }, + ) + + result = Flow.new(provider: provider).run!(server_url: @server_url) + + assert_equal(:authorized, result) + # The assertion is minted for the legacy authorization base, which is the only identity such a server has. + assert_equal({ audience: "https://srv.example.com", resource: "https://srv.example.com/mcp" }, received) + assert_equal("https://srv.example.com", provider.tokens["issuer"]) + assert_requested(:post, "https://srv.example.com/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "urn:ietf:params:oauth:grant-type:jwt-bearer" && + form["assertion"] == "id-jag-assertion" && + form["resource"] == "https://srv.example.com/mcp" && + !form.key?("scope") + end + end + def test_run_legacy_fallback_rejects_insecure_authorization_base # The Communication Security requirement still applies on the legacy path: a remote plain-http origin must not # become the authorization base URL. From f51ccfbe662f4f0320ceb5974d39eab317589e3d Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 17 Sep 2026 02:39:45 +0900 Subject: [PATCH 21/24] Let providers without a CIMD URL reader refresh their tokens ## Motivation and Context `Flow#refresh!` reads `client_id_metadata_document_url` from the provider unconditionally, once to decide whether a provider without stored `client_information` may still refresh and once to build the CIMD `client_id`. Only `Provider` defines that reader: `CrossAppAccessProvider` and `ClientCredentialsProvider` raise `NoMethodError` once a stored `refresh_token` is found. `MCP::Client::HTTP` calls `refresh!` on any `401` while a `refresh_token` is stored, so a `jwt-bearer` client whose authorization server issued one fails with `NoMethodError` on the first expired token instead of refreshing. The error is not an `AuthorizationError`, so the transport does not fall back to the grant either. RFC 7521 Section 4.1 makes a refresh token unusual for an assertion grant, not forbidden, and one in storage must not crash the client. The flow now reads the URL through a duck-typed helper, like `authorization_flow` and `client_assertion`, so a provider without the reader has no CIMD URL and refreshes with its stored `client_information`: the `refresh_token` grant is sent with the stored client authentication, and no new ID-JAG assertion is obtained. The authorization-code registration path reads the URL through the same helper. The docs now say that a refresh token issued anyway is used on the next `401`, and that the assertion provider is not called again on refresh. A refresh token stored without the `issuer` the SDK records is refused for these two providers before any request: unlike `Provider`, they have no tokens that predate the record, and a refresh asks no validator, so such a token would be presented to whatever authorization server discovery names. The refusal is an `AuthorizationError`, which sends the transport back through the grant, and the grant asks. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb` refresh through a `CrossAppAccessProvider` and a `ClientCredentialsProvider` holding a `refresh_token`, asserting the `refresh_token` grant, the Basic credentials, and that the assertion provider is not called. A transport test in `test/mcp/client/oauth/http_oauth_test.rb` sends a `401` to a `CrossAppAccessProvider` with a stored `refresh_token` and expects the refreshed bearer token on the retry. All three raise `NoMethodError` against the previous library. Further tests pin the issuer stamp on the refreshed tokens, the refusal of an authorization server that did not issue them for a `CrossAppAccessProvider`, a clean `AuthorizationError` for a provider with neither the reader nor stored `client_information`, and registration through DCR for a provider without the reader. A refresh token without a recorded issuer is refused for both providers before any request is sent. ## Breaking Changes None. --- docs/_client/authorization.md | 6 +- lib/mcp/client/oauth/flow.rb | 32 +++++- test/mcp/client/oauth/flow_test.rb | 132 +++++++++++++++++++++++ test/mcp/client/oauth/http_oauth_test.rb | 62 +++++++++++ 4 files changed, 227 insertions(+), 5 deletions(-) diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index b35b597..bb86458 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -40,6 +40,8 @@ pass an `MCP::Client::OAuth::Provider` to the transport instead of a static `Aut as the authorization base URL, its metadata is fetched from `/.well-known/oauth-authorization-server` without the RFC 8414 issuer byte-match (which the legacy spec predates), and when even that is absent the spec's default endpoints `/authorize`, `/token`, and `/register` at the origin are used with PKCE S256 assumed. - On subsequent 401s with a saved `refresh_token`, exchange it at the token endpoint before falling back to the full interactive flow (RFC 6749 Section 6). + `ClientCredentialsProvider` and `CrossAppAccessProvider` refresh the same way and fall back to their own grant instead; + their refresh also requires the `issuer` the SDK records on the tokens, so tokens stored without it run the grant again. - 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. @@ -192,7 +194,8 @@ and redaction policy before persisting them or displaying them to users. For a confidential machine-to-machine client (no user, no browser redirect), use `MCP::Client::OAuth::ClientCredentialsProvider` instead of `Provider`. The transport discovers the authorization server the same way, then exchanges the OAuth 2.1 `client_credentials` grant (RFC 6749 Section 4.4) at -the token endpoint. There is no authorization request, PKCE, or `offline_access`, because the grant does not issue a refresh token. +the token endpoint. There is no authorization request, PKCE, or `offline_access`, because the grant is not expected to issue a refresh token (RFC 6749 Section 4.4.3); +a refresh token the authorization server issues anyway is used on the next `401`. ```ruby provider = MCP::Client::OAuth::ClientCredentialsProvider.new( @@ -224,6 +227,7 @@ Keyword arguments: For enterprise MCP deployments where an identity provider (IdP) governs authorization (SEP-990), use `MCP::Client::OAuth::CrossAppAccessProvider` instead of `Provider`. The client exchanges an IdP-issued ID token for an Identity Assertion Authorization Grant (ID-JAG) at the IdP via RFC 8693 token exchange, then presents the ID-JAG to the MCP authorization server with the RFC 7523 `jwt-bearer` grant, authenticating with `client_secret_basic`. There is no authorization request, PKCE, DCR, or `offline_access`. +A refresh token the authorization server issues is exchanged on the next `401` with the stored client secret, without calling `assertion_provider` again. Mirrors `CrossAppAccessProvider` and `requestJwtAuthorizationGrant` in the TypeScript SDK. `MCP::Client::OAuth::IDJAGTokenExchange.request` performs the RFC 8693 exchange at the IdP token endpoint. Wrap it in a callable so the same provider can plug into diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 1093eba..c34d617 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -182,7 +182,7 @@ module MCP # Runs the OAuth 2.1 `client_credentials` grant (machine-to-machine, no user interaction) and persists # the resulting token. Shares the same discovery and security checks as `run!`; the only difference is # the grant exchanged at the token endpoint. There is no PKCE, redirect, or authorization request, - # and no `offline_access` augmentation because the grant does not issue a refresh token (OAuth 2.1 Section 4.3.3). + # and no `offline_access` augmentation because the grant is not expected to issue a refresh token (OAuth 2.1 Section 4.3.3). # The pre-registered `client_id` / `client_secret` come from the provider's stored `client_information`. # https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization def run_client_credentials!(as_metadata:, prm:, resource:, scope:, server_url:) @@ -275,19 +275,22 @@ module MCP # checks before talking to it. # # Returns `:refreshed` on success. Raises `AuthorizationError` when the provider has no refresh token, no client information, + # when a `client_credentials` or `jwt-bearer` provider's tokens record no issuer, # or when the token endpoint refuses the refresh request. # https://www.rfc-editor.org/rfc/rfc6749#section-6 def refresh!(server_url:, resource_metadata_url: nil) refresh_token = read_token("refresh_token") raise AuthorizationError, "Cannot refresh: no refresh_token in provider storage." unless refresh_token + ensure_refresh_token_issuer_recorded! + stored_client_info = @provider.client_information have_stored_client_info = stored_client_info.is_a?(Hash) && client_info_required_value(stored_client_info, "client_id") # A CIMD-configured provider stores no `client_information` on purpose # (the CIMD URL is re-resolved against the live AS metadata on every flow). # Allow refresh to proceed in that case so the `refresh_token` obtained via the CIMD flow remains usable. - have_cimd_url = !@provider.client_id_metadata_document_url.nil? + have_cimd_url = !provider_client_id_metadata_document_url.nil? unless have_stored_client_info || have_cimd_url raise AuthorizationError, "Cannot refresh: no client_information in provider storage." @@ -323,7 +326,7 @@ module MCP ensure_refreshable_client_information!(stored_client_info, as_metadata: as_metadata) stored_client_info elsif as_metadata["client_id_metadata_document_supported"] == true - { "client_id" => @provider.client_id_metadata_document_url } + { "client_id" => provider_client_id_metadata_document_url } else raise AuthorizationError, "Cannot refresh: provider has a CIMD URL but the authorization server no longer advertises " \ @@ -684,7 +687,7 @@ module MCP # (or the operator may rotate the CIMD URL), and a stale `client_information` entry would otherwise # keep sending the old CIMD URL forever. Re-evaluating on every flow re-reads the current AS metadata # and the current `provider.client_id_metadata_document_url`. - cimd_url = @provider.client_id_metadata_document_url + cimd_url = provider_client_id_metadata_document_url if cimd_url && as_metadata["client_id_metadata_document_supported"] == true return { "client_id" => cimd_url } end @@ -814,6 +817,18 @@ module MCP MESSAGE end + # `Provider` tolerates tokens stored before the issuer was recorded (see `ensure_token_issuer!`). + # The `client_credentials` and `jwt-bearer` providers have no such tokens, since their refresh is new, + # and a refresh asks no validator, so a token without an `issuer` would be presented to whatever + # authorization server discovery names now. Refusing it sends the transport back through the grant, + # which does ask. + def ensure_refresh_token_issuer_recorded! + return if provider_authorization_flow == :authorization_code + return unless read_token("issuer").nil? + + raise AuthorizationError, "Cannot refresh: the stored tokens record no issuer; re-authorization is required." + end + def ensure_refreshable_client_information!(client_info, as_metadata:) stored_issuer = client_info_required_value(client_info, "issuer") return if stored_issuer.nil? @@ -1020,6 +1035,15 @@ module MCP params end + # The Client ID Metadata Document URL, when the provider has one. Only `Provider` exposes the reader + # (CIMD replaces Dynamic Client Registration on the authorization-code flow), while `refresh!` serves + # every provider that holds a `refresh_token`, so the read is duck-typed like `authorization_flow`. + def provider_client_id_metadata_document_url + return unless @provider.respond_to?(: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:) authorization_endpoint = as_metadata["authorization_endpoint"] unless authorization_endpoint diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index c463fdc..799ecd0 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -557,6 +557,138 @@ module MCP assert_not_requested(:post, "#{@auth_base}/token") end + def test_refresh_uses_the_stored_credentials_of_a_cross_app_access_provider + # RFC 7521 Section 4.1 makes a refresh token unusual for an assertion grant, not forbidden; + # when the authorization server issued one, it is exchanged with the stored client secret + # and no new ID-JAG assertion is minted. + assertion_calls = 0 + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { + assertion_calls += 1 + "id-jag-assertion" + }, + ) + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + result = Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:refreshed, result) + assert_equal("test-token-from-flow", provider.access_token) + assert_equal(@auth_base, provider.tokens["issuer"]) + assert_equal(0, assertion_calls) + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "refresh_token" && + form["refresh_token"] == "saved-rt" && + !form.key?("assertion") && + req.headers["Authorization"] == "Basic " + Base64.strict_encode64("xaa-client:xaa-secret") + end + end + + def test_refresh_uses_the_stored_credentials_of_a_client_credentials_provider + # RFC 6749 Section 4.4.3 says a refresh token SHOULD NOT be issued for this grant, + # so one in storage is unusual, but it must be exchanged rather than crash the refresh. + provider = client_credentials_provider + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + result = Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:refreshed, result) + assert_equal(@auth_base, provider.tokens["issuer"]) + assert_requested(:post, "#{@auth_base}/token") do |req| + form = URI.decode_www_form(req.body).to_h + + form["grant_type"] == "refresh_token" && + form["refresh_token"] == "saved-rt" && + req.headers["Authorization"] == "Basic " + Base64.strict_encode64("cc-client:cc-secret") + end + end + + def test_refresh_refuses_an_authorization_server_that_did_not_issue_a_cross_app_access_token + # The token-level issuer binding is what protects these providers: their stored `client_information` carries no `issuer`, + # so `ensure_refreshable_client_information!` has nothing to compare. + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag-assertion" }, + ) + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => "https://old-as.example.com") + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/issued by a different authorization server/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + + def test_refresh_refuses_tokens_without_a_recorded_issuer_for_the_client_credentials_and_jwt_bearer_providers + # These providers had no refresh before their tokens recorded the issuer, so nothing older needs + # tolerating; without the record a refresh would go to whatever server discovery names, unasked. + cross_app = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag-assertion" }, + ) + [cross_app, client_credentials_provider].each do |provider| + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt") + + error = assert_raises(Flow::AuthorizationError, "should refuse #{provider.class}") do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/record no issuer/, error.message) + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + + assert_not_requested(:get, @prm_url) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_refresh_raises_cleanly_for_a_provider_with_neither_client_information_nor_a_cimd_url + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag-assertion" }, + ) + provider.storage.save_client_information(nil) + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/no client_information/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_registers_through_dcr_for_a_provider_without_a_cimd_url_reader + # Registration reads the CIMD URL through the same duck-typed helper as refresh, + # so a provider that lacks the reader goes to Dynamic Client Registration. + provider_class = Class.new(Provider) { undef_method :client_id_metadata_document_url } + state_value = nil + provider = provider_class.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_value = URI.decode_www_form(url.query).to_h.fetch("state") }, + callback_handler: -> { ["test-auth-code", state_value] }, + ) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_requested(:post, "#{@auth_base}/register") + end + def test_run_uses_authorization_code_grant_for_default_provider # A standard `Provider` declares `authorization_flow == :authorization_code`, # so `Flow` runs the interactive grant regardless of what `client_metadata[:grant_types]` happens to list. diff --git a/test/mcp/client/oauth/http_oauth_test.rb b/test/mcp/client/oauth/http_oauth_test.rb index 5b1be13..ce026c9 100644 --- a/test/mcp/client/oauth/http_oauth_test.rb +++ b/test/mcp/client/oauth/http_oauth_test.rb @@ -627,6 +627,68 @@ module MCP assert_equal("refreshed-token", provider.access_token) end + def test_send_request_refreshes_for_a_cross_app_access_provider_holding_a_refresh_token + # Refresh serves every provider, not only the authorization-code one that has a CIMD URL reader. + stub_request(:post, @mcp_url).with { |req| + req.headers["Authorization"] != "Bearer refreshed-token" + }.to_return( + status: 401, + headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") }, + body: "", + ) + + stub_request(:post, @mcp_url).with( + headers: { "Authorization" => "Bearer refreshed-token" } + ).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(jsonrpc: "2.0", id: "1", result: { ok: true }), + ) + + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + + stub_request(:get, "#{@auth_base}/.well-known/oauth-authorization-server").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate( + issuer: @auth_base, + token_endpoint: "#{@auth_base}/token", + grant_types_supported: ["urn:ietf:params:oauth:grant-type:jwt-bearer", "refresh_token"], + token_endpoint_auth_methods_supported: ["client_secret_basic"], + ), + ) + + 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: "refreshed-token", token_type: "Bearer", expires_in: 3600), + ) + + assertion_calls = 0 + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { + assertion_calls += 1 + "id-jag-assertion" + }, + ) + provider.save_tokens("access_token" => "stale-token", "refresh_token" => "saved-rt", "issuer" => @auth_base) + + transport = HTTP.new(url: @mcp_url, oauth: provider) + response = transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" }) + + assert_equal({ "ok" => true }, response["result"]) + assert_equal("refreshed-token", provider.access_token) + assert_equal(0, assertion_calls) + end + def test_send_request_surfaces_a_bad_token_request_params_hook_instead_of_reauthorizing # A provider whose `token_request_params` names a reserved key is misconfigured, not unauthorized: # the refresh attempt must raise rather than fall through to the interactive flow, From c9f381a3ee03768952099f1daf3fff9bc2dba5da Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 18 Sep 2026 01:15:37 +0900 Subject: [PATCH 22/24] Let a provider customize the HTTP client the OAuth flow uses ## Motivation and Context `MCP::Client::OAuth::Flow` takes an `http_client_factory:`, but `MCP::Client::HTTP` builds the flow itself and passes none, so an application using `oauth:` could not add middleware to the requests the flow makes: Protected Resource Metadata and authorization server metadata discovery, dynamic client registration, and every token request. The block that customizes the transport's Faraday connection does not help, because that connection is bound to the MCP server URL and carries the `headers:` meant for that server, and the authorization server is a different origin. The TypeScript SDK hands the transport's `fetch` to `auth()` and the Python SDK sends the provider's requests through the transport's httpx client, so both let an application instrument these requests; this SDK did not. (#558) Providers now take `http_client_customizer:`, a callable invoked with the Faraday connection the flow builds for its own requests, next to the other callables a provider already holds. It runs after the SDK's defaults (`Accept: application/json`, no redirect following) and between two middlewares of the SDK's own: `Flow::RequestedURLStamp`, registered before the customizer is invoked, records the URL the flow asked for on the request environment, and `Flow::RequestedOriginGuard`, registered last, sees what the customizer's middleware did to the request after that record. A request that would leave that origin, by following a redirect or rewriting the URL, or that arrives without the record, is refused with `Flow::DestinationMismatchError` before it reaches the adapter. The record lives on the environment rather than in `env.request.context`, which stays the application's to fill or replace. The error is an `ArgumentError` rather than an `AuthorizationError` so neither discovery nor the transport's refresh fallback can absorb a configuration mistake. The boundary resembles the one the Python SDK keeps for its own requests, which follows a redirect only within the origin; this flow follows none itself. `Flow.build_http_client` assembles that connection and `Flow.new(http_client_factory:)` keeps replacing it whole. Following done inside an adapter cannot be seen by any guard, so the documentation keeps that, `Accept-Encoding`, and `raise_error` off the list of things a customizer may add, and notes that a recording middleware sees client credentials and tokens. Fixes #558. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb`, `test/mcp/client/oauth/http_oauth_test.rb`, and the three provider test files. Against the library before this change, every one of them fails, on the unknown keyword or on the missing reader. ## Breaking Changes None. The customizer is opt-in, and the guard cannot fire on the SDK's own middleware-free connection. --- docs/_client/authorization.md | 45 +- docs/_client/transports.md | 3 + lib/mcp/client/oauth/bounded_body.rb | 2 +- .../oauth/client_credentials_provider.rb | 8 +- .../client/oauth/cross_app_access_provider.rb | 8 +- lib/mcp/client/oauth/flow.rb | 116 +++++- lib/mcp/client/oauth/id_jag_token_exchange.rb | 2 +- lib/mcp/client/oauth/provider.rb | 8 +- .../client/oauth/storage_backed_provider.rb | 15 + .../oauth/client_credentials_provider_test.rb | 19 + .../oauth/cross_app_access_provider_test.rb | 30 ++ test/mcp/client/oauth/flow_test.rb | 390 +++++++++++++++++- test/mcp/client/oauth/http_oauth_test.rb | 216 +++++++++- test/mcp/client/oauth/provider_test.rb | 20 + 14 files changed, 856 insertions(+), 26 deletions(-) diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index b35b597..314d712 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -95,6 +95,8 @@ Optional keyword arguments: - `scope`: Space-separated scopes to request when the server's `WWW-Authenticate` does not specify one. - `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. + See [Customizing the OAuth HTTP Client](#customizing-the-oauth-http-client). - `storage`: Object responding to `tokens`, `save_tokens(t)`, `client_information`, `save_client_information(info)`. Defaults to `MCP::Client::OAuth::InMemoryStorage`, which keeps credentials in process memory only. Persisted `client_information` is stamped with an `"issuer"` member binding it to the authorization server that issued it (SEP-2352): when the server's authorization server changes, the SDK discards the stale registration and its tokens and re-registers automatically @@ -216,7 +218,7 @@ Keyword arguments: - `private_key`, `signing_algorithm`: Required with `private_key_jwt` - the key (a PEM string or `OpenSSL::PKey::PKey`, never written to `storage`) signs the client assertion with `"ES256"` or `"RS256"`; `client_secret` must not be set, because the private key is the credential. -- `scope`, `storage`, `authorization_request_validator`, `token_request_params`: Optional, same meaning as on `Provider`. +- `scope`, `storage`, `authorization_request_validator`, `token_request_params`, `http_client_customizer`: Optional, same meaning as on `Provider`. Use `token_request_params` for a parameter the authorization server requires on the `client_credentials` grant, such as Auth0's `audience`. ### Cross-App Access (JWT Bearer) Grant @@ -254,7 +256,39 @@ Keyword arguments: - `assertion_provider`: Required. Callable invoked as `call(audience:, resource:)` and returning the ID-JAG assertion. `audience` is the MCP authorization server's validated issuer identifier; `resource` is the canonical MCP server URL (RFC 8707). Passing both through to `IDJAGTokenExchange.request` covers the common case. -- `scope`, `storage`, `authorization_request_validator`, `token_request_params`: Optional, same meaning as on `Provider`. +- `scope`, `storage`, `authorization_request_validator`, `token_request_params`, `http_client_customizer`: Optional, same meaning as on `Provider`. + +### Customizing the OAuth HTTP Client + +The requests the OAuth flow makes (Protected Resource Metadata discovery on the MCP server's origin, authorization server metadata discovery, +dynamic client registration, and every token request the flow sends, whether the first exchange, a refresh, or a step-up) go over a Faraday connection of their own, +not over the transport's connection: the transport's is bound to the MCP server URL and carries the `headers:` and the customizer block meant for that server. +To add middleware to the OAuth flow's connection, or to swap its adapter, pass `http_client_customizer:` to the provider: + +```ruby +provider = MCP::Client::OAuth::ClientCredentialsProvider.new( + client_id: "my-service", + client_secret: ENV.fetch("MCP_CLIENT_SECRET"), + http_client_customizer: ->(faraday) { faraday.use MyApp::Middleware::HttpRecorder }, +) +``` + +The callable receives the `Faraday::Connection` after the SDK has applied its defaults and registered the middleware that records the requested URL, +and before the SDK registers its origin guard, the same position the transport's customizer block has on the MCP server connection. +It may be invoked more than once, and from more than one thread at a time, so keep it free of side effects and safe to run concurrently; +today it runs once per authorization attempt, but that is not a promise. +A few constraints follow from the checks described below: + +- Do not add redirect-following middleware. Every destination check runs against the URL as written, so a request that middleware added by the customizer would send + to a different origin after the SDK has recorded the requested URL, whether by following a `3xx` or by rewriting the URL, is refused with `Flow::DestinationMismatchError` + before it reaches the adapter, as is a request that reaches the guard without that record. Middleware inserted ahead of the record with `builder.insert(0, ...)` + that rewrites the URL first or rebuilds the environment is outside the guard, as is following done inside an adapter, so leave both off. +- Leave `Accept-Encoding` unset. The response cap below is measured on decoded bytes, and claiming the header turns Net::HTTP's decoding off. +- Do not add Faraday's `raise_error` middleware. The flow reads statuses itself, both to tell an absent metadata document from a failed request + and to turn a token endpoint error into `Flow::InvalidGrantError`. +- With an adapter that does not stream through `on_data`, the response cap is applied once the body has been buffered rather than as it arrives. +- A middleware that records requests sees the client credentials on token requests (`Authorization: Basic`, `client_secret`, `client_assertion`), refresh tokens, + and the access tokens in token responses; redact them before they reach a log. ### Communication Security @@ -286,8 +320,11 @@ The range check compares IP literals and does not resolve hostnames, so it canno such as `https://vault.corp.internal/`. Resolving names here would not close that gap either, because the address the SDK looked up need not be the one the HTTP client connects to a moment later. The same-origin rule is what protects the `resource_metadata` URL, which is the only one of these a server supplies directly. -If you replace the OAuth HTTP client through `MCP::Client::OAuth::Flow.new(http_client_factory:)`, do not add redirect-following middleware. Every check above runs against -the URL as written, so a connection that follows a `3xx` on its own would reach hosts these rules just refused. +On the connection the SDK builds, a middleware that would send a request to a different origin, by following a `3xx` or by rewriting the URL, is refused before the request +goes out (see [Customizing the OAuth HTTP Client](#customizing-the-oauth-http-client)). A connection supplied through `MCP::Client::OAuth::Flow.new(http_client_factory:)` +replaces that one, the provider's `http_client_customizer` and the guard included, so do not add redirect-following middleware to it: every check above runs against +the URL as written, and a connection that follows a `3xx` on its own would reach hosts these rules just refused. +A factory that wants to keep them can return `MCP::Client::OAuth::Flow.build_http_client(customizer)`, the connection the SDK builds for itself. The SDK also bounds what those endpoints may return. A discovery, dynamic client registration, token, or token exchange response is refused once it passes 4 MiB, measured as the body arrives rather than after it has been buffered, so a compressed body that expands past the limit is refused partway through the expansion. diff --git a/docs/_client/transports.md b/docs/_client/transports.md index 4362666..d69e5e9 100644 --- a/docs/_client/transports.md +++ b/docs/_client/transports.md @@ -248,6 +248,9 @@ http_transport = MCP::Client::HTTP.new(url: "https://api.example.com/mcp") do |f end ``` +The block customizes only the connection to the MCP server. The connection the OAuth flow uses for its own requests is customized through +the provider's `http_client_customizer:` keyword instead; see [Customizing the OAuth HTTP Client](/client/authorization/#customizing-the-oauth-http-client). + {: .note } > Answers to server-to-client requests (a pong, an elicitation result) are POSTed from inside > the SSE streaming callback of another response, re-entering the connection on the same thread. diff --git a/lib/mcp/client/oauth/bounded_body.rb b/lib/mcp/client/oauth/bounded_body.rb index 4aab2ad..f3b5efa 100644 --- a/lib/mcp/client/oauth/bounded_body.rb +++ b/lib/mcp/client/oauth/bounded_body.rb @@ -26,7 +26,7 @@ module MCP # Faraday `on_data` streaming callback. The chunks arrive decompressed: the default `Net::HTTP` adapter negotiates # `Accept-Encoding` itself and reads the body through `Net::HTTPResponse#inflater`, so a small compressed body # that expands past the cap is refused partway through the expansion rather than after it. That holds only while - # the connection leaves `Accept-Encoding` to the adapter; see `Flow#default_http_client`. + # the connection leaves `Accept-Encoding` to the adapter; see `Flow.build_http_client`. def on_data proc do |chunk, _received_bytes, _env| @buffer << chunk diff --git a/lib/mcp/client/oauth/client_credentials_provider.rb b/lib/mcp/client/oauth/client_credentials_provider.rb index 22ef56a..4e1e1cf 100644 --- a/lib/mcp/client/oauth/client_credentials_provider.rb +++ b/lib/mcp/client/oauth/client_credentials_provider.rb @@ -46,6 +46,8 @@ module MCP # server requires beyond the grant itself (Auth0's `audience`, for example). # A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS` raises `Flow::InvalidTokenRequestParamsError`. # The Hash is copied and frozen. See `StorageBackedProvider#token_request_params`. + # - `http_client_customizer` - Callable invoked with the `Faraday::Connection` the flow builds for + # its own requests; see `StorageBackedProvider#http_client_customizer`. class ClientCredentialsProvider include StorageBackedProvider @@ -66,7 +68,8 @@ module MCP scope: nil, storage: nil, authorization_request_validator: nil, - token_request_params: nil + token_request_params: nil, + http_client_customizer: nil ) if blank?(client_id) raise InvalidCredentialsError, "client_id is required for the client_credentials grant." @@ -104,6 +107,8 @@ module MCP client_information["client_secret"] = client_secret end + http_client_customizer = validated_http_client_customizer(http_client_customizer) + @client_id = client_id @private_key = private_key @signing_algorithm = signing_algorithm @@ -111,6 +116,7 @@ module MCP @storage = storage || InMemoryStorage.new @authorization_request_validator = authorization_request_validator @token_request_params = frozen_token_request_params(token_request_params) + @http_client_customizer = http_client_customizer @storage.save_client_information(client_information) end diff --git a/lib/mcp/client/oauth/cross_app_access_provider.rb b/lib/mcp/client/oauth/cross_app_access_provider.rb index 422cbb5..44b5ffd 100644 --- a/lib/mcp/client/oauth/cross_app_access_provider.rb +++ b/lib/mcp/client/oauth/cross_app_access_provider.rb @@ -29,6 +29,8 @@ module MCP # the authorization server requires beyond the grant itself. A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS` raises # `Flow::InvalidTokenRequestParamsError`. The Hash is copied and frozen. # See `StorageBackedProvider#token_request_params`. + # - `http_client_customizer` - Callable invoked with the `Faraday::Connection` the flow builds for its own requests; + # see `StorageBackedProvider#http_client_customizer`. # # https://github.com/modelcontextprotocol/modelcontextprotocol/issues/990 class CrossAppAccessProvider @@ -46,7 +48,8 @@ module MCP scope: nil, storage: nil, authorization_request_validator: nil, - token_request_params: nil + token_request_params: nil, + http_client_customizer: nil ) if blank?(client_id) raise InvalidConfigurationError, "client_id is required for the jwt-bearer grant." @@ -60,11 +63,14 @@ module MCP raise InvalidConfigurationError, "assertion_provider must be callable as `call(audience:, resource:)` and return the ID-JAG assertion." end + http_client_customizer = validated_http_client_customizer(http_client_customizer) + @assertion_provider = assertion_provider @scope = scope @storage = storage || InMemoryStorage.new @authorization_request_validator = authorization_request_validator @token_request_params = frozen_token_request_params(token_request_params) + @http_client_customizer = http_client_customizer @storage.save_client_information( "client_id" => client_id, "client_secret" => client_secret, diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 1093eba..5183c6b 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -63,6 +63,71 @@ module MCP # a failed refresh as a reason to run the interactive flow. class InvalidTokenRequestParamsError < ArgumentError; end + # Raised by `RequestedOriginGuard` when middleware added through the provider's `http_client_customizer` + # would send a request to an origin other than the one the flow validated, or has dropped the record of + # the URL the flow asked for. An `ArgumentError` because the middleware is a configuration mistake, + # and deliberately outside `AuthorizationError`, which discovery treats as "nothing published" + # and `MCP::Client::HTTP` treats on a failed refresh as a reason to run the interactive flow. + class DestinationMismatchError < ArgumentError; end + + # Faraday middleware registered on the connection `build_http_client` assembles before the customizer + # is invoked, so with the usual `use` it sits ahead of the customizer's middleware and sees the URL exactly + # as the flow requested it, which it records on the request environment for `RequestedOriginGuard`. + # The guard covers what happens to a request after that record; middleware inserted ahead of it with + # `builder.insert(0, ...)` that rewrites the URL before it or rebuilds the environment is outside the guard. + # The record lives on the environment, not in `env.request.context`: that slot belongs to the application, + # which may fill it on the connection or replace it from a middleware of its own. Only the first URL seen + # on an environment is recorded: a middleware inserted ahead of this one that re-enters the stack after + # a `3xx` with the same environment, or with its `dup`, which shares the record, cannot replace it with + # the redirected URL. + class RequestedURLStamp + KEY = :mcp_oauth_requested_url + + def initialize(app) + @app = app + end + + def call(env) + env[KEY] ||= env.url.to_s + @app.call(env) + end + end + + # Faraday middleware registered last on that connection, so it sees `env.url` after any customizer-added + # middleware has rewritten it or followed a redirect. A request that would leave the origin the flow asked + # for is refused before it reaches the adapter, since every destination check ran against the URL as + # written; a same-origin change stays with the server those checks admitted. The record survives + # the `env.dup` that redirect-following middleware performs, and a request that arrives without it is + # refused as well, so a middleware that rebuilds the environment fails closed rather than open. + # The origin boundary resembles the one the Python SDK keeps for its own auth requests, which follows + # a redirect itself only within the origin; this flow follows none. + class RequestedOriginGuard + def initialize(app) + @app = app + end + + def call(env) + requested = env[RequestedURLStamp::KEY] + unless requested + raise DestinationMismatchError, <<~MESSAGE + Request to #{Discovery.canonicalize_origin_and_path(env.url.to_s).inspect} carries no record of \ + the URL the flow asked for; middleware that rebuilds the request environment is refused. + MESSAGE + end + + unless Discovery.same_origin?(env.url.to_s, requested) + raise DestinationMismatchError, <<~MESSAGE + Request to #{Discovery.canonicalize_origin_and_path(requested).inspect} would be sent to \ + #{Discovery.canonicalize_origin_and_path(env.url.to_s).inspect}, on a different origin; \ + middleware that follows redirects or rewrites URLs is refused. + MESSAGE + end + + @app.call(env) + end + end + private_constant :RequestedURLStamp, :RequestedOriginGuard + class << self # Returns why `params` cannot ride a token request as `token_request_params`, or `nil` when it can. # Shared by the provider constructors and the flow, which both refuse the value with `InvalidTokenRequestParamsError`, @@ -82,6 +147,33 @@ module MCP nil end + + # Builds the connection the flow uses for its own requests: the SDK's defaults, `RequestedURLStamp`, + # then `customizer` (a provider's `http_client_customizer`, called with the `Faraday::Connection`), + # then `RequestedOriginGuard` last so it sees what the customizer's middleware does to each request + # after the stamp recorded it. Every request on the connection passes through both, so a caller using + # it directly is held to the same origin rule. + # + # Deliberately built without redirect-following middleware. Every destination check in this class runs + # against the URL as written, before the request goes out, so a connection that transparently followed + # a `3xx` would let a server reach a host the checks just refused. The guard turns following at + # the middleware level into a refusal; following inside an adapter stays invisible, so a customizer must + # not enable it. + # + # `Accept-Encoding` is deliberately left unset. `Net::HTTP::GenericRequest` negotiates it and decodes + # the response only while the caller has not claimed that header; assigning it turns `decode_content` off, + # which would silently move `BoundedBody`'s cap onto compressed bytes and let a small body expand past it + # after the check. + def build_http_client(customizer = nil) + require "faraday" + + Faraday.new do |faraday| + faraday.headers["Accept"] = "application/json" + faraday.use(RequestedURLStamp) + customizer&.call(faraday) + faraday.use(RequestedOriginGuard) + end + end end def initialize(provider:, http_client_factory: nil) @@ -1255,22 +1347,18 @@ module MCP @http_client ||= @http_client_factory.call end - # Deliberately built without redirect-following middleware. Every destination check in - # this class runs against the URL as written, before the request goes out, so a connection - # that transparently followed a `3xx` would let a server reach a host the checks just refused. - # A caller passing `http_client_factory:` takes on that responsibility: add redirect following here - # and the guards above only cover the first hop. - # - # `Accept-Encoding` is deliberately left unset. `Net::HTTP::GenericRequest` negotiates it and decodes - # the response only while the caller has not claimed that header; assigning it turns `decode_content` off, - # which would silently move `BoundedBody`'s cap onto compressed bytes and let a small body expand past it - # after the check. + # A connection supplied through `http_client_factory:` replaces this one, the provider's customizer and + # `RequestedOriginGuard` included, so that caller takes on the redirect responsibility described on + # `build_http_client`; `bounded_request` caps its responses all the same. def default_http_client - require "faraday" + self.class.build_http_client(provider_http_client_customizer) + end - Faraday.new do |faraday| - faraday.headers["Accept"] = "application/json" - end + # `nil` for a provider that predates the hook or leaves it unset. + def provider_http_client_customizer + return unless @provider.respond_to?(:http_client_customizer) + + @provider.http_client_customizer end def response_body_string(response) diff --git a/lib/mcp/client/oauth/id_jag_token_exchange.rb b/lib/mcp/client/oauth/id_jag_token_exchange.rb index 57701ba..ea771a9 100644 --- a/lib/mcp/client/oauth/id_jag_token_exchange.rb +++ b/lib/mcp/client/oauth/id_jag_token_exchange.rb @@ -95,7 +95,7 @@ module MCP assertion end - # `Accept-Encoding` is deliberately left unset, for the same reason as `Flow#default_http_client`: + # `Accept-Encoding` is deliberately left unset, for the same reason as `Flow.build_http_client`: # claiming that header turns Net::HTTP's `decode_content` off and would move `BoundedBody`'s cap # onto compressed bytes. def default_http_client diff --git a/lib/mcp/client/oauth/provider.rb b/lib/mcp/client/oauth/provider.rb index 16f8c4e..803b5b9 100644 --- a/lib/mcp/client/oauth/provider.rb +++ b/lib/mcp/client/oauth/provider.rb @@ -53,6 +53,8 @@ module MCP # the grant itself. The authorization request is not affected. A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS` # raises `Flow::InvalidTokenRequestParamsError`. # The Hash is copied and frozen. See `StorageBackedProvider#token_request_params`. + # - `http_client_customizer` - Callable invoked with the `Faraday::Connection` the flow builds for + # its own requests; see `StorageBackedProvider#http_client_customizer`. class Provider include StorageBackedProvider @@ -93,7 +95,8 @@ module MCP storage: nil, client_id_metadata_document_url: nil, authorization_request_validator: nil, - token_request_params: nil + token_request_params: nil, + http_client_customizer: nil ) unless Discovery.secure_url?(redirect_uri) raise InsecureRedirectURIError, @@ -115,6 +118,8 @@ module MCP "per the MCP authorization specification and `draft-ietf-oauth-client-id-metadata-document`." end + http_client_customizer = validated_http_client_customizer(http_client_customizer) + @client_metadata = client_metadata @redirect_uri = redirect_uri @redirect_handler = redirect_handler @@ -124,6 +129,7 @@ module MCP @client_id_metadata_document_url = client_id_metadata_document_url @authorization_request_validator = authorization_request_validator @token_request_params = frozen_token_request_params(token_request_params) + @http_client_customizer = http_client_customizer end # Identifies the OAuth flow this provider drives. diff --git a/lib/mcp/client/oauth/storage_backed_provider.rb b/lib/mcp/client/oauth/storage_backed_provider.rb index 2be8acc..087f223 100644 --- a/lib/mcp/client/oauth/storage_backed_provider.rb +++ b/lib/mcp/client/oauth/storage_backed_provider.rb @@ -28,6 +28,13 @@ module MCP # with `Flow::InvalidTokenRequestParamsError`. attr_reader :token_request_params + # Optional callable invoked with the `Faraday::Connection` the flow builds for its own requests, + # after the SDK's defaults and before its origin guard (see `Flow.build_http_client`), so an application can + # add middleware to discovery, registration, and token requests or swap their adapter. `nil` (the default) + # keeps the default connection. The transport's own connection and customizer block never serve these requests: + # they are bound to the MCP server URL and carry headers meant for that server. + attr_reader :http_client_customizer + def access_token tokens&.dig("access_token") || tokens&.dig(:access_token) end @@ -66,6 +73,14 @@ module MCP params.each_with_object({}) { |(key, value), copy| copy[key.dup.freeze] = value.dup.freeze }.freeze end + + # Returns `customizer` when it is `nil` or callable and raises otherwise, so a connection object passed + # in place of a callable fails at construction rather than at the first `401`. + def validated_http_client_customizer(customizer) + return customizer if customizer.nil? || customizer.respond_to?(:call) + + raise ArgumentError, "http_client_customizer must respond to call (got #{customizer.class})." + end end end end diff --git a/test/mcp/client/oauth/client_credentials_provider_test.rb b/test/mcp/client/oauth/client_credentials_provider_test.rb index 5ff4d20..6c679e3 100644 --- a/test/mcp/client/oauth/client_credentials_provider_test.rb +++ b/test/mcp/client/oauth/client_credentials_provider_test.rb @@ -49,6 +49,25 @@ module MCP assert_equal("client_secret_post", provider.client_information["token_endpoint_auth_method"]) end + def test_initialize_keeps_the_http_client_customizer + customizer = ->(_faraday) {} + provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", http_client_customizer: customizer) + + assert_same(customizer, provider.http_client_customizer) + assert_nil(ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret").http_client_customizer) + end + + def test_initialize_rejects_a_non_callable_http_client_customizer_before_writing_credentials + storage = InMemoryStorage.new + + error = assert_raises(ArgumentError) do + ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", storage: storage, http_client_customizer: "recorder") + end + + assert_equal("http_client_customizer must respond to call (got String).", error.message) + assert_nil(storage.client_information) + end + def test_initialize_rejects_missing_client_id ["", " ", nil].each do |value| assert_raises(ClientCredentialsProvider::InvalidCredentialsError, "should reject #{value.inspect}") do diff --git a/test/mcp/client/oauth/cross_app_access_provider_test.rb b/test/mcp/client/oauth/cross_app_access_provider_test.rb index 1283b62..ed49b47 100644 --- a/test/mcp/client/oauth/cross_app_access_provider_test.rb +++ b/test/mcp/client/oauth/cross_app_access_provider_test.rb @@ -49,6 +49,36 @@ module MCP ) end + def test_initialize_keeps_the_http_client_customizer + customizer = ->(_faraday) {} + provider = CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag" }, + http_client_customizer: customizer, + ) + + assert_same(customizer, provider.http_client_customizer) + assert_nil(build_provider.http_client_customizer) + end + + def test_initialize_rejects_a_non_callable_http_client_customizer_before_writing_credentials + storage = InMemoryStorage.new + + error = assert_raises(ArgumentError) do + CrossAppAccessProvider.new( + client_id: "xaa-client", + client_secret: "xaa-secret", + assertion_provider: ->(**) { "id-jag" }, + storage: storage, + http_client_customizer: "recorder", + ) + end + + assert_equal("http_client_customizer must respond to call (got String).", error.message) + assert_nil(storage.client_information) + end + def test_initialize_rejects_missing_client_id assert_raises(CrossAppAccessProvider::InvalidConfigurationError) do CrossAppAccessProvider.new( diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index c463fdc..1d2a670 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -85,12 +85,13 @@ module MCP ) end - def client_credentials_provider(token_endpoint_auth_method: "client_secret_basic", token_request_params: nil) + def client_credentials_provider(token_endpoint_auth_method: "client_secret_basic", token_request_params: nil, http_client_customizer: nil) ClientCredentialsProvider.new( client_id: "cc-client", client_secret: "cc-secret", token_endpoint_auth_method: token_endpoint_auth_method, token_request_params: token_request_params, + http_client_customizer: http_client_customizer, ) end @@ -141,6 +142,118 @@ module MCP end end + # Records `[method, url]` for every request that passes through, standing in for the tracing middleware + # an application adds through `http_client_customizer`. + class RecordingMiddleware + def initialize(app, log) + @app = app + @log = log + end + + def call(env) + @log << [env.method, env.url.to_s] + @app.call(env) + end + end + + # Sends the request for `from` to `to` instead, the shape redirect-following middleware leaves behind: + # the flow asked for one URL and the adapter is handed another. + class URLRewritingMiddleware + def initialize(app, from:, to:) + @app = app + @from = from + @to = to + end + + def call(env) + env.url = URI(@to) if env.url.to_s == @from + @app.call(env) + end + end + + # Replaces the per-request context the way instrumentation middleware may; the guard must not depend on it. + class ContextReplacingMiddleware + def initialize(app) + @app = app + end + + def call(env) + env.request.context = { tag: "instrumented" } + @app.call(env) + end + end + + # Hands the next middleware an environment rebuilt from Faraday's own members only, dropping whatever + # an earlier middleware recorded on the original. + class EnvRebuildingMiddleware + def initialize(app) + @app = app + end + + def call(env) + @app.call(Faraday::Env.from(env.to_h)) + end + end + + # Follows a `3xx` the way `faraday-follow_redirects` does: duplicates the environment, points it at the `Location`, + # and re-enters the stack from its own position. + class RedirectFollowingMiddleware + def initialize(app) + @app = app + end + + def call(env) + response = @app.call(env) + location = response.headers["Location"] + return response unless location && (300..399).cover?(response.status) + + redirected = env.dup + redirected.url = URI(location) + redirected.response = nil + @app.call(redirected) + end + end + + # Replaces the request options wholesale, the way middleware that resets timeouts or contexts might. + class RequestOptionsReplacingMiddleware + def initialize(app) + @app = app + end + + def call(env) + env.request = Faraday::RequestOptions.new + @app.call(env) + end + end + + # Re-enters the stack once with the same environment after a `5xx`, the way retry middleware does. + class RetryingMiddleware + def initialize(app) + @app = app + end + + def call(env) + response = @app.call(env) + return response unless response.status >= 500 + + env.response = nil + @app.call(env) + end + end + + # Records the application's `trace_id` from the per-request context, as a tracing middleware would. + class ContextRecordingMiddleware + def initialize(app, log) + @app = app + @log = log + end + + def call(env) + @log << env.request.context&.dig(:trace_id) + @app.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 @@ -190,6 +303,278 @@ module MCP end end + def test_run_sends_every_authorization_server_request_through_the_provider_customizer + log = [] + provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.use(RecordingMiddleware, log) }) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_equal([[:get, @prm_url], [:get, @as_metadata_url], [:post, "#{@auth_base}/token"]], log) + + # The SDK's defaults are applied before the customizer runs. + assert_requested(:get, @prm_url, headers: { "Accept" => "application/json" }) + end + + def test_run_refuses_a_customized_connection_that_sends_a_request_off_the_origin + stub_request(:get, "https://other.example.com/prm.json").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + provider = client_credentials_provider( + http_client_customizer: ->(faraday) { faraday.use(URLRewritingMiddleware, from: @prm_url, to: "https://other.example.com/prm.json") }, + ) + + error = assert_raises(Flow::DestinationMismatchError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_equal(<<~MESSAGE, error.message) + Request to \"#{@prm_url}\" would be sent to \"https://other.example.com/prm.json\", on a different origin; \ + middleware that follows redirects or rewrites URLs is refused. + MESSAGE + + # The guard sits before the adapter, so the request never leaves. + assert_not_requested(:get, "https://other.example.com/prm.json") + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_run_keeps_a_same_origin_rewrite + moved_url = "https://srv.example.com/prm-moved.json" + stub_request(:get, moved_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + provider = client_credentials_provider( + http_client_customizer: ->(faraday) { faraday.use(URLRewritingMiddleware, from: @prm_url, to: moved_url) }, + ) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_requested(:get, moved_url) + end + + def test_run_refuses_a_rewritten_request_even_when_middleware_replaces_the_request_context + stub_request(:get, "https://other.example.com/prm.json").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + provider = client_credentials_provider( + http_client_customizer: ->(faraday) { + faraday.use(ContextReplacingMiddleware) + faraday.use(URLRewritingMiddleware, from: @prm_url, to: "https://other.example.com/prm.json") + }, + ) + + assert_raises(Flow::DestinationMismatchError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_not_requested(:get, "https://other.example.com/prm.json") + end + + def test_run_refuses_a_redirect_off_the_origin_followed_by_middleware_placed_ahead_of_the_stamp + # `builder.insert(0, ...)` is where middleware that wants to be outermost puts itself, so a follower can + # land ahead of the SDK's stamp and re-enter the stack; the record of the first URL must survive that. + stub_request(:get, @prm_url).to_return(status: 302, headers: { "Location" => "https://other.example.com/prm.json" }) + stub_request(:get, "https://other.example.com/prm.json").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + provider = client_credentials_provider( + http_client_customizer: ->(faraday) { faraday.builder.insert(0, RedirectFollowingMiddleware) }, + ) + + error = assert_raises(Flow::DestinationMismatchError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/would be sent to "https:\/\/other\.example\.com\/prm\.json", on a different origin/, error.message) + assert_not_requested(:get, "https://other.example.com/prm.json") + end + + def test_run_keeps_a_same_origin_redirect_followed_by_middleware_placed_ahead_of_the_stamp + moved_url = "https://srv.example.com/prm-moved.json" + stub_request(:get, @prm_url).to_return(status: 302, headers: { "Location" => moved_url }) + stub_request(:get, moved_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + provider = client_credentials_provider( + http_client_customizer: ->(faraday) { faraday.builder.insert(0, RedirectFollowingMiddleware) }, + ) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_requested(:get, moved_url) + end + + def test_run_refuses_a_request_whose_environment_was_rebuilt + provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.use(EnvRebuildingMiddleware) }) + + error = assert_raises(Flow::DestinationMismatchError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_equal(<<~MESSAGE, error.message) + Request to \"#{@prm_url}\" carries no record of the URL the flow asked for; \ + middleware that rebuilds the request environment is refused. + MESSAGE + assert_not_requested(:get, @prm_url) + end + + def test_run_refuses_a_rewritten_token_request + stub_request(:post, "https://other.example.com/token").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(access_token: "stolen", token_type: "Bearer"), + ) + provider = client_credentials_provider( + http_client_customizer: ->(faraday) { + faraday.use(URLRewritingMiddleware, from: "#{@auth_base}/token", to: "https://other.example.com/token") + }, + ) + + assert_raises(Flow::DestinationMismatchError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_not_requested(:post, "https://other.example.com/token") + assert_not_requested(:post, "#{@auth_base}/token") + assert_nil(provider.access_token) + end + + def test_run_keeps_the_application_request_context_for_every_request + trace_ids = [] + + result = run_authorization_flow( + http_client_customizer: ->(faraday) { + faraday.options.context = { trace_id: "incident-123" } + faraday.use(ContextRecordingMiddleware, trace_ids) + }, + ) + + assert_equal(:authorized, result) + # Protected Resource Metadata and authorization server metadata (GET), registration (JSON POST), + # and the token exchange (form POST) all carry it. + assert_equal(["incident-123"] * 4, trace_ids) + end + + def test_run_keeps_the_application_request_context_on_a_factory_connection + trace_ids = [] + factory = -> { + Faraday.new do |faraday| + faraday.options.context = { trace_id: "incident-123" } + faraday.use(ContextRecordingMiddleware, trace_ids) + end + } + + result = Flow.new(provider: client_credentials_provider, http_client_factory: factory).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_equal(["incident-123"] * 3, trace_ids) + end + + def test_run_refuses_a_rewritten_request_even_when_middleware_replaces_the_request_options + stub_request(:get, "https://other.example.com/prm.json").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + provider = client_credentials_provider( + http_client_customizer: ->(faraday) { + faraday.use(RequestOptionsReplacingMiddleware) + faraday.use(URLRewritingMiddleware, from: @prm_url, to: "https://other.example.com/prm.json") + }, + ) + + error = assert_raises(Flow::DestinationMismatchError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + # The mismatch, not a lost record: replacing the options must leave the record in place. + assert_match(/would be sent to "https:\/\/other\.example\.com\/prm\.json", on a different origin/, error.message) + assert_not_requested(:get, "https://other.example.com/prm.json") + end + + def test_run_refuses_a_redirect_off_the_origin_followed_by_middleware_placed_after_the_stamp + stub_request(:get, @prm_url).to_return(status: 302, headers: { "Location" => "https://other.example.com/prm.json" }) + stub_request(:get, "https://other.example.com/prm.json").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.use(RedirectFollowingMiddleware) }) + + error = assert_raises(Flow::DestinationMismatchError) do + Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + # The mismatch, not a lost record: the duplicate the follower re-enters with shares the record. + assert_match(/would be sent to "https:\/\/other\.example\.com\/prm\.json", on a different origin/, error.message) + assert_not_requested(:get, "https://other.example.com/prm.json") + end + + def test_run_keeps_a_same_origin_redirect_followed_by_middleware_placed_after_the_stamp + moved_url = "https://srv.example.com/prm-moved.json" + stub_request(:get, @prm_url).to_return(status: 302, headers: { "Location" => moved_url }) + stub_request(:get, moved_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.use(RedirectFollowingMiddleware) }) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_requested(:get, moved_url) + end + + def test_run_keeps_a_retry_by_middleware_placed_ahead_of_the_stamp + # The same environment re-enters the stack with the same URL, so the first record still matches. + stub_request(:get, @prm_url).to_return( + { status: 503 }, + { + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + }, + ) + provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.builder.insert(0, RetryingMiddleware) }) + + result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + assert_requested(:get, @prm_url, times: 2) + end + + def test_build_http_client_records_each_request_on_its_own + # A caller using the SDK-built connection directly may address different origins from one request + # to the next; each request gets its own record. + stub_request(:get, "https://a.example.com/one").to_return(status: 200, body: "one") + stub_request(:get, "https://b.example.com/two").to_return(status: 200, body: "two") + connection = Flow.build_http_client + + assert_equal("one", connection.get("https://a.example.com/one").body) + assert_equal("two", connection.get("https://b.example.com/two").body) + end + + def test_run_prefers_an_explicit_http_client_factory_over_the_customizer + provider = client_credentials_provider(http_client_customizer: ->(_faraday) { raise "the customizer must not run" }) + + result = Flow.new(provider: provider, http_client_factory: -> { Faraday.new }).run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:authorized, result) + end + def test_run_client_credentials_with_client_secret_post_sends_credentials_in_body # `client_secret_post` puts the credentials in the form body rather # than an HTTP Basic header (RFC 6749 Section 2.3.1). @@ -586,7 +971,7 @@ module MCP # 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: {}) + def run_authorization_flow(redirect_uri: "http://localhost:0/callback", client_metadata_extra: {}, http_client_customizer: nil) state_holder = {} provider = Provider.new( client_metadata: { @@ -598,6 +983,7 @@ module MCP 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]] }, + http_client_customizer: http_client_customizer, ) 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 5b1be13..1edab1e 100644 --- a/test/mcp/client/oauth/http_oauth_test.rb +++ b/test/mcp/client/oauth/http_oauth_test.rb @@ -11,6 +11,46 @@ module MCP class Client module OAuth class HTTPOAuthTest < Minitest::Test + # Records `[method, url]` for every request that passes through, standing in for the tracing + # middleware an application adds through `http_client_customizer`. + class RecordingMiddleware + def initialize(app, log) + @app = app + @log = log + end + + def call(env) + @log << [env.method, env.url.to_s] + @app.call(env) + end + end + + # Sends the request for `from` to `to` instead, the shape redirect-following middleware leaves behind. + class URLRewritingMiddleware + def initialize(app, from:, to:) + @app = app + @from = from + @to = to + end + + def call(env) + env.url = URI(@to) if env.url.to_s == @from + @app.call(env) + end + end + + # Hands the next middleware an environment rebuilt from Faraday's own members only, dropping whatever + # an earlier middleware recorded on the original. + class EnvRebuildingMiddleware + def initialize(app) + @app = app + end + + def call(env) + @app.call(Faraday::Env.from(env.to_h)) + end + end + def setup WebMock.enable! @mcp_url = "https://srv.example.com/mcp" @@ -101,6 +141,91 @@ module MCP assert_equal("test-token-after-flow", provider.access_token) end + def test_send_request_runs_the_oauth_flow_through_the_provider_customizer + stub_request(:post, @mcp_url).with { |req| + req.headers["Authorization"].nil? + }.to_return( + status: 401, + headers: { + "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}"), + }, + body: "", + ) + + stub_request(:post, @mcp_url).with( + headers: { "Authorization" => "Bearer test-token-after-flow" } + ).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(jsonrpc: "2.0", id: "1", result: { ok: true }), + ) + + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + + stub_request(:get, "#{@auth_base}/.well-known/oauth-authorization-server").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"], + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + ), + ) + + stub_request(:post, "#{@auth_base}/register").to_return( + status: 201, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(client_id: "test-client"), + ) + + stub_request(:post, "#{@auth_base}/token").to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(access_token: "test-token-after-flow", token_type: "Bearer", expires_in: 3600), + ) + + log = [] + state_holder = {} + provider = Provider.new( + client_metadata: { + client_name: "ruby-sdk-test", + 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]] }, + http_client_customizer: ->(faraday) { faraday.use(RecordingMiddleware, log) }, + ) + + transport = HTTP.new(url: @mcp_url, oauth: provider) + response = transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" }) + + assert_equal({ "ok" => true }, response["result"]) + assert_equal( + [ + [:get, @prm_url], + [:get, "#{@auth_base}/.well-known/oauth-authorization-server"], + [:post, "#{@auth_base}/register"], + [:post, "#{@auth_base}/token"], + ], + log, + ) + end + def test_send_request_does_not_follow_a_resource_metadata_challenge_off_the_server_origin # End to end over the transport, which is where the header is actually parsed: # a server that answers 401 must not be able to name an unrelated host in @@ -678,6 +803,94 @@ module MCP assert_equal("saved-rt", provider.tokens["refresh_token"]) end + def test_send_request_surfaces_a_refused_refresh_without_falling_back + # A customizer middleware that would carry the refresh request off the origin is refused before + # the request leaves, and that refusal is not one of the failures that start the interactive flow + # or discard the stored tokens. + stub_request(:post, @mcp_url).to_return( + status: 401, + headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") }, + body: "", + ) + + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]), + ) + + stub_request(:get, "#{@auth_base}/.well-known/oauth-authorization-server").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"], + ), + ) + + stub_request(:post, "#{@auth_base}/register").to_raise(StandardError.new("DCR should not be called.")) + stub_request(:post, "https://other.example.com/token").to_raise(StandardError.new("the rewritten request must not leave.")) + + provider = build_provider( + http_client_customizer: ->(faraday) { + faraday.use(URLRewritingMiddleware, from: "#{@auth_base}/token", to: "https://other.example.com/token") + }, + ) + provider.save_client_information("client_id" => "test-client") + provider.save_tokens("access_token" => "stale-token", "refresh_token" => "saved-rt") + + transport = HTTP.new(url: @mcp_url, oauth: provider) + + assert_raises(MCP::Client::OAuth::Flow::DestinationMismatchError) do + transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" }) + end + + assert_not_requested(:post, "https://other.example.com/token") + assert_not_requested(:post, "#{@auth_base}/token") + assert_not_requested(:post, "#{@auth_base}/register") + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + + def test_send_request_surfaces_a_refresh_whose_environment_was_rebuilt_without_falling_back + # The missing-record refusal takes the same route as the origin mismatch: out of `send_request`, + # with no interactive fallback and the stored tokens intact. + stub_request(:post, @mcp_url).to_return( + status: 401, + headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") }, + body: "", + ) + stub_request(:get, @prm_url).to_raise(StandardError.new("the rebuilt request must not leave.")) + stub_request(:post, "#{@auth_base}/register").to_raise(StandardError.new("DCR should not be called.")) + + interactive_flow_started = false + provider = Provider.new( + client_metadata: { redirect_uris: ["http://localhost:0/callback"] }, + redirect_uri: "http://localhost:0/callback", + redirect_handler: ->(_url) { interactive_flow_started = true }, + callback_handler: -> { ["code", "state"] }, + http_client_customizer: ->(faraday) { faraday.use(EnvRebuildingMiddleware) }, + ) + provider.save_client_information("client_id" => "test-client") + provider.save_tokens("access_token" => "stale-token", "refresh_token" => "saved-rt") + + transport = HTTP.new(url: @mcp_url, oauth: provider) + + error = assert_raises(MCP::Client::OAuth::Flow::DestinationMismatchError) do + transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" }) + end + + assert_match(/carries no record of the URL the flow asked for/, error.message) + refute(interactive_flow_started, "the refusal must not fall back to the interactive flow") + assert_not_requested(:get, @prm_url) + assert_not_requested(:post, "#{@auth_base}/token") + assert_not_requested(:post, "#{@auth_base}/register") + assert_equal({ "access_token" => "stale-token", "refresh_token" => "saved-rt" }, provider.tokens) + end + def test_send_request_preserves_refresh_token_when_refresh_hits_a_transient_failure # A 5xx (or any non-`invalid_grant`) from the token endpoint indicates # a transient AS outage, NOT that the refresh token is dead. @@ -1656,12 +1869,13 @@ module MCP provider end - def build_provider + def build_provider(http_client_customizer: nil) Provider.new( client_metadata: { redirect_uris: ["http://localhost:0/callback"] }, redirect_uri: "http://localhost:0/callback", redirect_handler: ->(_url) {}, callback_handler: -> { ["code", "state"] }, + http_client_customizer: http_client_customizer, ) end end diff --git a/test/mcp/client/oauth/provider_test.rb b/test/mcp/client/oauth/provider_test.rb index d9fa64e..7425131 100644 --- a/test/mcp/client/oauth/provider_test.rb +++ b/test/mcp/client/oauth/provider_test.rb @@ -38,6 +38,26 @@ module MCP end end + def test_initialize_defaults_http_client_customizer_to_nil + assert_nil(Provider.new(**args_for("https://app.example.com/callback")).http_client_customizer) + end + + def test_initialize_keeps_the_http_client_customizer + customizer = ->(_faraday) {} + provider = Provider.new(**args_for("https://app.example.com/callback"), http_client_customizer: customizer) + + assert_same(customizer, provider.http_client_customizer) + end + + def test_initialize_rejects_a_non_callable_http_client_customizer + # A connection object in place of a callable is the likely mistake. + error = assert_raises(ArgumentError) do + Provider.new(**args_for("https://app.example.com/callback"), http_client_customizer: Object.new) + end + + assert_equal("http_client_customizer must respond to call (got Object).", error.message) + 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, From 5081cc562618c518ddf587f2dfa72447175c075b Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 18 Sep 2026 03:49:09 +0900 Subject: [PATCH 23/24] Surface unreachable Protected Resource Metadata instead of falling back to legacy discovery ## Motivation and Context `Flow#locate_authorization_server` treated every failure to fetch Protected Resource Metadata as "the server publishes none" and took the legacy 2025-03-26 path, where the MCP server's origin becomes the authorization base URL. A request that never reached the server, or a `5xx` or `429`, says nothing about what the server publishes, so a transient failure moved the flow to a different authorization server, discarded client information and tokens bound to the real one on the way, and surfaced as an unrelated error, such as a failed registration at the origin, rather than as the fetch failure itself. The comment claimed both reference SDKs behave that way, which is not so: outside browsers the TypeScript SDK propagates network errors from discovery, and the Python SDK propagates them and refuses the legacy path after a `5xx` or `429` from any candidate. Discovery now keeps trying candidate URLs until one serves a JSON object and classifies the outcome once they are exhausted. Like the Python SDK, it remembers a `5xx` or `429` from any candidate before deciding on the legacy path; moving on to the next candidate after a network error is what the loop already did. `MetadataNotPublishedError` (a `4xx` other than `429`, or a body that is not a JSON object) is the only failure that selects the legacy path; `MetadataUnreachableError` (a network error, or a `5xx` or `429`, from any candidate) is raised instead. Both subclass `AuthorizationError`, so callers rescuing that class keep working, and `MCP::Client::HTTP` still falls back from a failed refresh to the full flow, which repeats discovery and raises the same way unless the server has recovered. A body over the response cap is refused outright with a plain `AuthorizationError`, as it already was on the modern path. The message lists every candidate's failure, with the URL stripped of userinfo, query and fragment and cut to a fixed length, but otherwise spelled as requested, and with transport and parser messages bounded, as token endpoint diagnostics already are. The default legacy endpoints for an origin whose own metadata document is absent are unchanged. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb` and `test/mcp/client/oauth/discovery_test.rb`. Against the library before this change, discovery in the scenarios these tests set up took the legacy path instead of raising. ## Breaking Changes When no Protected Resource Metadata candidate serves a JSON object, a request that failed to reach the server, or returned a `5xx` or `429`, now raises instead of attempting the legacy 2025-03-26 discovery at the MCP server's origin, and a Protected Resource Metadata body over the response cap is refused instead of triggering that fallback. --- docs/_client/authorization.md | 2 + lib/mcp/client/oauth/discovery.rb | 23 +++ lib/mcp/client/oauth/flow.rb | 111 ++++++++---- test/mcp/client/oauth/discovery_test.rb | 13 ++ test/mcp/client/oauth/flow_test.rb | 226 +++++++++++++++++++++++- 5 files changed, 332 insertions(+), 43 deletions(-) diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index ebd3014..11e9215 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -39,6 +39,8 @@ pass an `MCP::Client::OAuth::Provider` to the transport instead of a static `Aut - Fall back to the legacy 2025-03-26 discovery when the server publishes no Protected Resource Metadata, matching the TypeScript and Python SDKs: the MCP server's origin acts as the authorization base URL, its metadata is fetched from `/.well-known/oauth-authorization-server` and must name that origin as its `issuer` (RFC 8414 Section 3.3), and when even that is absent the spec's default endpoints `/authorize`, `/token`, and `/register` at the origin are used with PKCE S256 assumed. + When no Protected Resource Metadata candidate serves a JSON object, a request that could not reach the server, or that returned a `5xx` or `429`, + raises `Flow::MetadataUnreachableError` instead of triggering that fallback, and a body over the response cap is refused outright. - On subsequent 401s with a saved `refresh_token`, exchange it at the token endpoint before falling back to the full interactive flow (RFC 6749 Section 6). - 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. diff --git a/lib/mcp/client/oauth/discovery.rb b/lib/mcp/client/oauth/discovery.rb index 04030cc..a8ad3be 100644 --- a/lib/mcp/client/oauth/discovery.rb +++ b/lib/mcp/client/oauth/discovery.rb @@ -336,6 +336,29 @@ module MCP uri.to_s end + # Drops userinfo, query and fragment from `url` and leaves the host and path spelled as given, for reporting + # a URL next to the request that failed, where it must still match the server's access log; `URI#to_s` + # still lowercases the scheme and drops an explicit default port. Unlike `canonicalize_origin_and_path` + # it neither normalizes nor resolves dot segments, so its cost stays linear in the URL length, which matters + # for a URL the server chose. A URL that does not parse is not echoed, since the raw value could carry + # the very credentials being dropped. + def redact_url(url) + uri = URI.parse(url.to_s) + + uri.fragment = nil + uri.query = nil + # `URI::Generic#userinfo=` is a no-op on Ruby 2.7 (the project's minimum supported version), + # so clear the components individually. + if uri.respond_to?(:user) && (uri.user || uri.password) + uri.user = nil + uri.password = nil + end + + uri.to_s + rescue URI::Error + "[unparseable URL]" + end + # Returns true when `prm` (a PRM `resource` URL) covers `server` (the MCP endpoint URL): # same scheme/host/port, with PRM's path being a prefix of the server's path. When PRM # also advertises a query string, the server's query MUST be identical to it (otherwise diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 0d38f2d..37c0cce 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -16,6 +16,8 @@ module MCP class Flow TOKEN_ENDPOINT_ERROR_MAX_LENGTH = 128 TOKEN_ENDPOINT_ERROR_DESCRIPTION_MAX_LENGTH = 512 + METADATA_DIAGNOSTIC_MAX_LENGTH = 128 + METADATA_URL_MAX_LENGTH = 2048 # 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. @@ -57,6 +59,17 @@ module MCP # or authorization server metadata failure by rescuing a class rather than by matching the message text. class AuthorizationRefusedError < AuthorizationError; end + # Raised by metadata discovery when every candidate URL answered that nothing usable is published there + # (a `4xx` other than `429`, a redirect that was not followed, or a body that is not a JSON object). + # The only discovery failure that may select the legacy 2025-03-26 path. + class MetadataNotPublishedError < AuthorizationError; end + + # Raised by metadata discovery when the answer says nothing about what is published: the request failed to + # reach the server, or a candidate answered `5xx` or `429`. Falling back on this would move + # the flow to a different authorization server because of a transient failure, so it is surfaced instead, + # as the TypeScript SDK does for network errors outside browsers and the Python SDK does for both. + class MetadataUnreachableError < AuthorizationError; end + # Raised for a `token_request_params` value the SDK refuses: a reserved key, a Hash comparing keys by identity, # or anything but a Hash of Strings. An `ArgumentError` because the value is a configuration mistake, # not a failed authorization, and deliberately outside `AuthorizationError`, which `MCP::Client::HTTP` treats on @@ -375,7 +388,12 @@ module MCP # # Legacy path (2025-03-26 backwards compatibility): when the server publishes no PRM, `prm` is nil # and the MCP server's own origin acts as the authorization base URL, matching the TypeScript and Python SDKs. - # Any PRM discovery failure (404s, network errors, malformed documents) selects the legacy path, mirroring both SDKs' behavior. + # Only a discovery answer saying that nothing usable is published (`MetadataNotPublishedError`: a `4xx` other than `429`, + # a redirect that was not followed, or a body that is not a JSON object) selects the legacy path. + # A request that failed to reach the server, or returned a `5xx` or `429`, says nothing about what the server publishes, + # so once no candidate has served a usable document it is surfaced instead (`MetadataUnreachableError`), + # as both SDKs do for network errors (the TypeScript SDK outside browsers) and the Python SDK does for server errors. + # A body over the response cap is refused outright and never reaches the fallback either. # https://modelcontextprotocol.io/specification/2025-03-26/basic/authorization#fallbacks-for-servers-without-metadata-discovery def locate_authorization_server(server_url:, resource_metadata_url:) prm = begin @@ -383,7 +401,7 @@ module MCP server_url: server_url, resource_metadata_url: resource_metadata_url, ) - rescue AuthorizationError + rescue MetadataNotPublishedError nil end @@ -504,44 +522,63 @@ module MCP first end - # Walks candidate metadata URLs and returns the parsed JSON body of - # the first 2xx response. Raises `AuthorizationError` for transport - # failures (`Faraday::Error`) and malformed bodies (`JSON::ParserError`) - # so callers do not have to handle raw Faraday/JSON exceptions. + # Walks candidate metadata URLs and returns the parsed body of the first 2xx response that is a JSON object; + # the caller checks its fields. Candidates are tried until one serves such a body, since a later one may still + # be usable when an earlier one is broken or down (the URL from `WWW-Authenticate` against the well-known path, + # or the OAuth document against the OpenID one). Once the candidates are exhausted, an answer that said nothing + # about what is published (a network error, a `5xx`, or a `429`) outranks the rest and raises + # `MetadataUnreachableError`; otherwise (any other status, such as a `4xx` other than `429` or a redirect that + # was not followed, a body that is not JSON, or not a JSON object) `MetadataNotPublishedError`. + # A body over the cap is refused outright by `bounded_request` with a plain `AuthorizationError`, + # before any classification. Each failure is listed with its URL stripped of userinfo, query and fragment + # and cut to `METADATA_URL_MAX_LENGTH`, but otherwise spelled as requested, so it can be matched against + # a server's access log, and with exception text bounded, since the message lands in every log destination + # the error passes through. def fetch_metadata_json(urls, label:) - last_error = nil + failures = [] + inconclusive = false urls.each do |url| response = begin http_get(url) rescue Faraday::Error => e - last_error = "GET #{url} raised #{e.class}: #{e.message}" + detail = bounded_diagnostic(e.message, limit: METADATA_DIAGNOSTIC_MAX_LENGTH) + failures << "GET #{reported_url(url)} raised #{[e.class, detail].compact.join(": ")}" + inconclusive = true next end - if response.status >= 200 && response.status < 300 - parsed = begin - JSON.parse(response_body_string(response)) - rescue JSON::ParserError => e - raise AuthorizationError, "Failed to parse #{label} from #{url}: #{e.message}." - end - - # Even valid JSON can be the wrong shape (a top-level array, - # a bare `null`, a string, ...). The discovery callers index by - # name (`prm["authorization_servers"]`, etc.), so anything that - # is not a Hash would raise `TypeError` / `NoMethodError` - # downstream. Surface that as `AuthorizationError` instead so - # callers see a single, documented error type. - unless parsed.is_a?(Hash) - raise AuthorizationError, - "#{label} from #{url} is not a JSON object (got #{parsed.class})." - end - - return parsed + unless response.status >= 200 && response.status < 300 + failures << "GET #{reported_url(url)} returned #{response.status}" + inconclusive = true if response.status >= 500 || response.status == 429 + next end - last_error = "GET #{url} returned #{response.status}" + parsed = begin + JSON.parse(response_body_string(response)) + rescue JSON::ParserError => e + detail = bounded_diagnostic(e.message, limit: METADATA_DIAGNOSTIC_MAX_LENGTH) || e.class.name + failures << "GET #{reported_url(url)} returned a body that is not JSON: #{detail}" + next + end + + # Even valid JSON can be the wrong shape (a top-level array, a bare `null`, a string, ...). + # The discovery callers index by name (`prm["authorization_servers"]`, etc.), so anything that + # is not a Hash would raise `TypeError` / `NoMethodError` downstream. + unless parsed.is_a?(Hash) + failures << "GET #{reported_url(url)} returned a body that is not a JSON object (got #{parsed.class})" + next + end + + return parsed + end + + message = "Failed to fetch #{label}: #{failures.join("; ")}." + + if inconclusive + raise MetadataUnreachableError, message + else + raise MetadataNotPublishedError, message end - raise AuthorizationError, "Failed to fetch #{label}: #{last_error}." end def ensure_pkce_supported!(as_metadata) @@ -1188,8 +1225,8 @@ module MCP parsed = {} unless parsed.is_a?(Hash) error_class = parsed["error"] == "invalid_grant" ? InvalidGrantError : AuthorizationError - error = token_endpoint_diagnostic(parsed["error"], limit: TOKEN_ENDPOINT_ERROR_MAX_LENGTH) - description = token_endpoint_diagnostic(parsed["error_description"], limit: TOKEN_ENDPOINT_ERROR_DESCRIPTION_MAX_LENGTH) + error = bounded_diagnostic(parsed["error"], limit: TOKEN_ENDPOINT_ERROR_MAX_LENGTH) + description = bounded_diagnostic(parsed["error_description"], limit: TOKEN_ENDPOINT_ERROR_DESCRIPTION_MAX_LENGTH) message += " #{[error, description].compact.join(": ")}" if error || description error_class.new(message, http_status: response.status, error: error, error_description: description) @@ -1198,17 +1235,23 @@ module MCP error_class.new("Token endpoint returned status #{response.status}.", http_status: response.status) end - def token_endpoint_diagnostic(value, limit:) + def bounded_diagnostic(value, limit:) return unless value.is_a?(String) - # RFC 6749 permits printable ASCII except double quotes and backslashes. - # Replace other characters to keep provider text on one log line. + # RFC 6749 permits printable ASCII except double quotes and backslashes in token endpoint error fields. + # Replace other characters to keep text received off the network on one log line. value = value.scrub(" ").gsub(/[^\x20-\x21\x23-\x5B\x5D-\x7E]/, " ").strip return if value.empty? value.length > limit ? "#{value[0, limit - 3]}..." : value end + # A candidate URL as it goes into a failure string: redacted, and cut so a URL the server chose cannot + # grow the message without limit. + def reported_url(url) + bounded_diagnostic(Discovery.redact_url(url), limit: METADATA_URL_MAX_LENGTH) + end + # Per RFC 6749 Section 2.3.1, the `client_id` and `client_secret` MUST be # `application/x-www-form-urlencoded` encoded before they are joined with # `:` and base64-encoded for the `Authorization: Basic` header. This is diff --git a/test/mcp/client/oauth/discovery_test.rb b/test/mcp/client/oauth/discovery_test.rb index c8d9fda..d58c7f2 100644 --- a/test/mcp/client/oauth/discovery_test.rb +++ b/test/mcp/client/oauth/discovery_test.rb @@ -259,6 +259,19 @@ module MCP ) end + def test_redact_url_drops_credentials_query_and_fragment_but_keeps_the_spelling + # Reported next to a failed request, the URL must still match the server's access log, so the host + # and path are left alone: no lowercasing of the host, no dot-segment resolution, no decoding. + assert_equal( + "https://Srv.Example.COM:8443/tenant/%2e%2e/prm%2Ejson", + Discovery.redact_url("https://user:pass@Srv.Example.COM:8443/tenant/%2e%2e/prm%2Ejson?token=abc#frag"), + ) + end + + def test_redact_url_does_not_echo_a_url_it_cannot_parse + assert_equal("[unparseable URL]", Discovery.redact_url("https://user:pass@srv.example.com/pr m.json?token=abc")) + end + def test_canonicalize_url_normalizes_query_to_match_faraday # Faraday rewrites `env.url` before sending a request: it sorts # parameters by name, uppercases percent-encoded hex, and drops diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index 26adb60..5a41b3b 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -1084,9 +1084,9 @@ module MCP end def test_run_falls_back_to_legacy_discovery_when_prm_is_not_a_json_object - # Valid JSON but the wrong shape. Any PRM discovery failure selects the legacy 2025-03-26 path - # (matching the TypeScript and Python SDKs); here the legacy path also dead-ends, surfacing - # a domain error rather than a raw `TypeError` from indexing the array. + # Valid JSON but the wrong shape counts as nothing usable being published, so it selects the legacy 2025-03-26 path + # (matching the TypeScript and Python SDKs); here the legacy path also dead-ends, surfacing a domain error rather than + # a raw `TypeError` from indexing the array. stub_request(:get, @prm_url).to_return( status: 200, headers: { "Content-Type" => "application/json" }, @@ -1118,6 +1118,211 @@ module MCP assert_requested(:get, "https://srv.example.com/.well-known/oauth-authorization-server") end + def test_run_surfaces_a_network_error_during_prm_discovery_instead_of_falling_back + # A request that never reached the server says nothing about whether it publishes PRM, so once + # the other candidates answer 404 the flow stops instead of moving to the legacy authorization base; + # the TypeScript and Python SDKs propagate the error the same way. + challenge_url = "https://srv.example.com/prm.json" + stub_request(:get, challenge_url).to_raise(Faraday::ConnectionFailed.new("connection refused")) + stub_prm_not_found + + error = assert_raises(Flow::MetadataUnreachableError) do + Flow.new(provider: build_legacy_discovery_provider({})).run!(server_url: @server_url, resource_metadata_url: challenge_url) + end + + assert_match(/Faraday::ConnectionFailed/, error.message) + assert_not_requested(:get, "https://srv.example.com/.well-known/oauth-authorization-server") + assert_not_requested(:post, "https://srv.example.com/register") + end + + def test_run_reports_a_prm_candidate_url_without_its_query_in_the_discovery_error + # The candidate URL comes from the server's `WWW-Authenticate` challenge and lands in every log line + # the error reaches, so the query is dropped, while the path keeps the spelling that was requested + # (`%2E` is not decoded) so the line can be matched against the server's access log. + challenge_url = "https://srv.example.com/prm%2Ejson?token=abc" + stub_request(:get, challenge_url).to_raise(Faraday::ConnectionFailed.new("connection refused")) + stub_prm_not_found + + error = assert_raises(Flow::MetadataUnreachableError) do + Flow.new(provider: build_legacy_discovery_provider({})).run!(server_url: @server_url, resource_metadata_url: challenge_url) + end + + assert_includes(error.message, "GET https://srv.example.com/prm%2Ejson raised Faraday::ConnectionFailed: connection refused; GET") + refute_includes(error.message, "token=abc") + end + + def test_run_cuts_an_overlong_prm_candidate_url_in_the_discovery_error + # The challenge URL is the server's to choose, so its length is bounded before it lands in the message. + challenge_url = "https://srv.example.com/#{"a" * 5000}/prm.json" + stub_request(:get, challenge_url).to_raise(Faraday::ConnectionFailed.new("connection refused")) + stub_prm_not_found + + error = assert_raises(Flow::MetadataUnreachableError) do + Flow.new(provider: build_legacy_discovery_provider({})).run!(server_url: @server_url, resource_metadata_url: challenge_url) + end + + assert_includes(error.message, "GET #{challenge_url[0, Flow::METADATA_URL_MAX_LENGTH - 3]}... raised Faraday::ConnectionFailed") + refute_includes(error.message, "a" * (Flow::METADATA_URL_MAX_LENGTH - 3)) + end + + def test_run_bounds_the_transport_error_message_in_the_discovery_error + # A caller-supplied client can raise with server-chosen text in the message (Faraday's `raise_error` middleware repeats + # the status line and the request URL), so it is bounded like the parser's message. + stub_request(:get, @prm_url).to_raise(Faraday::ConnectionFailed.new("x" * 5000)) + stub_request(:get, "https://srv.example.com/.well-known/oauth-protected-resource").to_return(status: 404) + + error = assert_raises(Flow::MetadataUnreachableError) do + Flow.new(provider: build_legacy_discovery_provider({})).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_includes(error.message, "raised Faraday::ConnectionFailed: #{"x" * 125}...; GET") + refute_includes(error.message, "x" * 126) + end + + def test_run_bounds_the_parser_message_in_the_discovery_error + # json before 2.10 repeats the remaining source in `JSON::ParserError#message`, so a malformed body + # could otherwise put up to the response cap into the message once per candidate. + stub_request(:get, @prm_url).to_return(status: 200, headers: { "Content-Type" => "application/json" }, body: "{") + stub_request(:get, "https://srv.example.com/.well-known/oauth-protected-resource").to_return(status: 503) + JSON.stubs(:parse).raises(JSON::ParserError, "unexpected token at '#{"{" * 5000}'") + + error = assert_raises(Flow::MetadataUnreachableError) do + Flow.new(provider: build_legacy_discovery_provider({})).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + # `METADATA_DIAGNOSTIC_MAX_LENGTH` is 128: the first 125 characters survive, then three dots. + assert_includes(error.message, <<~MESSAGE.chomp) + returned a body that is not JSON: unexpected token at '#{"{" * 104}...; \ + GET https://srv.example.com/.well-known/oauth-protected-resource returned 503. + MESSAGE + + refute_includes(error.message, "{" * 105) + end + + def test_run_surfaces_a_server_error_during_prm_discovery_instead_of_falling_back + # A `5xx` or `429` says nothing about what the server publishes either, even when a later candidate answers 404; + # the Python SDK refuses the legacy path the same way. + [503, 429].each do |status| + stub_request(:get, @prm_url).to_return(status: status) + stub_request(:get, "https://srv.example.com/.well-known/oauth-protected-resource").to_return(status: 404) + + error = assert_raises(Flow::MetadataUnreachableError, "status #{status}") do + Flow.new(provider: build_legacy_discovery_provider({})).run!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/returned #{status}/, error.message) + end + + assert_not_requested(:get, "https://srv.example.com/.well-known/oauth-authorization-server") + assert_not_requested(:post, "https://srv.example.com/register") + end + + def test_run_keeps_a_server_error_in_mind_when_a_later_candidate_is_unusable + # An unusable document from a later candidate must not turn an earlier `503` into "nothing published". + challenge_url = "https://srv.example.com/prm.json" + stub_request(:get, challenge_url).to_return(status: 503) + stub_request(:get, @prm_url).to_return(status: 200, headers: { "Content-Type" => "application/json" }, body: "[]") + stub_request(:get, "https://srv.example.com/.well-known/oauth-protected-resource").to_return(status: 404) + + error = assert_raises(Flow::MetadataUnreachableError) do + Flow.new(provider: build_legacy_discovery_provider({})).run!(server_url: @server_url, resource_metadata_url: challenge_url) + end + + assert_match(/returned 503/, error.message) + assert_not_requested(:get, "https://srv.example.com/.well-known/oauth-authorization-server") + end + + def test_run_uses_a_later_prm_candidate_when_the_challenge_url_serves_an_unusable_document + # The setup stubs a valid document at the well-known path; a broken one at the challenge URL is skipped. + challenge_url = "https://srv.example.com/prm.json" + stub_request(:get, challenge_url).to_return(status: 200, headers: { "Content-Type" => "application/json" }, body: "[]") + + result = Flow.new(provider: build_legacy_discovery_provider({})).run!(server_url: @server_url, resource_metadata_url: challenge_url) + + assert_equal(:authorized, result) + assert_requested(:get, @prm_url) + end + + def test_run_recovers_authorization_server_metadata_from_a_later_candidate_after_a_network_error + # The OAuth document being unreachable must not stop the OpenID one from being tried. + stub_request(:get, @as_metadata_url).to_raise(Faraday::TimeoutError.new("execution expired")) + stub_request(:get, "#{@auth_base}/.well-known/openid-configuration").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"], + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + ), + ) + + result = run_authorization_flow + + assert_equal(:authorized, result) + assert_requested(:get, "#{@auth_base}/.well-known/openid-configuration") + end + + def test_run_recovers_authorization_server_metadata_from_a_later_candidate_after_an_unusable_document + # An OAuth document that is not JSON, or not a JSON object, must not stop the OpenID one from being tried. + stub_request(:get, "#{@auth_base}/.well-known/openid-configuration").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"], + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + ), + ) + + ["{", "[]"].each do |body| + stub_request(:get, @as_metadata_url).to_return(status: 200, headers: { "Content-Type" => "application/json" }, body: body) + + assert_equal(:authorized, run_authorization_flow, "body #{body.inspect}") + end + + assert_requested(:get, "#{@auth_base}/.well-known/openid-configuration", times: 2) + end + + def test_refresh_surfaces_a_network_error_during_prm_discovery_instead_of_falling_back + stub_request(:get, @prm_url).to_raise(Faraday::TimeoutError.new("execution expired")) + stub_request(:get, "https://srv.example.com/.well-known/oauth-protected-resource").to_return(status: 404) + provider = build_legacy_discovery_provider({}) + provider.save_client_information("client_id" => "test-client") + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt") + + assert_raises(Flow::MetadataUnreachableError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_not_requested(:get, "https://srv.example.com/.well-known/oauth-authorization-server") + assert_not_requested(:post, "https://srv.example.com/token") + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + + def test_refresh_surfaces_a_server_error_during_prm_discovery_instead_of_falling_back + stub_request(:get, @prm_url).to_return(status: 503) + stub_request(:get, "https://srv.example.com/.well-known/oauth-protected-resource").to_return(status: 404) + provider = build_legacy_discovery_provider({}) + provider.save_client_information("client_id" => "test-client") + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt") + + error = assert_raises(Flow::MetadataUnreachableError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + + assert_match(/returned 503/, error.message) + assert_not_requested(:post, "https://srv.example.com/token") + assert_equal("saved-rt", provider.tokens["refresh_token"]) + end + # Builds a provider for the legacy-discovery tests, capturing the authorization URL so tests can assert # which endpoint was used. def build_legacy_discovery_provider(holder) @@ -1511,10 +1716,10 @@ module MCP end def test_run_refuses_a_protected_resource_metadata_body_over_the_cap - # PRM discovery failures select the legacy path by design, so the refusal shows up as - # the fallback rather than as a raise. The padded document is valid JSON naming - # an authorization server, so contacting that server is exactly what would happen - # if the body had been read: the assertion below fails if the cap stops working. + # An oversized document is refused outright rather than treated as unpublished, so the flow stops + # without the legacy fallback. The padded document is valid JSON naming an authorization server, + # so contacting that server is exactly what would happen if the body had been read: + # the assertion below fails if the cap stops working. stub_request(:any, %r{\Ahttps://srv\.example\.com/}).to_return(status: 404) stub_request(:get, @prm_url).to_return( status: 200, @@ -1526,9 +1731,11 @@ module MCP ), ) - assert_raises(Flow::AuthorizationError) { run_authorization_flow } + error = assert_raises(Flow::AuthorizationError) { run_authorization_flow } + assert_match(/exceeds \d+ bytes/, error.message) assert_not_requested(:get, @as_metadata_url) + assert_not_requested(:get, "https://srv.example.com/.well-known/oauth-authorization-server") end def test_run_refuses_a_dynamic_client_registration_body_over_the_cap @@ -1632,6 +1839,7 @@ module MCP headers: { "Content-Type" => "application/json" }, body: "null", ) + stub_request(:get, "#{@auth_base}/.well-known/openid-configuration").to_return(status: 404) provider = Provider.new( client_metadata: { @@ -3283,7 +3491,7 @@ module MCP end def test_token_endpoint_errors_fall_back_when_diagnostic_extraction_raises - Flow.any_instance.stubs(:token_endpoint_diagnostic).raises(ArgumentError, "sensitive provider text") + Flow.any_instance.stubs(:bounded_diagnostic).raises(ArgumentError, "sensitive provider text") { "invalid_grant" => Flow::InvalidGrantError, "invalid_client" => Flow::AuthorizationError }.each do |code, klass| error = refresh_token_endpoint_error(JSON.generate(error: code, error_description: "details")) From 8173a38280b0c7c31b2ef5bc3e7dfd1f4ce7fa8b Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 19 Sep 2026 17:08:22 +0900 Subject: [PATCH 24/24] Validate the src and sizes of an icon against the specification ## Motivation and Context `MCP::Icon` validated `theme` but accepted anything for `src` and `sizes`: `src:` defaulted to `nil`, so `MCP::Icon.new.to_h` was `{}`, an icon with no source, and a `sizes` given as a String reached `to_h` unchanged and serialized as a JSON string where the specification's `Icon` type requires an array of strings. Nothing on the server side reported either; the server answered every request with 200 and the failure surfaced in a client, with no reference to the field responsible. The TypeScript SDK types `sizes` as `string[]` and `src` as required, and the Python SDK's model refuses such an icon at construction, so this SDK was the one that let it through. `src` is now a required keyword and must be a non-empty String, `sizes` must be `nil` or an Array of Strings, and `mime_type` must be `nil` or a String, each refused with `ArgumentError` at construction the way `theme` already was. The checks stop at the schema's types, as the reference SDKs do: the scheme of a URL and the `WxH` form of a size stay the caller's to get right. A message names the class of the rejected value, never the value, since `src` may carry a `data:` URI of any length. Coercing a String into a one-element Array was considered and rejected: it would hide the caller's mistake that the class exists to surface. The `theme` check let `false` through its truthiness test and serialized it; it now refuses everything but `nil`, `"light"`, and `"dark"`. Fixes #562. ## How Has This Been Tested? New tests in `test/mcp/icon_test.rb`. Against the previous library, an icon without `src` and an icon whose `sizes` is a String are both constructed and serialized. ## Breaking Changes `MCP::Icon.new` now raises `ArgumentError` where earlier releases produced an icon the specification's schema rejects: `src:` is required and must be a non-empty String, so a call without it, or with `nil`, an empty String, or a non-String, fails; `sizes:` must be `nil` or an Array of Strings, so a String, or an Array holding `nil` or a non-String, fails; `mime_type:` must be `nil` or a String; `theme:` must be `nil`, `"light"`, or `"dark"`, so `false` fails where it used to be serialized. Leaving the optional keywords out, or passing `nil` for them, is unchanged. --- lib/mcp/icon.rb | 37 ++++++++++++++++- test/mcp/icon_test.rb | 94 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/lib/mcp/icon.rb b/lib/mcp/icon.rb index 3397c33..56ac6ad 100644 --- a/lib/mcp/icon.rb +++ b/lib/mcp/icon.rb @@ -1,13 +1,35 @@ # frozen_string_literal: true module MCP + # An icon attached to a server, tool, prompt, or resource, per the specification's `Icon` type: + # https://modelcontextprotocol.io/specification/2026-07-28/basic#icons + # + # Each argument is checked against the schema's type, as the TypeScript and Python SDKs check theirs, + # so an icon that would serialize into something a client rejects fails here instead. What a value means + # is not judged: `src` may be any non-empty `String` (the schema types it as a URI, which an empty `String` + # is not, and the specification allows an HTTP/HTTPS URL or a `data:` URI), and a size may be any `String` + # (the specification expects `WxH` or `"any"`). class Icon SUPPORTED_THEMES = ["light", "dark"].freeze attr_reader :mime_type, :sizes, :src, :theme - def initialize(mime_type: nil, sizes: nil, src: nil, theme: nil) - raise ArgumentError, 'The value of theme must specify "light" or "dark".' if theme && !SUPPORTED_THEMES.include?(theme) + def initialize(mime_type: nil, sizes: nil, src:, theme: nil) + unless src.is_a?(String) && !src.empty? + raise ArgumentError, "The value of src must be a non-empty String (got #{src.class})." + end + + unless mime_type.nil? || mime_type.is_a?(String) + raise ArgumentError, "The value of mime_type must be a String (got #{mime_type.class})." + end + + if (problem = sizes_problem(sizes)) + raise ArgumentError, "The value of sizes must be an Array of Strings such as [\"48x48\"] or [\"any\"] (#{problem})." + end + + unless theme.nil? || SUPPORTED_THEMES.include?(theme) + raise ArgumentError, 'The value of theme must specify "light" or "dark".' + end @mime_type = mime_type @sizes = sizes @@ -18,5 +40,16 @@ module MCP def to_h { mimeType: mime_type, sizes: sizes, src: src, theme: theme }.compact end + + private + + def sizes_problem(sizes) + return if sizes.nil? + return "got #{sizes.class}" unless sizes.is_a?(Array) + + index = sizes.index { |size| !size.is_a?(String) } + + "got #{sizes[index].class} inside the Array" if index + end end end diff --git a/test/mcp/icon_test.rb b/test/mcp/icon_test.rb index e8c837c..f2fbc9a 100644 --- a/test/mcp/icon_test.rb +++ b/test/mcp/icon_test.rb @@ -15,32 +15,112 @@ module MCP assert_equal({ mimeType: "image/png", sizes: ["48x48", "96x96"], src: "https://example.com", theme: "light" }, icon.to_h) end - def test_initialization_by_default - icon = Icon.new + def test_initialization_with_only_src + icon = Icon.new(src: "https://example.com/icon.png") assert_nil(icon.mime_type) assert_nil(icon.sizes) - assert_nil(icon.src) + assert_equal("https://example.com/icon.png", icon.src) assert_nil(icon.theme) - assert_equal({}, icon.to_h) + assert_equal({ src: "https://example.com/icon.png" }, icon.to_h) + end + + def test_src_is_required + exception = assert_raises(ArgumentError) do + Icon.new + end + assert_equal("missing keyword: :src", exception.message) + end + + def test_src_rejects_nil_and_an_empty_string + [nil, ""].each do |src| + exception = assert_raises(ArgumentError) do + Icon.new(src: src) + end + assert_equal("The value of src must be a non-empty String (got #{src.class}).", exception.message) + end + end + + def test_src_rejects_a_non_string + exception = assert_raises(ArgumentError) do + Icon.new(src: :icon) + end + assert_equal("The value of src must be a non-empty String (got Symbol).", exception.message) + end + + def test_sizes_accepts_an_array_of_strings + [["48x48", "96x96"], ["any"], []].each do |sizes| + icon = Icon.new(sizes: sizes, src: "https://example.com/icon.png") + + assert_equal(sizes, icon.to_h[:sizes]) + end + end + + # https://github.com/modelcontextprotocol/ruby-sdk/issues/562 + def test_sizes_rejects_a_string + exception = assert_raises(ArgumentError) do + Icon.new(mime_type: "image/png", sizes: "51x51", src: "https://example.com/icon.png") + end + assert_equal( + 'The value of sizes must be an Array of Strings such as ["48x48"] or ["any"] (got String).', + exception.message, + ) + end + + def test_sizes_rejects_an_array_holding_a_non_string + { + ["48x48", 96] => "Integer", + [nil] => "NilClass", + ["48x48", nil] => "NilClass", + [nil, 96] => "NilClass", + }.each do |sizes, offender| + exception = assert_raises(ArgumentError) do + Icon.new(sizes: sizes, src: "https://example.com/icon.png") + end + assert_equal( + "The value of sizes must be an Array of Strings such as [\"48x48\"] or [\"any\"] (got #{offender} inside the Array).", + exception.message, + ) + end + end + + def test_src_accepts_any_scheme_and_sizes_accept_any_string + string_subclass = Class.new(String) + icon = Icon.new(sizes: ["unconventional", +"48x48", string_subclass.new("any")], src: "custom:icon") + + assert_equal({ sizes: ["unconventional", "48x48", "any"], src: "custom:icon" }, icon.to_h) + end + + def test_mime_type_rejects_a_non_string + exception = assert_raises(ArgumentError) do + Icon.new(mime_type: :png, src: "https://example.com/icon.png") + end + assert_equal("The value of mime_type must be a String (got Symbol).", exception.message) end def test_valid_theme_for_light assert_nothing_raised do - Icon.new(theme: "light") + Icon.new(src: "https://example.com/icon.png", theme: "light") end end def test_valid_theme_for_dark assert_nothing_raised do - Icon.new(theme: "dark") + Icon.new(src: "https://example.com/icon.png", theme: "dark") end end def test_invalid_theme exception = assert_raises(ArgumentError) do - Icon.new(theme: "unexpected") + Icon.new(src: "https://example.com/icon.png", theme: "unexpected") + end + assert_equal('The value of theme must specify "light" or "dark".', exception.message) + end + + def test_theme_rejects_false + exception = assert_raises(ArgumentError) do + Icon.new(src: "https://example.com/icon.png", theme: false) end assert_equal('The value of theme must specify "light" or "dark".', exception.message) end