<%= @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 %> <% elsif @client.metadata_document? %>Name set by the app at <%= URI.parse(@client.client_id).host %>.
diff --git a/config/environments/development.rb b/config/environments/development.rb index 8e65afa06b..eac6d0315a 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -100,6 +100,7 @@ config.x.oauth.acceptance_enabled = ENV["OAUTH_DARK"].blank? config.x.oauth.cimd_enabled = true config.x.oauth.cimd_allowed_hosts = %w[ mcp.claude.ai claude.ai claude.com chatgpt.com ] + 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 6e0ad7d2c1..dea1e20c28 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -136,6 +136,12 @@ config.x.oauth.cimd_enabled = ENV["OAUTH_CIMD_ENABLED"] == "true" config.x.oauth.cimd_allowed_hosts = ENV.fetch("OAUTH_CIMD_ALLOWED_HOSTS", "").split + # 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 791dfc538f..7b5fec764c 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -72,6 +72,7 @@ config.x.oauth.acceptance_enabled = true config.x.oauth.cimd_enabled = true config.x.oauth.cimd_allowed_hosts = %w[ mcp.claude.ai claude.ai claude.com chatgpt.com ] + 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/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. diff --git a/saas/config/deploy.yml b/saas/config/deploy.yml index 4b2c057438..552326acf0 100644 --- a/saas/config/deploy.yml +++ b/saas/config/deploy.yml @@ -54,6 +54,11 @@ env: # Configuration, not a credential, and inert on its own: nothing is fetched until # OAUTH_CIMD_ENABLED is "true", which no destination sets. OAUTH_CIMD_ALLOWED_HOSTS: "mcp.claude.ai claude.ai claude.com chatgpt.com" + # 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..d0072ac2fc --- /dev/null +++ b/test/integration/oauth_dcr_redirect_allowlist_test.rb @@ -0,0 +1,163 @@ +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 + + # 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" + 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:" + assert_select "strong", text: "cursor://anysphere.cursor-mcp" + + 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 "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) + + 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 1b845e770a..89eca65cf6 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,55 @@ 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:/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 + test "dynamically registered clients reject plain http for non-loopback hosts" do client = Oauth::Client.new( name: "External", @@ -42,7 +92,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 +102,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 +112,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 +123,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 +141,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 +151,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