From e81bc92c5e8493c4378045f1199165ddf6eb528b 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 --- CHANGELOG.md | 1 + .../shopify_app/callback_controller.rb | 8 +- test/controllers/callback_controller_test.rb | 106 ++++++++++++------ 3 files changed, 79 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b7fe7340..796ccc5b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ Unreleased ---------- * Make type param for webhooks route optional. This will fix a bug with CLI initiated webhooks.[1786](https://github.com/Shopify/shopify_app/pull/1786) +* Always register webhooks with offline sessions.[1788](https://github.com/Shopify/shopify_app/pull/1788) 21.10.0 (January 24, 2024) ---------- 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..d133fcb76 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", @@ -47,17 +49,27 @@ class CallbackControllerTest < ActionController::TestCase ShopifyApp::SessionRepository.stubs(:store_session) end + teardown do + SessionRepository.shop_storage.clear + end + 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" } + get :callback, 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 +81,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 +98,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 +130,18 @@ 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_oauth(session: mock_session) + ShopifyApp::SessionRepository.shop_storage.store(@stubbed_session) + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + + mock_session = online_session + mock_oauth(session: online_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 +167,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 +190,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 +201,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 +223,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 +234,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,10 +297,28 @@ 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 + ShopifyApp.configure do |config| + config.webhooks = [{ topic: "carts/update", address: "example-app.com/webhooks" }] + end + ShopifyApp::SessionRepository.shop_storage.store(@stubbed_session) + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + + mock_oauth(session: online_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 @@ -309,7 +328,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 +341,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 +367,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