diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ff82c71b..d1b19bbaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased ---------- +* Add the `login_callback_url` config to allow overwriting that route as well, and mount the engine routes based on the configurations. [#1445](https://github.com/Shopify/shopify_app/pull/1445) + 19.0.2 (April 27, 2022) ---------- @@ -27,7 +29,7 @@ BREAKING, please see migration notes. 18.1.0 (Jan 28, 2022) ---------- * Support Rails 7 [#1354](https://github.com/Shopify/shopify_app/pull/1354) -* Fix webhooks handling in Ruby 3 [#1342](https://github.com/Shopify/shopify_app/pull/1342) +* Fix webhooks handling in Ruby 3 [#1342](https://github.com/Shopify/shopify_app/pull/1342) * Update to Ruby 3 and drop support to Ruby 2.5 [#1359](https://github.com/Shopify/shopify_app/pull/1359) 18.0.4 (Jan 27, 2022) @@ -51,7 +53,7 @@ BREAKING, please see migration notes. 18.0.0 (May 3, 2021) ---------- -* Support OmniAuth 2.x +* Support OmniAuth 2.x * If your app has custom OmniAuth configuration, please refer to the [OmniAuth 2.0 upgrade guide](https://github.com/omniauth/omniauth/wiki/Upgrading-to-2.0). * Support App Bridge version 2.x in the Embedded App layout. [#1241](https://github.com/Shopify/shopify_app/pull/1241) @@ -81,7 +83,7 @@ BREAKING, please see migration notes. 17.0.4 (January 25, 2021) ---------- -* Redirect user to login page if shopify domain is not found in the `EnsureAuthenticatedLinks` concern [#1158](https://github.com/Shopify/shopify_app/pull/1158) +* Redirect user to login page if shopify domain is not found in the `EnsureAuthenticatedLinks` concern [#1158](https://github.com/Shopify/shopify_app/pull/1158) 17.0.3 (January 22, 2021) ---------- diff --git a/app/controllers/shopify_app/sessions_controller.rb b/app/controllers/shopify_app/sessions_controller.rb index eb6d61551..cfd54f467 100644 --- a/app/controllers/shopify_app/sessions_controller.rb +++ b/app/controllers/shopify_app/sessions_controller.rb @@ -44,9 +44,11 @@ def authenticate end def start_oauth + callback_url = ShopifyApp.configuration.login_callback_url.gsub(%r{^/}, "") + auth_attributes = ShopifyAPI::Auth::Oauth.begin_auth( shop: sanitized_shop_name, - redirect_path: "/auth/shopify/callback", + redirect_path: "/#{callback_url}", is_online: user_session_expected? ) cookies.encrypted[auth_attributes[:cookie].name] = { diff --git a/config/routes.rb b/config/routes.rb index 2d84072d5..899888d57 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,14 +1,28 @@ # frozen_string_literal: true ShopifyApp::Engine.routes.draw do + login_url = ShopifyApp.configuration.login_url.gsub(/^#{ShopifyApp.configuration.root_url}/, "") + login_callback_url = ShopifyApp.configuration.login_callback_url.gsub(/^#{ShopifyApp.configuration.root_url}/, "") + controller :sessions do - get "login" => :new, :as => :login - post "login" => :create, :as => :authenticate + get login_url => :new, :as => :login + post login_url => :create, :as => :authenticate get "logout" => :destroy, :as => :logout + + # Kept to prevent apps relying on these routes from breaking + if login_url.gsub(%r{^/}, "") != "login" + get "login" => :new, :as => :default_login + post "login" => :create, :as => :default_authenticate + end end controller :callback do - get "auth/shopify/callback" => :callback + get login_callback_url => :callback + + # Kept to prevent apps relying on these routes from breaking + if login_callback_url.gsub(%r{^/}, "") != "auth/shopify/callback" + get "auth/shopify/callback" => :default_callback + end end namespace :webhooks do diff --git a/lib/shopify_app/configuration.rb b/lib/shopify_app/configuration.rb index 1a6f735d8..a8e983eda 100644 --- a/lib/shopify_app/configuration.rb +++ b/lib/shopify_app/configuration.rb @@ -24,6 +24,7 @@ class Configuration # customise urls attr_accessor :root_url attr_writer :login_url + attr_writer :login_callback_url # customise ActiveJob queue names attr_accessor :scripttags_manager_queue_name @@ -50,6 +51,11 @@ def login_url @login_url || File.join(@root_url, "login") end + def login_callback_url + # Not including @root_url to keep historic behaviour + @login_callback_url || File.join("auth/shopify/callback") + end + def user_session_repository=(klass) ShopifyApp::SessionRepository.user_storage = klass end diff --git a/test/routes/callback_routes_test.rb b/test/routes/callback_routes_test.rb index b6747b3d4..b3b199431 100644 --- a/test/routes/callback_routes_test.rb +++ b/test/routes/callback_routes_test.rb @@ -9,7 +9,33 @@ class CallbackRoutesTest < ActionController::TestCase ShopifyApp.configuration = nil end + teardown do + ShopifyApp.configuration = nil + end + test "auth_shopify_callback routes to callback#callback" do assert_routing "/auth/shopify/callback", controller: "shopify_app/callback", action: "callback" end + + test "callback route doesn't change with custom root URL because it is in an engine" do + ShopifyApp.configuration.root_url = "/new-root" + Rails.application.reload_routes! + + assert_routing "/auth/shopify/callback", controller: "shopify_app/callback", action: "callback" + end + + test "callback route changes with custom callback URL" do + ShopifyApp.configuration.login_callback_url = "/other-callback" + Rails.application.reload_routes! + + assert_routing "/other-callback", controller: "shopify_app/callback", action: "callback" + end + + test "callback route strips out root URL if both are set since it runs in an engine" do + ShopifyApp.configuration.root_url = "/new-root" + ShopifyApp.configuration.login_callback_url = "/new-root/other-callback" + Rails.application.reload_routes! + + assert_routing "/other-callback", controller: "shopify_app/callback", action: "callback" + end end diff --git a/test/routes/sessions_routes_test.rb b/test/routes/sessions_routes_test.rb index 176ff3581..414535b67 100644 --- a/test/routes/sessions_routes_test.rb +++ b/test/routes/sessions_routes_test.rb @@ -9,6 +9,10 @@ class SessionsRoutesTest < ActionController::TestCase ShopifyApp.configuration = nil end + teardown do + ShopifyApp.configuration = nil + end + test "login routes to sessions#new" do assert_routing "/login", { controller: "shopify_app/sessions", action: "new" } end @@ -20,4 +24,29 @@ class SessionsRoutesTest < ActionController::TestCase test "logout routes to sessions#destroy" do assert_routing "/logout", { controller: "shopify_app/sessions", action: "destroy" } end + + test "login route doesn't change with custom root URL because it is in an engine" do + ShopifyApp.configuration.root_url = "/new-root" + Rails.application.reload_routes! + + assert_routing "/login", { controller: "shopify_app/sessions", action: "new" } + assert_routing({ method: "post", path: "/login" }, { controller: "shopify_app/sessions", action: "create" }) + end + + test "login route changes with custom login URL" do + ShopifyApp.configuration.login_url = "/other-login" + Rails.application.reload_routes! + + assert_routing "/other-login", { controller: "shopify_app/sessions", action: "new" } + assert_routing({ method: "post", path: "/other-login" }, { controller: "shopify_app/sessions", action: "create" }) + end + + test "login route strips out root URL if both are set since it runs in an engine" do + ShopifyApp.configuration.root_url = "/new-root" + ShopifyApp.configuration.login_url = "/new-root/other-login" + Rails.application.reload_routes! + + assert_routing "/other-login", { controller: "shopify_app/sessions", action: "new" } + assert_routing({ method: "post", path: "/other-login" }, { controller: "shopify_app/sessions", action: "create" }) + end end