Merge pull request #985 from Shopify/clear-stale-ids

Clear stale session IDs in callback
Shopify · May 14, 2020 · 61f68da · 61f68da
2 parents b325381 + fe90ddc
commit 61f68da
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 5 deletions.
diff --git a/app/controllers/shopify_app/callback_controller.rb b/app/controllers/shopify_app/callback_controller.rb
@@ -92,9 +92,11 @@ def set_shopify_session
 
       session[:shopify_user] = associated_user
       if session[:shopify_user].present?
+        session[:shop_id] = nil if shop_session && shop_session.domain != shop_name
         session[:user_id] = ShopifyApp::SessionRepository.store_user_session(session_store, associated_user)
       else
         session[:shop_id] = ShopifyApp::SessionRepository.store_shop_session(session_store)
+        session[:user_id] = nil if user_session && user_session.domain != shop_name
       end
       session[:shopify_domain] = shop_name
       session[:user_session] = auth_hash&.extra&.session

diff --git a/test/controllers/callback_controller_test.rb b/test/controllers/callback_controller_test.rb
@@ -66,6 +66,66 @@ class CallbackControllerTest < ActionController::TestCase
       assert_nil session[:shopify_user]
     end
 
+    test '#callback keeps the user_id if shop session is for the same shop' do
+      mock_shopify_omniauth
+      session[:user_id] = 'valid-user-id'
+      user_shop_session = ShopifyAPI::Session.new(
+        domain: TEST_SHOPIFY_DOMAIN,
+        token: '1234',
+        api_version: nil,
+      )
+      ShopifyApp::SessionRepository.stubs(:retrieve_user_session).with('valid-user-id').returns(user_shop_session)
+
+      get :callback, params: { shop: 'shop' }
+      assert_equal 'valid-user-id', session[:user_id]
+      assert_not_nil session[:shop_id]
+    end
+
+    test '#callback clears stale user_id if shop session is for a different shop' do
+      mock_shopify_omniauth
+      session[:user_id] = 'valid-user-id'
+      user_shop_session = ShopifyAPI::Session.new(
+        domain: 'other-shop.myshopify.io',
+        token: '1234',
+        api_version: nil,
+      )
+      ShopifyApp::SessionRepository.stubs(:retrieve_user_session).with('valid-user-id').returns(user_shop_session)
+
+      get :callback, params: { shop: 'shop' }
+      assert_nil session[:user_id]
+      assert_not_nil session[:shop_id]
+    end
+
+    test '#callback keeps shop_id if user session is for the same shop' do
+      mock_shopify_user_omniauth
+      session[:shop_id] = 'valid-shop-id'
+      shop_session = ShopifyAPI::Session.new(
+        domain: TEST_SHOPIFY_DOMAIN,
+        token: '1234',
+        api_version: nil,
+      )
+      ShopifyApp::SessionRepository.stubs(:retrieve_shop_session).with('valid-shop-id').returns(shop_session)
+
+      get :callback, params: { shop: 'shop' }
+      assert_not_nil session[:user_id]
+      assert_equal 'valid-shop-id', session[:shop_id]
+    end
+
+    test '#callback clears a stale shop_id if user session is for a different shop' do
+      mock_shopify_user_omniauth
+      session[:shop_id] = 'valid-shop-id'
+      other_shop_session = ShopifyAPI::Session.new(
+        domain: 'other-domain.myshopify.io',
+        token: '1234',
+        api_version: nil,
+      )
+      ShopifyApp::SessionRepository.stubs(:retrieve_shop_session).with('valid-shop-id').returns(other_shop_session)
+
+      get :callback, params: { shop: 'shop' }
+      assert_not_nil session[:user_id]
+      assert_nil session[:shop_id]
+    end
+
     test '#callback sets up a shopify session with a user for online mode' do
       mock_shopify_user_omniauth
 
@@ -149,15 +209,14 @@ class CallbackControllerTest < ActionController::TestCase
     end
 
     test '#install_webhooks still uses the shop token for user strategy' do
-      shop_session = ShopifyAPI::Session.new(domain: 'shop', token: '1234', api_version: '2019-1')
-      ShopifyApp::SessionRepository.expects(:retrieve_shop_session).returns(shop_session)
-      user_session = ShopifyAPI::Session.new(domain: 'shop', token: '4321', api_version: '2019-1')
-      ShopifyApp::SessionRepository.expects(:retrieve_user_session).returns(user_session)
+      shop_session = ShopifyAPI::Session.new(domain: 'shop', token: '4321', api_version: '2019-1')
+      ShopifyApp::SessionRepository.stubs(:retrieve_shop_session).with('135').returns(shop_session)
+
       ShopifyApp.configure do |config|
         config.webhooks = [{ topic: 'carts/update', address: 'example-app.com/webhooks' }]
       end
 
-      ShopifyApp::WebhooksManager.expects(:queue).with(TEST_SHOPIFY_DOMAIN, '1234', ShopifyApp.configuration.webhooks)
+      ShopifyApp::WebhooksManager.expects(:queue).with(TEST_SHOPIFY_DOMAIN, '4321', ShopifyApp.configuration.webhooks)
 
       session[:shop_id] = '135'
       mock_shopify_user_omniauth