diff --git a/CHANGELOG.md b/CHANGELOG.md index 99ca540e7..cf7e6ec5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Note: For changes to the API, see https://shopify.dev/changelog?filter=api ## Unreleased - Add support for 2026-04 API version +- Add shop validation ## 16.1.0 (2026-01-15) - Add support for 2026-01 API version diff --git a/lib/shopify_api/auth/client_credentials.rb b/lib/shopify_api/auth/client_credentials.rb index 5775c9686..1ab1c8964 100644 --- 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 index 0d52fd032..d6a3eb908 100644 --- 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 index 4741ae0d8..47f036b4f 100644 --- a/lib/shopify_api/auth/token_exchange.rb +++ b/lib/shopify_api/auth/token_exchange.rb @@ -36,10 +36,11 @@ def exchange_token(shop:, session_token:, requested_token_type:) raise ShopifyAPI::Errors::UnsupportedOauthError, "Cannot perform OAuth Token Exchange for non embedded apps." unless ShopifyAPI::Context.embedded? - # Validate the session token content - ShopifyAPI::Auth::JwtPayload.new(session_token) + # Validate the session token and use the shop from the token's `dest` claim + jwt_payload = ShopifyAPI::Auth::JwtPayload.new(session_token) + dest_shop = jwt_payload.shop - shop_session = ShopifyAPI::Auth::Session.new(shop: shop) + shop_session = ShopifyAPI::Auth::Session.new(shop: dest_shop) body = { client_id: ShopifyAPI::Context.api_key, client_secret: ShopifyAPI::Context.api_secret_key, @@ -74,7 +75,7 @@ def exchange_token(shop:, session_token:, requested_token_type:) session_params = T.cast(response.body, T::Hash[String, T.untyped]).to_h Session.from( - shop: shop, + shop: dest_shop, access_token_response: Oauth::AccessTokenResponse.from_hash(session_params), ) end @@ -91,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, @@ -120,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 index 671b3f2f9..28f8e5c86 100644 --- 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 new file mode 100644 index 000000000..277dc2ea8 --- /dev/null +++ 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 new file mode 100644 index 000000000..a077842d5 --- /dev/null +++ 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 index 29e26f049..b5c6995d8 100644 --- 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 index 9d5a7391d..6b1fd8310 100644 --- 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 index c3989bf8b..36c9e5631 100644 --- a/test/auth/token_exchange_test.rb +++ b/test/auth/token_exchange_test.rb @@ -174,6 +174,39 @@ def test_exchange_token_offline_token assert_equal(expected_session, session) end + def test_exchange_token_uses_shop_from_session_token_dest_claim + modify_context(is_embedded: true, expiring_offline_access_tokens: false) + + # Pass a different shop than what's in the JWT dest claim + different_shop = "other-shop.myshopify.com" + + # The request should go to the shop from the JWT dest claim, not the passed shop + stub_request(:post, "https://#{@shop}/admin/oauth/access_token") + .with(body: @non_expiring_offline_token_exchange_request) + .to_return(body: @offline_token_response.to_json, headers: { content_type: "application/json" }) + + expected_session = ShopifyAPI::Auth::Session.new( + id: "offline_#{@shop}", + shop: @shop, + access_token: @offline_token_response[:access_token], + scope: @offline_token_response[:scope], + is_online: false, + expires: nil, + shopify_session_id: @offline_token_response[:session], + refresh_token: nil, + refresh_token_expires: nil, + ) + + session = ShopifyAPI::Auth::TokenExchange.exchange_token( + shop: different_shop, + session_token: @session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, + ) + + assert_equal(expected_session, session) + assert_equal(@shop, session.shop) + end + def test_exchange_token_expiring_offline_token modify_context(is_embedded: true, expiring_offline_access_tokens: true) stub_request(:post, "https://#{@shop}/admin/oauth/access_token") @@ -229,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 index 2df7d3a57..9f6c10c3d 100644 --- 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 new file mode 100644 index 000000000..41cf9bcdd --- /dev/null +++ b/test/utils/shop_validator_test.rb @@ -0,0 +1,69 @@ +# 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 + result = ShopifyAPI::Utils::ShopValidator.validate!(" test-shop.myshopify.com ") + assert_equal("test-shop.myshopify.com", result) + 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