diff --git a/CHANGELOG.md b/CHANGELOG.md index 56f20cf2f..9eed0e4d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Note: For changes to the API, see https://shopify.dev/changelog?filter=api ## Unreleased +- [#1472](https://github.com/Shopify/shopify-api-ruby/pull/1472) `ShopifyAPI::Auth.embedded_app_url` now raises `ShopifyAPI::Errors::InvalidShopError` unless the decoded `host` is a trusted Shopify Admin host (see `ShopifyAPI::Utils::ShopValidator`). URLs for supported Admin hosts are unchanged. +- [#1472](https://github.com/Shopify/shopify-api-ruby/pull/1472) `ShopifyAPI::Auth::Oauth.begin_auth` now validates and normalizes `shop` with `ShopifyAPI::Utils::ShopValidator.sanitize!`, and raises `ShopifyAPI::Errors::InvalidShopError` for untrusted domains. +- [#1472](https://github.com/Shopify/shopify-api-ruby/pull/1472) `ShopifyAPI::Utils::ShopValidator` now rejects hosts with non-hostname characters and reads the store handle from the segment after `/store/` in Admin URLs (`admin.shopify.com/store/`), rejecting a missing or invalid handle. ## 16.3.0 (2026-08-04) - [#1443](https://github.com/Shopify/shopify-api-ruby/pull/1443) Add `ShopifyAPI::Utils::ShopValidator` with `sanitize_shop_domain` and `sanitize!`. diff --git a/lib/shopify_api/auth.rb b/lib/shopify_api/auth.rb index 2fd16babb..0650ba2d1 100644 --- a/lib/shopify_api/auth.rb +++ b/lib/shopify_api/auth.rb @@ -1,10 +1,19 @@ # typed: strict # frozen_string_literal: true +require "addressable/uri" + module ShopifyAPI module Auth extend T::Sig + # Hostname characters, plus an optional port. Anything else before the first "/" is rejected. + EMBEDDED_HOST_AUTHORITY_REGEX = T.let(/\A[a-zA-Z0-9._-]+(:\d{1,5})?\z/, Regexp) + + # Printable ASCII only: no whitespace, control bytes or non-ASCII bytes. + EMBEDDED_HOST_ALLOWED_BYTES_REGEX = T.let(/\A[\x21-\x7e]+\z/n, Regexp) + private_constant :EMBEDDED_HOST_AUTHORITY_REGEX, :EMBEDDED_HOST_ALLOWED_BYTES_REGEX + class << self extend T::Sig @@ -19,8 +28,29 @@ def embedded_app_url(host) end decoded_host = Base64.decode64(host) + unless trusted_embedded_host?(decoded_host) + raise Errors::InvalidShopError, "host must decode to a trusted Shopify Admin host" + end + "https://#{decoded_host}/apps/#{Context.api_key}" end + + private + + sig { params(decoded_host: String).returns(T::Boolean) } + def trusted_embedded_host?(decoded_host) + return false unless decoded_host.b.match?(EMBEDDED_HOST_ALLOWED_BYTES_REGEX) + + authority = T.must(decoded_host.split("/", 2).first) + return false unless authority.match?(EMBEDDED_HOST_AUTHORITY_REGEX) + + uri = Addressable::URI.parse("https://#{decoded_host}") + return false unless uri.userinfo.nil? && uri.authority == authority + + !Utils::ShopValidator.sanitize_shop_domain(uri.to_s).nil? + rescue Addressable::URI::InvalidURIError + false + end end end end diff --git a/lib/shopify_api/auth/oauth.rb b/lib/shopify_api/auth/oauth.rb index 75897ddab..7a9a54717 100644 --- a/lib/shopify_api/auth/oauth.rb +++ b/lib/shopify_api/auth/oauth.rb @@ -33,6 +33,8 @@ def begin_auth(shop:, redirect_path:, is_online: true, scope_override: nil) end raise Errors::UnsupportedOauthError, "Cannot perform OAuth for private apps." if Context.private? + shop = Utils::ShopValidator.sanitize!(shop) + state = SecureRandom.alphanumeric(NONCE_LENGTH) cookie = SessionCookie.new(value: state, expires: Time.now + 60) diff --git a/lib/shopify_api/utils/shop_validator.rb b/lib/shopify_api/utils/shop_validator.rb index 860fcb846..a182e0800 100644 --- a/lib/shopify_api/utils/shop_validator.rb +++ b/lib/shopify_api/utils/shop_validator.rb @@ -17,6 +17,10 @@ module ShopValidator T::Array[String], ) + HOSTNAME_REGEX = T.let(/\A[a-z0-9_-]+(\.[a-z0-9_-]+)*\.?\z/, Regexp) + STORE_HANDLE_REGEX = T.let(/\A[a-z0-9][a-z0-9-]*\z/, Regexp) + private_constant :HOSTNAME_REGEX, :STORE_HANDLE_REGEX + class << self extend T::Sig @@ -29,6 +33,7 @@ class << self def sanitize_shop_domain(shop_domain, myshopify_domain: nil) uri = uri_from_shop_domain(shop_domain, myshopify_domain) return nil if uri.nil? || uri.host.nil? || uri.host.empty? + return nil unless T.cast(uri.host, String).match?(HOSTNAME_REGEX) trusted_domains(myshopify_domain).each do |trusted_domain| host = T.cast(uri.host, String) @@ -107,9 +112,13 @@ def unified_admin?(uri) T.cast(uri.host, String).split(".").first == "admin" end - sig { params(uri: Addressable::URI).returns(String) } + sig { params(uri: Addressable::URI).returns(T.nilable(String)) } def myshopify_domain_from_unified_admin(uri) - shop = uri.path.to_s.split("/").last + segments = uri.path.to_s.split("/") + store_index = segments.index("store") + shop = store_index && segments[store_index + 1] + return unless shop&.match?(STORE_HANDLE_REGEX) + "#{shop}.myshopify.com" end end diff --git a/test/auth/oauth_test.rb b/test/auth/oauth_test.rb index ad26e30a3..364446217 100644 --- a/test/auth/oauth_test.rb +++ b/test/auth/oauth_test.rb @@ -134,6 +134,62 @@ def test_begin_auth_private_app end end + def test_begin_auth_rejects_untrusted_shops + assert_raises(ShopifyAPI::Errors::InvalidShopError) do + ShopifyAPI::Auth::Oauth.begin_auth(shop: "example.com", redirect_path: "/redirect") + end + end + + def test_begin_auth_rejects_unified_admin_urls_with_invalid_handles + [ + "admin.shopify.com/store/bad.handle", + "https://admin.shopify.com/store/bad\\handle", + ].each do |shop| + assert_raises(ShopifyAPI::Errors::InvalidShopError, "expected #{shop.inspect} to be rejected") do + ShopifyAPI::Auth::Oauth.begin_auth(shop: shop, redirect_path: "/redirect") + end + end + end + + def test_begin_auth_normalizes_full_shop_urls + [ + "https://test-shop.myshopify.com", + "https://admin.shopify.com/store/test-shop", + ].each do |shop| + result = ShopifyAPI::Auth::Oauth.begin_auth(shop: shop, redirect_path: "/redirect") + verify_oauth_begin(auth_route: result[:auth_route], cookie: result[:cookie], is_online: true) + end + end + + def test_begin_auth_supports_my_shop_dev_without_dev_server + result = ShopifyAPI::Auth::Oauth.begin_auth(shop: "test-shop.my.shop.dev", redirect_path: "/redirect") + result_uri = URI.parse(result[:auth_route]) + + assert_equal("test-shop.my.shop.dev", result_uri.host) + assert_equal("/admin/oauth/authorize", result_uri.path) + end + + def test_begin_auth_supports_my_shop_dev_with_dev_server + dev_server = Class.new do + def initialize(name) + @name = name + end + + def host!(nonstandard_host_prefix:) + "#{nonstandard_host_prefix}.#{@name}.shop.dev" + end + end + Object.const_set(:DevServer, dev_server) + + result = ShopifyAPI::Auth::Oauth.begin_auth(shop: "test-shop.my.shop.dev", redirect_path: "/redirect") + result_uri = URI.parse(result[:auth_route]) + + assert_equal("admin.admin-web.shop.dev", result_uri.host) + assert_equal("/store/test-shop/oauth/authorize", result_uri.path) + ensure + Object.send(:remove_const, :DevServer) if Object.const_defined?(:DevServer, false) + end + def test_validate_auth_callback_offline modify_context(is_embedded: false, expiring_offline_access_tokens: false) diff --git a/test/auth_test.rb b/test/auth_test.rb index eea872714..bf607617d 100644 --- a/test/auth_test.rb +++ b/test/auth_test.rb @@ -18,6 +18,67 @@ def test_valid_host ) end + def test_supported_admin_hosts_are_unchanged + [ + "some-shopify-store.myshopify.com/admin", + "admin.shopify.com/store/some-shopify-store", + "admin.web.my-spin.user.us.spin.dev/store/shop1", + "shop1.my.shop.dev/admin", + "shop1.myshopify.io:3000/admin", + ].each do |host| + assert_equal( + "https://#{host}/apps/#{ShopifyAPI::Context.api_key}", + ShopifyAPI::Auth.embedded_app_url(Base64.strict_encode64(host)), + ) + end + end + + def test_unpadded_host_is_still_decoded + host = "admin.shopify.com/store/some-shop1" + encoded_host = Base64.strict_encode64(host).delete("=") + + refute_equal(Base64.strict_encode64(host), encoded_host) + assert_equal( + "https://#{host}/apps/#{ShopifyAPI::Context.api_key}", + ShopifyAPI::Auth.embedded_app_url(encoded_host), + ) + end + + def test_untrusted_hosts_are_rejected + [ + "example.com/admin", + "myshopify.com/admin", + "some-shopify-store.myshopify.com.example.com/admin", + ].each do |host| + assert_raises(ShopifyAPI::Errors::InvalidShopError, "expected #{host.inspect} to be rejected") do + ShopifyAPI::Auth.embedded_app_url(Base64.strict_encode64(host)) + end + end + end + + def test_invalid_embedded_host_forms_are_rejected + [ + "example.com%2F.myshopify.com/admin", + "https://some-shopify-store.myshopify.com/admin", + ].each do |host| + assert_raises(ShopifyAPI::Errors::InvalidShopError, "expected #{host.inspect} to be rejected") do + ShopifyAPI::Auth.embedded_app_url(Base64.strict_encode64(host)) + end + end + end + + def test_malformed_or_empty_hosts_are_rejected + [ + "some-shopify-store.myshopify.com/admin\n", + "some-shopify-store.myshopify.com/admin\xFF".b, + "", + ].each do |host| + assert_raises(ShopifyAPI::Errors::InvalidShopError, "expected #{host.inspect} to be rejected") do + ShopifyAPI::Auth.embedded_app_url(Base64.strict_encode64(host)) + end + end + end + def test_no_host assert_raises(ShopifyAPI::Errors::MissingRequiredArgumentError) do ShopifyAPI::Auth.embedded_app_url(nil) diff --git a/test/utils/shop_validator_test.rb b/test/utils/shop_validator_test.rb index 60c66a641..837356f41 100644 --- a/test/utils/shop_validator_test.rb +++ b/test/utils/shop_validator_test.rb @@ -93,6 +93,59 @@ def test_unified_admin_store_url_maps_to_myshopify_host ) end + def test_unified_admin_store_url_accepts_valid_handles + { + "https://admin.shopify.com/store/shop1" => "shop1.myshopify.com", + "https://admin.shopify.com/store/cool-shop/" => "cool-shop.myshopify.com", + "https://Admin.Shopify.com/store/Cool-Shop" => "cool-shop.myshopify.com", + "admin.shopify.com/store/1-shop" => "1-shop.myshopify.com", + "https://admin.shopify.com/store/shop1/apps/cool_app_hansel" => "shop1.myshopify.com", + "https://admin.web.my-spin.user.us.spin.dev/store/shop1" => "shop1.myshopify.com", + }.each do |shop, expected| + assert_equal(expected, ShopifyAPI::Utils::ShopValidator.sanitize!(shop)) + end + end + + def test_unified_admin_store_url_rejects_invalid_handles + [ + "https://admin.shopify.com/store/bad.handle", + "https://admin.shopify.com/store/bad_handle", + "https://admin.shopify.com/store/-bad-handle", + "https://admin.shopify.com/store/bad%2Ehandle", + "https://admin.shopify.com/store/bad\\handle", + "https://admin.shopify.com/store/bad:handle", + "https://admin.shopify.com/store/", + "https://admin.shopify.com/apps/cool-app", + "https://admin.shopify.com", + ].each do |shop| + assert_nil( + ShopifyAPI::Utils::ShopValidator.sanitize_shop_domain(shop), + "expected #{shop.inspect} to be rejected", + ) + assert_raises(ShopifyAPI::Errors::InvalidShopError, "expected #{shop.inspect} to be rejected") do + ShopifyAPI::Utils::ShopValidator.sanitize!(shop) + end + end + end + + def test_rejects_hosts_with_non_hostname_characters + [ + "bad%5Chost.myshopify.com", + "bad%2Fhost.myshopify.com", + "bad%40host.myshopify.com", + "bad;host.myshopify.com", + "bad,host.myshopify.com", + "bad!host.myshopify.com", + "bad+host.myshopify.com", + "bad=host.myshopify.com", + ].each do |shop| + assert_nil( + ShopifyAPI::Utils::ShopValidator.sanitize_shop_domain(shop), + "expected #{shop.inspect} to be rejected", + ) + end + end + def test_sanitize_shop_domain_with_custom_myshopify_domain assert_equal( "mystore.myshopify.com",