diff --git a/CHANGELOG.md b/CHANGELOG.md index 10202c1de..a6036fab4 100644 --- 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 index b5af10e16..1bbfeb3cf 100644 --- 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 index 2ed254a41..2b85e63ad 100644 --- 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 index b076962ee..fb62a702e 100644 --- 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 index 8b285e579..440226862 100644 --- 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 new file mode 100644 index 000000000..4221a0e21 --- /dev/null +++ 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 index 35896f37e..7c01cb858 100644 --- 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 new file mode 100644 index 000000000..76ac7ad1b --- /dev/null +++ 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 index fb6a32b16..1bb47188b 100644 --- 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,7 +12,6 @@ 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( @@ -20,7 +19,6 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase 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) @@ -28,7 +26,6 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase 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,7 +35,6 @@ 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) @@ -46,7 +42,6 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase shop: instance.shopify_domain, access_token: instance.shopify_token, scope: TEST_MERCHANT_SCOPES, - expires: TEST_EXPIRES_AT, ) user_id = TEST_SHOPIFY_USER_ID @@ -54,7 +49,6 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase 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