diff --git a/CHANGELOG.md b/CHANGELOG.md index f9cb771b2..13fcccad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Unreleased ---------- * Fixes a bug with `EnsureAuthenticatedLinks` causing deep links to not work [#1549](https://github.com/Shopify/shopify_app/pull/1549) * Ensure online token is properly used when using `current_shopify_session` [#1566](https://github.com/Shopify/shopify_app/pull/1566) +* Detect the use of incompatible controller concerns [#1560](https://github.com/Shopify/shopify_app/pull/1560). Depending on how your app is structured, this may result in an error on startup even though the app was working correctly. However, it is helping exposing an existing flaw that may have caused issues in future. 21.2.0 (Oct 25, 2022) ---------- diff --git a/app/controllers/concerns/shopify_app/require_known_shop.rb b/app/controllers/concerns/shopify_app/require_known_shop.rb index b5268cc93..4999a9e3a 100644 --- a/app/controllers/concerns/shopify_app/require_known_shop.rb +++ b/app/controllers/concerns/shopify_app/require_known_shop.rb @@ -6,6 +6,12 @@ module RequireKnownShop include ShopifyApp::RedirectForEmbedded included do + if ancestors.include?(ShopifyApp::LoginProtection) + raise ConfigurationError, + "You are attempting to use both RequireKnownShop and LoginProtection in the same controller. "\ + "These are not compatible." + end + before_action :check_shop_domain before_action :check_shop_known end diff --git a/lib/shopify_app/controller_concerns/login_protection.rb b/lib/shopify_app/controller_concerns/login_protection.rb index 980cce04c..e7c473165 100644 --- a/lib/shopify_app/controller_concerns/login_protection.rb +++ b/lib/shopify_app/controller_concerns/login_protection.rb @@ -9,6 +9,12 @@ module LoginProtection include ShopifyApp::SanitizedParams included do + if ancestors.include?(ShopifyApp::RequireKnownShop) + raise ConfigurationError, + "You are attempting to use both RequireKnownShop and LoginProtection in the same controller. "\ + "These are not compatible." + end + after_action :set_test_cookie rescue_from ShopifyAPI::Errors::HttpResponseError, with: :handle_http_error end diff --git a/test/controllers/concerns/require_known_shop_test.rb b/test/controllers/concerns/require_known_shop_test.rb index 9347c4fdc..73173d8bd 100644 --- a/test/controllers/concerns/require_known_shop_test.rb +++ b/test/controllers/concerns/require_known_shop_test.rb @@ -60,4 +60,20 @@ def index assert_response :ok end + + test "detects incompatible controller concerns" do + assert_raises ShopifyApp::ConfigurationError do + Class.new(ApplicationController) do + include ShopifyApp::RequireKnownShop + include ShopifyApp::LoginProtection + end + end + + assert_raises ShopifyApp::ConfigurationError do + Class.new(ApplicationController) do + include ShopifyApp::RequireKnownShop + include ShopifyApp::Authenticated # since this indirectly includes LoginProtection + end + end + end end diff --git a/test/dummy/app/controllers/application_controller.rb b/test/dummy/app/controllers/application_controller.rb index 921861742..7944f9f99 100644 --- a/test/dummy/app/controllers/application_controller.rb +++ b/test/dummy/app/controllers/application_controller.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true class ApplicationController < ActionController::Base - include ShopifyApp::LoginProtection end diff --git a/test/dummy/app/controllers/home_controller.rb b/test/dummy/app/controllers/home_controller.rb index c92198787..6021d3724 100644 --- a/test/dummy/app/controllers/home_controller.rb +++ b/test/dummy/app/controllers/home_controller.rb @@ -1,6 +1,9 @@ # frozen_string_literal: true class HomeController < ApplicationController + include ShopifyApp::EmbeddedApp + include ShopifyApp::RequireKnownShop + def index "index" end diff --git a/test/shopify_app/controller_concerns/login_protection_test.rb b/test/shopify_app/controller_concerns/login_protection_test.rb index 9cc755319..bd024c08b 100644 --- a/test/shopify_app/controller_concerns/login_protection_test.rb +++ b/test/shopify_app/controller_concerns/login_protection_test.rb @@ -466,6 +466,15 @@ class LoginProtectionControllerTest < ActionController::TestCase end end + test "detects incompatible controller concerns" do + assert_raises ShopifyApp::ConfigurationError do + Class.new(ApplicationController) do + include ShopifyApp::LoginProtection + include ShopifyApp::RequireKnownShop + end + end + end + private def assert_fullpage_redirected(shop_domain, _response)