From 8c72defa202ab57da0532a434fdb95cc12116a57 Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Mon, 25 Mar 2024 06:54:25 -0600 Subject: [PATCH 1/8] Add token exchange concern --- Gemfile.lock | 1 + .../shopify_app/ensure_has_session.rb | 16 +- .../concerns/shopify_app/ensure_installed.rb | 10 +- .../shop_access_scopes_verification.rb | 6 +- lib/shopify_app.rb | 1 + .../retrieve_session_from_token_exchange.rb | 141 ++++++++++++++++++ 6 files changed, 166 insertions(+), 9 deletions(-) create mode 100644 lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb diff --git a/Gemfile.lock b/Gemfile.lock index 9c49bc64d..d665e2225 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -256,6 +256,7 @@ GEM PLATFORMS arm64-darwin-21 arm64-darwin-22 + arm64-darwin-23 x86_64-darwin-19 x86_64-darwin-20 x86_64-darwin-21 diff --git a/app/controllers/concerns/shopify_app/ensure_has_session.rb b/app/controllers/concerns/shopify_app/ensure_has_session.rb index 7e3159fd3..740ab1efe 100644 --- a/app/controllers/concerns/shopify_app/ensure_has_session.rb +++ b/app/controllers/concerns/shopify_app/ensure_has_session.rb @@ -6,14 +6,20 @@ module EnsureHasSession included do include ShopifyApp::Localization - include ShopifyApp::LoginProtection + + if ShopifyApp.configuration.use_new_embedded_auth_strategy? + include ShopifyApp::RetrieveSessionFromTokenExchange + around_action :activate_shopify_session + else + include ShopifyApp::LoginProtection + before_action :login_again_if_different_user_or_shop + around_action :activate_shopify_session + after_action :add_top_level_redirection_headers + end + include ShopifyApp::CsrfProtection include ShopifyApp::EmbeddedApp include ShopifyApp::EnsureBilling - - before_action :login_again_if_different_user_or_shop - around_action :activate_shopify_session - after_action :add_top_level_redirection_headers end end end diff --git a/app/controllers/concerns/shopify_app/ensure_installed.rb b/app/controllers/concerns/shopify_app/ensure_installed.rb index 21b624a37..8ad3797b4 100644 --- a/app/controllers/concerns/shopify_app/ensure_installed.rb +++ b/app/controllers/concerns/shopify_app/ensure_installed.rb @@ -15,9 +15,13 @@ module EnsureInstalled raise message end - before_action :check_shop_domain - before_action :check_shop_known - before_action :validate_non_embedded_session + # TODO: Add support to use new embedded auth strategy here when invalid + # session token can be handled by AppBridge app reload + unless ShopifyApp.configuration.use_new_embedded_auth_strategy? + before_action :check_shop_domain + before_action :check_shop_known + before_action :validate_non_embedded_session + end end def current_shopify_domain diff --git a/app/controllers/concerns/shopify_app/shop_access_scopes_verification.rb b/app/controllers/concerns/shopify_app/shop_access_scopes_verification.rb index fd380a72f..c813f3f65 100644 --- a/app/controllers/concerns/shopify_app/shop_access_scopes_verification.rb +++ b/app/controllers/concerns/shopify_app/shop_access_scopes_verification.rb @@ -6,7 +6,11 @@ module ShopAccessScopesVerification include ShopifyApp::RedirectForEmbedded included do - before_action :login_on_scope_changes + # Embedded auth strategy uses Shopify managed install to ensure latest access scopes, + # This will be handled automatically through token exchange + unless ShopifyApp.configuration.use_new_embedded_auth_strategy? + before_action :login_on_scope_changes + end end protected diff --git a/lib/shopify_app.rb b/lib/shopify_app.rb index 751a56bdf..5ed841e48 100644 --- a/lib/shopify_app.rb +++ b/lib/shopify_app.rb @@ -52,6 +52,7 @@ def self.use_webpacker? require "shopify_app/controller_concerns/payload_verification" require "shopify_app/controller_concerns/app_proxy_verification" require "shopify_app/controller_concerns/webhook_verification" + require "shopify_app/controller_concerns/retrieve_session_from_token_exchange" # jobs require "shopify_app/jobs/webhooks_manager_job" diff --git a/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb b/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb new file mode 100644 index 000000000..a5b1ce6e6 --- /dev/null +++ b/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +module ShopifyApp + module RetrieveSessionFromTokenExchange + extend ActiveSupport::Concern + + def activate_shopify_session + if current_shopify_session.blank? + retrieve_session_from_token_exchange + end + + if ShopifyApp.configuration.check_session_expiry_date && current_shopify_session.expired? + retrieve_session_from_token_exchange + end + + begin + ShopifyApp::Logger.debug("Activating Shopify session") + ShopifyAPI::Context.activate_session(current_shopify_session) + yield + ensure + ShopifyApp::Logger.debug("Deactivating session") + ShopifyAPI::Context.deactivate_session + end + end + + def current_shopify_session + @current_shopify_session ||= begin + session_id = ShopifyAPI::Utils::SessionUtils.current_session_id( + request.headers["HTTP_AUTHORIZATION"], + nil, + online_token_configured?, + ) + return nil unless session_id + + ShopifyApp::SessionRepository.load_session(session_id) + end + end + + def current_shopify_domain + return if params[:shop].blank? + + ShopifyApp::Utils.sanitize_shop_domain(params[:shop]) + end + + private + + def retrieve_session_from_token_exchange + # TODO: Right now JWT Middleware only updates env['jwt.shopify_domain'] from request headers tokens, + # which won't work for new installs. + # we need to update the middleware to also update the env['jwt.shopify_domain'] from the query params + domain = ShopifyApp::JWT.new(session_token).shopify_domain + + ShopifyApp::Logger.info("Performing Token Exchange for [#{domain}] - (Offline)") + session = exchange_token( + shop: domain, # TODO: use jwt_shopify_domain ? + session_token: session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, + ) + + if session && online_token_configured? + ShopifyApp::Logger.info("Performing Token Exchange for [#{domain}] - (Online)") + exchange_token( + shop: domain, # TODO: use jwt_shopify_domain ? + session_token: session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, + ) + end + + # TODO: Refactor and add post authenticate tasks + # ShopifyApp.configuration.post_authenticate_tasks.perform(session) if session + end + + def exchange_token(shop:, session_token:, requested_token_type:) + if session_token.blank? + # respond_to_invalid_session_token + return + end + + begin + session = ShopifyAPI::Auth::TokenExchange.exchange_token( + shop: shop, + session_token: session_token, + requested_token_type: requested_token_type, + ) + rescue ShopifyAPI::Errors::InvalidJwtTokenError + # respond_to_invalid_session_token + return + rescue ShopifyAPI::Errors::HttpResponseError => error + ShopifyApp::Logger.info( + "A #{error.code} error (#{error.class}) occurred during the token exchange. Response: #{error.response.body}", + ) + raise + rescue => error + ShopifyApp::Logger.info("An error occurred during the token exchange: #{error.message}") + raise + end + + if session + begin + ShopifyApp::SessionRepository.store_session(session) + rescue ActiveRecord::RecordNotUnique + ShopifyApp::Logger.debug("Session not stored due to concurrent token exchange calls") + end + end + + session + end + + def session_token + @session_token ||= id_token_header + end + + def id_token_header + request.headers["HTTP_AUTHORIZATION"]&.match(/^Bearer (.+)$/)&.[](1) + end + + def online_token_configured? + !ShopifyApp.configuration.user_session_repository.blank? && ShopifyApp::SessionRepository.user_storage.present? + end + + def respond_to_invalid_session_token + # TODO: Implement this method to handle invalid session tokens + + # if request.xhr? + # response.set_header("X-Shopify-Retry-Invalid-Session-Request", 1) + # unauthorized_response = { message: :unauthorized } + # render(json: { errors: [unauthorized_response] }, status: :unauthorized) + # else + # patch_session_token_url = "#{ShopifyAPI::Context.host}/patch_session_token" + # patch_session_token_params = request.query_parameters.except(:id_token) + + # bounce_url = "#{ShopifyAPI::Context.host}#{request.path}?#{patch_session_token_params.to_query}" + + # # App Bridge will trigger a fetch to the URL in shopify-reload, with a new session token in headers + # patch_session_token_params["shopify-reload"] = bounce_url + + # redirect_to("#{patch_session_token_url}?#{patch_session_token_params.to_query}", allow_other_host: true) + # end + end + end +end From 0efe87fdc0e2b52095b53a41715c936434f03024 Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Mon, 25 Mar 2024 09:26:41 -0600 Subject: [PATCH 2/8] Extract to configuration --- lib/shopify_app/configuration.rb | 4 ++++ .../controller_concerns/login_protection.rb | 2 +- .../retrieve_session_from_token_exchange.rb | 8 ++------ test/shopify_app/configuration_test.rb | 17 +++++++++++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/lib/shopify_app/configuration.rb b/lib/shopify_app/configuration.rb index 27f6316a5..07299dabe 100644 --- a/lib/shopify_app/configuration.rb +++ b/lib/shopify_app/configuration.rb @@ -126,6 +126,10 @@ def user_access_scopes def use_new_embedded_auth_strategy? wip_new_embedded_auth_strategy && embedded_app? end + + def online_token_configured? + !ShopifyApp.configuration.user_session_repository.blank? && ShopifyApp::SessionRepository.user_storage.present? + end end class BillingConfiguration diff --git a/lib/shopify_app/controller_concerns/login_protection.rb b/lib/shopify_app/controller_concerns/login_protection.rb index 3f0d50dad..c9b96cf4f 100644 --- a/lib/shopify_app/controller_concerns/login_protection.rb +++ b/lib/shopify_app/controller_concerns/login_protection.rb @@ -263,7 +263,7 @@ def shop_session end def online_token_configured? - !ShopifyApp.configuration.user_session_repository.blank? && ShopifyApp::SessionRepository.user_storage.present? + ShopifyApp.configuration.online_token_configured? end def user_session_expected? diff --git a/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb b/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb index a5b1ce6e6..b10423b5c 100644 --- a/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb +++ b/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb @@ -28,7 +28,7 @@ def current_shopify_session session_id = ShopifyAPI::Utils::SessionUtils.current_session_id( request.headers["HTTP_AUTHORIZATION"], nil, - online_token_configured?, + ShopifyApp.configuration.online_token_configured?, ) return nil unless session_id @@ -57,7 +57,7 @@ def retrieve_session_from_token_exchange requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, ) - if session && online_token_configured? + if session && ShopifyApp.configuration.online_token_configured? ShopifyApp::Logger.info("Performing Token Exchange for [#{domain}] - (Online)") exchange_token( shop: domain, # TODO: use jwt_shopify_domain ? @@ -114,10 +114,6 @@ def id_token_header request.headers["HTTP_AUTHORIZATION"]&.match(/^Bearer (.+)$/)&.[](1) end - def online_token_configured? - !ShopifyApp.configuration.user_session_repository.blank? && ShopifyApp::SessionRepository.user_storage.present? - end - def respond_to_invalid_session_token # TODO: Implement this method to handle invalid session tokens diff --git a/test/shopify_app/configuration_test.rb b/test/shopify_app/configuration_test.rb index aaaddb9ce..fcbd95b5d 100644 --- a/test/shopify_app/configuration_test.rb +++ b/test/shopify_app/configuration_test.rb @@ -254,4 +254,21 @@ class ConfigurationTest < ActiveSupport::TestCase refute ShopifyApp.configuration.use_new_embedded_auth_strategy? end + + test "#online_token_configured? is true when user_session_repository is set" do + ShopifyApp.configure do |config| + config.user_session_repository = "ShopifyApp::InMemoryUserSessionStore" + end + + assert ShopifyApp.configuration.online_token_configured? + end + + test "#online_token_configured? is false when user storage is nil" do + ShopifyApp.configure do |config| + config.user_session_repository = "ShopifyApp::InMemoryUserSessionStore" + end + ShopifyApp::SessionRepository.user_storage = nil + + refute ShopifyApp.configuration.online_token_configured? + end end From b53fe3d85a8ca644f5111c0dd8d9759c5ee8f66d Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Mon, 25 Mar 2024 09:40:02 -0600 Subject: [PATCH 3/8] Rename concern to TokenExchange --- .../concerns/shopify_app/ensure_has_session.rb | 2 +- lib/shopify_app.rb | 2 +- ...on_from_token_exchange.rb => token_exchange.rb} | 14 +++++++++----- 3 files changed, 11 insertions(+), 7 deletions(-) rename lib/shopify_app/controller_concerns/{retrieve_session_from_token_exchange.rb => token_exchange.rb} (93%) diff --git a/app/controllers/concerns/shopify_app/ensure_has_session.rb b/app/controllers/concerns/shopify_app/ensure_has_session.rb index 740ab1efe..cc0f3067a 100644 --- a/app/controllers/concerns/shopify_app/ensure_has_session.rb +++ b/app/controllers/concerns/shopify_app/ensure_has_session.rb @@ -8,7 +8,7 @@ module EnsureHasSession include ShopifyApp::Localization if ShopifyApp.configuration.use_new_embedded_auth_strategy? - include ShopifyApp::RetrieveSessionFromTokenExchange + include ShopifyApp::TokenExchange around_action :activate_shopify_session else include ShopifyApp::LoginProtection diff --git a/lib/shopify_app.rb b/lib/shopify_app.rb index 5ed841e48..2f109ec00 100644 --- a/lib/shopify_app.rb +++ b/lib/shopify_app.rb @@ -52,7 +52,7 @@ def self.use_webpacker? require "shopify_app/controller_concerns/payload_verification" require "shopify_app/controller_concerns/app_proxy_verification" require "shopify_app/controller_concerns/webhook_verification" - require "shopify_app/controller_concerns/retrieve_session_from_token_exchange" + require "shopify_app/controller_concerns/token_exchange" # jobs require "shopify_app/jobs/webhooks_manager_job" diff --git a/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb b/lib/shopify_app/controller_concerns/token_exchange.rb similarity index 93% rename from lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb rename to lib/shopify_app/controller_concerns/token_exchange.rb index b10423b5c..86a506f4c 100644 --- a/lib/shopify_app/controller_concerns/retrieve_session_from_token_exchange.rb +++ b/lib/shopify_app/controller_concerns/token_exchange.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module ShopifyApp - module RetrieveSessionFromTokenExchange + module TokenExchange extend ActiveSupport::Concern def activate_shopify_session @@ -28,7 +28,7 @@ def current_shopify_session session_id = ShopifyAPI::Utils::SessionUtils.current_session_id( request.headers["HTTP_AUTHORIZATION"], nil, - ShopifyApp.configuration.online_token_configured?, + online_token_configured?, ) return nil unless session_id @@ -57,7 +57,7 @@ def retrieve_session_from_token_exchange requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, ) - if session && ShopifyApp.configuration.online_token_configured? + if session && online_token_configured? ShopifyApp::Logger.info("Performing Token Exchange for [#{domain}] - (Online)") exchange_token( shop: domain, # TODO: use jwt_shopify_domain ? @@ -86,12 +86,12 @@ def exchange_token(shop:, session_token:, requested_token_type:) # respond_to_invalid_session_token return rescue ShopifyAPI::Errors::HttpResponseError => error - ShopifyApp::Logger.info( + ShopifyApp::Logger.error( "A #{error.code} error (#{error.class}) occurred during the token exchange. Response: #{error.response.body}", ) raise rescue => error - ShopifyApp::Logger.info("An error occurred during the token exchange: #{error.message}") + ShopifyApp::Logger.error("An error occurred during the token exchange: #{error.message}") raise end @@ -133,5 +133,9 @@ def respond_to_invalid_session_token # redirect_to("#{patch_session_token_url}?#{patch_session_token_params.to_query}", allow_other_host: true) # end end + + def online_token_configured? + ShopifyApp.configuration.online_token_configured? + end end end From fcef5f27becf409a9ac2f1fae05c3be17fb855db Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Mon, 25 Mar 2024 11:22:32 -0600 Subject: [PATCH 4/8] Move common action 'check_shop_domain' --- app/controllers/concerns/shopify_app/ensure_installed.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/concerns/shopify_app/ensure_installed.rb b/app/controllers/concerns/shopify_app/ensure_installed.rb index 8ad3797b4..b23ec502f 100644 --- a/app/controllers/concerns/shopify_app/ensure_installed.rb +++ b/app/controllers/concerns/shopify_app/ensure_installed.rb @@ -15,10 +15,11 @@ module EnsureInstalled raise message end - # TODO: Add support to use new embedded auth strategy here when invalid - # session token can be handled by AppBridge app reload + before_action :check_shop_domain + unless ShopifyApp.configuration.use_new_embedded_auth_strategy? - before_action :check_shop_domain + # TODO: Add support to use new embedded auth strategy here when invalid + # session token can be handled by AppBridge app reload before_action :check_shop_known before_action :validate_non_embedded_session end From c3c6fb5aad95e76940712f01f70e52617b0937ac Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Tue, 26 Mar 2024 18:54:33 -0600 Subject: [PATCH 5/8] Add test case to EnsureHasSessionTest --- .../concerns/ensure_has_session_test.rb | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/test/controllers/concerns/ensure_has_session_test.rb b/test/controllers/concerns/ensure_has_session_test.rb index e65c2c503..9f23021f6 100644 --- a/test/controllers/concerns/ensure_has_session_test.rb +++ b/test/controllers/concerns/ensure_has_session_test.rb @@ -3,20 +3,36 @@ require "test_helper" class EnsureHasSessionTest < ActionController::TestCase - class EnsureHasSessionTestController < ActionController::Base - include ShopifyApp::EnsureHasSession + test "includes all the needed concerns" do + ShopifyApp.configuration.stubs(:use_new_embedded_auth_strategy?).returns(false) - def index - end + controller = define_controller + assert_common_concerns_are_included(controller) + assert controller.include?(ShopifyApp::LoginProtection), "ShopifyApp::LoginProtection" + refute controller.include?(ShopifyApp::TokenExchange), "ShopifyApp::TokenExchange" end - tests EnsureHasSessionTestController + test "includes TokenExchange when use_new_embedded_auth_strategy is true" do + ShopifyApp.configuration.stubs(:use_new_embedded_auth_strategy?).returns(true) - test "includes all the needed concerns" do - EnsureHasSessionTestController.include?(ShopifyApp::Localization) - EnsureHasSessionTestController.include?(ShopifyApp::LoginProtection) - EnsureHasSessionTestController.include?(ShopifyApp::CsrfProtection) - EnsureHasSessionTestController.include?(ShopifyApp::EmbeddedApp) - EnsureHasSessionTestController.include?(ShopifyApp::EnsureBilling) + controller = define_controller + assert_common_concerns_are_included(controller) + assert controller.include?(ShopifyApp::TokenExchange), "ShopifyApp::TokenExchange" + refute controller.include?(ShopifyApp::LoginProtection), "ShopifyApp::LoginProtection" + end + + private + + def assert_common_concerns_are_included(controller) + assert controller.include?(ShopifyApp::Localization), "ShopifyApp::Localization" + assert controller.include?(ShopifyApp::CsrfProtection), "ShopifyApp::CsrfProtection" + assert controller.include?(ShopifyApp::EmbeddedApp), "ShopifyApp::EmbeddedApp" + assert controller.include?(ShopifyApp::EnsureBilling), "ShopifyApp::EnsureBilling" + end + + def define_controller + Class.new(ActionController::Base) do + include ShopifyApp::EnsureHasSession + end end end From 525aeea0714689465f6e7a8e8c82129857ba6313 Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Wed, 27 Mar 2024 08:07:55 -0600 Subject: [PATCH 6/8] Add tests for token exchange concern --- .../token_exchange_test.rb | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 test/shopify_app/controller_concerns/token_exchange_test.rb diff --git a/test/shopify_app/controller_concerns/token_exchange_test.rb b/test/shopify_app/controller_concerns/token_exchange_test.rb new file mode 100644 index 000000000..b7960cba7 --- /dev/null +++ b/test/shopify_app/controller_concerns/token_exchange_test.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require "test_helper" +require "action_controller" +require "action_controller/base" + +class TokenExchangeController < ActionController::Base + include ShopifyApp::TokenExchange + + around_action :activate_shopify_session + + def index + render(plain: "OK") + end +end + +class TokenExchangeControllerTest < ActionController::TestCase + tests TokenExchangeController + + setup do + @shop = "my-shop.myshopify.com" + ShopifyApp::JWT.any_instance.stubs(:shopify_domain).returns(@shop) + + @session_token = "this-is-a-session-token" + @session_token_in_header = "Bearer #{@session_token}" + request.headers["HTTP_AUTHORIZATION"] = @session_token_in_header + + ShopifyApp::SessionRepository.shop_storage = ShopifyApp::InMemoryShopSessionStore + ShopifyApp::SessionRepository.user_storage = nil + + @offline_session = ShopifyAPI::Auth::Session.new(id: "offline_session_1", shop: @shop) + + @user = ShopifyAPI::Auth::AssociatedUser.new( + id: 1, + first_name: "Hello", + last_name: "World", + email: "Email", + email_verified: true, + account_owner: true, + locale: "en", + collaborator: false, + ) + @online_session = ShopifyAPI::Auth::Session.new( + id: "online_session_1", + shop: @shop, + is_online: true, + associated_user: @user, + ) + end + + test "Exchanges offline token when session doesn't exist" do + with_application_test_routes do + ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).twice.with( + @session_token_in_header, + nil, + false, + ).returns(nil, "offline_#{@shop}") + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, + ).returns(@offline_session) + + ShopifyAPI::Context.expects(:activate_session).with(@offline_session) + + get :index, params: { shop: @shop } + end + end + + test "Exchanges online token if user session store is configured" do + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + + with_application_test_routes do + ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).twice.with( + @session_token_in_header, + nil, + true, + ).returns(nil, "online_#{@user.id}") + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, + ).returns(@offline_session) + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, + ).returns(@online_session) + + ShopifyAPI::Context.expects(:activate_session).with(@online_session) + + get :index, params: { shop: @shop } + end + end + + private + + def with_application_test_routes + with_routing do |set| + set.draw do + get "/" => "token_exchange#index" + end + yield + end + end +end From 6079cf40364f692d4cf9a796dd29fd37e9b1827a Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Wed, 27 Mar 2024 09:04:49 -0600 Subject: [PATCH 7/8] Add more tests for token exchange --- .../controller_concerns/token_exchange.rb | 1 + .../token_exchange_test.rb | 102 +++++++++++++++++- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/lib/shopify_app/controller_concerns/token_exchange.rb b/lib/shopify_app/controller_concerns/token_exchange.rb index 86a506f4c..dc5522779 100644 --- a/lib/shopify_app/controller_concerns/token_exchange.rb +++ b/lib/shopify_app/controller_concerns/token_exchange.rb @@ -10,6 +10,7 @@ def activate_shopify_session end if ShopifyApp.configuration.check_session_expiry_date && current_shopify_session.expired? + @current_shopify_session = nil retrieve_session_from_token_exchange end diff --git a/test/shopify_app/controller_concerns/token_exchange_test.rb b/test/shopify_app/controller_concerns/token_exchange_test.rb index b7960cba7..484dff528 100644 --- a/test/shopify_app/controller_concerns/token_exchange_test.rb +++ b/test/shopify_app/controller_concerns/token_exchange_test.rb @@ -28,8 +28,6 @@ class TokenExchangeControllerTest < ActionController::TestCase ShopifyApp::SessionRepository.shop_storage = ShopifyApp::InMemoryShopSessionStore ShopifyApp::SessionRepository.user_storage = nil - @offline_session = ShopifyAPI::Auth::Session.new(id: "offline_session_1", shop: @shop) - @user = ShopifyAPI::Auth::AssociatedUser.new( id: 1, first_name: "Hello", @@ -46,6 +44,10 @@ class TokenExchangeControllerTest < ActionController::TestCase is_online: true, associated_user: @user, ) + @offline_session = ShopifyAPI::Auth::Session.new(id: "offline_session_1", shop: @shop) + + @offline_session_id = "offline_#{@shop}" + @online_session_id = "online_#{@user.id}" end test "Exchanges offline token when session doesn't exist" do @@ -54,7 +56,7 @@ class TokenExchangeControllerTest < ActionController::TestCase @session_token_in_header, nil, false, - ).returns(nil, "offline_#{@shop}") + ).returns(nil, @offline_session_id) ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( shop: @shop, @@ -76,7 +78,75 @@ class TokenExchangeControllerTest < ActionController::TestCase @session_token_in_header, nil, true, - ).returns(nil, "online_#{@user.id}") + ).returns(nil, @online_session_id) + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, + ).returns(@offline_session) + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, + ).returns(@online_session) + + ShopifyAPI::Context.expects(:activate_session).with(@online_session) + + get :index, params: { shop: @shop } + end + end + + test "Use existing shop session if it exists" do + ShopifyApp::SessionRepository.store_shop_session(@offline_session) + + with_application_test_routes do + ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).with( + @session_token_in_header, + nil, + false, + ).returns(@offline_session_id) + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).never + + ShopifyAPI::Context.expects(:activate_session).with(@offline_session) + + get :index, params: { shop: @shop } + end + end + + test "Use existing user session if it exists" do + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + ShopifyApp::SessionRepository.store_user_session(@online_session, @user) + + with_application_test_routes do + ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).with( + @session_token_in_header, + nil, + true, + ).returns(@online_session_id) + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).never + + ShopifyAPI::Context.expects(:activate_session).with(@online_session) + + get :index, params: { shop: @shop } + end + end + + test "Exchange token again if current user session is expired" do + ShopifyApp.configuration.check_session_expiry_date = true + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + ShopifyApp::SessionRepository.store_user_session(@online_session, @user) + @online_session.stubs(:expired?).returns(true) + + with_application_test_routes do + ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).twice.with( + @session_token_in_header, + nil, + true, + ).returns(@online_session_id) ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( shop: @shop, @@ -90,6 +160,30 @@ class TokenExchangeControllerTest < ActionController::TestCase requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, ).returns(@online_session) + ShopifyApp::SessionRepository.shop_storage.expects(:store).with(@offline_session) + ShopifyApp::SessionRepository.user_storage.expects(:store).with(@online_session, @user) + + ShopifyAPI::Context.expects(:activate_session).with(@online_session) + + get :index, params: { shop: @shop } + end + end + + test "Don't exchange token if current user session is not expired" do + ShopifyApp.configuration.check_session_expiry_date = true + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + ShopifyApp::SessionRepository.store_user_session(@online_session, @user) + @online_session.stubs(:expired?).returns(false) + + with_application_test_routes do + ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).with( + @session_token_in_header, + nil, + true, + ).returns(@online_session_id) + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).never + ShopifyAPI::Context.expects(:activate_session).with(@online_session) get :index, params: { shop: @shop } From 5448d55ba581a669da6b2dd763e2fa15ba57632b Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Wed, 27 Mar 2024 11:32:55 -0600 Subject: [PATCH 8/8] Replace expects with stubs --- .../controller_concerns/token_exchange_test.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/shopify_app/controller_concerns/token_exchange_test.rb b/test/shopify_app/controller_concerns/token_exchange_test.rb index 484dff528..a02414c77 100644 --- a/test/shopify_app/controller_concerns/token_exchange_test.rb +++ b/test/shopify_app/controller_concerns/token_exchange_test.rb @@ -52,7 +52,7 @@ class TokenExchangeControllerTest < ActionController::TestCase test "Exchanges offline token when session doesn't exist" do with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).twice.with( + ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( @session_token_in_header, nil, false, @@ -74,7 +74,7 @@ class TokenExchangeControllerTest < ActionController::TestCase ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).twice.with( + ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( @session_token_in_header, nil, true, @@ -102,7 +102,7 @@ class TokenExchangeControllerTest < ActionController::TestCase ShopifyApp::SessionRepository.store_shop_session(@offline_session) with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).with( + ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( @session_token_in_header, nil, false, @@ -121,7 +121,7 @@ class TokenExchangeControllerTest < ActionController::TestCase ShopifyApp::SessionRepository.store_user_session(@online_session, @user) with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).with( + ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( @session_token_in_header, nil, true, @@ -142,7 +142,7 @@ class TokenExchangeControllerTest < ActionController::TestCase @online_session.stubs(:expired?).returns(true) with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).twice.with( + ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( @session_token_in_header, nil, true, @@ -176,7 +176,7 @@ class TokenExchangeControllerTest < ActionController::TestCase @online_session.stubs(:expired?).returns(false) with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.expects(:current_session_id).with( + ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( @session_token_in_header, nil, true,