Merge pull request #1818 from Shopify/login-to-install

Redirect to shopify managed install path during login
Shopify · Mar 27, 2024 · 5c51437 · 5c51437
2 parents 932f5f3 + 9a7dd5c
commit 5c51437
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 34 deletions.
diff --git a/app/controllers/shopify_app/sessions_controller.rb b/app/controllers/shopify_app/sessions_controller.rb
@@ -37,23 +37,39 @@ 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?
         ShopifyApp::Logger.debug("Embedded URL within / 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
@@ -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
@@ -305,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 "/"
@@ -374,6 +386,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)