Skip to content

Commit

Permalink
allow trusted shopify domains when sanitizing (Shopify#1608)
Browse files Browse the repository at this point in the history
* allow trusted shopify domains when sanitizing

* better tests for trusting sanitized domains

* changelog
  • Loading branch information
nelsonwittwer authored and fabriazza committed Feb 1, 2023
1 parent 73a57db commit e334ef2
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
10 changes: 9 additions & 1 deletion lib/shopify_app/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions test/shopify_app/utils_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down

0 comments on commit e334ef2

Please sign in to comment.