diff --git a/CHANGELOG.md b/CHANGELOG.md index 68f8e9b..5dc43a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,55 @@ +## [0.6.1] - 2026-07-20 + +Two additive fixes for hosted MCP clients whose OAuth setup could not complete +against the 0.6.0 bridge: it now answers the metadata documents at the path-APPENDED +discovery locations as well as the RFC 8414 path-inserted ones, and its Dynamic +Client Registration response now states the registration (per RFC 7591) instead of +returning a bare `client_id`. Both are bridge-gated and change nothing for a host +with the bridge off, or for a client that already worked. + +### Fixed + +- **Authorization-server discovery for clients that path-APPEND the well-known + segment.** For an MCP endpoint at `https://host/mcp`, the issuer is path-ful, + and RFC 8414 §3.1 places its metadata at the path-INSERTED + `/.well-known/oauth-authorization-server/mcp` — which the bridge already served. + But some MCP clients (observed with a hosted client in the wild) instead request + the path-APPENDED `https://host/mcp/.well-known/oauth-authorization-server`, got + a 404 with no fallback, never obtained a `registration_endpoint`, and reported a + registration failure. The bridge now answers the metadata at BOTH forms. + +- **Dynamic Client Registration response now states the registration.** The stub + returned only a `client_id`. A strict client validates that the `redirect_uris` + it registered come back and abandons a registration that drops them — a plausible + cause of the same "couldn't register" failure, independent of the discovery gap + above. `register` now returns the client's `redirect_uris` and `client_name` + alongside `client_id_issued_at` (RFC 7591 §3.2.1). Echoing a `redirect_uri` + AUTHORIZES nothing: the bridge still stores no client, and `authorize`/`token` + check every one against the host allowlist independently, so a value reflected + here is not thereby permitted. + + The `token_endpoint_auth_method` and the grant/response types are **substituted, + not echoed** — RFC 7591 §3.2.1 states the metadata as REGISTERED and lets a server + replace what it does not support. Reflecting the client's request would contradict + the discovery document that named this endpoint (`token_endpoint_auth_methods_supported: + ["none"]`, `grant_types_supported: ["authorization_code"]`) and promise a flow the + token endpoint rejects — a reflected `refresh_token` is a refresh answered + `unsupported_grant_type`. The supported sets are now named once and shared by both + documents, so they cannot drift apart. + +### Added + +- **Path-appended metadata routes under the engine mount** (drawn only when + `oauth_bridge?`): `GET /.well-known/oauth-authorization-server`, + `GET /.well-known/oauth-protected-resource`, and the OIDC discovery alias + `GET /.well-known/openid-configuration` (which returns the + authorization-server document). All stay UNDER the mount, so they claim nothing + origin-global and cannot collide with an OAuth provider the host already runs; + the RFC 8414 path-inserted documents the host draws at the origin root are + unchanged. Every identifier is still derived from the live request origin, so + the document served at an appended location is byte-identical to the inserted + one, and both carry `Cache-Control: no-store`. + ## [0.6.0] - 2026-07-16 An OAuth 2.1 authorization bridge for the authority role, so hosted MCP clients diff --git a/README.md b/README.md index 3ffbc28..c22cff9 100644 --- a/README.md +++ b/README.md @@ -543,16 +543,26 @@ origin-global.** The flow endpoints live under the engine's mount `/oauth/*` — as an app with Doorkeeper for its own API does — you keep every one of those routes. -The metadata documents are **path-scoped** to the mount -(`/.well-known/oauth-protected-resource/mcp`), never the bare -`/.well-known/oauth-authorization-server`. That matters: the bare paths are -origin-global and mean *"the authorization server of this whole origin"*, which -belongs to a provider you already run, not to an MCP server sharing the host. +The metadata documents are **path-scoped** to the mount, never the bare +origin-global `/.well-known/oauth-authorization-server`. That matters: the bare +paths are origin-global and mean *"the authorization server of this whole origin"*, +which belongs to a provider you already run, not to an MCP server sharing the host. RFC 8414 §3.1 exists for exactly this — *"Using path components enables supporting multiple issuers per host"* — and the MCP authorization spec (2025-11-25) requires a client given a path-ful issuer to try the path-**inserted** URLs, with no root fallback. So the issuer is your MCP endpoint URL, and both documents hang off it. +A path-ful issuer, though, has two readings of *where* under the origin its +metadata lives, and MCP clients disagree: some **insert** the well-known segment +before the resource path (`/.well-known/oauth-authorization-server/mcp` — the RFC +form the host draws), others **append** it after +(`/mcp/.well-known/oauth-authorization-server`). The bridge serves both: the +inserted forms at the origin root (`draw_oauth_metadata_routes`), and the appended +forms — plus the `openid-configuration` OIDC alias — under the engine mount, +automatically whenever the bridge is on. Both stay path-scoped and claim nothing +origin-global; a client discovers the authorization server whichever convention it +follows. + If your MCP endpoint IS its origin root (a dedicated MCP domain), there is no path to insert and you get the bare paths — correct there, since your server really is that origin's only authorization server. Set `oauth_resource_path = "/"`. diff --git a/config/routes.rb b/config/routes.rb index c43c8a7..4267785 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -38,17 +38,31 @@ # route above: the routes file is evaluated through the routes_reloader, after # the host's initializers/to_prepare, so the config is already set. # - # The two metadata documents are NOT here: a client looks for them at the origin - # root, which an engine mounted under a path cannot draw. The host draws them - # with `McpToolkit.draw_oauth_metadata_routes(self)`. - # `format: false` on each, as on the metadata routes the host draws: without it - # Rails' optional `(.:format)` segment matches, so `/mcp/oauth/authorize.json` - # reaches the action, finds no JSON template, and 500s — an unauthenticated - # error on a public endpoint, for a format the bridge never speaks. + # `format: false` on each: without it Rails' optional `(.:format)` segment + # matches, so `/mcp/oauth/authorize.json` reaches the action, finds no JSON + # template, and 500s — an unauthenticated error on a public endpoint, for a + # format the bridge never speaks. if McpToolkit.config.oauth_bridge? get "oauth/authorize", to: "oauth#authorize", format: false post "oauth/authorize", to: "oauth#approve", format: false post "oauth/token", to: "oauth#token", format: false post "oauth/register", to: "oauth#register", format: false + + # The metadata documents, ALSO served path-APPENDED to the mount + # (`/mcp/.well-known/oauth-authorization-server`), in addition to the RFC 8414 + # §3.1 / RFC 9728 §3.1 path-INSERTED locations the host draws at the origin + # root via `McpToolkit.draw_oauth_metadata_routes`. A path-ful issuer has two + # readings of where its metadata lives, and MCP clients disagree: some INSERT + # the well-known segment before the resource path (the RFC form), others APPEND + # it after. Observed in the wild — a hosted client requesting + # `/.well-known/oauth-authorization-server` and getting a 404 it could not + # recover from, so it never reached registration. Serving both meets either + # convention. Unlike the origin-root bare paths, these stay UNDER the mount, so + # they claim nothing origin-global and cannot collide with an OAuth provider + # the host already runs. `openid-configuration` is the OIDC discovery alias a + # client may probe instead; it answers the same authorization-server document. + get ".well-known/oauth-authorization-server", to: "oauth#authorization_server", format: false + get ".well-known/oauth-protected-resource", to: "oauth#protected_resource", format: false + get ".well-known/openid-configuration", to: "oauth#authorization_server", format: false end end diff --git a/lib/mcp_toolkit/oauth/controller_methods.rb b/lib/mcp_toolkit/oauth/controller_methods.rb index 22f44e0..f057e6f 100644 --- a/lib/mcp_toolkit/oauth/controller_methods.rb +++ b/lib/mcp_toolkit/oauth/controller_methods.rb @@ -37,6 +37,12 @@ module McpToolkit::Oauth::ControllerMethods # base64url of a SHA-256, which is always exactly 43 of the same alphabet. PKCE_VALUE = /\A[A-Za-z0-9\-._~]{43,128}\z/ + # What the token endpoint actually honours. Named once because the discovery + # document and the registration response MUST agree — a client reads the first + # to find the second, so a disagreement is this server contradicting itself. + SUPPORTED_GRANT_TYPES = %w[authorization_code].freeze + SUPPORTED_RESPONSE_TYPES = %w[code].freeze + included do # Safe to disable: the token endpoint is called server-to-server without a CSRF # token, and `approve` never acts on ambient authority — it reads no session and @@ -86,22 +92,38 @@ def authorization_server authorization_endpoint: mcp_oauth_endpoint_url("authorize"), token_endpoint: mcp_oauth_endpoint_url("token"), registration_endpoint: mcp_oauth_endpoint_url("register"), - response_types_supported: ["code"], - grant_types_supported: ["authorization_code"], + response_types_supported: SUPPORTED_RESPONSE_TYPES, + grant_types_supported: SUPPORTED_GRANT_TYPES, code_challenge_methods_supported: ["S256"], token_endpoint_auth_methods_supported: ["none"] } end - # Stateless: persisting a `client_id` nothing reads would only grow a table of - # strings the bridge never consults. + # Stateless: nothing here is persisted (no endpoint reads a `client_id`). The + # response still names the `redirect_uris` the client sent, because a strict + # client validates that they come back and abandons a registration that drops + # them — the failure a hosted client hit against the pre-0.6.1 stub, which + # returned only a `client_id`. That echo AUTHORIZES nothing: `authorize` and + # `token` check every `redirect_uri` against the host's allowlist independently. + # + # The auth method and the grant/response types are SUBSTITUTED rather than + # echoed — RFC 7591 §3.2.1 states the metadata as REGISTERED, and lets a server + # replace what it does not support. Reflecting the client's request instead + # would contradict `authorization_server`, which is where that client got this + # endpoint, and would promise a flow `token` rejects: a `refresh_token` echoed + # back is a refresh answered `unsupported_grant_type`. No `client_secret` is + # issued, so `none` is the only auth method a client here could perform. def register - render json: { + body = { client_id: SecureRandom.uuid, + client_id_issued_at: Time.now.to_i, + redirect_uris: mcp_oauth_param_list(:redirect_uris), token_endpoint_auth_method: "none", - grant_types: ["authorization_code"], - response_types: ["code"] - }, status: :created + grant_types: mcp_oauth_registered_subset(:grant_types, SUPPORTED_GRANT_TYPES), + response_types: mcp_oauth_registered_subset(:response_types, SUPPORTED_RESPONSE_TYPES) + } + body[:client_name] = params[:client_name].to_s if params[:client_name].present? + render json: body, status: :created end # `formats: [:html]` because there is only an HTML template and `Accept` picks @@ -151,6 +173,19 @@ def mcp_oauth_config McpToolkit.config end + # A registration parameter as a plain array of strings — RFC 7591 lists arrive + # as JSON arrays; empty when the client sent none. + def mcp_oauth_param_list(key) + Array(params[key]).map(&:to_s) + end + + # What the client asked for, narrowed to what this server honours. A client that + # asked for NOTHING supported gets the default rather than an empty array, which + # would state a registration able to do nothing at all. + def mcp_oauth_registered_subset(key, supported) + (mcp_oauth_param_list(key) & supported).presence || supported + end + # ---- request validation --------------------------------------------------- # Halts both legs before their action runs. Both problems RENDER rather than diff --git a/lib/mcp_toolkit/version.rb b/lib/mcp_toolkit/version.rb index edfcda2..4d8be86 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.0" + VERSION = "0.6.1" end diff --git a/spec/mcp_toolkit/engine_spec.rb b/spec/mcp_toolkit/engine_spec.rb index a1aa099..0ab22c7 100644 --- a/spec/mcp_toolkit/engine_spec.rb +++ b/spec/mcp_toolkit/engine_spec.rb @@ -179,7 +179,14 @@ def drawn_formats [:get, "oauth/authorize", "oauth#authorize"], [:post, "oauth/authorize", "oauth#approve"], [:post, "oauth/token", "oauth#token"], - [:post, "oauth/register", "oauth#register"] + [:post, "oauth/register", "oauth#register"], + # The metadata documents served path-APPENDED under the mount, for clients + # that append the well-known segment after the resource path rather than + # inserting it. Gated with the rest of the bridge; format segment disabled + # like the rest (the checks below fold these in via `oauth_routes`). + [:get, ".well-known/oauth-authorization-server", "oauth#authorization_server"], + [:get, ".well-known/oauth-protected-resource", "oauth#protected_resource"], + [:get, ".well-known/openid-configuration", "oauth#authorization_server"] ] it "does NOT draw the OAuth bridge for an authority that has not opted in" do 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 82307b9..8865350 100644 --- a/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb +++ b/spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb @@ -161,8 +161,31 @@ def token_info = render(plain: "HOST_TOKEN_INFO") session.get("/.well-known/oauth-authorization-server") result["bare_as_status"] = session.last_response.status - # 4. Client registration. - session.post("/mcp/oauth/register", JSON.generate({ redirect_uris: [REDIRECT_URI] }), + # 3b. The SAME documents, served path-APPENDED under the mount, for a client + # that appends the well-known segment after the resource path instead of + # inserting it. These stay under the mount, so they are additive to the + # inserted forms and claim nothing origin-global. + session.get("/mcp/.well-known/oauth-authorization-server") + result["appended_as_status"] = session.last_response.status + result["appended_as"] = JSON.parse(session.last_response.body) + result["appended_as_cache_control"] = session.last_response.headers["Cache-Control"] + + session.get("/mcp/.well-known/oauth-protected-resource") + result["appended_prm_status"] = session.last_response.status + result["appended_prm"] = JSON.parse(session.last_response.body) + + session.get("/mcp/.well-known/openid-configuration") + result["appended_oidc_status"] = session.last_response.status + result["appended_oidc"] = JSON.parse(session.last_response.body) + + # 4. Client registration. Asks for more than the bridge honours, so the + # response below exercises substitution over a real request cycle rather + # than the defaults an empty body would take. + session.post("/mcp/oauth/register", + JSON.generate({ redirect_uris: [REDIRECT_URI], + grant_types: %w[authorization_code refresh_token], + response_types: %w[code token], + token_endpoint_auth_method: "client_secret_post" }), "CONTENT_TYPE" => "application/json") result["register_status"] = session.last_response.status result["register"] = JSON.parse(session.last_response.body) @@ -323,7 +346,10 @@ def token_info = render(plain: "HOST_TOKEN_INFO") it "keeps every one of its own endpoints under the engine mount" do expect(@result.fetch("engine_paths")).to contain_exactly( "/", "/", "/", "/health", "/tokens/introspect", - "/oauth/authorize", "/oauth/authorize", "/oauth/token", "/oauth/register" + "/oauth/authorize", "/oauth/authorize", "/oauth/token", "/oauth/register", + "/.well-known/oauth-authorization-server", + "/.well-known/oauth-protected-resource", + "/.well-known/openid-configuration" ) end end @@ -473,6 +499,36 @@ def token_info = render(plain: "HOST_TOKEN_INFO") expect(@result.fetch("bare_prm_status")).to eq(404) expect(@result.fetch("bare_as_status")).to eq(404) end + + # A client that appends the well-known segment after the resource path gets the + # same documents as one that inserts it — the byte-identical issuer + endpoints, + # and the same no-store — so discovery succeeds under either convention. + it "also answers authorization-server metadata at the path-APPENDED location" do + expect(@result.fetch("appended_as_status")).to eq(200) + expect(@result.fetch("appended_as")).to include( + "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" + ) + expect(@result.fetch("appended_as_cache_control")).to eq("no-store") + end + + it "also answers protected-resource metadata at the path-APPENDED location" do + expect(@result.fetch("appended_prm_status")).to eq(200) + expect(@result.fetch("appended_prm")).to include( + "resource" => "http://example.org/mcp", + "authorization_servers" => ["http://example.org/mcp"] + ) + end + + it "answers the OIDC discovery alias with the authorization-server document" do + expect(@result.fetch("appended_oidc_status")).to eq(200) + expect(@result.fetch("appended_oidc")).to include( + "issuer" => "http://example.org/mcp", + "registration_endpoint" => "http://example.org/mcp/oauth/register" + ) + end end describe "registration" do @@ -480,6 +536,29 @@ def token_info = render(plain: "HOST_TOKEN_INFO") expect(@result.fetch("register_status")).to eq(201) expect(@result.fetch("register")["client_id"]).to be_a(String).and be_present end + + # RFC 7591 §3.2.1: a strict client validates that the redirect_uris it + # registered come back, and abandons a registration that drops them. Echoing + # them registers nothing — the allowlist still gates authorize/token — but a + # client that requires the echo can now complete registration. + it "echoes the submitted redirect_uris" do + register = @result.fetch("register") + + expect(register["redirect_uris"]).to eq(["https://client.example/callback"]) + expect(register["client_id_issued_at"]).to be_a(Integer) + end + + # The request asked for refresh_token, an implicit response type and a secret + # this server never issues. Stating them back would promise flows the token + # endpoint rejects, and contradict the very document that named this endpoint. + it "states only what it honours, whatever the client asked to register" do + register = @result.fetch("register") + + expect(register["grant_types"]).to eq(["authorization_code"]) + expect(register["response_types"]).to eq(["code"]) + expect(register["token_endpoint_auth_method"]).to eq("none") + expect(register).not_to have_key("client_id_expires_at") + end end describe "the authorization page" do diff --git a/spec/mcp_toolkit/oauth/controller_methods_spec.rb b/spec/mcp_toolkit/oauth/controller_methods_spec.rb index d78a3b4..2e1919e 100644 --- a/spec/mcp_toolkit/oauth/controller_methods_spec.rb +++ b/spec/mcp_toolkit/oauth/controller_methods_spec.rb @@ -210,6 +210,81 @@ def issue_code(overrides = {}) expect(controller.rendered[:options][:json][:client_id]).not_to eq(first) end + + # RFC 7591 §3.2.1: a strict client validates that the redirect_uris it sent + # come back, and abandons a registration that drops them. Echoing them + # authorizes nothing — authorize/token still gate every redirect_uri against + # the allowlist (see the #authorize examples below). + it "echoes the submitted redirect_uris and client_name" do + controller.params = { redirect_uris: [redirect_uri], client_name: "Some Client" } + controller.register + + expect(controller.rendered[:options][:json]).to include( + redirect_uris: [redirect_uri], + client_name: "Some Client" + ) + expect(controller.rendered[:options][:json][:client_id_issued_at]).to be_a(Integer) + end + + # RFC 7591 §3.2.1 states the metadata as REGISTERED, not as requested. Echoing + # a grant this server rejects would promise the client a flow #token answers + # `unsupported_grant_type`, and echoing an auth method would confirm a + # credential no client here holds — no client_secret is ever issued. + it "substitutes grant/response types it does not support rather than echoing them" do + controller.params = { + grant_types: %w[authorization_code refresh_token], + response_types: %w[code token], + token_endpoint_auth_method: "client_secret_post" + } + controller.register + + expect(controller.rendered[:options][:json]).to include( + grant_types: ["authorization_code"], + response_types: ["code"], + token_endpoint_auth_method: "none" + ) + end + + it "falls back to the supported defaults when the client asks for nothing it honours" do + controller.params = { grant_types: ["client_credentials"], response_types: ["token"] } + controller.register + + expect(controller.rendered[:options][:json]).to include( + grant_types: ["authorization_code"], + response_types: ["code"] + ) + end + + it "defaults the grant/response types and auth method when the client sends none" do + controller.register + + expect(controller.rendered[:options][:json]).to include( + redirect_uris: [], + grant_types: ["authorization_code"], + response_types: ["code"], + token_endpoint_auth_method: "none" + ) + end + + # The client fetched this endpoint FROM the discovery document, so the two + # disagreeing is the server contradicting itself — the defect the substitution + # above exists to prevent, pinned against both documents at once. + it "states nothing the discovery document does not advertise as supported" do + controller.params = { + grant_types: %w[authorization_code refresh_token], + response_types: %w[code token], + token_endpoint_auth_method: "private_key_jwt" + } + controller.register + registered = controller.rendered[:options][:json] + controller.authorization_server + advertised = controller.rendered[:options][:json] + + expect(registered[:grant_types] - advertised[:grant_types_supported]).to be_empty + expect(registered[:response_types] - advertised[:response_types_supported]).to be_empty + expect(advertised[:token_endpoint_auth_methods_supported]) + .to include(registered[:token_endpoint_auth_method]) + end end describe "#authorize" do