Validate store sessions (#1612)

* Check shop offline session is still valid when embedded

* only redirect on 401 and use better naming

* use guard clause instead of conditional filter

* redirect to shop_login if token failed

* update tests to match expected behavior

* Changelog updates

* raise error if not 401

* better redirect test

CHANGELOG.md

@@ -1,5 +1,6 @@
 Unreleased
 ----------
+* Validates shop's offline session token is still valid when using `EnsureInstalled`[#1612](https://github.com/Shopify/shopify_app/pull/1612)
 
 21.3.1 (Dec 12, 2022)
 ----------

app/controllers/concerns/shopify_app/ensure_installed.rb

@@ -17,6 +17,7 @@ module EnsureInstalled
 
       before_action :check_shop_domain
       before_action :check_shop_known
+      before_action :validate_non_embedded_session
     end
 
     def current_shopify_domain
@@ -30,14 +31,18 @@ def current_shopify_domain
       @shopify_domain
     end
 
+    def installed_shop_session
+      @installed_shop_session ||= SessionRepository.retrieve_shop_session_by_shopify_domain(current_shopify_domain)
+    end
+
     private
 
     def check_shop_domain
       redirect_to(ShopifyApp.configuration.login_url) unless current_shopify_domain
     end
 
     def check_shop_known
-      @shop = SessionRepository.retrieve_shop_session_by_shopify_domain(current_shopify_domain)
+      @shop = installed_shop_session
       unless @shop
         if embedded_param?
           redirect_for_embedded
@@ -58,5 +63,15 @@ def shop_login
 
       url.to_s
     end
+
+    def validate_non_embedded_session
+      return if loaded_directly_from_admin?
+
+      client = ShopifyAPI::Clients::Rest::Admin.new(session: installed_shop_session)
+      client.get(path: "shop")
+    rescue ShopifyAPI::Errors::HttpResponseError => error
+      redirect_to(shop_login) if error.code == 401
+      raise error if error.code != 401
+    end
   end
 end

lib/shopify_app/controller_concerns/redirect_for_embedded.rb

@@ -11,7 +11,11 @@ def embedded_redirect_url?
     end
 
     def embedded_param?
-      embedded_redirect_url? && params[:embedded].present? && params[:embedded] == "1"
+      embedded_redirect_url? && params[:embedded].present? && loaded_directly_from_admin?
+    end
+
+    def loaded_directly_from_admin?
+      ShopifyApp.configuration.embedded_app? && params[:embedded] == "1"
     end
 
     def redirect_for_embedded

test/controllers/concerns/ensure_installed_test.rb

@@ -52,7 +52,12 @@ def index
   end
 
   test "returns :ok if the shop is installed" do
-    ShopifyApp::SessionRepository.expects(:retrieve_shop_session_by_shopify_domain).returns(true)
+    session = mock
+    ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)
+
+    client = mock
+    ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
+    client.expects(:get)
 
     shopify_domain = "shop1.myshopify.com"
 
@@ -61,6 +66,79 @@ def index
     assert_response :ok
   end
 
+  test "redirects to login_url (oauth path) to reinstall the app if the store's session token is no longer valid" do
+    session = mock
+    ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)
+
+    client = mock
+    ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
+    uninstalled_http_error = ShopifyAPI::Errors::HttpResponseError.new(
+      response: ShopifyAPI::Clients::HttpResponse.new(
+        code: 401,
+        headers: {},
+        body: "Invalid API key or access token (unrecognized login or wrong password)",
+      ),
+    )
+    client.expects(:get).with(path: "shop").raises(uninstalled_http_error)
+
+    shopify_domain = "shop1.myshopify.com"
+    host = "https://tunnel.vision.for.webhooks.com"
+
+    get :index, params: { shop: shopify_domain, host: host }
+
+    url = URI(ShopifyApp.configuration.login_url)
+    url.query = URI.encode_www_form(
+      shop: shopify_domain,
+      host: host,
+      return_to: request.fullpath,
+    )
+    assert_redirected_to url.to_s
+  end
+
+  test "throws an error if the shopify error isn't a 401" do
+    session = mock
+    ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)
+
+    client = mock
+    ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
+    uninstalled_http_error = ShopifyAPI::Errors::HttpResponseError.new(
+      response: ShopifyAPI::Clients::HttpResponse.new(
+        code: 404,
+        headers: {},
+        body: "Insert generic message about how we can't find your requests here.",
+      ),
+    )
+    client.expects(:get).with(path: "shop").raises(uninstalled_http_error)
+
+    shopify_domain = "shop1.myshopify.com"
+
+    assert_raises ShopifyAPI::Errors::HttpResponseError do
+      get :index, params: { shop: shopify_domain }
+    end
+  end
+
+  test "throws an error if the request session validation API check fails with an" do
+    session = mock
+    ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)
+
+    client = mock
+    ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
+    client.expects(:get).with(path: "shop").raises(RuntimeError)
+
+    shopify_domain = "shop1.myshopify.com"
+
+    assert_raises RuntimeError do
+      get :index, params: { shop: shopify_domain }
+    end
+  end
+
+  test "does not perform a session validation check if coming from an embedded" do
+    ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain)
+    ShopifyAPI::Clients::Rest::Admin.expects(:new).never
+
+    get :index, params: { shop: "shop1.myshopify.com" }
+  end
+
   test "detects incompatible controller concerns" do
     version = "22.0.0"
     ShopifyApp::Logger.expects(:deprecated).with(regexp_matches(/incompatible concerns/), version)

test/controllers/concerns/require_known_shop_test.rb

@@ -52,7 +52,12 @@ def index
   end
 
   test "returns :ok if the shop is installed" do
-    ShopifyApp::SessionRepository.expects(:retrieve_shop_session_by_shopify_domain).returns(true)
+    session = mock
+    ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)
+
+    client = mock
+    ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
+    client.expects(:get)
 
     shopify_domain = "shop1.myshopify.com"