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..cc0f3067a 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::TokenExchange + 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..b23ec502f 100644 --- a/app/controllers/concerns/shopify_app/ensure_installed.rb +++ b/app/controllers/concerns/shopify_app/ensure_installed.rb @@ -16,8 +16,13 @@ module EnsureInstalled end before_action :check_shop_domain - before_action :check_shop_known - before_action :validate_non_embedded_session + + unless ShopifyApp.configuration.use_new_embedded_auth_strategy? + # 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 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..2f109ec00 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/token_exchange" # jobs require "shopify_app/jobs/webhooks_manager_job" 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/token_exchange.rb b/lib/shopify_app/controller_concerns/token_exchange.rb new file mode 100644 index 000000000..dc5522779 --- /dev/null +++ b/lib/shopify_app/controller_concerns/token_exchange.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +module ShopifyApp + module TokenExchange + 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? + @current_shopify_session = nil + 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.error( + "A #{error.code} error (#{error.class}) occurred during the token exchange. Response: #{error.response.body}", + ) + raise + rescue => error + ShopifyApp::Logger.error("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 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 + + def online_token_configured? + ShopifyApp.configuration.online_token_configured? + end + end +end 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 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 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..a02414c77 --- /dev/null +++ b/test/shopify_app/controller_concerns/token_exchange_test.rb @@ -0,0 +1,203 @@ +# 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 + + @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, + ) + @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 + with_application_test_routes do + ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( + @session_token_in_header, + nil, + false, + ).returns(nil, @offline_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::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.stubs(:current_session_id).with( + @session_token_in_header, + nil, + true, + ).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.stubs(: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.stubs(: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.stubs(:current_session_id).with( + @session_token_in_header, + nil, + true, + ).returns(@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) + + 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.stubs(: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 + + private + + def with_application_test_routes + with_routing do |set| + set.draw do + get "/" => "token_exchange#index" + end + yield + end + end +end