diff --git a/CHANGELOG.md b/CHANGELOG.md
index ac0f5d050..7be409bad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,6 @@
Unreleased
----------
+* Fix session deletion for users with customized session storage[#1773](https://github.com/Shopify/shopify_app/pull/1773)
21.9.0 (January 16, 2023)
----------
diff --git a/docs/shopify_app/sessions.md b/docs/shopify_app/sessions.md
index b5af10e16..140e4efa1 100644
--- a/docs/shopify_app/sessions.md
+++ b/docs/shopify_app/sessions.md
@@ -4,23 +4,27 @@ Sessions are used to make contextual API calls for either a shop (offline sessio
#### Table of contents
-- [Sessions](#sessions-1)
- - [Types of session tokens](#types-of-session-tokens) - Shop (offline) v.s. User (online)
- - [Session token storage](#session-token-storage)
- - [Shop (offline) token storage](#shop-offline-token-storage)
- - [User (online) token storage](#user-online-token-storage)
- - [In-Memory Session Storage for Testing](#in-memory-session-storage-for-testing)
- - [Customizing Session Storage with `ShopifyApp::SessionRepository`](#customizing-session-storage-with-shopifyappsessionrepository)
- - [Loading Sessions](#loading-sessions)
+- [Sessions](#sessions)
+ - [Table of contents](#table-of-contents)
+ - [Sessions](#sessions-1)
+ - [Types of session tokens](#types-of-session-tokens)
+ - [Session token storage](#session-token-storage)
+ - [Shop (offline) token storage](#shop-offline-token-storage)
+ - [User (online) token storage](#user-online-token-storage)
+ - [In-memory Session Storage for testing](#in-memory-session-storage-for-testing)
+ - [Customizing Session Storage with `ShopifyApp::SessionRepository`](#customizing-session-storage-with-shopifyappsessionrepository)
+ - [⚠️ Custom Session Storage Requirements](#️--custom-session-storage-requirements)
+ - [Available `ActiveSupport::Concerns` that contains implementation of the above methods](#available-activesupportconcerns-that-contains-implementation-of-the-above-methods)
+ - [Loading Sessions](#loading-sessions)
- [Getting Sessions with Controller Concerns](#getting-sessions-with-controller-concerns)
- - [Shop session - "EnsureInstalled" ](#shop-sessions---ensureinstalled)
- - [User session - "EnsureHasSession" ](#user-sessions---ensurehassession)
- - [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)
-- [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)
+ - [**Shop Sessions - `EnsureInstalled`**](#shop-sessions---ensureinstalled)
+ - [User Sessions - `EnsureHasSession`](#user-sessions---ensurehassession)
+ - [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)
+ - [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)
## Sessions
#### Types of session tokens
@@ -103,6 +107,7 @@ The custom **Shop** repository must implement the following methods:
| `self.store(auth_session)` | `auth_session` (ShopifyAPI::Auth::Session) | - |
| `self.retrieve(id)` | `id` (String) | ShopifyAPI::Auth::Session |
| `self.retrieve_by_shopify_domain(shopify_domain)` | `shopify_domain` (String) | ShopifyAPI::Auth::Session |
+| `self.destroy_by_shopify_domain(shopify_domain)` | `shopify_domain` (String) | - |
The custom **User** repository must implement the following methods:
| Method | Parameters | Return Type |
@@ -110,6 +115,7 @@ The custom **User** repository must implement the following methods:
| `self.store(auth_session, user)` |
`auth_session` (ShopifyAPI::Auth::Session)
`user` (ShopifyAPI::Auth::AssociatedUser) | - |
| `self.retrieve(id)` | `id` (String) | `ShopifyAPI::Auth::Session` |
| `self.retrieve_by_shopify_user_id(user_id)` | `user_id` (String) | `ShopifyAPI::Auth::Session` |
+| `self.destroy_by_shopify_user_id(user_id)` | `user_id` (String) | - |
These methods are already implemented as a part of the `User` and `Shop` models generated from this gem's generator.
diff --git a/lib/shopify_app/session/session_repository.rb b/lib/shopify_app/session/session_repository.rb
index e02ff4771..d1652e7b1 100644
--- a/lib/shopify_app/session/session_repository.rb
+++ b/lib/shopify_app/session/session_repository.rb
@@ -23,6 +23,14 @@ def retrieve_user_session_by_shopify_user_id(user_id)
user_storage.retrieve_by_shopify_user_id(user_id)
end
+ def destroy_shop_session_by_domain(shopify_domain)
+ shop_storage.destroy_by_shopify_domain(shopify_domain)
+ end
+
+ def destroy_user_session_by_shopify_user_id(user_id)
+ user_storage.destroy_by_shopify_user_id(user_id)
+ end
+
def store_shop_session(session)
shop_storage.store(session)
end
@@ -73,18 +81,17 @@ def load_session(id)
def delete_session(id)
match = id.match(/^offline_(.*)/)
- record = if match
+ if match
domain = match[1]
ShopifyApp::Logger.debug("Destroying session by domain - domain: #{domain}")
- Shop.find_by(shopify_domain: match[1])
+ destroy_shop_session_by_domain(domain)
+
else
shopify_user_id = id.split("_").last
ShopifyApp::Logger.debug("Destroying session by user - user_id: #{shopify_user_id}")
- User.find_by(shopify_user_id: shopify_user_id)
+ destroy_user_session_by_shopify_user_id(shopify_user_id)
end
- record.destroy
-
true
end
diff --git a/lib/shopify_app/session/shop_session_storage.rb b/lib/shopify_app/session/shop_session_storage.rb
index a1cc27ef7..f3cc08171 100644
--- a/lib/shopify_app/session/shop_session_storage.rb
+++ b/lib/shopify_app/session/shop_session_storage.rb
@@ -27,6 +27,10 @@ def retrieve_by_shopify_domain(domain)
construct_session(shop)
end
+ def destroy_by_shopify_domain(domain)
+ destroy_by(shopify_domain: domain)
+ end
+
private
def construct_session(shop)
diff --git a/lib/shopify_app/session/shop_session_storage_with_scopes.rb b/lib/shopify_app/session/shop_session_storage_with_scopes.rb
index 5312608e5..24458e421 100644
--- a/lib/shopify_app/session/shop_session_storage_with_scopes.rb
+++ b/lib/shopify_app/session/shop_session_storage_with_scopes.rb
@@ -29,6 +29,10 @@ def retrieve_by_shopify_domain(domain)
construct_session(shop)
end
+ def destroy_by_shopify_domain(domain)
+ destroy_by(shopify_domain: domain)
+ end
+
private
def construct_session(shop)
diff --git a/lib/shopify_app/session/user_session_storage.rb b/lib/shopify_app/session/user_session_storage.rb
index 78a77d1af..1cba9b2bf 100644
--- a/lib/shopify_app/session/user_session_storage.rb
+++ b/lib/shopify_app/session/user_session_storage.rb
@@ -28,6 +28,10 @@ def retrieve_by_shopify_user_id(user_id)
construct_session(user)
end
+ def destroy_by_shopify_user_id(user_id)
+ destroy_by(shopify_user_id: user_id)
+ end
+
private
def construct_session(user)
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 440226862..446ef5544 100644
--- a/lib/shopify_app/session/user_session_storage_with_scopes.rb
+++ b/lib/shopify_app/session/user_session_storage_with_scopes.rb
@@ -30,6 +30,10 @@ def retrieve_by_shopify_user_id(user_id)
construct_session(user)
end
+ def destroy_by_shopify_user_id(user_id)
+ destroy_by(shopify_user_id: user_id)
+ end
+
private
def construct_session(user)
diff --git a/test/shopify_app/session/session_repository_test.rb b/test/shopify_app/session/session_repository_test.rb
index 1c18bce48..2ed23a37f 100644
--- a/test/shopify_app/session/session_repository_test.rb
+++ b/test/shopify_app/session/session_repository_test.rb
@@ -140,21 +140,21 @@ class SessionRepositoryTest < ActiveSupport::TestCase
end
test(".delete_session destroys a shop record") do
- shop = MockShopInstance.new(shopify_domain: "shop", shopify_token: "token")
+ SessionRepository.shop_storage = InMemoryShopSessionStore
+ mock_session_id = "offline_abra-shop"
- Shop.expects(:find_by).with(shopify_domain: "shop").returns(shop)
- shop.expects(:destroy)
+ InMemoryShopSessionStore.expects(:destroy_by_shopify_domain).with("abra-shop")
- SessionRepository.delete_session("offline_shop")
+ SessionRepository.delete_session(mock_session_id)
end
test(".delete_session destroys a user record") do
- user = MockUserInstance.new(shopify_domain: "shop", shopify_token: "token")
+ SessionRepository.user_storage = InMemoryUserSessionStore
+ mock_session_id = "test_shop.myshopify.com_1234"
- User.expects(:find_by).with(shopify_user_id: "1234").returns(user)
- user.expects(:destroy)
+ InMemoryUserSessionStore.expects(:destroy_by_shopify_user_id).with("1234")
- SessionRepository.delete_session("shop_1234")
+ SessionRepository.delete_session(mock_session_id)
end
private
diff --git a/test/shopify_app/session/shop_session_storage_test.rb b/test/shopify_app/session/shop_session_storage_test.rb
index cecad0696..02bfc6524 100644
--- a/test/shopify_app/session/shop_session_storage_test.rb
+++ b/test/shopify_app/session/shop_session_storage_test.rb
@@ -40,6 +40,12 @@ class ShopSessionStorageTest < ActiveSupport::TestCase
assert_equal expected_session.access_token, session.access_token
end
+ test ".destroy_by_shopify_domain destroys shop session records by JWT" do
+ ShopMockSessionStore.expects(:destroy_by).with(shopify_domain: TEST_SHOPIFY_DOMAIN)
+
+ ShopMockSessionStore.destroy_by_shopify_domain(TEST_SHOPIFY_DOMAIN)
+ end
+
test ".store can store shop session records" do
mock_shop_instance = MockShopInstance.new(id: 12345)
mock_shop_instance.stubs(:save!).returns(true)
diff --git a/test/shopify_app/session/shop_session_storage_with_scopes_test.rb b/test/shopify_app/session/shop_session_storage_with_scopes_test.rb
index 87f1e08a2..663b1a9aa 100644
--- a/test/shopify_app/session/shop_session_storage_with_scopes_test.rb
+++ b/test/shopify_app/session/shop_session_storage_with_scopes_test.rb
@@ -46,6 +46,12 @@ class ShopSessionStorageWithScopesTest < ActiveSupport::TestCase
assert_equal expected_session.scope, session.scope
end
+ test ".destroy_by_shopify_domain destroys shop session records by JWT" do
+ ShopMockSessionStoreWithScopes.expects(:destroy_by).with(shopify_domain: TEST_SHOPIFY_DOMAIN)
+
+ ShopMockSessionStoreWithScopes.destroy_by_shopify_domain(TEST_SHOPIFY_DOMAIN)
+ end
+
test ".store can store shop session records" do
mock_shop_instance = MockShopInstance.new(id: 12345)
mock_shop_instance.stubs(:save!).returns(true)
diff --git a/test/shopify_app/session/user_session_storage_test.rb b/test/shopify_app/session/user_session_storage_test.rb
index 7f399f361..6f1037209 100644
--- a/test/shopify_app/session/user_session_storage_test.rb
+++ b/test/shopify_app/session/user_session_storage_test.rb
@@ -46,6 +46,12 @@ class UserSessionStorageTest < ActiveSupport::TestCase
assert_equal expected_session.access_token, session.access_token
end
+ test ".destroy_by_shopify_user_id destroys user session by shopify_user_id" do
+ UserMockSessionStore.expects(:destroy_by).with(shopify_user_id: TEST_SHOPIFY_USER_ID)
+
+ UserMockSessionStore.destroy_by_shopify_user_id(TEST_SHOPIFY_USER_ID)
+ 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)
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 1bb47188b..f1e5457e3 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
@@ -51,6 +51,12 @@ class UserSessionStorageWithScopesTest < ActiveSupport::TestCase
assert_equal expected_session.scope, session.scope
end
+ test ".destroy_by_shopify_user_id destroys user session by shopify_user_id" do
+ UserMockSessionStoreWithScopes.expects(:destroy_by).with(shopify_user_id: TEST_SHOPIFY_USER_ID)
+
+ UserMockSessionStoreWithScopes.destroy_by_shopify_user_id(TEST_SHOPIFY_USER_ID)
+ 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)