diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d455ba99..ba81e1fb9 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/app/controllers/shopify_app/callback_controller.rb b/app/controllers/shopify_app/callback_controller.rb index 4dbfe64ae..b700d6624 100644 --- a/app/controllers/shopify_app/callback_controller.rb +++ b/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) diff --git a/lib/shopify_app/controller_concerns/login_protection.rb b/lib/shopify_app/controller_concerns/login_protection.rb index 60d852898..aeb94a225 100644 --- a/lib/shopify_app/controller_concerns/login_protection.rb +++ b/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 diff --git a/lib/shopify_app/session/session_repository.rb b/lib/shopify_app/session/session_repository.rb index 3aefcb544..854c75937 100644 --- a/lib/shopify_app/session/session_repository.rb +++ b/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 diff --git a/lib/shopify_app/session/user_session_storage_with_scopes.rb b/lib/shopify_app/session/user_session_storage_with_scopes.rb index c14402b81..1b69681ac 100644 --- a/lib/shopify_app/session/user_session_storage_with_scopes.rb +++ b/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 diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index c8d175390..6bcd1292e 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/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) diff --git a/test/shopify_app/controller_concerns/login_protection_test.rb b/test/shopify_app/controller_concerns/login_protection_test.rb index 93c26864e..1e59aabe3 100644 --- a/test/shopify_app/controller_concerns/login_protection_test.rb +++ b/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 diff --git a/test/shopify_app/session/session_repository_test.rb b/test/shopify_app/session/session_repository_test.rb index 4af294f31..bf33d9236 100644 --- a/test/shopify_app/session/session_repository_test.rb +++ b/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 diff --git a/test/shopify_app/session/shop_session_storage_test.rb b/test/shopify_app/session/shop_session_storage_test.rb index ec3cbf76b..6b71ef77a 100644 --- a/test/shopify_app/session/shop_session_storage_test.rb +++ b/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 diff --git a/test/shopify_app/session/user_session_storage_test.rb b/test/shopify_app/session/user_session_storage_test.rb index 20b64bfd8..fd93a9ce7 100644 --- a/test/shopify_app/session/user_session_storage_test.rb +++ b/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, diff --git a/test/shopify_app/session/user_session_storage_with_scopes_test.rb b/test/shopify_app/session/user_session_storage_with_scopes_test.rb index 2158513be..55bb77e5b 100644 --- a/test/shopify_app/session/user_session_storage_with_scopes_test.rb +++ b/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