Always validate shop domain

gonzaloriestra · gonzaloriestra · commit a31c59dc27c8 · 2026-04-09T13:02:35.000+02:00
diff --git a/lib/shopify_api/auth/client_credentials.rb b/lib/shopify_api/auth/client_credentials.rb
@@ -22,7 +22,8 @@ def client_credentials(shop:)
               "ShopifyAPI::Context not setup, please call ShopifyAPI::Context.setup"
           end
 
-          shop_session = ShopifyAPI::Auth::Session.new(shop: shop)
+          validated_shop = Utils::ShopValidator.validate!(shop)
+          shop_session = ShopifyAPI::Auth::Session.new(shop: validated_shop)
           body = {
             client_id: ShopifyAPI::Context.api_key,
             client_secret: ShopifyAPI::Context.api_secret_key,
@@ -42,7 +43,7 @@ def client_credentials(shop:)
           response_hash = T.cast(response.body, T::Hash[String, T.untyped]).to_h
 
           Session.from(
-            shop: shop,
+            shop: validated_shop,
             access_token_response: Oauth::AccessTokenResponse.from_hash(response_hash),
           )
         end
diff --git a/lib/shopify_api/auth/refresh_token.rb b/lib/shopify_api/auth/refresh_token.rb
@@ -21,7 +21,8 @@ def refresh_access_token(shop:, refresh_token:)
               "ShopifyAPI::Context not setup, please call ShopifyAPI::Context.setup"
           end
 
-          shop_session = ShopifyAPI::Auth::Session.new(shop:)
+          validated_shop = Utils::ShopValidator.validate!(shop)
+          shop_session = ShopifyAPI::Auth::Session.new(shop: validated_shop)
           body = {
             client_id: ShopifyAPI::Context.api_key,
             client_secret: ShopifyAPI::Context.api_secret_key,
@@ -47,7 +48,7 @@ def refresh_access_token(shop:, refresh_token:)
           session_params = T.cast(response.body, T::Hash[String, T.untyped]).to_h
 
           Session.from(
-            shop:,
+            shop: validated_shop,
             access_token_response: Oauth::AccessTokenResponse.from_hash(session_params),
           )
         end
diff --git a/lib/shopify_api/auth/token_exchange.rb b/lib/shopify_api/auth/token_exchange.rb
@@ -92,7 +92,8 @@ def migrate_to_expiring_token(shop:, non_expiring_offline_token:)
               "ShopifyAPI::Context not setup, please call ShopifyAPI::Context.setup"
           end
 
-          shop_session = ShopifyAPI::Auth::Session.new(shop: shop)
+          validated_shop = Utils::ShopValidator.validate!(shop)
+          shop_session = ShopifyAPI::Auth::Session.new(shop: validated_shop)
           body = {
             client_id: ShopifyAPI::Context.api_key,
             client_secret: ShopifyAPI::Context.api_secret_key,
@@ -121,7 +122,7 @@ def migrate_to_expiring_token(shop:, non_expiring_offline_token:)
           session_params = T.cast(response.body, T::Hash[String, T.untyped]).to_h
 
           Session.from(
-            shop: shop,
+            shop: validated_shop,
             access_token_response: Oauth::AccessTokenResponse.from_hash(session_params),
           )
         end
diff --git a/lib/shopify_api/clients/graphql/storefront.rb b/lib/shopify_api/clients/graphql/storefront.rb
@@ -19,9 +19,10 @@ def initialize(shop, private_token: nil, public_token: nil, api_version: nil)
             raise ArgumentError, "Storefront client requires either private_token or public_token to be provided"
           end
 
+          validated_shop = Utils::ShopValidator.validate!(shop)
           session = Auth::Session.new(
-            id: shop,
-            shop: shop,
+            id: validated_shop,
+            shop: validated_shop,
             access_token: "",
             is_online: false,
           )
diff --git a/lib/shopify_api/errors/invalid_shop_error.rb b/lib/shopify_api/errors/invalid_shop_error.rb
@@ -0,0 +1,9 @@
+# typed: strict
+# frozen_string_literal: true
+
+module ShopifyAPI
+  module Errors
+    class InvalidShopError < StandardError
+    end
+  end
+end
diff --git a/lib/shopify_api/utils/shop_validator.rb b/lib/shopify_api/utils/shop_validator.rb
@@ -0,0 +1,31 @@
+# typed: strict
+# frozen_string_literal: true
+
+module ShopifyAPI
+  module Utils
+    class ShopValidator
+      extend T::Sig
+
+      SHOPIFY_OWNED_SUFFIXES = T.let([
+        ".myshopify.com",
+        ".myshopify.io",
+      ].freeze, T::Array[String])
+
+      class << self
+        extend T::Sig
+
+        sig { params(shop: String).returns(String) }
+        def validate!(shop)
+          cleaned = shop.to_s.strip.downcase.gsub(%r{\A(https?://)?}, "").gsub(%r{/\z}, "")
+
+          if cleaned.empty? || !SHOPIFY_OWNED_SUFFIXES.any? { |suffix| cleaned.end_with?(suffix) }
+            raise Errors::InvalidShopError,
+              "shop must end with one of #{SHOPIFY_OWNED_SUFFIXES.join(", ")}, got: #{shop.inspect}"
+          end
+
+          cleaned
+        end
+      end
+    end
+  end
+end
diff --git a/test/auth/client_credentials_test.rb b/test/auth/client_credentials_test.rb
@@ -30,6 +30,12 @@ def test_client_credentials_context_not_setup
         end
       end
 
+      def test_client_credentials_rejects_non_shopify_domain
+        assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+          ShopifyAPI::Auth::ClientCredentials.client_credentials(shop: "attacker.example")
+        end
+      end
+
       def test_client_credentials_offline_token
         stub_request(:post, "https://#{@shop}/admin/oauth/access_token")
           .with(body: @client_credentials_request)
diff --git a/test/auth/refresh_token_test.rb b/test/auth/refresh_token_test.rb
@@ -29,6 +29,15 @@ def setup
         }
       end
 
+      def test_refresh_access_token_rejects_non_shopify_domain
+        assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+          ShopifyAPI::Auth::RefreshToken.refresh_access_token(
+            shop: "attacker.example",
+            refresh_token: @refresh_token,
+          )
+        end
+      end
+
       def test_refresh_access_token_success
         stub_request(:post, "https://#{@shop}/admin/oauth/access_token")
           .with(body: @refresh_token_request)
diff --git a/test/auth/token_exchange_test.rb b/test/auth/token_exchange_test.rb
@@ -262,6 +262,15 @@ def test_exchange_token_online_token
         assert_equal(expected_session, session)
       end
 
+      def test_migrate_to_expiring_token_rejects_non_shopify_domain
+        assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+          ShopifyAPI::Auth::TokenExchange.migrate_to_expiring_token(
+            shop: "attacker.example",
+            non_expiring_offline_token: "old-offline-token-123",
+          )
+        end
+      end
+
       def test_migrate_to_expiring_token_context_not_setup
         modify_context(api_key: "", api_secret_key: "", host: "")
 
diff --git a/test/clients/graphql/storefront_test.rb b/test/clients/graphql/storefront_test.rb
@@ -32,6 +32,12 @@ def build_client
           end
         end
 
+        def test_rejects_non_shopify_domain
+          assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+            ShopifyAPI::Clients::Graphql::Storefront.new("attacker.example", public_token: "token")
+          end
+        end
+
         def test_can_query_using_private_token
           query = <<~QUERY
             {
diff --git a/test/utils/shop_validator_test.rb b/test/utils/shop_validator_test.rb
@@ -0,0 +1,68 @@
+# typed: false
+# frozen_string_literal: true
+
+require_relative "../test_helper"
+
+module ShopifyAPITest
+  module Utils
+    class ShopValidatorTest < Test::Unit::TestCase
+      def test_accepts_valid_myshopify_com_domain
+        assert_equal("test-shop.myshopify.com", ShopifyAPI::Utils::ShopValidator.validate!("test-shop.myshopify.com"))
+      end
+
+      def test_accepts_valid_myshopify_io_domain
+        assert_equal("test-shop.myshopify.io", ShopifyAPI::Utils::ShopValidator.validate!("test-shop.myshopify.io"))
+      end
+
+      def test_strips_https_scheme
+        assert_equal("test-shop.myshopify.com", ShopifyAPI::Utils::ShopValidator.validate!("https://test-shop.myshopify.com"))
+      end
+
+      def test_strips_http_scheme
+        assert_equal("test-shop.myshopify.com", ShopifyAPI::Utils::ShopValidator.validate!("http://test-shop.myshopify.com"))
+      end
+
+      def test_strips_trailing_slash
+        assert_equal("test-shop.myshopify.com", ShopifyAPI::Utils::ShopValidator.validate!("test-shop.myshopify.com/"))
+      end
+
+      def test_normalizes_to_lowercase
+        assert_equal("test-shop.myshopify.com", ShopifyAPI::Utils::ShopValidator.validate!("Test-Shop.MyShopify.com"))
+      end
+
+      def test_strips_whitespace
+        assert_equal("test-shop.myshopify.com", ShopifyAPI::Utils::ShopValidator.validate!("  test-shop.myshopify.com  "))
+      end
+
+      def test_rejects_attacker_controlled_domain
+        assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+          ShopifyAPI::Utils::ShopValidator.validate!("attacker.example")
+        end
+      end
+
+      def test_rejects_empty_string
+        assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+          ShopifyAPI::Utils::ShopValidator.validate!("")
+        end
+      end
+
+      def test_rejects_non_shopify_domain
+        assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+          ShopifyAPI::Utils::ShopValidator.validate!("evil.com")
+        end
+      end
+
+      def test_rejects_shopify_suffix_as_subdomain_of_attacker
+        assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+          ShopifyAPI::Utils::ShopValidator.validate!("myshopify.com.evil.com")
+        end
+      end
+
+      def test_rejects_similar_looking_domain
+        assert_raises(ShopifyAPI::Errors::InvalidShopError) do
+          ShopifyAPI::Utils::ShopValidator.validate!("test-shop.notmyshopify.com")
+        end
+      end
+    end
+  end
+end
