diff --git a/lib/shopify_app/controller_concerns/login_protection.rb b/lib/shopify_app/controller_concerns/login_protection.rb index 1a897e75f..5a8dc74d3 100644 --- a/lib/shopify_app/controller_concerns/login_protection.rb +++ b/lib/shopify_app/controller_concerns/login_protection.rb @@ -108,6 +108,11 @@ def host end def redirect_to_login + if defined?(ShopifySecurityBase::CurrentTenant) + ShopifyApp::Logger.debug("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant") + ShopifySecurityBase::CurrentTenant.tenant = ShopifySecurityBase::NilTenant.new + end + if requested_by_javascript? add_top_level_redirection_headers(ignore_response_code: true) ShopifyApp::Logger.debug("Login redirect request is a XHR") diff --git a/lib/shopify_app/controller_concerns/token_exchange.rb b/lib/shopify_app/controller_concerns/token_exchange.rb index 06c8307d8..db090beca 100644 --- a/lib/shopify_app/controller_concerns/token_exchange.rb +++ b/lib/shopify_app/controller_concerns/token_exchange.rb @@ -63,6 +63,11 @@ def respond_to_invalid_shopify_id_token(error) ShopifyApp::Logger.debug("Responding to invalid Shopify ID token: #{error.message}") return if performed? + if defined?(ShopifySecurityBase::CurrentTenant) + ShopifyApp::Logger.debug("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant") + ShopifySecurityBase::CurrentTenant.tenant = ShopifySecurityBase::NilTenant.new + end + if request.headers["HTTP_AUTHORIZATION"].blank? if embedded? redirect_to_bounce_page diff --git a/test/shopify_app/controller_concerns/login_protection_test.rb b/test/shopify_app/controller_concerns/login_protection_test.rb index bc80a8132..8362a339e 100644 --- a/test/shopify_app/controller_concerns/login_protection_test.rb +++ b/test/shopify_app/controller_concerns/login_protection_test.rb @@ -618,6 +618,35 @@ class LoginProtectionControllerTest < ActionController::TestCase end end + test "#redirect_to_login sets current tenant to nil before redirecting when ShopifySecurityBase::CurrentTenant is defined" do + ::ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(nil) + ShopifyApp::Logger.stubs(:debug) + + with_shopify_security_base_current_tenant do |current_tenant_class, nil_tenant| + with_application_test_routes do + current_tenant_class.expects(:tenant=).with(nil_tenant) + ShopifyApp::Logger.expects(:debug) + .with("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant") + + get :index, params: { shop: "foobar" } + assert_redirected_to "/login?shop=foobar.myshopify.com" + end + end + end + + test "#redirect_to_login ignores ShopifySecurityBase::CurrentTenant when not defined" do + ::ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(nil) + ShopifyApp::Logger.stubs(:debug) + + with_application_test_routes do + ShopifyApp::Logger.expects(:debug) + .with("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant").never + + get :index, params: { shop: "foobar" } + assert_redirected_to "/login?shop=foobar.myshopify.com" + end + end + private def assert_fullpage_redirected(shop_domain, expect_embedded) @@ -656,4 +685,22 @@ def with_custom_login_url(url) ensure ShopifyApp.configure { |config| config.login_url = original_url } end + + def with_shopify_security_base_current_tenant + shopify_security_base_module = Module.new + ShopifyApp::LoginProtection.const_set("ShopifySecurityBase", shopify_security_base_module) + current_tenant_class = stub(:current_tenant_class, "tenant=": nil) + shopify_security_base_module.const_set("CurrentTenant", current_tenant_class) + nil_tenant_class = stub(:nil_tenant_class) + shopify_security_base_module.const_set("NilTenant", nil_tenant_class) + + nil_tenant = stub(:nil_tenant) + nil_tenant_class.stubs(:new).returns(nil_tenant) + + yield(current_tenant_class, nil_tenant) + ensure + if ShopifyApp::LoginProtection.const_defined?("ShopifySecurityBase") + ShopifyApp::LoginProtection.send(:remove_const, "ShopifySecurityBase") + end + end end diff --git a/test/shopify_app/controller_concerns/token_exchange_test.rb b/test/shopify_app/controller_concerns/token_exchange_test.rb index 81bc3f791..a7245166e 100644 --- a/test/shopify_app/controller_concerns/token_exchange_test.rb +++ b/test/shopify_app/controller_concerns/token_exchange_test.rb @@ -370,6 +370,41 @@ class TokenExchangeControllerTest < ActionController::TestCase end end + test "sets current tenant to nil before redirecting when ShopifySecurityBase::CurrentTenant is defined" do + ShopifyAPI::Utils::SessionUtils.stubs(:session_id_from_shopify_id_token) + .raises(ShopifyAPI::Errors::InvalidJwtTokenError) + request.headers["HTTP_AUTHORIZATION"] = nil + + ShopifyApp::Logger.stubs(:debug) + + with_shopify_security_base_current_tenant do |current_tenant_class, nil_tenant| + with_application_test_routes do + current_tenant_class.expects(:tenant=).with(nil_tenant) + ShopifyApp::Logger.expects(:debug) + .with("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant") + + get :index, params: { shop: @shop, host: Base64.encode64("#{@shop}/admin") } + assert_response :redirect + end + end + end + + test "ignores ShopifySecurityBase::CurrentTenant when not defined when redirecting" do + ShopifyAPI::Utils::SessionUtils.stubs(:session_id_from_shopify_id_token) + .raises(ShopifyAPI::Errors::InvalidJwtTokenError) + request.headers["HTTP_AUTHORIZATION"] = nil + + ShopifyApp::Logger.stubs(:debug) + + with_application_test_routes do + ShopifyApp::Logger.expects(:debug) + .with("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant").never + + get :index, params: { shop: @shop, host: Base64.encode64("#{@shop}/admin") } + assert_response :redirect + end + end + private def with_application_test_routes @@ -383,4 +418,22 @@ def with_application_test_routes yield end end + + def with_shopify_security_base_current_tenant + shopify_security_base_module = Module.new + ShopifyApp::TokenExchange.const_set("ShopifySecurityBase", shopify_security_base_module) + current_tenant_class = stub(:current_tenant_class, "tenant=": nil) + shopify_security_base_module.const_set("CurrentTenant", current_tenant_class) + nil_tenant_class = stub(:nil_tenant_class) + shopify_security_base_module.const_set("NilTenant", nil_tenant_class) + + nil_tenant = stub(:nil_tenant) + nil_tenant_class.stubs(:new).returns(nil_tenant) + + yield(current_tenant_class, nil_tenant) + ensure + if ShopifyApp::TokenExchange.const_defined?("ShopifySecurityBase") + ShopifyApp::TokenExchange.send(:remove_const, "ShopifySecurityBase") + end + end end