Move expiry date to a new concern

Shopify · Nov 27, 2023 · 8c56b41 · 8c56b41
1 parent 4d24cc3
commit 8c56b41
Show file tree

Hide file tree

Showing 9 changed files with 239 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,6 @@
 Unreleased
 ----------
-* Support for storing the user session expiry date and added a check to trigger a re-auth when it is expired. The DB migration can be generated with `rails generate shopify_app:user_model --skip` [#1757](https://github.com/Shopify/shopify_app/pull/1757)
+* Added a `UserSessionStorageWithScopesAndExpiry` concern for storing the user session expiry date and added a check to trigger a re-auth when it is expired. The DB migration can be generated with `rails generate shopify_app:user_model --skip` [#1757](https://github.com/Shopify/shopify_app/pull/1757)
 
 21.8.0 (Nov 13, 2023)
 ----------

diff --git a/docs/shopify_app/sessions.md b/docs/shopify_app/sessions.md
@@ -18,7 +18,7 @@ Sessions are used to make contextual API calls for either a shop (offline sessio
       - [Getting Sessions from a Shop or User model record - "with_shopify_session"](#getting-sessions-from-a-shop-or-user-model-record---with_shopify_session)
 - [Access scopes](#access-scopes)
   - [`ShopifyApp::ShopSessionStorageWithScopes`](#shopifyappshopsessionstoragewithscopes)
-  - [``ShopifyApp::UserSessionStorageWithScopes``](#shopifyappusersessionstoragewithscopes)
+  - [``ShopifyApp::UserSessionStorageWithScopesAndExpiry``](#shopifyappusersessionstoragewithscopesandexpiry)
 - [Migrating from shop-based to user-based token strategy](#migrating-from-shop-based-to-user-based-token-strategy)
 - [Migrating from ShopifyApi::Auth::SessionStorage to ShopifyApp::SessionStorage](#migrating-from-shopifyapiauthsessionstorage-to-shopifyappsessionstorage)
 
@@ -114,7 +114,7 @@ The custom **User** repository must implement the following methods:
 
 These methods are already implemented as a part of the `User` and `Shop` models generated from this gem's generator.
 - `Shop` model includes the [ShopSessionStorageWithScopes](https://github.com/Shopify/shopify_app/blob/main/lib/shopify_app/session/shop_session_storage_with_scopes.rb) concern.
-- `User` model includes the [UserSessionStorageWithScopes](https://github.com/Shopify/shopify_app/blob/main/lib/shopify_app/session/user_session_storage_with_scopes.rb) concern.
+- `User` model includes the [UserMockSessionStoreWithScopesAndExpiry](https://github.com/Shopify/shopify_app/blob/main/lib/shopify_app/session/user_session_storage_with_scopes_and_expiry.rb) concern.
 
 ##### Available `ActiveSupport::Concerns` that contains implementation of the above methods
 Simply include these concerns if you want to use the implementation, and overwrite methods for custom implementation
@@ -124,6 +124,7 @@ Simply include these concerns if you want to use the implementation, and overwri
   - [ShopSessionStorage](https://github.com/Shopify/shopify_app/blob/main/lib/shopify_app/session/shop_session_storage.rb)
 
 - `User` storage
+  - [UserSessionStorageWithScopesAndExpiry](https://github.com/Shopify/shopify_app/blob/main/lib/shopify_app/session/user_session_storage_with_scopes_and_expiry.rb)
   - [UserSessionStorageWithScopes](https://github.com/Shopify/shopify_app/blob/main/lib/shopify_app/session/user_session_storage_with_scopes.rb)
   - [UserSessionStorage](https://github.com/Shopify/shopify_app/blob/main/lib/shopify_app/session/user_session_storage.rb)
 
@@ -198,7 +199,7 @@ end
 ```
 
 ## Access scopes
-If you want to customize how access scopes are stored for shops and users, you can implement the `access_scopes` getters and setters in the models that include `ShopifyApp::ShopSessionStorageWithScopes` and `ShopifyApp::UserSessionStorageWithScopes` as shown:
+If you want to customize how access scopes are stored for shops and users, you can implement the `access_scopes` getters and setters in the models that include `ShopifyApp::ShopSessionStorageWithScopes` and `ShopifyApp::UserSessionStorageWithScopesAndExpiry` as shown:
 
 ### `ShopifyApp::ShopSessionStorageWithScopes`
 ```ruby
@@ -214,10 +215,10 @@ class Shop < ActiveRecord::Base
 end
 ```
 
-### `ShopifyApp::UserSessionStorageWithScopes`
+### `ShopifyApp::UserSessionStorageWithScopesAndExpiry`
 ```ruby
 class User < ActiveRecord::Base
-  include ShopifyApp::UserSessionStorageWithScopes
+  include ShopifyApp::UserSessionStorageWithScopesAndExpiry
 
   def access_scopes=(scopes)
     # Store access scopes

diff --git a/lib/generators/shopify_app/user_model/templates/user.rb b/lib/generators/shopify_app/user_model/templates/user.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class User < ActiveRecord::Base
-  include ShopifyApp::UserSessionStorageWithScopes
+  include ShopifyApp::UserSessionStorageWithScopesAndExpiry
 
   def api_version
     ShopifyApp.configuration.api_version

diff --git a/lib/shopify_app.rb b/lib/shopify_app.rb
@@ -77,6 +77,7 @@ def self.use_webpacker?
   require "shopify_app/session/shop_session_storage_with_scopes"
   require "shopify_app/session/user_session_storage"
   require "shopify_app/session/user_session_storage_with_scopes"
+  require "shopify_app/session/user_session_storage_with_scopes_and_expiry"
 
   # access scopes strategies
   require "shopify_app/access_scopes/shop_strategy"

diff --git a/lib/shopify_app/session/user_session_storage_with_scopes.rb b/lib/shopify_app/session/user_session_storage_with_scopes.rb
@@ -15,7 +15,6 @@ def store(auth_session, user)
         user.shopify_token = auth_session.access_token
         user.shopify_domain = auth_session.shop
         user.access_scopes = auth_session.scope.to_s
-        user.expires_at = auth_session.expires
 
         user.save!
         user.id
@@ -53,7 +52,6 @@ def construct_session(user)
           scope: user.access_scopes,
           associated_user_scope: user.access_scopes,
           associated_user: associated_user,
-          expires: user.expires_at,
         )
       end
     end
@@ -69,18 +67,5 @@ def access_scopes
     rescue NotImplementedError, NoMethodError
       raise NotImplementedError, "#access_scopes= must be defined to hook into stored access scopes"
     end
-
-    def expires_at=(expires_at)
-      super(expires_at)
-    rescue NotImplementedError, NoMethodError
-      ShopifyApp::Logger.warn("#expires_at= must be defined to handle storing the session expiry date")
-    end
-
-    def expires_at
-      super
-    rescue NotImplementedError, NoMethodError
-      ShopifyApp::Logger.warn("#expires_at must be defined to leverage the session expiry date")
-      nil
-    end
   end
 end
diff --git a/lib/shopify_app/session/user_session_storage_with_scopes_and_expiry.rb b/lib/shopify_app/session/user_session_storage_with_scopes_and_expiry.rb
@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+module ShopifyApp
+  module UserSessionStorageWithScopesAndExpiry
+    extend ActiveSupport::Concern
+    include ::ShopifyApp::UserSessionStorageWithScopes
+
+    included do
+      validates :shopify_domain, presence: true
+    end
+
+    class_methods do
+      def store(auth_session, user)
+        user = find_or_initialize_by(shopify_user_id: user.id)
+        user.shopify_token = auth_session.access_token
+        user.shopify_domain = auth_session.shop
+        user.access_scopes = auth_session.scope.to_s
+        user.expires_at = auth_session.expires
+
+        user.save!
+        user.id
+      end
+
+      def retrieve(id)
+        user = find_by(id: id)
+        construct_session(user)
+      end
+
+      def retrieve_by_shopify_user_id(user_id)
+        user = find_by(shopify_user_id: user_id)
+        construct_session(user)
+      end
+
+      private
+
+      def construct_session(user)
+        return unless user
+
+        associated_user = ShopifyAPI::Auth::AssociatedUser.new(
+          id: user.shopify_user_id,
+          first_name: "",
+          last_name: "",
+          email: "",
+          email_verified: false,
+          account_owner: false,
+          locale: "",
+          collaborator: false,
+        )
+
+        ShopifyAPI::Auth::Session.new(
+          shop: user.shopify_domain,
+          access_token: user.shopify_token,
+          scope: user.access_scopes,
+          associated_user_scope: user.access_scopes,
+          associated_user: associated_user,
+          expires: user.expires_at,
+        )
+      end
+    end
+
+    def expires_at=(expires_at)
+      super(expires_at)
+    rescue NotImplementedError, NoMethodError
+      raise NotImplementedError, "#expires_at= must be defined to handle storing the expiry date"
+    end
+
+    def expires_at
+      super
+    rescue NotImplementedError, NoMethodError
+      raise NotImplementedError, "#expires_at must be defined to leverage the session expiry date"
+    end
+  end
+end
diff --git a/test/generators/user_model_generator_test.rb b/test/generators/user_model_generator_test.rb
@@ -16,7 +16,7 @@ class UserModelGeneratorTest < Rails::Generators::TestCase
     run_generator
     assert_file "app/models/user.rb" do |user|
       assert_match "class User < ActiveRecord::Base", user
-      assert_match "include ShopifyApp::UserSessionStorageWithScopes", user
+      assert_match "include ShopifyApp::UserSessionStorageWithScopesAndExpiry", user
       assert_match(/def api_version\n\s*ShopifyApp\.configuration\.api_version\n\s*end/, user)
     end
   end

diff --git a/test/shopify_app/session/user_session_storage_with_scopes_and_expiry_test.rb b/test/shopify_app/session/user_session_storage_with_scopes_and_expiry_test.rb
@@ -0,0 +1,156 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class UserMockSessionStoreWithScopesAndExpiry < ActiveRecord::Base
+  include ShopifyApp::UserSessionStorageWithScopesAndExpiry
+end
+
+module ShopifyApp
+  class UserSessionStorageWithScopeAndExpiryTest < ActiveSupport::TestCase
+    TEST_SHOPIFY_USER_ID = 42
+    TEST_SHOPIFY_DOMAIN = "example.myshopify.com"
+    TEST_SHOPIFY_USER_TOKEN = "some-user-token-42"
+    TEST_MERCHANT_SCOPES = "read_orders, write_products"
+    TEST_EXPIRES_AT = Time.now
+
+    test ".retrieve returns user session by id" do
+      UserMockSessionStoreWithScopesAndExpiry.stubs(:find_by).returns(MockUserInstance.new(
+        shopify_user_id: TEST_SHOPIFY_USER_ID,
+        shopify_domain: TEST_SHOPIFY_DOMAIN,
+        shopify_token: TEST_SHOPIFY_USER_TOKEN,
+        scopes: TEST_MERCHANT_SCOPES,
+        expires_at: TEST_EXPIRES_AT,
+      ))
+
+      session = UserMockSessionStoreWithScopesAndExpiry.retrieve(shopify_user_id: TEST_SHOPIFY_USER_ID)
+
+      assert_equal TEST_SHOPIFY_DOMAIN, session.shop
+      assert_equal TEST_SHOPIFY_USER_TOKEN, session.access_token
+      assert_equal ShopifyAPI::Auth::AuthScopes.new(TEST_MERCHANT_SCOPES), session.scope
+      assert_equal TEST_EXPIRES_AT, session.expires
+    end
+
+    test ".retrieve_by_shopify_user_id returns user session by shopify_user_id" do
+      instance = MockUserInstance.new(
+        shopify_user_id: TEST_SHOPIFY_USER_ID,
+        shopify_domain: TEST_SHOPIFY_DOMAIN,
+        shopify_token: TEST_SHOPIFY_USER_TOKEN,
+        api_version: ShopifyApp.configuration.api_version,
+        scopes: TEST_MERCHANT_SCOPES,
+        expires_at: TEST_EXPIRES_AT,
+      )
+      UserMockSessionStoreWithScopesAndExpiry.stubs(:find_by).with(shopify_user_id: TEST_SHOPIFY_USER_ID)
+        .returns(instance)
+
+      expected_session = ShopifyAPI::Auth::Session.new(
+        shop: instance.shopify_domain,
+        access_token: instance.shopify_token,
+        scope: TEST_MERCHANT_SCOPES,
+        expires: TEST_EXPIRES_AT,
+      )
+
+      user_id = TEST_SHOPIFY_USER_ID
+      session = UserMockSessionStoreWithScopesAndExpiry.retrieve_by_shopify_user_id(user_id)
+      assert_equal expected_session.shop, session.shop
+      assert_equal expected_session.access_token, session.access_token
+      assert_equal expected_session.scope, session.scope
+      assert_equal expected_session.expires, session.expires
+    end
+
+    test ".store can store user session record" do
+      mock_user_instance = MockUserInstance.new(shopify_user_id: 100)
+      mock_user_instance.stubs(:save!).returns(true)
+
+      UserMockSessionStoreWithScopesAndExpiry.stubs(:find_or_initialize_by).returns(mock_user_instance)
+
+      saved_id = UserMockSessionStoreWithScopesAndExpiry.store(
+        mock_session(
+          shop: mock_user_instance.shopify_domain,
+          scope: TEST_MERCHANT_SCOPES,
+        ),
+        mock_associated_user,
+      )
+
+      assert_equal "a-new-user_token!", mock_user_instance.shopify_token
+      assert_equal mock_user_instance.id, saved_id
+    end
+
+    test ".retrieve returns nil for non-existent user" do
+      user_id = "non-existent-user"
+      UserMockSessionStoreWithScopesAndExpiry.stubs(:find_by).with(id: user_id).returns(nil)
+
+      refute UserMockSessionStoreWithScopesAndExpiry.retrieve(user_id)
+    end
+
+    test ".retrieve_by_user_id returns nil for non-existent user" do
+      user_id = "non-existent-user"
+      UserMockSessionStoreWithScopesAndExpiry.stubs(:find_by).with(shopify_user_id: user_id).returns(nil)
+
+      refute UserMockSessionStoreWithScopesAndExpiry.retrieve_by_shopify_user_id(user_id)
+    end
+
+    test ".retrieve throws NotImplementedError when access_scopes getter is not implemented" do
+      mock_user = MockUserInstance.new(
+        shopify_user_id: TEST_SHOPIFY_USER_ID,
+        shopify_domain: TEST_SHOPIFY_DOMAIN,
+        shopify_token: TEST_SHOPIFY_USER_TOKEN,
+      )
+      mock_user.stubs(:access_scopes).raises(NotImplementedError)
+      UserMockSessionStoreWithScopesAndExpiry.stubs(:find_by).returns(mock_user)
+
+      assert_raises NotImplementedError do
+        UserMockSessionStoreWithScopesAndExpiry.retrieve(1)
+      end
+    end
+
+    test ".store throws NotImplementedError when expires_at setter is not implemented" do
+      mock_user = MockUserInstance.new(
+        shopify_user_id: TEST_SHOPIFY_USER_ID,
+        shopify_domain: TEST_SHOPIFY_DOMAIN,
+        shopify_token: TEST_SHOPIFY_USER_TOKEN,
+      )
+      mock_user.stubs(:expires_at=).raises(NotImplementedError)
+      UserMockSessionStoreWithScopesAndExpiry.stubs(:find_or_initialize_by).returns(mock_user)
+
+      assert_raises NotImplementedError do
+        UserMockSessionStoreWithScopesAndExpiry.store(
+          mock_session(
+            shop: mock_user.shopify_domain,
+            scope: TEST_MERCHANT_SCOPES,
+          ),
+          mock_associated_user,
+        )
+      end
+    end
+
+    test ".retrieve throws NotImplementedError when expires_at getter is not implemented" do
+      mock_user = MockUserInstance.new(
+        shopify_user_id: TEST_SHOPIFY_USER_ID,
+        shopify_domain: TEST_SHOPIFY_DOMAIN,
+        shopify_token: TEST_SHOPIFY_USER_TOKEN,
+      )
+      mock_user.stubs(:expires_at).raises(NotImplementedError)
+      UserMockSessionStoreWithScopesAndExpiry.stubs(:find_by).returns(mock_user)
+
+      assert_raises NotImplementedError do
+        UserMockSessionStoreWithScopesAndExpiry.retrieve(1)
+      end
+    end
+
+    private
+
+    def mock_associated_user
+      ShopifyAPI::Auth::AssociatedUser.new(
+        id: 100,
+        first_name: "John",
+        last_name: "Doe",
+        email: "johndoe@email.com",
+        email_verified: true,
+        account_owner: false,
+        locale: "en",
+        collaborator: true,
+      )
+    end
+  end
+end
diff --git a/test/shopify_app/session/user_session_storage_with_scopes_test.rb b/test/shopify_app/session/user_session_storage_with_scopes_test.rb
@@ -12,23 +12,20 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase
     TEST_SHOPIFY_DOMAIN = "example.myshopify.com"
     TEST_SHOPIFY_USER_TOKEN = "some-user-token-42"
     TEST_MERCHANT_SCOPES = "read_orders, write_products"
-    TEST_EXPIRES_AT = Time.now
 
     test ".retrieve returns user session by id" do
       UserMockSessionStoreWithScopes.stubs(:find_by).returns(MockUserInstance.new(
         shopify_user_id: TEST_SHOPIFY_USER_ID,
         shopify_domain: TEST_SHOPIFY_DOMAIN,
         shopify_token: TEST_SHOPIFY_USER_TOKEN,
         scopes: TEST_MERCHANT_SCOPES,
-        expires_at: TEST_EXPIRES_AT,
       ))
 
       session = UserMockSessionStoreWithScopes.retrieve(shopify_user_id: TEST_SHOPIFY_USER_ID)
 
       assert_equal TEST_SHOPIFY_DOMAIN, session.shop
       assert_equal TEST_SHOPIFY_USER_TOKEN, session.access_token
       assert_equal ShopifyAPI::Auth::AuthScopes.new(TEST_MERCHANT_SCOPES), session.scope
-      assert_equal TEST_EXPIRES_AT, session.expires
     end
 
     test ".retrieve_by_shopify_user_id returns user session by shopify_user_id" do
@@ -38,23 +35,20 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase
         shopify_token: TEST_SHOPIFY_USER_TOKEN,
         api_version: ShopifyApp.configuration.api_version,
         scopes: TEST_MERCHANT_SCOPES,
-        expires_at: TEST_EXPIRES_AT,
       )
       UserMockSessionStoreWithScopes.stubs(:find_by).with(shopify_user_id: TEST_SHOPIFY_USER_ID).returns(instance)
 
       expected_session = ShopifyAPI::Auth::Session.new(
         shop: instance.shopify_domain,
         access_token: instance.shopify_token,
         scope: TEST_MERCHANT_SCOPES,
-        expires: TEST_EXPIRES_AT,
       )
 
       user_id = TEST_SHOPIFY_USER_ID
       session = UserMockSessionStoreWithScopes.retrieve_by_shopify_user_id(user_id)
       assert_equal expected_session.shop, session.shop
       assert_equal expected_session.access_token, session.access_token
       assert_equal expected_session.scope, session.scope
-      assert_equal expected_session.expires, session.expires
     end
 
     test ".store can store user session record" do