From e334ef2432ba11642f2bac10b06531d021a6445a Mon Sep 17 00:00:00 2001 From: Nelson Date: Mon, 12 Dec 2022 13:27:22 -0500 Subject: [PATCH] allow trusted shopify domains when sanitizing (#1608) * allow trusted shopify domains when sanitizing * better tests for trusting sanitized domains * changelog --- CHANGELOG.md | 4 ++++ lib/shopify_app/utils.rb | 10 +++++++++- test/shopify_app/utils_test.rb | 7 +++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c382219bb..4798f7e66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ Unreleased ---------- +21.3.1 (Dec 12, 2022) +---------- +* Fix bug with stores using the new unified admin that were falsely being flagged as phishing attempts [#1608](https://github.com/Shopify/shopify_app/pull/1608) + 21.3.0 (Dec 9, 2022) ---------- * Move covered scopes check into user access strategy [#1600](https://github.com/Shopify/shopify_app/pull/1600) diff --git a/lib/shopify_app/utils.rb b/lib/shopify_app/utils.rb index c5044b0b4..e94465000 100644 --- a/lib/shopify_app/utils.rb +++ b/lib/shopify_app/utils.rb @@ -2,14 +2,22 @@ module ShopifyApp module Utils + TRUSTED_SHOPIFY_DOMAINS = [ + "shopify.com", + "myshopify.io", + "myshopify.com", + ].freeze + def self.sanitize_shop_domain(shop_domain) myshopify_domain = ShopifyApp.configuration.myshopify_domain name = shop_domain.to_s.downcase.strip name += ".#{myshopify_domain}" if !name.include?(myshopify_domain.to_s) && !name.include?(".") name.sub!(%r|https?://|, "") + trusted_domains = TRUSTED_SHOPIFY_DOMAINS.dup.push(myshopify_domain) u = URI("http://#{name}") - u.host if u.host&.match(/^[a-z0-9][a-z0-9\-]*[a-z0-9]\.#{Regexp.escape(myshopify_domain)}$/) + regex = /^[a-z0-9][a-z0-9\-]*[a-z0-9]\.(#{trusted_domains.join("|")})$/ + u.host if u.host&.match(regex) rescue URI::InvalidURIError nil end diff --git a/test/shopify_app/utils_test.rb b/test/shopify_app/utils_test.rb index a95520959..a463d06ec 100644 --- a/test/shopify_app/utils_test.rb +++ b/test/shopify_app/utils_test.rb @@ -26,6 +26,13 @@ class UtilsTest < ActiveSupport::TestCase assert ShopifyApp::Utils.sanitize_shop_domain("MY-shop.myshopify.com") end + test "unified admin is still trusted as a sanitzed domain" do + ShopifyApp.configuration.stubs(:myshpoify_domain).returns("totally.cool.domain.com") + assert ShopifyApp::Utils.sanitize_shop_domain("admin.shopify.com/some_shoppe_over_the_rainbow") + assert ShopifyApp::Utils.sanitize_shop_domain("some-shoppe-over-the-rainbow.myshopify.com") + assert ShopifyApp::Utils.sanitize_shop_domain("some-shoppe-over-the-rainbow.myshopify.io") + end + ["myshop.com", "myshopify.com", "shopify.com", "two words", "store.myshopify.com.evil.com", "/foo/bar", "foo.myshopify.io.evil.ru", "%0a123.myshopify.io", "foo.bar.myshopify.io",].each do |bad_url|