From 5cb6dc0911c01005d23a97a374b51edc243e5dee Mon Sep 17 00:00:00 2001
From: Paulo Margarido <64600052+paulomarg@users.noreply.github.com>
Date: Thu, 8 Feb 2024 09:48:42 -0500
Subject: [PATCH 1/2] Use a 401 with headers for login redirect

After a 401 response from Shopify, we were simply attempting to redirect
to Shopify, which wouldn't work for XHR requests for embedded apps.

We can instead use redirect_to_login, which is aware of the context and
will return the appropriate response in all cases.
---
 CHANGELOG.md                                            | 1 +
 lib/shopify_app/controller_concerns/login_protection.rb | 7 ++++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7b7fe7340..cafbbab7f 100644
--- 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
index ac73a44de..c60a51750 100644
--- 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)

From 55b9ea569e6feedb7089db05e0c4adc0d5abebad Mon Sep 17 00:00:00 2001
From: Paulo Margarido <64600052+paulomarg@users.noreply.github.com>
Date: Thu, 8 Feb 2024 14:19:30 -0500
Subject: [PATCH 2/2] Adding tests for new 401 handling

---
 .../controller_concerns/login_protection_test.rb    | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/test/shopify_app/controller_concerns/login_protection_test.rb b/test/shopify_app/controller_concerns/login_protection_test.rb
index 7f11ac758..36e25716f 100644
--- 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"