Merge pull request #1787 from Shopify/catch_401_errors_in_login_prote…

…ction Properly handle 401 errors in XHR requests with LoginProtection
Shopify · Feb 9, 2024 · d4cfb7e · d4cfb7e
2 parents 8fc434a + 55b9ea5
commit d4cfb7e
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 Unreleased
 ----------
 * Make type param for webhooks route optional. This will fix a bug with CLI initiated webhooks.[1786](https://github.com/Shopify/shopify_app/pull/1786)
+* Fix redirecting to login when we catch a 401 response from Shopify, so that it can also handle cases where the app is already embedded when that happens.[1787](https://github.com/Shopify/shopify_app/pull/1787)
 
 21.10.0 (January 24, 2024)
 ----------

diff --git a/lib/shopify_app/controller_concerns/login_protection.rb b/lib/shopify_app/controller_concerns/login_protection.rb
@@ -147,10 +147,11 @@ def redirect_to_login
     end
 
     def close_session
-      clear_shopify_session
       ShopifyApp::Logger.debug("Closing session")
-      ShopifyApp::Logger.debug("Redirecting to #{login_url_with_optional_shop}")
-      redirect_to(login_url_with_optional_shop)
+      clear_shopify_session
+
+      ShopifyApp::Logger.debug("Redirecting to login")
+      redirect_to_login
     end
 
     def handle_http_error(error)

diff --git a/test/shopify_app/controller_concerns/login_protection_test.rb b/test/shopify_app/controller_concerns/login_protection_test.rb
@@ -409,11 +409,22 @@ class LoginProtectionControllerTest < ActionController::TestCase
       cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] = "cookie"
 
       get :raise_unauthorized, params: { shop: "foobar" }
-      assert_redirected_to "/login?shop=foobar.myshopify.com"
+      assert_redirected_to(
+        "/login?return_to=%2Fraise_unauthorized%3Fshop%3Dfoobar.myshopify.com&shop=foobar.myshopify.com",
+      )
       assert_nil cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME]
     end
   end
 
+  test "#activate_shopify_session when rescuing from unauthorized access, breaks out of iframe in XHR requests" do
+    with_application_test_routes do
+      get :raise_unauthorized, params: { shop: "foobar" }, xhr: true
+      assert_equal 401, response.status
+      assert_match "1", response.headers["X-Shopify-API-Request-Failure-Reauthorize"]
+      assert_match "/login?shop=foobar", response.headers["X-Shopify-API-Request-Failure-Reauthorize-Url"]
+    end
+  end
+
   test "#activate_shopify_session when rescuing from non 401 errors, does not close session" do
     with_application_test_routes do
       cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] = "cookie"