Fixing online token creation

CHANGELOG.md

@@ -1,3 +1,7 @@
+Unreleased
+----------
+* Fix regression in apps using online tokens. [#1413](https://github.com/Shopify/shopify_app/pull/1413)
+
 19.0.1 (April 11, 2022)
 ----------
 * Bump Shopify API (https://github.com/Shopify/shopify_api) to version 10.0.2. This update includes patch fixes since the initial v10 release.

app/controllers/shopify_app/callback_controller.rb

@@ -27,6 +27,12 @@ def callback
         value: auth_result[:cookie].value,
       }
 
+      session[:shopify_user_id] = auth_result[:session].associated_user.id if auth_result[:session].online?
+
+      if start_user_token_flow?(auth_result[:session])
+        return respond_with_user_token_flow
+      end
+
       perform_post_authenticate_jobs(auth_result[:session])
 
       respond_successfully
@@ -43,6 +49,25 @@ def respond_with_error
       redirect_to(login_url_with_optional_shop)
     end
 
+    def respond_with_user_token_flow
+      redirect_to(login_url_with_optional_shop)
+    end
+
+    def start_user_token_flow?(shopify_session)
+      return false unless ShopifyApp::SessionRepository.user_storage.present?
+      return false if shopify_session.online?
+      update_user_access_scopes?(shopify_session)
+    end
+
+    def update_user_access_scopes?(shopify_session)
+      return true if session[:shopify_user_id].nil?
+      user_access_scopes_strategy.update_access_scopes?(shopify_user_id: session[:shopify_user_id])
+    end
+
+    def user_access_scopes_strategy
+      ShopifyApp.configuration.user_access_scopes_strategy
+    end
+
     def perform_post_authenticate_jobs(session)
       install_webhooks(session)
       install_scripttags(session)

lib/shopify_app/controller_concerns/login_protection.rb

@@ -232,7 +232,13 @@ def session_shop_conflicts_with_params
       current_shopify_session && params[:shop].is_a?(String) && current_shopify_session.shop != params[:shop]
     end
 
+    def shop_session
+      ShopifyApp::SessionRepository.retrieve_shop_session_by_shopify_domain(sanitize_shop_param(params))
+    end
+
     def user_session_expected?
+      return false if shop_session.nil?
+      return false if ShopifyApp.configuration.shop_access_scopes_strategy.update_access_scopes?(shop_session.shop)
       !ShopifyApp.configuration.user_session_repository.blank? && ShopifyApp::SessionRepository.user_storage.present?
     end
   end

lib/shopify_app/session/session_repository.rb

@@ -46,7 +46,7 @@ def user_storage
       # ShopifyAPI::Auth::SessionStorage override
       def store_session(session)
         if session.online?
-          user_storage.store(session, session.associated_user.id.to_s)
+          user_storage.store(session, session.associated_user)
         else
           shop_storage.store(session)
         end

lib/shopify_app/session/user_session_storage_with_scopes.rb

@@ -11,7 +11,7 @@ module UserSessionStorageWithScopes
 
     class_methods do
       def store(auth_session, user)
-        user = find_or_initialize_by(shopify_user_id: user[:id])
+        user = find_or_initialize_by(shopify_user_id: user.id)
         user.shopify_token = auth_session.access_token
         user.shopify_domain = auth_session.shop
         user.access_scopes = auth_session.scope.to_s

test/controllers/sessions_controller_test.rb

@@ -97,11 +97,11 @@ class SessionsControllerTest < ActionController::TestCase
       assert_redirected_to "/auth-route"
     end
 
-    test "#new starts OAuth requesting online token if user session is expected" do
+    test "#new starts OAuth requesting offline token if user session is expected but there is no shop session" do
       ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore
 
       ShopifyAPI::Auth::Oauth.expects(:begin_auth)
-        .with(shop: "my-shop.myshopify.com", redirect_path: "/auth/shopify/callback", is_online: true)
+        .with(shop: "my-shop.myshopify.com", redirect_path: "/auth/shopify/callback", is_online: false)
         .returns({
           cookie: ShopifyAPI::Auth::Oauth::SessionCookie.new(value: "", expires: Time.now),
           auth_route: "/auth-route",
@@ -110,6 +110,30 @@ class SessionsControllerTest < ActionController::TestCase
       get :new, params: { shop: "my-shop", top_level: true }
     end
 
+    test "#new starts OAuth requesting online token if user session is expected and there is a shop session" do
+      shop = "my-shop.myshopify.com"
+
+      mock_session = mock
+      mock_session.stubs(:shop).returns(shop)
+      mock_session.stubs(:access_token).returns("a-new-user_token!")
+      mock_session.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new("read_products,write_orders"))
+
+      ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore
+      ShopifyApp::SessionRepository.shop_storage = ShopifyApp::InMemoryShopSessionStore
+      ShopifyApp::SessionRepository.shop_storage.stubs(:retrieve_by_shopify_domain)
+        .with(shop)
+        .returns(mock_session)
+
+      ShopifyAPI::Auth::Oauth.expects(:begin_auth)
+        .with(shop: shop, redirect_path: "/auth/shopify/callback", is_online: true)
+        .returns({
+          cookie: ShopifyAPI::Auth::Oauth::SessionCookie.new(value: "", expires: Time.now),
+          auth_route: "/auth-route",
+        })
+
+      get :new, params: { shop: shop, top_level: true }
+    end
+
     test "#new starts OAuth requesting online token if user session is unexpected" do
       ShopifyAPI::Auth::Oauth.expects(:begin_auth)
         .with(shop: "my-shop.myshopify.com", redirect_path: "/auth/shopify/callback", is_online: false)

test/shopify_app/controller_concerns/login_protection_test.rb

@@ -60,6 +60,11 @@ class LoginProtectionControllerTest < ActionController::TestCase
   end
 
   test "#current_shopify_session loads online session if user session expected" do
+    shop = "my-shop.myshopify.com"
+    ShopifyApp::SessionRepository.shop_storage.stubs(:retrieve_by_shopify_domain)
+      .with(shop)
+      .returns(mock_session(shop))
+
     cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] = "cookie"
     request.headers["HTTP_AUTHORIZATION"] = "Bearer token"
 
@@ -72,7 +77,7 @@ class LoginProtectionControllerTest < ActionController::TestCase
       .returns(nil)
 
     with_application_test_routes do
-      get :index
+      get :index, params: { shop: shop }
     end
   end
 
@@ -161,6 +166,10 @@ class LoginProtectionControllerTest < ActionController::TestCase
   end
 
   test "#current_shopify_session redirects to login if the loaded session doesn't have enough scope" do
+    shop = "my-shop.myshopify.com"
+    ShopifyApp::SessionRepository.shop_storage.stubs(:retrieve_by_shopify_domain)
+      .with(shop)
+      .returns(mock_session(shop))
     ShopifyAPI::Context.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new(["scope1", "scope2"]))
 
     cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] = "cookie"
@@ -173,18 +182,22 @@ class LoginProtectionControllerTest < ActionController::TestCase
         is_online: true
       )
       .returns(
-        ShopifyAPI::Auth::Session.new(shop: "some-shop", scope: ["scope1"])
+        ShopifyAPI::Auth::Session.new(shop: shop, scope: ["scope1"])
       )
 
     with_application_test_routes do
-      get :index
+      get :index, params: { shop: shop }
 
-      assert_redirected_to "/login"
+      assert_redirected_to "/login?shop=my-shop.myshopify.com"
       assert_nil cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME]
     end
   end
 
   test "#current_shopify_session does not redirect when sufficient scope" do
+    shop = "my-shop.myshopify.com"
+    ShopifyApp::SessionRepository.shop_storage.stubs(:retrieve_by_shopify_domain)
+      .with(shop)
+      .returns(mock_session(shop))
     ShopifyAPI::Context.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new(["scope1"]))
 
     cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] = "cookie"
@@ -201,7 +214,7 @@ class LoginProtectionControllerTest < ActionController::TestCase
       )
 
     with_application_test_routes do
-      get :index
+      get :index, params: { shop: shop }
       assert_response :ok
       assert_equal "cookie", cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME]
     end
@@ -440,4 +453,26 @@ def with_custom_login_url(url)
   ensure
     ShopifyApp.configure { |config| config.login_url = original_url }
   end
+
+  def mock_session(shop = "my-shop.myshopify.com")
+    mock_session = mock
+    mock_session.stubs(:shop).returns(shop)
+    mock_session.stubs(:access_token).returns("a-new-user_token!")
+    mock_session.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new("read_products,write_orders"))
+
+    mock_session
+  end
+
+  def mock_associated_user
+    ShopifyAPI::Auth::AssociatedUser.new(
+      id: 100,
+      first_name: "John",
+      last_name: "Doe",
+      email: "johndoe@email.com",
+      email_verified: true,
+      account_owner: false,
+      locale: "en",
+      collaborator: true
+    )
+  end
 end

test/shopify_app/session/session_repository_test.rb

@@ -106,7 +106,7 @@ class SessionRepositoryTest < ActiveSupport::TestCase
       user = mock_associated_user
       session.stubs(:associated_user).returns(user)
 
-      InMemoryUserSessionStore.expects(:store).with(session, user.id.to_s)
+      InMemoryUserSessionStore.expects(:store).with(session, user)
 
       SessionRepository.store_session(session)
     end

test/shopify_app/session/shop_session_storage_test.rb

@@ -49,6 +49,7 @@ class ShopSessionStorageTest < ActiveSupport::TestCase
       mock_auth_hash = mock
       mock_auth_hash.stubs(:shop).returns(mock_shop_instance.shopify_domain)
       mock_auth_hash.stubs(:access_token).returns("a-new-token!")
+      mock_auth_hash.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new("read_products,write_orders"))
       saved_id = ShopMockSessionStore.store(mock_auth_hash)
 
       assert_equal "a-new-token!", mock_shop_instance.shopify_token

test/shopify_app/session/user_session_storage_test.rb

@@ -54,6 +54,7 @@ class UserSessionStorageTest < ActiveSupport::TestCase
       mock_auth_hash = mock
       mock_auth_hash.stubs(:shop).returns(mock_user_instance.shopify_domain)
       mock_auth_hash.stubs(:access_token).returns("a-new-user_token!")
+      mock_auth_hash.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new("read_products,write_orders"))
 
       associated_user = {
         id: 100,

test/shopify_app/session/user_session_storage_with_scopes_test.rb

@@ -57,16 +57,12 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase
 
       UserMockSessionStoreWithScopes.stubs(:find_or_initialize_by).returns(mock_user_instance)
 
-      mock_auth_hash = mock
-      mock_auth_hash.stubs(:shop).returns(mock_user_instance.shopify_domain)
-      mock_auth_hash.stubs(:access_token).returns("a-new-user_token!")
-      mock_auth_hash.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new(TEST_MERCHANT_SCOPES))
+      mock_session = mock
+      mock_session.stubs(:shop).returns(mock_user_instance.shopify_domain)
+      mock_session.stubs(:access_token).returns("a-new-user_token!")
+      mock_session.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new(TEST_MERCHANT_SCOPES))
 
-      associated_user = {
-        id: 100,
-      }
-
-      saved_id = UserMockSessionStoreWithScopes.store(mock_auth_hash, user: associated_user)
+      saved_id = UserMockSessionStoreWithScopes.store(mock_session, mock_associated_user)
 
       assert_equal "a-new-user_token!", mock_user_instance.shopify_token
       assert_equal mock_user_instance.id, saved_id
@@ -99,5 +95,20 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase
         UserMockSessionStoreWithScopes.retrieve(1)
       end
     end
+
+    private
+
+    def mock_associated_user
+      ShopifyAPI::Auth::AssociatedUser.new(
+        id: 100,
+        first_name: "John",
+        last_name: "Doe",
+        email: "johndoe@email.com",
+        email_verified: true,
+        account_owner: false,
+        locale: "en",
+        collaborator: true
+      )
+    end
   end
 end