From 47b90eea3158d194ead32df73807a44364d4bdc5 Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Mon, 25 Mar 2024 11:16:21 -0600 Subject: [PATCH 1/2] Redirect to shopify managed install path during login if using new embedded auth strategy --- .../shopify_app/sessions_controller.rb | 22 ++++++++++++++++--- test/controllers/sessions_controller_test.rb | 19 ++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/app/controllers/shopify_app/sessions_controller.rb b/app/controllers/shopify_app/sessions_controller.rb index 19dbd4b1a..080abdb9a 100644 --- a/app/controllers/shopify_app/sessions_controller.rb +++ b/app/controllers/shopify_app/sessions_controller.rb @@ -37,6 +37,22 @@ def destroy def authenticate return render_invalid_shop_error unless sanitized_shop_name.present? + if ShopifyApp.configuration.use_new_embedded_auth_strategy? + ShopifyApp::Logger.debug("Starting OAuth - Redirecting to Shopify managed install") + start_install + else + ShopifyApp::Logger.debug("Starting OAuth - Redirecting to begin auth") + start_oauth + end + end + + def start_install + shop_name = sanitized_shop_name.split(".").first + install_path = "https://admin.shopify.com/store/#{shop_name}/oauth/install?client_id=#{ShopifyApp.configuration.api_key}" + redirect_to(install_path, allow_other_host: true) + end + + def start_oauth copy_return_to_param_to_session if embedded_redirect_url? @@ -44,16 +60,16 @@ def authenticate if embedded_param? redirect_for_embedded else - start_oauth + redirect_to_begin_oauth end elsif top_level? - start_oauth + redirect_to_begin_oauth else redirect_auth_to_top_level end end - def start_oauth + def redirect_to_begin_oauth callback_url = ShopifyApp.configuration.login_callback_url.gsub(%r{^/}, "") ShopifyApp::Logger.debug("Starting OAuth with the following callback URL: #{callback_url}") diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 2cfb3326f..3a2165671 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -9,12 +9,15 @@ def perform; end end module ShopifyApp + APP_API_KEY = "my_app_api_key" class SessionsControllerTest < ActionController::TestCase setup do @routes = ShopifyApp::Engine.routes ShopifyApp.configuration.api_version = ShopifyAPI::LATEST_SUPPORTED_ADMIN_VERSION ShopifyApp::SessionRepository.shop_storage = ShopifyApp::InMemoryShopSessionStore ShopifyApp::SessionRepository.user_storage = nil + ShopifyApp.configuration.wip_new_embedded_auth_strategy = false + ShopifyApp.configuration.api_key = APP_API_KEY ShopifyAppConfigurer.setup_context # need to reset context after config changes I18n.locale = :en @@ -374,6 +377,22 @@ class SessionsControllerTest < ActionController::TestCase assert_equal "Cerrar sesión", flash[:notice] end + [ + "my-shop", + "my-shop.myshopify.com", + "https://my-shop.myshopify.com", + "http://my-shop.myshopify.com", + "https://admin.shopify.com/store/my-shop", + ].each do |good_url| + test "#create redirects to Shopify managed install path instead if use_new_embedded_auth_strategy is enabled - #{good_url}" do + ShopifyApp.configuration.wip_new_embedded_auth_strategy = true + + post :create, params: { shop: good_url } + + assert_redirected_to "https://admin.shopify.com/store/my-shop/oauth/install?client_id=#{APP_API_KEY}" + end + end + private def assert_redirected_to_top_level(shop_domain, expected_url = nil) From 9a7dd5c111b84150b532d8e0e58cf2e635728994 Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Wed, 27 Mar 2024 11:39:26 -0600 Subject: [PATCH 2/2] Add test coverage --- test/controllers/sessions_controller_test.rb | 71 +++++++++++--------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 3a2165671..7ff9ecdf6 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -308,45 +308,54 @@ class SessionsControllerTest < ActionController::TestCase end [ - "myshop.com", - "myshopify.com", - "shopify.com", - "two words", - "store.myshopify.com.evil.com", - "/foo/bar", - ].each do |bad_url| - test "#create should return an error for a non-myshopify URL (#{bad_url})" do - post :create, params: { shop: bad_url } - assert_response :redirect - assert_redirected_to "/" - assert_equal I18n.t("invalid_shop_url"), flash[:error] + true, + false, + ].each do |use_new_embedded_auth_strategy| + [ + "myshop.com", + "myshopify.com", + "shopify.com", + "two words", + "store.myshopify.com.evil.com", + "/foo/bar", + ].each do |bad_url| + test "#create should return an error for a non-myshopify URL (#{bad_url}) - + when use new embedded auth strategy is #{use_new_embedded_auth_strategy}" do + ShopifyApp.configuration.stubs(:use_new_embedded_auth_strategy?).returns(use_new_embedded_auth_strategy) + post :create, params: { shop: bad_url } + assert_response :redirect + assert_redirected_to "/" + assert_equal I18n.t("invalid_shop_url"), flash[:error] + end end - end - [ - "myshop.com", - "myshopify.com", - "shopify.com", - "two words", - "store.myshopify.com.evil.com", - "/foo/bar", - ].each do |bad_url| - test "#create should return an error for a non-myshopify URL (#{bad_url}) with embedded param" do - ShopifyApp.configuration.embedded_redirect_url = "/a-redirect-page" - post :create, params: { shop: bad_url, embedded: 1 } + [ + "myshop.com", + "myshopify.com", + "shopify.com", + "two words", + "store.myshopify.com.evil.com", + "/foo/bar", + ].each do |bad_url| + test "#create should return an error for a non-myshopify URL (#{bad_url}) with embedded param - + when use new embedded auth strategy is #{use_new_embedded_auth_strategy}" do + ShopifyApp.configuration.embedded_redirect_url = "/a-redirect-page" + post :create, params: { shop: bad_url, embedded: 1 } + assert_response :redirect + assert_redirected_to "/" + assert_equal I18n.t("invalid_shop_url"), flash[:error] + end + end + + test "#create should return an error for a non-myshopify URL when using JWT authentication - + when use new embedded auth strategy is #{use_new_embedded_auth_strategy}" do + post :create, params: { shop: "invalid domain" } assert_response :redirect assert_redirected_to "/" assert_equal I18n.t("invalid_shop_url"), flash[:error] end end - test "#create should return an error for a non-myshopify URL when using JWT authentication" do - post :create, params: { shop: "invalid domain" } - assert_response :redirect - assert_redirected_to "/" - assert_equal I18n.t("invalid_shop_url"), flash[:error] - end - test "#create should render the login page if the shop param doesn't exist" do post :create assert_redirected_to "/"