From ce87d586c54f58f6c88a73c4dd66818a8521c5c1 Mon Sep 17 00:00:00 2001 From: Paulo Margarido <64600052+paulomarg@users.noreply.github.com> Date: Thu, 8 Feb 2024 15:54:18 -0500 Subject: [PATCH] Always register webhooks with offline sessions --- .../shopify_app/callback_controller.rb | 8 +- test/controllers/callback_controller_test.rb | 90 ++++++++++++------- 2 files changed, 64 insertions(+), 34 deletions(-) diff --git a/app/controllers/shopify_app/callback_controller.rb b/app/controllers/shopify_app/callback_controller.rb index ae938e6b0..4b23fdf97 100644 --- a/app/controllers/shopify_app/callback_controller.rb +++ b/app/controllers/shopify_app/callback_controller.rb @@ -130,8 +130,12 @@ def user_access_scopes_strategy end def perform_post_authenticate_jobs(session) - install_webhooks(session) - install_scripttags(session) + # Ensure we use the offline session to install webhooks and scripttags + offline_session = session.online? ? shop_session : session + + install_webhooks(offline_session) + install_scripttags(offline_session) + perform_after_authenticate_job(session) end diff --git a/test/controllers/callback_controller_test.rb b/test/controllers/callback_controller_test.rb index daa87e3d0..66c4d1024 100644 --- a/test/controllers/callback_controller_test.rb +++ b/test/controllers/callback_controller_test.rb @@ -21,6 +21,8 @@ def perform; end end module ShopifyApp + SHOP_DOMAIN = "shop.myshopify.io" + class CallbackControllerTest < ActionController::TestCase setup do @routes = ShopifyApp::Engine.routes @@ -28,12 +30,12 @@ class CallbackControllerTest < ActionController::TestCase ShopifyApp::SessionRepository.user_storage = nil ShopifyAppConfigurer.setup_context I18n.locale = :en - @stubbed_session = ShopifyAPI::Auth::Session.new(shop: "shop", access_token: "token") + @stubbed_session = ShopifyAPI::Auth::Session.new(shop: SHOP_DOMAIN, access_token: "token") @stubbed_cookie = ShopifyAPI::Auth::Oauth::SessionCookie.new(value: "", expires: Time.now) @host = "little-shoppe-of-horrors.#{ShopifyApp.configuration.myshopify_domain}" host = Base64.strict_encode64(@host + "/admin") @callback_params = { - shop: "shop", + shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", @@ -50,14 +52,14 @@ class CallbackControllerTest < ActionController::TestCase test "#callback flashes error in Spanish" do I18n.expects(:t).with("could_not_log_in") get :callback, - params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } + params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } end test "#callback rescued errors of ShopifyAPI::Error will not emit a deprecation notice" do ShopifyAPI::Auth::Oauth.expects(:validate_auth_callback).raises(ShopifyAPI::Errors::MissingRequiredArgumentError) assert_not_deprecated do get :callback, - params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } + params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } end assert_equal flash[:error], "Could not log in to Shopify store" end @@ -69,7 +71,7 @@ class CallbackControllerTest < ActionController::TestCase ShopifyApp::Logger.expects(:deprecated).never get :callback, - params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } + params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } end test "#callback rescued non-shopify errors will be deprecated" do @@ -86,7 +88,7 @@ class CallbackControllerTest < ActionController::TestCase assert_within_deprecation_schedule(version) ShopifyApp::Logger.expects(:deprecated).with(message, version) get :callback, - params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } + params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } end test "#callback calls ShopifyAPI::Auth::Oauth.validate_auth_callback" do @@ -118,29 +120,14 @@ class CallbackControllerTest < ActionController::TestCase end test "#callback sets the shopify_user_id in the Rails session when session is online" do - associated_user = ShopifyAPI::Auth::AssociatedUser.new( - id: 42, - first_name: "LeeeEEeeeeee3roy", - last_name: "Jenkins", - email: "dat_email@tho.com", - email_verified: true, - locale: "en", - collaborator: true, - account_owner: true, - ) - mock_session = ShopifyAPI::Auth::Session.new( - shop: "shop", - access_token: "token", - is_online: true, - associated_user: associated_user, - ) + mock_session = online_session mock_oauth(session: mock_session) get :callback, params: @callback_params - assert_equal session[:shopify_user_id], associated_user.id + assert_equal session[:shopify_user_id], mock_session.associated_user.id end test "#callback DOES NOT set the shopify_user_id in the Rails session when session is offline" do - mock_session = ShopifyAPI::Auth::Session.new(shop: "shop", access_token: "token", is_online: false) + mock_session = ShopifyAPI::Auth::Session.new(shop: SHOP_DOMAIN, access_token: "token", is_online: false) mock_oauth(session: mock_session) get :callback, params: @callback_params assert_nil session[:shopify_user_id] @@ -166,7 +153,7 @@ class CallbackControllerTest < ActionController::TestCase config.webhooks = [{ topic: "carts/update", address: "example-app.com/webhooks" }] end - ShopifyApp::WebhooksManager.expects(:queue).with("shop", "token") + ShopifyApp::WebhooksManager.expects(:queue).with(SHOP_DOMAIN, "token") mock_oauth get :callback, params: @callback_params @@ -189,7 +176,7 @@ class CallbackControllerTest < ActionController::TestCase config.after_authenticate_job = { job: Shopify::AfterAuthenticateJob, inline: true } end - Shopify::AfterAuthenticateJob.expects(:perform_now).with(shop_domain: "shop") + Shopify::AfterAuthenticateJob.expects(:perform_now).with(shop_domain: SHOP_DOMAIN) mock_oauth get :callback, params: @callback_params @@ -200,7 +187,7 @@ class CallbackControllerTest < ActionController::TestCase config.after_authenticate_job = { job: Shopify::AfterAuthenticateJob, inline: false } end - Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: "shop") + Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: SHOP_DOMAIN) mock_oauth get :callback, params: @callback_params @@ -222,7 +209,7 @@ class CallbackControllerTest < ActionController::TestCase config.after_authenticate_job = { job: Shopify::AfterAuthenticateJob } end - Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: "shop") + Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: SHOP_DOMAIN) mock_oauth get :callback, params: @callback_params @@ -233,7 +220,7 @@ class CallbackControllerTest < ActionController::TestCase config.after_authenticate_job = { job: "Shopify::AfterAuthenticateJob", inline: false } end - Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: "shop") + Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: SHOP_DOMAIN) mock_oauth get :callback, params: @callback_params @@ -296,12 +283,32 @@ class CallbackControllerTest < ActionController::TestCase config.webhooks = [{ topic: "carts/update", address: "example-app.com/webhooks" }] end - ShopifyApp::WebhooksManager.expects(:queue).with("shop", "token") + ShopifyApp::WebhooksManager.expects(:queue).with(SHOP_DOMAIN, "token") get :callback, params: @callback_params assert_response 302 end + test "#callback performs install_webhook job with an offline session after an online session OAuth" do + shop_storage = ShopifyApp::InMemoryShopSessionStore + + ShopifyApp.configure do |config| + config.webhooks = [{ topic: "carts/update", address: "example-app.com/webhooks" }] + config.shop_session_repository = shop_storage + config.user_session_repository = ShopifyApp::InMemoryUserSessionStore + end + mock_oauth(session: online_session) + shop_storage.store(@stubbed_session) + + ShopifyApp::WebhooksManager.expects(:queue).with(SHOP_DOMAIN, "token") + + get :callback, params: @callback_params + assert_response 302 + rescue => e + shop_storage.clear + raise e + end + test "#callback performs install_scripttags job after authentication" do mock_oauth @@ -309,7 +316,7 @@ class CallbackControllerTest < ActionController::TestCase config.scripttags = [{ event: "onload", src: "https://example.com/fancy.js" }] end - ShopifyApp::ScripttagsManager.expects(:queue).with("shop", "token", ShopifyApp.configuration.scripttags) + ShopifyApp::ScripttagsManager.expects(:queue).with(SHOP_DOMAIN, "token", ShopifyApp.configuration.scripttags) get :callback, params: @callback_params assert_response 302 @@ -322,7 +329,7 @@ class CallbackControllerTest < ActionController::TestCase config.after_authenticate_job = { job: Shopify::AfterAuthenticateJob, inline: true } end - Shopify::AfterAuthenticateJob.expects(:perform_now).with(shop_domain: "shop") + Shopify::AfterAuthenticateJob.expects(:perform_now).with(shop_domain: SHOP_DOMAIN) get :callback, params: @callback_params assert_response 302 @@ -348,5 +355,24 @@ def mock_oauth(cookie: @stubbed_cookie, session: @stubbed_session) session: session, }) end + + def online_session + associated_user = ShopifyAPI::Auth::AssociatedUser.new( + id: 42, + first_name: "LeeeEEeeeeee3roy", + last_name: "Jenkins", + email: "dat_email@tho.com", + email_verified: true, + locale: "en", + collaborator: true, + account_owner: true, + ) + ShopifyAPI::Auth::Session.new( + shop: SHOP_DOMAIN, + access_token: "online-token", + is_online: true, + associated_user: associated_user, + ) + end end end