Merge pull request #71 from Shopify/get-shop-params-from-session

Get shop from session by default, fallback to shop param

lib/omniauth/shopify/version.rb

@@ -1,5 +1,5 @@
 module OmniAuth
   module Shopify
-    VERSION = "1.2.1"
+    VERSION = "2.0.0"
   end
 end

lib/omniauth/strategies/shopify.rb

@@ -23,8 +23,16 @@ class Shopify < OmniAuth::Strategies::OAuth2
       option :per_user_permissions, false
 
       option :setup, proc { |env|
-        request = Rack::Request.new(env)
-        env['omniauth.strategy'].options[:client_options][:site] = "https://#{request.GET['shop']}"
+        strategy = env['omniauth.strategy']
+
+        shopify_auth_params = strategy.session['shopify.omniauth_params'] && strategy.session['shopify.omniauth_params'].with_indifferent_access
+        shop = if shopify_auth_params && shopify_auth_params['shop']
+          "https://#{shopify_auth_params['shop']}"
+        else
+          ''
+        end
+
+        strategy.options[:client_options][:site] = shop
       }
 
       uid { URI.parse(options[:client_options][:site]).host }

omniauth-shopify-oauth2.gemspec

@@ -18,6 +18,7 @@ Gem::Specification.new do |s|
   s.required_ruby_version = '>= 2.1.9'
 
   s.add_runtime_dependency 'omniauth-oauth2', '~> 1.5.0'
+  s.add_runtime_dependency 'activesupport'
 
   s.add_development_dependency 'minitest', '~> 5.6'
   s.add_development_dependency 'fakeweb', '~> 1.3'

test/integration_test.rb

@@ -34,7 +34,7 @@ def test_authorize_overrides_site_with_https_scheme
       env['omniauth.strategy'].options[:client_options][:site] = "http://#{params['shop']}"
     }
 
-    response = authorize('snowdevil.myshopify.com')
+    response = request.get('https://app.example.com/auth/shopify?shop=snowdevil.myshopify.com')
     assert_match %r{\A#{Regexp.quote(shopify_authorize_url)}}, response.location
   end
 
@@ -48,6 +48,7 @@ def test_site_validation
       'user@snowdevil.myshopify.com',   # shop contains user
       'snowdevil.myshopify.com:22',     # shop contains port
     ].each do |shop, valid|
+      @shop = shop
       response = authorize(shop)
       assert_auth_failure(response, 'invalid_site')
 
@@ -133,7 +134,10 @@ def test_callback_rejects_body_params
 
     response = request.get("https://app.example.com/auth/shopify/callback?#{Rack::Utils.build_query(params)}",
                            input: body,
-                           "CONTENT_TYPE" => 'application/x-www-form-urlencoded')
+                           "CONTENT_TYPE" => 'application/x-www-form-urlencoded',
+                           'rack.session' => {
+                              'shopify.omniauth_params' => { shop: 'snowdevil.myshopify.com' }
+                            })
 
     assert_auth_failure(response, 'invalid_signature')
   end
@@ -148,25 +152,33 @@ def test_provider_options
                 env['omniauth.strategy'].options[:client_options][:site] = "https://#{shop}"
               }
 
-    response = authorize('snowdevil')
+    response = request.get("https://app.example.com/auth/shopify?shop=snowdevil.myshopify.dev:3000")
     assert_equal 302, response.status
     assert_match %r{\A#{Regexp.quote("https://snowdevil.myshopify.dev:3000/admin/oauth/authorize?")}}, response.location
     redirect_params = Rack::Utils.parse_query(URI(response.location).query)
     assert_equal 'read_products,read_orders,write_content', redirect_params['scope']
     assert_equal 'https://app.example.com/admin/auth/legacy/callback', redirect_params['redirect_uri']
   end
 
+  def test_default_setup_reads_shop_from_session
+    build_app
+    response = authorize('snowdevil.myshopify.com')
+    assert_equal 302, response.status
+    assert_match %r{\A#{Regexp.quote("https://snowdevil.myshopify.com/admin/oauth/authorize?")}}, response.location
+    redirect_params = Rack::Utils.parse_query(URI(response.location).query)
+    assert_equal 'https://app.example.com/auth/shopify/callback', redirect_params['redirect_uri']
+  end
+
   def test_unnecessary_read_scopes_are_removed
     build_app scope: 'read_content,read_products,write_products',
               callback_path: '/admin/auth/legacy/callback',
               myshopify_domain: 'myshopify.dev:3000',
               setup: lambda { |env|
                 shop = Rack::Request.new(env).GET['shop']
-                shop += ".myshopify.dev:3000" unless shop.include?(".")
                 env['omniauth.strategy'].options[:client_options][:site] = "https://#{shop}"
               }
 
-    response = authorize('snowdevil')
+    response = request.get("https://app.example.com/auth/shopify?shop=snowdevil.myshopify.dev:3000")
     assert_equal 302, response.status
     redirect_params = Rack::Utils.parse_query(URI(response.location).query)
     assert_equal 'read_content,write_products', redirect_params['scope']
@@ -345,11 +357,17 @@ def build_app(options={})
     @app = Rack::Session::Cookie.new(app, secret: SecureRandom.hex(64))
   end
 
+  def shop
+    @shop ||= 'snowdevil.myshopify.com'
+  end
+
   def authorize(shop)
-    request.get("https://app.example.com/auth/shopify?shop=#{CGI.escape(shop)}", opts)
+    @opts['rack.session']['shopify.omniauth_params'] = { shop: shop }
+    request.get('https://app.example.com/auth/shopify', opts)
   end
 
   def callback(params)
+    @opts['rack.session']['shopify.omniauth_params'] = { shop: shop }
     request.get("https://app.example.com/auth/shopify/callback?#{Rack::Utils.build_query(params)}", opts)
   end
 

test/test_helper.rb

@@ -5,6 +5,7 @@
 require 'minitest/autorun'
 require 'fakeweb'
 require 'json'
+require 'active_support/core_ext/hash'
 
 OmniAuth.config.logger = Logger.new(nil)
 FakeWeb.allow_net_connect = false