From 00338c2cfc7b9a35f2e42a582aac881687f54379 Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 28 Aug 2026 10:32:33 +0200 Subject: [PATCH 1/2] Implement RFC 9207 issuer identification in the OAuth bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `iss` to the authorization response and advertises `authorization_response_iss_parameter_supported` in the authorization server metadata. RFC 9207 is mixed-up-authorization-server defence: it names which server produced a response, so a client registered with several cannot be induced to redeem a code at the wrong one. It is also the fix for hosted clients that choose their redirect URI from whether the server supports it. ChatGPT connectors use the stable `https://chatgpt.com/connector_platform_oauth_redirect` when the server conforms, and a per-connector `https://chatgpt.com/connector/oauth/{callback_id}` when it does not — a URI no exact-match allowlist can express. Confirmed in production: 9 rejections over two days, all naming the per-connector form. No control is relaxed. The redirect allowlist is untouched and still exact-match (RFC 9700 §2.1); conforming makes the client ask for a URI that is already on it. The alternative — matching the vendor's callbacks by pattern — would let an attacker name their own destination inside the prefix, so the README now records why that is not the fix. - `iss` is response-owned alongside `code`/`state`: a caller that seeds one into its own redirect_uri has it replaced, not appended. - `iss` must byte-match the advertised `issuer` (clients compare by exact string). Both derive from `mcp_oauth_issuer`, which reads `request.base_url` and so honours X-Forwarded-Host — a spec pins the redirect against the discovery document rather than a literal, so any divergence fails here. - `approve` is the only response this bridge redirects; every error path renders (400/422), so there is no error response for `iss` to be absent from. 619 examples/0 failures (all 4 new assertions verified red without the change), RuboCop 65/0, Brakeman 0. --- CHANGELOG.md | 40 +++++++++++++++++++ README.md | 19 +++++++++ lib/mcp_toolkit/oauth/controller_methods.rb | 21 +++++++++- lib/mcp_toolkit/version.rb | 2 +- .../oauth/bridge_end_to_end_spec.rb | 32 +++++++++++++-- 5 files changed, 108 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc43a3..d841fb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,43 @@ +## [0.6.2] - 2026-08-28 + +RFC 9207 authorization server issuer identification. Additive hardening, and the +fix for hosted clients that choose their redirect URI based on whether the +authorization server supports it. + +### Added + +- **`iss` in the authorization response, and `authorization_response_iss_parameter_supported` + in the metadata** (RFC 9207). The parameter names which authorization server + produced a response, so a client registered with several cannot be induced to + redeem a code at the wrong one — the mixed-up authorization server attack. + + This also unblocks **ChatGPT connectors**, which pick their `redirect_uri` from + whether the server meets RFC 9207: when it does, ChatGPT uses the stable + `https://chatgpt.com/connector_platform_oauth_redirect`; when it does not, it + mints a per-connector `https://chatgpt.com/connector/oauth/{callback_id}` that + no exact-match allowlist can express, and the connection fails with an + unregistered `redirect_uri`. Observed in production against a real customer + before this release. + + Note for hosts: a client that picks its callback at *connector-creation* time + reads the metadata then. An existing connector created against a pre-0.6.2 + server keeps the URI it already chose — **it has to be re-created** to pick up + the change. + + `iss` is emitted on the one response this bridge redirects (`approve`); every + error path renders rather than redirecting, so there is no error response for it + to be absent from. It is byte-identical to the advertised `issuer` — clients + compare by exact string and do not normalise trailing slashes, paths, ports or + casing — and a spec pins the redirect against the discovery document rather than + against a literal. + + `iss` is now response-owned alongside `code` and `state`: a caller that seeds one + into its own `redirect_uri` has it replaced, not appended. A value the caller + chose would defeat the point of the parameter. + + **No control was relaxed.** The redirect allowlist is unchanged and still exact-match + (RFC 9700 §2.1); this release makes conforming clients ask for a URI already on it. + ## [0.6.1] - 2026-07-20 Two additive fixes for hosted MCP clients whose OAuth setup could not complete diff --git a/README.md b/README.md index c22cff9..f6458b0 100644 --- a/README.md +++ b/README.md @@ -493,6 +493,25 @@ So **every target must be named by exact string**, with exactly one exception: | Private-use scheme (`cursor://…`, `com.example.app:/cb`) | Exact string, in `oauth_allowed_redirect_uris` | Keeps the code on the device, but its URI is a fixed string — so just name it. | | Loopback (`http://127.0.0.1:*`, `localhost`, `[::1]`) | `oauth_allow_loopback_redirects` | The only target that **cannot** be named: the client picks an ephemeral port at runtime (RFC 8252 §7.3). And it resolves on the operator's own machine, so the attack above cannot reach it. | +### If a client's callback looks impossible to name, check RFC 9207 first + +Some hosted clients mint a **per-connector** callback (`https://vendor.example/connector/oauth/{id}`), +which no exact-match list can express — and the obvious response, matching the +host and path by pattern, is the wrong one. A prefix turns an attacker into +someone who can **name their own destination inside it**: they create their own +connector at that vendor, put their callback in the authorize URL, and the code +goes somewhere they control. Exact matching leaves a weaker residual (a code sent +to the vendor's *legitimate* shared callback, separated by the client binding +`state` to the initiating session — RFC 6819 §4.4.1.7); a prefix removes the need +for any of that to go wrong. + +At least one such client picks the per-connector form **only when the +authorization server does not implement RFC 9207**, and uses a single stable +callback when it does. Since 0.6.2 this gem implements it, so the fix for that +class of failure is to be conforming, not to be permissive. Before relaxing the +policy for a client that "cannot be named", check whether it is asking you for a +capability instead. + The loopback exception exists because an allowlist entry is *impossible* there, not because native clients are trusted. A private-use scheme keeps the code on the device too, but nothing forces it to be unnamed — and whole **schemes** cannot be diff --git a/lib/mcp_toolkit/oauth/controller_methods.rb b/lib/mcp_toolkit/oauth/controller_methods.rb index f057e6f..e67959e 100644 --- a/lib/mcp_toolkit/oauth/controller_methods.rb +++ b/lib/mcp_toolkit/oauth/controller_methods.rb @@ -31,7 +31,10 @@ module McpToolkit::Oauth::ControllerMethods # Query parameters the callback response owns: whatever a client put in its own # redirect_uri, these are set by the redirect and not carried over from it. - RESPONSE_OWNED_QUERY_KEYS = %w[code state].freeze + # `iss` is here for the same reason as `code`: RFC 9207 exists so a client can + # tell WHICH authorization server answered, which it cannot do if the value is + # one the caller seeded into its own redirect_uri. + RESPONSE_OWNED_QUERY_KEYS = %w[code state iss].freeze # RFC 7636 §4.1: 43–128 unreserved characters. The challenge is §4.2's # base64url of a SHA-256, which is always exactly 43 of the same alphabet. @@ -95,7 +98,8 @@ def authorization_server response_types_supported: SUPPORTED_RESPONSE_TYPES, grant_types_supported: SUPPORTED_GRANT_TYPES, code_challenge_methods_supported: ["S256"], - token_endpoint_auth_methods_supported: ["none"] + token_endpoint_auth_methods_supported: ["none"], + authorization_response_iss_parameter_supported: true } end @@ -402,12 +406,25 @@ def mcp_oauth_endpoint_url(action) # `URI`, so the host part of what is emitted is byte-identical to what the # policy approved. The query IS re-encoded (`?a=1?b=2` normalises to # `?a=1%3Fb%3D2`), which is the point — that is where `code` gets stripped. + # `iss` is RFC 9207 issuer identification: it names which authorization server + # produced this response, so a client registered with several cannot be tricked + # into redeeming a code at the wrong one (mixed-up authorization server). It is + # emitted unconditionally — RFC 9207 §2 makes it a MUST for every authorization + # response sent to the redirect_uri, and `approve` is the only one this bridge + # sends: every error path renders (400/422) rather than redirecting, so there is + # no error response for it to be absent from. + # + # It MUST be byte-identical to the `issuer` in the authorization server metadata + # — clients compare by exact string and do not normalise trailing slashes, + # paths, ports or casing. Both come from `mcp_oauth_issuer` for that reason, and + # a spec pins them against each other. def mcp_oauth_callback_url(code) redirect_uri = params[:redirect_uri].to_s base, _, existing = redirect_uri.partition("?") pairs = mcp_oauth_preserved_query_pairs(existing) pairs << ["code", code] pairs << ["state", params[:state].to_s] if params[:state].present? + pairs << ["iss", mcp_oauth_issuer] "#{base}?#{URI.encode_www_form(pairs)}" end diff --git a/lib/mcp_toolkit/version.rb b/lib/mcp_toolkit/version.rb index 4d8be86..bcf86f9 100644 --- a/lib/mcp_toolkit/version.rb +++ b/lib/mcp_toolkit/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module McpToolkit - VERSION = "0.6.1" + VERSION = "0.6.2" end diff --git a/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb b/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb index 8865350..ad8e0e8 100644 --- a/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb +++ b/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb @@ -215,6 +215,7 @@ def token_info = render(plain: "HOST_TOKEN_INFO") result["approve_location_host"] = location && location.split("?").first code = location && Rack::Utils.parse_query(URI.parse(location).query)["code"] result["approve_state"] = location && Rack::Utils.parse_query(URI.parse(location).query)["state"] + result["approve_iss"] = location && Rack::Utils.parse_query(URI.parse(location).query)["iss"] # 6b. A bad paste must not issue a code. session.post("/mcp/oauth/authorize", authorize_query.merge(access_token: "wrong-token")) @@ -304,12 +305,13 @@ def token_info = render(plain: "HOST_TOKEN_INFO") # 12. A loopback client controls its own query, so it can pass `?code=`. The # response owns that parameter — ours must be the only one. - polluted = "http://127.0.0.1:54321/cb?code=ATTACKER&tenant=acme" + polluted = "http://127.0.0.1:54321/cb?code=ATTACKER&iss=https://attacker.example&tenant=acme" session.post("/mcp/oauth/authorize", authorize_query.merge(redirect_uri: polluted, access_token: VALID_TOKEN)) polluted_location = session.last_response.headers["Location"] polluted_query = Rack::Utils.parse_query(URI.parse(polluted_location.to_s).query) result["polluted_code_values"] = Array(polluted_query["code"]) + result["polluted_iss_values"] = Array(polluted_query["iss"]) result["polluted_keeps_client_query"] = polluted_query["tenant"] puts JSON.generate(result) @@ -438,6 +440,16 @@ def token_info = render(plain: "HOST_TOKEN_INFO") expect(codes.first).not_to eq("ATTACKER") expect(@result.fetch("polluted_keeps_client_query")).to eq("acme") end + + # A seeded `iss` is the one that matters most: RFC 9207 exists so a client can + # tell which authorization server answered, and a caller-supplied value would + # let the caller answer that question instead of us. + it "emits exactly one iss, ours, over a caller-seeded one" do + values = @result.fetch("polluted_iss_values") + + expect(values.size).to eq(1) + expect(values.first).to eq("http://example.org/mcp") + end end # RFC 6749 §5.1 (both headers on a token response) and RFC 9700 §4.12 (303 after @@ -489,7 +501,8 @@ def token_info = render(plain: "HOST_TOKEN_INFO") "issuer" => "http://example.org/mcp", "authorization_endpoint" => "http://example.org/mcp/oauth/authorize", "token_endpoint" => "http://example.org/mcp/oauth/token", - "code_challenge_methods_supported" => ["S256"] + "code_challenge_methods_supported" => ["S256"], + "authorization_response_iss_parameter_supported" => true ) end @@ -509,7 +522,8 @@ def token_info = render(plain: "HOST_TOKEN_INFO") "issuer" => "http://example.org/mcp", "authorization_endpoint" => "http://example.org/mcp/oauth/authorize", "token_endpoint" => "http://example.org/mcp/oauth/token", - "registration_endpoint" => "http://example.org/mcp/oauth/register" + "registration_endpoint" => "http://example.org/mcp/oauth/register", + "authorization_response_iss_parameter_supported" => true ) expect(@result.fetch("appended_as_cache_control")).to eq("no-store") end @@ -593,6 +607,18 @@ def token_info = render(plain: "HOST_TOKEN_INFO") expect(@result.fetch("approve_bad_token_status")).to eq(422) expect(@result.fetch("approve_bad_token_redirected")).to be(false) end + + # RFC 9207 §2. The value is useless unless a client can match it against the + # metadata by EXACT STRING — no normalising of trailing slashes, paths, ports + # or casing — so this pins the redirect against the discovery document the + # client actually read, rather than against a literal. Both derive from + # `request.base_url`, which honours X-Forwarded-Host: if that ever makes them + # disagree, they disagree HERE and not in a customer's connector. + it "carries an iss that byte-matches the advertised issuer" do + expect(@result.fetch("approve_iss")).to eq(@result.fetch("as").fetch("issuer")) + expect(@result.fetch("approve_iss")).to eq(@result.fetch("appended_as").fetch("issuer")) + expect(@result.fetch("approve_iss")).to eq(@result.fetch("prm").fetch("authorization_servers").first) + end end describe "the exchange" do From 39be614f9f0bd33122ad2dd657bad6a1e9fadddf Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 28 Aug 2026 10:35:52 +0200 Subject: [PATCH 2/2] Trim the RFC 9207 comments to the budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rationale (mixed-up authorization server, RFC 9207 §2's MUST, why `approve` is the only redirect that can carry `iss`) was written into the source as a 12-line block minutes after working it out. It is already in the previous commit message and the CHANGELOG, and the code shows what matters: `iss` is `mcp_oauth_issuer`, the same method the metadata renders. Also renames the loopback describe, which said "its own code" while now covering a seeded `iss` too. 24 added comment lines -> 2. --- lib/mcp_toolkit/oauth/controller_methods.rb | 15 --------------- spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb | 13 +++---------- 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/lib/mcp_toolkit/oauth/controller_methods.rb b/lib/mcp_toolkit/oauth/controller_methods.rb index e67959e..c323129 100644 --- a/lib/mcp_toolkit/oauth/controller_methods.rb +++ b/lib/mcp_toolkit/oauth/controller_methods.rb @@ -31,9 +31,6 @@ module McpToolkit::Oauth::ControllerMethods # Query parameters the callback response owns: whatever a client put in its own # redirect_uri, these are set by the redirect and not carried over from it. - # `iss` is here for the same reason as `code`: RFC 9207 exists so a client can - # tell WHICH authorization server answered, which it cannot do if the value is - # one the caller seeded into its own redirect_uri. RESPONSE_OWNED_QUERY_KEYS = %w[code state iss].freeze # RFC 7636 §4.1: 43–128 unreserved characters. The challenge is §4.2's @@ -406,18 +403,6 @@ def mcp_oauth_endpoint_url(action) # `URI`, so the host part of what is emitted is byte-identical to what the # policy approved. The query IS re-encoded (`?a=1?b=2` normalises to # `?a=1%3Fb%3D2`), which is the point — that is where `code` gets stripped. - # `iss` is RFC 9207 issuer identification: it names which authorization server - # produced this response, so a client registered with several cannot be tricked - # into redeeming a code at the wrong one (mixed-up authorization server). It is - # emitted unconditionally — RFC 9207 §2 makes it a MUST for every authorization - # response sent to the redirect_uri, and `approve` is the only one this bridge - # sends: every error path renders (400/422) rather than redirecting, so there is - # no error response for it to be absent from. - # - # It MUST be byte-identical to the `issuer` in the authorization server metadata - # — clients compare by exact string and do not normalise trailing slashes, - # paths, ports or casing. Both come from `mcp_oauth_issuer` for that reason, and - # a spec pins them against each other. def mcp_oauth_callback_url(code) redirect_uri = params[:redirect_uri].to_s base, _, existing = redirect_uri.partition("?") diff --git a/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb b/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb index ad8e0e8..ee2889b 100644 --- a/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb +++ b/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb @@ -432,7 +432,7 @@ def token_info = render(plain: "HOST_TOKEN_INFO") # A loopback redirect_uri is not exact-matched, so its query is the caller's to # choose. The parameters this response owns are not. - describe "a loopback redirect_uri carrying its own code" do + describe "a loopback redirect_uri carrying its own response parameters" do it "emits exactly one code, ours, and keeps the client's own query" do codes = @result.fetch("polluted_code_values") @@ -441,9 +441,6 @@ def token_info = render(plain: "HOST_TOKEN_INFO") expect(@result.fetch("polluted_keeps_client_query")).to eq("acme") end - # A seeded `iss` is the one that matters most: RFC 9207 exists so a client can - # tell which authorization server answered, and a caller-supplied value would - # let the caller answer that question instead of us. it "emits exactly one iss, ours, over a caller-seeded one" do values = @result.fetch("polluted_iss_values") @@ -608,12 +605,8 @@ def token_info = render(plain: "HOST_TOKEN_INFO") expect(@result.fetch("approve_bad_token_redirected")).to be(false) end - # RFC 9207 §2. The value is useless unless a client can match it against the - # metadata by EXACT STRING — no normalising of trailing slashes, paths, ports - # or casing — so this pins the redirect against the discovery document the - # client actually read, rather than against a literal. Both derive from - # `request.base_url`, which honours X-Forwarded-Host: if that ever makes them - # disagree, they disagree HERE and not in a customer's connector. + # Compared against the document, not a literal: both derive from + # `request.base_url`, so X-Forwarded-Host divergence has to fail here. it "carries an iss that byte-matches the advertised issuer" do expect(@result.fetch("approve_iss")).to eq(@result.fetch("as").fetch("issuer")) expect(@result.fetch("approve_iss")).to eq(@result.fetch("appended_as").fetch("issuer"))