From 0367b0e1a78b83e1c30d2c26ddf2d3895f221346 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 23 Oct 2026 12:15:42 -0700 Subject: [PATCH 1/4] Accept self-registered redirects only on loopback, cursor:// and claude://, and allowlisted https hosts Dynamic client registration is open and unauthenticated, so an https callback on any host let anyone register a client that sends codes to a host they choose. Self-registered clients now name an http loopback redirect, a cursor:// or claude:// app redirect, or https on a host in OAUTH_DCR_ALLOWED_HTTPS_HOSTS, as bc3's strict allowlist did before bc3#13778. Unset, no https host is allowed. SaaS lists the Claude, ChatGPT, Cursor and Grok callback hosts. The host check runs only on hosts a registration adds, so a client keeps working after its host leaves the list. --- app/controllers/oauth/clients_controller.rb | 19 +-- app/models/oauth.rb | 56 +++++++++ app/models/oauth/client.rb | 26 +++- config/environments/development.rb | 1 + config/environments/production.rb | 6 + config/environments/test.rb | 1 + saas/config/deploy.yml | 5 + .../oauth_dcr_redirect_allowlist_test.rb | 117 ++++++++++++++++++ test/models/oauth/client_test.rb | 69 +++++++++-- .../oauth_availability_test_helper.rb | 10 ++ 10 files changed, 286 insertions(+), 24 deletions(-) create mode 100644 test/integration/oauth_dcr_redirect_allowlist_test.rb diff --git a/app/controllers/oauth/clients_controller.rb b/app/controllers/oauth/clients_controller.rb index a736d6aa92..d37adcb550 100644 --- a/app/controllers/oauth/clients_controller.rb +++ b/app/controllers/oauth/clients_controller.rb @@ -49,7 +49,7 @@ def bounded_redirect_uris?(uris) def validate_redirect_uri_origins unless performed? || all_registrable_uris?(params[:redirect_uris]) - oauth_error "invalid_redirect_uri", "Only https or local loopback redirect URIs are allowed for dynamic registration" + oauth_error "invalid_redirect_uri", registrable_uris_description end end @@ -70,21 +70,22 @@ def all_registrable_uris?(uris) def registrable_uri?(uri) parsed = URI.parse(uri) - parsed.fragment.nil? && (loopback_uri?(parsed) || https_uri?(parsed)) + Oauth.dcr_redirect_shape?(parsed) && Oauth.dcr_vetted_https_redirect?(parsed) rescue URI::InvalidURIError false end - def registered_client_name - (params[:client_name] if params[:client_name].is_a?(String)).presence || "MCP Client" - end + def registrable_uris_description + schemes = Oauth::DCR_CUSTOM_SCHEMES.map { |scheme| "#{scheme}://" }.join(" or ") + hosts = Oauth.dcr_allowed_https_hosts - def loopback_uri?(parsed) - parsed.scheme == "http" && Oauth.plain_authority?(parsed) && Oauth.loopback_host?(parsed.host) + "Dynamic registration accepts local loopback redirect URIs (http://127.0.0.1, http://[::1] or http://localhost), " \ + "#{schemes} app redirects, " \ + "#{hosts.any? ? "and https redirects on #{hosts.join(", ")}" : "and no https redirects on this server"}" end - def https_uri?(parsed) - parsed.scheme == "https" && Oauth.plain_authority?(parsed) && !Oauth.loopback_host?(parsed.host) + def registered_client_name + (params[:client_name] if params[:client_name].is_a?(String)).presence || "MCP Client" end def validated_scopes diff --git a/app/models/oauth.rb b/app/models/oauth.rb index 882a7f875e..95b6b1073e 100644 --- a/app/models/oauth.rb +++ b/app/models/oauth.rb @@ -44,6 +44,62 @@ def self.plain_host?(host) end end + # Dynamic client registration is open and unauthenticated, so a remote + # callback is the sink an attacker who registers a client would point codes + # at. Self-registered clients may therefore name only: + # + # - an http loopback redirect (RFC 8252 §7.3), served on the user's device; + # - a private-use scheme from DCR_CUSTOM_SCHEMES (RFC 8252 §7.1), dispatched + # by the operating system to an app on the user's device; + # - an https redirect on a vetted vendor host (dcr_allowed_https_hosts). + # + # Only cursor and claude are accepted as schemes: they are the desktop MCP + # clients confirmed to send one. Dotted reverse-DNS schemes get no pass on + # shape, since some dotted schemes route to a remote handler (z39.50r, + # iris.beep). Add a scheme or a host only when a real client is confirmed + # to need it. The consent screen, which always shows where the code goes, + # stays the backstop. + DCR_CUSTOM_SCHEMES = %w[ cursor claude ] + + # The vetted https callback hosts, from OAUTH_DCR_ALLOWED_HTTPS_HOSTS in + # production. None by default, so an install that never names one accepts + # only on-device redirects. + def self.dcr_allowed_https_hosts + Array(Rails.configuration.x.oauth.dcr_allowed_https_hosts).map(&:downcase) + end + + def self.dcr_allowed_https_host?(host) + dcr_allowed_https_hosts.include?(host.to_s.downcase) + end + + # Whether a redirect has the shape a self-registered client may hold: no + # fragment, and http loopback, https on a plain host, or an allowlisted + # private-use scheme with a target. Whether its https host is vetted is + # judged only when a registration adds it (Oauth::Client), so a client + # stays valid after its host leaves the list. + def self.dcr_redirect_shape?(uri) + uri.fragment.nil? && \ + case uri.scheme&.downcase + when "http" then plain_authority?(uri) && loopback_host?(uri.host) + when "https" then plain_authority?(uri) && !loopback_host?(uri.host) + when *DCR_CUSTOM_SCHEMES then private_use_redirect?(uri) + else false + end + end + + # Hierarchical (cursor://host/path, never the opaque cursor:path, which + # can't carry the callback's query), with no userinfo, and naming a host + # or a path. + def self.private_use_redirect?(uri) + uri.opaque.nil? && uri.userinfo.nil? && (uri.host.present? || uri.path.present?) && \ + (uri.host.blank? || plain_host?(uri.host)) + end + + # A remote https redirect a registration may add: an allowlisted host. + def self.dcr_vetted_https_redirect?(uri) + uri.scheme&.downcase != "https" || dcr_allowed_https_host?(uri.host) + end + def self.table_name_prefix "oauth_" end diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb index 28d103eb52..fb20ba55a9 100644 --- a/app/models/oauth/client.rb +++ b/app/models/oauth/client.rb @@ -28,6 +28,7 @@ class Oauth::Client < ApplicationRecord validates :redirect_uris, presence: true, length: { maximum: MAXIMUM_REDIRECT_URIS } validates :token_endpoint_auth_method, inclusion: { in: AUTH_METHODS } validate :redirect_uris_are_valid + validate :added_https_hosts_are_vetted, if: -> { dynamically_registered? && (new_record? || redirect_uris_changed?) } # A confidential client's secret is stored as a digest (TokenDigester). The # raw secret exists only in memory on the record that generated it, for the @@ -126,8 +127,8 @@ def validate_redirect_uri(uri) errors.add :redirect_uris, "must not contain fragments" end - if dynamically_registered? && !valid_loopback_uri?(parsed) && !valid_https_uri?(parsed) - errors.add :redirect_uris, "must be an https or local loopback URI for dynamically registered clients" + if dynamically_registered? && !Oauth.dcr_redirect_shape?(parsed) + errors.add :redirect_uris, "must be a local loopback, https or allowed app-scheme URI for dynamically registered clients" end rescue URI::InvalidURIError errors.add :redirect_uris, "includes an invalid URI" @@ -139,12 +140,25 @@ def loopback_uri?(uri) false end - def valid_loopback_uri?(parsed) - parsed.scheme == "http" && Oauth.plain_authority?(parsed) && Oauth.loopback_host?(parsed.host) + # Judged only on the https hosts a write adds, never on those the client + # already holds, so taking a host off the list leaves every client that + # registered it working (Oauth.dcr_allowed_https_hosts). + def added_https_hosts_are_vetted + unvetted = https_hosts(redirect_uris) - https_hosts(new_record? ? [] : redirect_uris_was) + unvetted.reject! { |host| Oauth.dcr_allowed_https_host?(host) } + + if unvetted.any? + errors.add :redirect_uris, "names an https host not allowed for dynamic registration: #{unvetted.join(", ")}" + end end - def valid_https_uri?(parsed) - parsed.scheme == "https" && Oauth.plain_authority?(parsed) && !Oauth.loopback_host?(parsed.host) + def https_hosts(uris) + Array(uris).filter_map do |uri| + parsed = URI.parse(uri.to_s) + parsed.host.to_s.downcase if parsed.scheme&.downcase == "https" + rescue URI::InvalidURIError + nil + end.uniq end # Only the port may vary, and only for an http loopback redirect (RFC 8252 diff --git a/config/environments/development.rb b/config/environments/development.rb index bad753c8f3..465b7e7a9e 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -98,6 +98,7 @@ # OAuth is lit in development; preview the dark posture with OAUTH_DARK=1. # See Oauth::Availability. config.x.oauth.acceptance_enabled = ENV["OAUTH_DARK"].blank? + config.x.oauth.dcr_allowed_https_hosts = ENV.fetch("OAUTH_DCR_ALLOWED_HTTPS_HOSTS", "").split(/[\s,]+/).compact_blank # Canonical host for mailer URLs (emails always link here, not personal Tailscale URLs) config.action_mailer.default_url_options = { host: "#{config.hosts.first}:3006" } diff --git a/config/environments/production.rb b/config/environments/production.rb index 5437a54f60..7c91529339 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -129,6 +129,12 @@ config.x.oauth.issuance_enabled = ENV["OAUTH_ISSUANCE_ENABLED"] != "false" config.x.oauth.pilot_client_ids = ENV.fetch("OAUTH_PILOT_CLIENT_IDS", "").split(",").map(&:strip).compact_blank + # Dynamic client registration accepts https redirects only on these hosts + # (OAUTH_DCR_ALLOWED_HTTPS_HOSTS, separated by spaces or commas), beside + # loopback and the cursor:// and claude:// app schemes. Unset, it accepts + # no https redirect at all. See Oauth.dcr_redirect_shape?. + config.x.oauth.dcr_allowed_https_hosts = ENV.fetch("OAUTH_DCR_ALLOWED_HTTPS_HOSTS", "").split(/[\s,]+/).compact_blank + # Skip DNS rebinding protection for the default health check endpoint. # config.host_authorization = { exclude: ->(request) { request.path == "/up" } } end diff --git a/config/environments/test.rb b/config/environments/test.rb index f405825b57..d28904dae9 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -70,6 +70,7 @@ # OAuth is lit in tests; see OauthAvailabilityTestHelper to go dark. config.x.oauth.acceptance_enabled = true + config.x.oauth.dcr_allowed_https_hosts = %w[ connector.example.com ] # Enable multi-tenant mode for tests config.x.multi_tenant.enabled = true diff --git a/saas/config/deploy.yml b/saas/config/deploy.yml index 213eb16700..f2ae72fb7a 100644 --- a/saas/config/deploy.yml +++ b/saas/config/deploy.yml @@ -50,6 +50,11 @@ env: # because development runs the app and its cell as one user, where it must stay unset. HOTCELL_ROOT: /run/hotcell HOTCELL_GROUP: 10001 + # The vendor hosts whose https callbacks dynamic client registration accepts, beside loopback and the + # cursor:// and claude:// schemes: Claude's web and desktop connectors (claude.ai, claude.com), ChatGPT + # (chatgpt.com), Cursor and the Grok Bot (www.cursor.com), and Grok's own connector (grok.com). Add a host + # only when a real client is confirmed to need it. + OAUTH_DCR_ALLOWED_HTTPS_HOSTS: "claude.ai claude.com chatgpt.com www.cursor.com grok.com" proxy: ssl: true diff --git a/test/integration/oauth_dcr_redirect_allowlist_test.rb b/test/integration/oauth_dcr_redirect_allowlist_test.rb new file mode 100644 index 0000000000..a034278e2d --- /dev/null +++ b/test/integration/oauth_dcr_redirect_allowlist_test.rb @@ -0,0 +1,117 @@ +require "test_helper" + +class OauthDcrRedirectAllowlistTest < ActionDispatch::IntegrationTest + include OauthAvailabilityTestHelper + + test "registration accepts https only on an allowed host" do + with_dcr_allowed_https_hosts "claude.ai", "chatgpt.com" do + register "https://claude.ai/api/mcp/auth_callback" + assert_response :created + + register "https://connector.example.com/callback" + assert_response :bad_request + assert_equal "invalid_redirect_uri", response.parsed_body["error"] + assert_match "https redirects on claude.ai, chatgpt.com", response.parsed_body["error_description"] + end + end + + test "one unallowed host refuses the whole registration" do + with_dcr_allowed_https_hosts "claude.ai" do + assert_no_difference "Oauth::Client.count" do + register "http://127.0.0.1:8888/callback", "https://claude.ai/api/mcp/auth_callback", "https://elsewhere.example/callback" + end + + assert_response :bad_request + assert_equal "invalid_redirect_uri", response.parsed_body["error"] + end + end + + test "with no allowed hosts, registration takes loopback and app schemes but no https" do + with_dcr_allowed_https_hosts do + register "https://claude.ai/api/mcp/auth_callback" + assert_response :bad_request + assert_match "no https redirects on this server", response.parsed_body["error_description"] + + register "http://127.0.0.1:8888/callback", "http://[::1]:8888/callback", "http://localhost:8888/callback" + assert_response :created + end + end + + test "registration accepts the cursor and claude app schemes and no others" do + register "cursor://anysphere.cursor-mcp/oauth/callback", "claude://claude.ai/mcp-auth-callback/sdk" + assert_response :created + + %w[ com.example.app://callback vscode://callback cursor:callback ].each do |uri| + register uri + assert_response :bad_request, uri + assert_equal "invalid_redirect_uri", response.parsed_body["error"], uri + end + end + + test "an app-scheme client gets its code at its app redirect" do + sign_in_as :david + register "cursor://anysphere.cursor-mcp/oauth/callback" + client = Oauth::Client.find_by!(client_id: response.parsed_body["client_id"]) + + untenanted do + get new_oauth_authorization_path, params: { + client_id: client.client_id, + redirect_uri: "cursor://anysphere.cursor-mcp/oauth/callback", + response_type: "code", + code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + code_challenge_method: "S256", + scope: "read" + } + end + assert_response :success + assert_includes response.headers["Content-Security-Policy"], "cursor:" + + untenanted do + post oauth_authorization_path, params: { + client_id: client.client_id, + redirect_uri: "cursor://anysphere.cursor-mcp/oauth/callback", + response_type: "code", + code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + code_challenge_method: "S256", + scope: "read", + state: "xyz123" + } + end + + assert_response :redirect + location = URI.parse(response.location) + assert_equal "cursor", location.scheme + assert_equal "anysphere.cursor-mcp", location.host + assert Rack::Utils.parse_query(location.query)["code"].present? + end + + test "a client registered before its host left the allowlist still authorizes" do + sign_in_as :david + client = Oauth::Client.create!(name: "Hosted", redirect_uris: %w[ https://connector.example.com/callback ], dynamically_registered: true) + + with_dcr_allowed_https_hosts do + untenanted do + post oauth_authorization_path, params: { + client_id: client.client_id, + redirect_uri: "https://connector.example.com/callback", + response_type: "code", + code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + code_challenge_method: "S256", + scope: "read", + state: "xyz123" + } + end + end + + assert_response :redirect + assert_match %r{\Ahttps://connector\.example\.com/callback\?}, response.location + assert Rack::Utils.parse_query(URI.parse(response.location).query)["code"].present? + end + + private + def register(*redirect_uris) + untenanted do + post oauth_clients_path, params: { client_name: "Connector", redirect_uris: redirect_uris }, as: :json + end + end +end diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb index fdbae59c0d..e217e13e0d 100644 --- a/test/models/oauth/client_test.rb +++ b/test/models/oauth/client_test.rb @@ -1,6 +1,7 @@ require "test_helper" class Oauth::ClientTest < ActiveSupport::TestCase + include OauthAvailabilityTestHelper test "generates client_id on create" do client = Oauth::Client.create!(name: "Test", redirect_uris: %w[ http://127.0.0.1:8888/callback ]) assert_equal 32, client.client_id.length @@ -35,6 +36,54 @@ class Oauth::ClientTest < ActiveSupport::TestCase assert client.valid? end + test "dynamically registered clients may add an https host only from the allowlist" do + with_dcr_allowed_https_hosts "connector.example.com" do + vetted = Oauth::Client.new(name: "Vetted", redirect_uris: %w[ https://CONNECTOR.example.com/callback ], dynamically_registered: true) + unvetted = Oauth::Client.new(name: "Unvetted", redirect_uris: %w[ https://elsewhere.example/callback ], dynamically_registered: true) + + assert vetted.valid? + assert_not unvetted.valid? + assert_includes unvetted.errors[:redirect_uris], "names an https host not allowed for dynamic registration: elsewhere.example" + end + end + + test "dynamically registered clients accept no https host when none is allowed" do + with_dcr_allowed_https_hosts do + client = Oauth::Client.new(name: "Hosted", redirect_uris: %w[ https://connector.example.com/callback ], dynamically_registered: true) + + assert_not client.valid? + end + end + + test "a client keeps an https host taken off the allowlist, but can't add another" do + client = Oauth::Client.create!(name: "Hosted", redirect_uris: %w[ https://connector.example.com/callback ], dynamically_registered: true) + + with_dcr_allowed_https_hosts "other.example.com" do + assert client.reload.update(name: "Renamed") + assert client.update(redirect_uris: %w[ https://connector.example.com/callback http://127.0.0.1:8888/callback ]) + assert_not client.update(redirect_uris: %w[ https://connector.example.com/callback https://elsewhere.example/callback ]) + end + end + + test "manually registered clients aren't held to the dynamic registration allowlist" do + with_dcr_allowed_https_hosts do + assert Oauth::Client.new(name: "First party", redirect_uris: %w[ https://partner.example.com/callback ]).valid? + end + end + + test "dynamically registered clients may use the cursor and claude app schemes" do + %w[ cursor://anysphere.cursor-mcp/oauth/callback claude://claude.ai/mcp-auth-callback/sdk CURSOR://anysphere.cursor-mcp/oauth/callback ].each do |uri| + assert Oauth::Client.new(name: "App", redirect_uris: [ uri ], dynamically_registered: true).valid?, uri + end + end + + test "dynamically registered clients reject other app schemes and malformed app redirects" do + %w[ com.example.app://callback z39.50r://host/db iris.beep://host/x javascript://x/%0aalert(1) vscode://callback + cursor:callback cursor://user@anysphere.cursor-mcp/oauth/callback cursor://anysphere.cursor-mcp/oauth/callback#frag cursor: ].each do |uri| + assert_not Oauth::Client.new(name: "App", redirect_uris: [ uri ], dynamically_registered: true).valid?, uri + end + end + test "dynamically registered clients reject plain http for non-loopback hosts" do client = Oauth::Client.new( name: "External", @@ -42,7 +91,7 @@ class Oauth::ClientTest < ActiveSupport::TestCase dynamically_registered: true ) assert_not client.valid? - assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients" + assert_includes client.errors[:redirect_uris], "must be a local loopback, https or allowed app-scheme URI for dynamically registered clients" end test "dynamically registered clients reject https loopback regardless of host case" do @@ -52,7 +101,7 @@ class Oauth::ClientTest < ActiveSupport::TestCase dynamically_registered: true ) assert_not client.valid? - assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients" + assert_includes client.errors[:redirect_uris], "must be a local loopback, https or allowed app-scheme URI for dynamically registered clients" end test "dynamically registered clients reject percent-encoded https loopback" do @@ -62,7 +111,7 @@ class Oauth::ClientTest < ActiveSupport::TestCase dynamically_registered: true ) assert_not client.valid? - assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients" + assert_includes client.errors[:redirect_uris], "must be a local loopback, https or allowed app-scheme URI for dynamically registered clients" end test "dynamically registered clients name their redirect authority plainly" do @@ -73,12 +122,14 @@ class Oauth::ClientTest < ActiveSupport::TestCase client = Oauth::Client.new(name: "Unplain", redirect_uris: [ uri ], dynamically_registered: true) assert_not client.valid?, uri - assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients", uri + assert_includes client.errors[:redirect_uris], "must be a local loopback, https or allowed app-scheme URI for dynamically registered clients", uri end - %w[ https://connector.example.com:8443/callback https://Connector.Example.com/callback http://[::1]:8888/callback http://localhost/callback - https://203.0.113.7/callback https://3com.example/callback https://10.0.0.1/callback ].each do |uri| - assert Oauth::Client.new(name: "Plain", redirect_uris: [ uri ], dynamically_registered: true).valid?, uri + with_dcr_allowed_https_hosts "connector.example.com", "203.0.113.7", "3com.example", "10.0.0.1" do + %w[ https://connector.example.com:8443/callback https://Connector.Example.com/callback http://[::1]:8888/callback http://localhost/callback + https://203.0.113.7/callback https://3com.example/callback https://10.0.0.1/callback ].each do |uri| + assert Oauth::Client.new(name: "Plain", redirect_uris: [ uri ], dynamically_registered: true).valid?, uri + end end end @@ -89,7 +140,7 @@ class Oauth::ClientTest < ActiveSupport::TestCase dynamically_registered: true ) assert_not client.valid? - assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients" + assert_includes client.errors[:redirect_uris], "must be a local loopback, https or allowed app-scheme URI for dynamically registered clients" end test "dynamically registered clients reject custom schemes" do @@ -99,7 +150,7 @@ class Oauth::ClientTest < ActiveSupport::TestCase dynamically_registered: true ) assert_not client.valid? - assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients" + assert_includes client.errors[:redirect_uris], "must be a local loopback, https or allowed app-scheme URI for dynamically registered clients" end test "redirect URIs must not contain fragments" do diff --git a/test/test_helpers/oauth_availability_test_helper.rb b/test/test_helpers/oauth_availability_test_helper.rb index 6860126319..c32c95f318 100644 --- a/test/test_helpers/oauth_availability_test_helper.rb +++ b/test/test_helpers/oauth_availability_test_helper.rb @@ -10,4 +10,14 @@ def with_oauth_availability(acceptance: true, issuance: true, pilot_client_ids: ensure %i[ acceptance_enabled issuance_enabled pilot_client_ids ].each { |key| config[key] = saved[key] } end + + def with_dcr_allowed_https_hosts(*hosts) + config = Rails.application.config.x.oauth + saved = config.dcr_allowed_https_hosts + + config.dcr_allowed_https_hosts = hosts + yield + ensure + config.dcr_allowed_https_hosts = saved + end end From 5363306e1a843a1995ad6217fb63fba8049f9424 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 23 Oct 2026 12:25:56 -0700 Subject: [PATCH 2/4] Require a host on app-scheme redirects, and http for loopback A hostless cursor:/callback showed consent no destination, so app-scheme redirects now name a plain host, and consent shows it with its scheme. An app scheme naming a loopback host (cursor://localhost) was treated as loopback, which earned it pre-consent error redirects; loopback now means http. --- app/models/oauth.rb | 7 +++--- app/models/oauth/client.rb | 5 ++++- app/views/oauth/authorizations/new.html.erb | 4 ++-- .../oauth_dcr_redirect_allowlist_test.rb | 22 +++++++++++++++++++ test/models/oauth/client_test.rb | 3 ++- 5 files changed, 33 insertions(+), 8 deletions(-) diff --git a/app/models/oauth.rb b/app/models/oauth.rb index 95b6b1073e..cef1d4e4ea 100644 --- a/app/models/oauth.rb +++ b/app/models/oauth.rb @@ -88,11 +88,10 @@ def self.dcr_redirect_shape?(uri) end # Hierarchical (cursor://host/path, never the opaque cursor:path, which - # can't carry the callback's query), with no userinfo, and naming a host - # or a path. + # can't carry the callback's query), with no userinfo, and naming a plain + # host, which consent shows as where the code goes. def self.private_use_redirect?(uri) - uri.opaque.nil? && uri.userinfo.nil? && (uri.host.present? || uri.path.present?) && \ - (uri.host.blank? || plain_host?(uri.host)) + uri.opaque.nil? && uri.userinfo.nil? && plain_host?(uri.host.to_s) end # A remote https redirect a registration may add: an allowlisted host. diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb index fb20ba55a9..5fcabdc410 100644 --- a/app/models/oauth/client.rb +++ b/app/models/oauth/client.rb @@ -134,8 +134,11 @@ def validate_redirect_uri(uri) errors.add :redirect_uris, "includes an invalid URI" end + # http only: an app scheme naming a loopback host (cursor://localhost) is + # dispatched to an app, not served on loopback (RFC 8252 §7.3). def loopback_uri?(uri) - Oauth.loopback_host?(URI.parse(uri).host) + parsed = URI.parse(uri) + parsed.scheme&.downcase == "http" && Oauth.loopback_host?(parsed.host) rescue URI::InvalidURIError false end diff --git a/app/views/oauth/authorizations/new.html.erb b/app/views/oauth/authorizations/new.html.erb index 40bd97e190..cfeb8dc894 100644 --- a/app/views/oauth/authorizations/new.html.erb +++ b/app/views/oauth/authorizations/new.html.erb @@ -18,10 +18,10 @@

<%= @client.name %>

<% if @client.dynamically_registered? %> <% redirect = URI.parse(@redirect_uri) %> - <% if Oauth.loopback_host?(redirect.host) %> + <% if redirect.scheme&.downcase == "http" && Oauth.loopback_host?(redirect.host) %>

Registered by a local tool. Only authorize if you trust it.

<% else %> -

Self-registered app that will receive access at <%= redirect.port == redirect.default_port ? redirect.host : "#{redirect.host}:#{redirect.port}" %>. Only authorize if you trust it.

+

Self-registered app that will receive access at <%= "#{redirect.scheme.downcase}://" unless redirect.scheme.casecmp?("https") %><%= redirect.port == redirect.default_port ? redirect.host : "#{redirect.host}:#{redirect.port}" %>. Only authorize if you trust it.

<% end %> <% end %> diff --git a/test/integration/oauth_dcr_redirect_allowlist_test.rb b/test/integration/oauth_dcr_redirect_allowlist_test.rb index a034278e2d..150cc385ee 100644 --- a/test/integration/oauth_dcr_redirect_allowlist_test.rb +++ b/test/integration/oauth_dcr_redirect_allowlist_test.rb @@ -65,6 +65,7 @@ class OauthDcrRedirectAllowlistTest < ActionDispatch::IntegrationTest end assert_response :success assert_includes response.headers["Content-Security-Policy"], "cursor:" + assert_select "strong", text: "cursor://anysphere.cursor-mcp" untenanted do post oauth_authorization_path, params: { @@ -85,6 +86,27 @@ class OauthDcrRedirectAllowlistTest < ActionDispatch::IntegrationTest assert Rack::Utils.parse_query(location.query)["code"].present? end + test "an app scheme naming a loopback host gets no pre-consent redirect" do + sign_in_as :david + register "cursor://localhost/callback" + assert_response :created + client = Oauth::Client.find_by!(client_id: response.parsed_body["client_id"]) + + untenanted do + get new_oauth_authorization_path, params: { + client_id: client.client_id, + redirect_uri: "cursor://localhost/callback", + response_type: "code", + code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + code_challenge_method: "plain", + scope: "read" + } + end + + assert_response :bad_request + assert_nil response.location + end + test "a client registered before its host left the allowlist still authorizes" do sign_in_as :david client = Oauth::Client.create!(name: "Hosted", redirect_uris: %w[ https://connector.example.com/callback ], dynamically_registered: true) diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb index e217e13e0d..1bd40c149e 100644 --- a/test/models/oauth/client_test.rb +++ b/test/models/oauth/client_test.rb @@ -79,7 +79,8 @@ class Oauth::ClientTest < ActiveSupport::TestCase test "dynamically registered clients reject other app schemes and malformed app redirects" do %w[ com.example.app://callback z39.50r://host/db iris.beep://host/x javascript://x/%0aalert(1) vscode://callback - cursor:callback cursor://user@anysphere.cursor-mcp/oauth/callback cursor://anysphere.cursor-mcp/oauth/callback#frag cursor: ].each do |uri| + cursor:callback cursor:/callback cursor:///callback cursor://user@anysphere.cursor-mcp/oauth/callback cursor://anysphere.cursor-mcp/oauth/callback#frag cursor: + cursor://%65vil/callback ].each do |uri| assert_not Oauth::Client.new(name: "App", redirect_uris: [ uri ], dynamically_registered: true).valid?, uri end end From d9c1872acc9cd965e1b6cc9fa80d13463e3e612c Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 24 Oct 2026 14:41:08 -0700 Subject: [PATCH 3/4] Pin that DCR refuses callbacks on hosts 37signals runs Consent names a self-registered app by where its access returns, so a callback on fizzy.do, basecamp.com, 37signals.com or hey.com would read as ours. The allowlist names only vendor hosts, so ours are already refused; this holds that with production's list. --- .../oauth_dcr_redirect_allowlist_test.rb | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/integration/oauth_dcr_redirect_allowlist_test.rb b/test/integration/oauth_dcr_redirect_allowlist_test.rb index 150cc385ee..d0072ac2fc 100644 --- a/test/integration/oauth_dcr_redirect_allowlist_test.rb +++ b/test/integration/oauth_dcr_redirect_allowlist_test.rb @@ -26,6 +26,30 @@ class OauthDcrRedirectAllowlistTest < ActionDispatch::IntegrationTest end end + # Consent names a self-registered app by where its access returns, so a + # callback on a host we run would read as ours. The allowlist names only + # vendor hosts (production's, from saas/config/deploy.yml), so ours are + # refused, in any case or subdomain. + test "registration refuses https callbacks on hosts 37signals runs" do + with_dcr_allowed_https_hosts "claude.ai", "claude.com", "chatgpt.com", "www.cursor.com", "grok.com" do + %w[ + https://app.fizzy.do/callback + https://FIZZY.do/callback + https://3.basecamp.com/callback + https://basecamp.com/callback + https://launchpad.37signals.com/callback + https://app.hey.com/callback + ].each do |uri| + assert_no_difference "Oauth::Client.count" do + register uri + end + + assert_response :bad_request, uri + assert_equal "invalid_redirect_uri", response.parsed_body["error"], uri + end + end + end + test "with no allowed hosts, registration takes loopback and app schemes but no https" do with_dcr_allowed_https_hosts do register "https://claude.ai/api/mcp/auth_callback" From a581962c3fcb217e7cdc7e3efc3ae970e7ac4234 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 24 Oct 2026 15:45:45 -0700 Subject: [PATCH 4/4] Document OAUTH_DCR_ALLOWED_HTTPS_HOSTS for self-hosted installs Self-registered clients may now use only on-device callbacks, the cursor:// and claude:// schemes, or an https host the operator lists. An install that sets nothing refuses web-based clients' registrations after upgrading, so docker-deployment.md says how to list them. --- docs/docker-deployment.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/docker-deployment.md b/docs/docker-deployment.md index b025084020..ade6cc8e45 100644 --- a/docs/docker-deployment.md +++ b/docs/docker-deployment.md @@ -166,6 +166,14 @@ To turn it on, set `OAUTH_ACCEPTANCE_ENABLED=true`. If you need to stop new connections in an emergency, also set `OAUTH_ISSUANCE_ENABLED=false`: nothing new can connect and no token is refreshed, while access tokens already issued keep working until they expire, an hour after issue. People can always disconnect apps from their profile, whichever way these are set. +Apps that register themselves (dynamic client registration, which Cursor, Claude Desktop and most MCP clients use) may send people back only to a callback Fizzy accepts: + +- a callback on the person's own device: `http://127.0.0.1`, `http://[::1]` or `http://localhost`, on any port; +- the `cursor://` and `claude://` desktop app schemes; +- an `https` callback on a host listed in `OAUTH_DCR_ALLOWED_HTTPS_HOSTS`, separated by spaces or commas, for example `OAUTH_DCR_ALLOWED_HTTPS_HOSTS="claude.ai claude.com chatgpt.com www.cursor.com grok.com"`. Each entry matches that exact host only, not its subdomains. + +`OAUTH_DCR_ALLOWED_HTTPS_HOSTS` is empty unless you set it, so web-based clients such as claude.ai and ChatGPT can't register until you list their callback hosts. Apps registered before you change the list keep their callbacks; the list is checked only at registration. + ## Importing an existing Fizzy account You can move an account between Fizzy instances by exporting it on the old instance and uploading the export zip to the new one during signup.