Move Session Storage to shopify_app (#1055)

* make session storage optional * deprecate session storage * logging * cleaner deprecation strategy * Clarify statement about packages (#1057) Co-authored-by: Andy Waite <[email protected]> * Constrain Zeitwerk to 2.6.5 (#1059) Co-authored-by: Andy Waite <[email protected]> * Update lib/shopify_api/context.rb Co-authored-by: Andy Waite <[email protected]> * Update lib/shopify_api/utils/session_utils.rb Co-authored-by: Kevin O'Sullivan <[email protected]> * clean up deprecation logging * raise error if trying to use deprecrated utils without session_storage * throw error if misconfigured session_storage * Update lib/shopify_api/context.rb Co-authored-by: Kevin O'Sullivan <[email protected]> * change to more helpful upgrade instructions * rubocop line length * Update lib/shopify_api/context.rb Co-authored-by: Kevin O'Sullivan <[email protected]> * Update lib/shopify_api/context.rb Co-authored-by: Kevin O'Sullivan <[email protected]> * rubocop Co-authored-by: Andy Waite <[email protected]> Co-authored-by: Andy Waite <[email protected]> Co-authored-by: Kevin O'Sullivan <[email protected]>
Shopify · Nov 18, 2022 · 5b5e0b4 · 5b5e0b4
1 parent 27ab629
commit 5b5e0b4
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 48 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 Note: For changes to the API, see https://shopify.dev/changelog?filter=api
 
 ## Unreleased
+- [#1055](https://github.com/Shopify/shopify-api-ruby/pull/1055) Makes session_storage optional. Configuring the API with session_storage is now deprecated.  Session persistence is handled by the [shopify_app gem](https://github.com/Shopify/shopify_app) if using Rails.
 
 ## Version 12.2.1
 - [#1045](https://github.com/Shopify/shopify-api-ruby/pull/1045) Fixes bug with host/host_name requirement.

diff --git a/lib/shopify_api/auth/oauth.rb b/lib/shopify_api/auth/oauth.rb
@@ -93,10 +93,7 @@ def validate_auth_callback(cookies:, auth_query:)
             )
           end
 
-          unless Context.session_storage.store_session(session)
-            raise Errors::SessionStorageError,
-              "Session could not be saved. Please check your session storage implementation."
-          end
+          Context.session_storage&.store_session(session)
 
           { session: session, cookie: cookie }
         end

diff --git a/lib/shopify_api/context.rb b/lib/shopify_api/context.rb
@@ -9,13 +9,13 @@ class Context
     @api_secret_key = T.let("", String)
     @api_version = T.let(LATEST_SUPPORTED_ADMIN_VERSION, String)
     @scope = T.let(Auth::AuthScopes.new, Auth::AuthScopes)
-    @session_storage = T.let(ShopifyAPI::Auth::FileSessionStorage.new, ShopifyAPI::Auth::SessionStorage)
     @is_private = T.let(false, T::Boolean)
     @private_shop = T.let(nil, T.nilable(String))
     @is_embedded = T.let(true, T::Boolean)
     @logger = T.let(Logger.new($stdout), Logger)
     @notified_missing_resources_folder = T.let({}, T::Hash[String, T::Boolean])
     @active_session = T.let(Concurrent::ThreadLocalVar.new { nil }, Concurrent::ThreadLocalVar)
+    @session_storage = T.let(nil, T.nilable(ShopifyAPI::Auth::SessionStorage))
     @user_agent_prefix = T.let(nil, T.nilable(String))
     @old_api_secret_key = T.let(nil, T.nilable(String))
 
@@ -32,8 +32,8 @@ class << self
           scope: T.any(T::Array[String], String),
           is_private: T::Boolean,
           is_embedded: T::Boolean,
-          session_storage: ShopifyAPI::Auth::SessionStorage,
           logger: Logger,
+          session_storage: T.nilable(ShopifyAPI::Auth::SessionStorage),
           host_name: T.nilable(String),
           host: T.nilable(String),
           private_shop: T.nilable(String),
@@ -48,8 +48,8 @@ def setup(
         scope:,
         is_private:,
         is_embedded:,
-        session_storage:,
         logger: Logger.new($stdout),
+        session_storage: nil,
         host_name: nil,
         host: ENV["HOST"] || "https://#{host_name}",
         private_shop: nil,
@@ -69,6 +69,12 @@ def setup(
         @scope = Auth::AuthScopes.new(scope)
         @is_embedded = is_embedded
         @session_storage = session_storage
+        if @session_storage
+          ::ShopifyAPI::Context.logger.warn("The use of SessionStorage in the API library has been deprecated. " \
+            "The ShopifyAPI will no longer have responsibility for session persistence. " \
+            "Upgrading to `shopify_app` 21.3 will allow you to remove session_storage" \
+            " from the API library Context configuration.")
+        end
         @logger = logger
         @private_shop = private_shop
         @user_agent_prefix = user_agent_prefix
@@ -111,7 +117,7 @@ def load_rest_resources(api_version:)
       sig { returns(Auth::AuthScopes) }
       attr_reader :scope
 
-      sig { returns(ShopifyAPI::Auth::SessionStorage) }
+      sig { returns(T.nilable(ShopifyAPI::Auth::SessionStorage)) }
       attr_reader :session_storage
 
       sig { returns(Logger) }

diff --git a/lib/shopify_api/utils/session_utils.rb b/lib/shopify_api/utils/session_utils.rb
@@ -17,12 +17,13 @@ class << self
           ).returns(T.nilable(Auth::Session))
         end
         def load_current_session(auth_header: nil, cookies: nil, is_online: false)
+          validate_session_storage_for_deprecated_utils
           return load_private_session if Context.private?
 
           session_id = current_session_id(auth_header, cookies, is_online)
           return nil unless session_id
 
-          Context.session_storage.load_session(session_id)
+          T.must(Context.session_storage).load_session(session_id)
         end
 
         sig do
@@ -33,10 +34,12 @@ def load_current_session(auth_header: nil, cookies: nil, is_online: false)
           ).returns(T::Boolean)
         end
         def delete_current_session(auth_header: nil, cookies: nil, is_online: false)
+          validate_session_storage_for_deprecated_utils
+
           session_id = current_session_id(auth_header, cookies, is_online)
           return false unless session_id
 
-          Context.session_storage.delete_session(session_id)
+          T.must(Context.session_storage).delete_session(session_id)
         end
 
         sig do
@@ -46,8 +49,10 @@ def delete_current_session(auth_header: nil, cookies: nil, is_online: false)
           ).returns(T.nilable(Auth::Session))
         end
         def load_offline_session(shop:, include_expired: false)
+          validate_session_storage_for_deprecated_utils
+
           session_id = offline_session_id(shop)
-          session = Context.session_storage.load_session(session_id)
+          session = T.must(Context.session_storage).load_session(session_id)
           return nil if session && !include_expired && session.expires && T.must(session.expires) < Time.now
 
           session
@@ -59,23 +64,10 @@ def load_offline_session(shop:, include_expired: false)
           ).returns(T::Boolean)
         end
         def delete_offline_session(shop:)
-          session_id = offline_session_id(shop)
-          Context.session_storage.delete_session(session_id)
-        end
-
-        private
-
-        sig { returns(Auth::Session) }
-        def load_private_session
-          unless Context.private_shop
-            raise Errors::SessionNotFoundError, "Could not load private shop, Context.private_shop is nil."
-          end
+          validate_session_storage_for_deprecated_utils
 
-          Auth::Session.new(
-            shop: T.must(Context.private_shop),
-            access_token: Context.api_secret_key,
-            scope: Context.scope.to_a,
-          )
+          session_id = offline_session_id(shop)
+          T.must(Context.session_storage).delete_session(session_id)
         end
 
         sig do
@@ -129,6 +121,29 @@ def offline_session_id(shop)
         def cookie_session_id(cookies)
           cookies[Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME]
         end
+
+        private
+
+        sig { void }
+        def validate_session_storage_for_deprecated_utils
+          unless Context.session_storage
+            raise ShopifyAPI::Errors::SessionStorageError,
+              "session_storage is required in ShopifyAPI::Context when using deprecated Session utility methods."
+          end
+        end
+
+        sig { returns(Auth::Session) }
+        def load_private_session
+          unless Context.private_shop
+            raise Errors::SessionNotFoundError, "Could not load private shop, Context.private_shop is nil."
+          end
+
+          Auth::Session.new(
+            shop: T.must(Context.private_shop),
+            access_token: Context.api_secret_key,
+            scope: Context.scope.to_a,
+          )
+        end
       end
     end
   end

diff --git a/test/auth/oauth_test.rb b/test/auth/oauth_test.rb
@@ -285,20 +285,6 @@ def test_validate_auth_callback_bad_http_response
         end
       end
 
-      def test_validate_auth_callback_save_session_fails
-        stub_request(:post, "https://#{@shop}/admin/oauth/access_token")
-          .with(body: @access_token_request)
-          .to_return(body: @offline_token_response.to_json, headers: { content_type: "application/json" })
-
-        modify_context(is_embedded: true, session_storage: TestHelpers::FakeSessionStorage.new(
-          sessions: { @session_cookie => ShopifyAPI::Auth::Session.new(shop: @shop, id: @session_cookie,
-            state: @callback_state) }, error_on_save: true
-        ))
-        assert_raises(ShopifyAPI::Errors::SessionStorageError) do
-          ShopifyAPI::Auth::Oauth.validate_auth_callback(cookies: @cookies, auth_query: @auth_query)
-        end
-      end
-
       private
 
       def verify_oauth_begin(auth_route:, cookie:, is_online:, scope: ShopifyAPI::Context.scope)

diff --git a/test/utils/session_utils_test.rb b/test/utils/session_utils_test.rb
@@ -26,20 +26,20 @@ def setup
         @cookie_id = "1234-this-is-a-cookie-session-id"
         @cookies = { ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME => @cookie_id }
         @online_session = ShopifyAPI::Auth::Session.new(id: @cookie_id, shop: @shop, is_online: true)
-        ShopifyAPI::Context.session_storage.store_session(@online_session)
+        T.must(ShopifyAPI::Context.session_storage).store_session(@online_session)
 
         @online_embedded_session_id = "#{@shop}_#{@jwt_payload[:sub]}"
         @online_embedded_session = ShopifyAPI::Auth::Session.new(id: @online_embedded_session_id, shop: @shop,
           is_online: true)
-        ShopifyAPI::Context.session_storage.store_session(@online_embedded_session)
+        T.must(ShopifyAPI::Context.session_storage).store_session(@online_embedded_session)
 
         @offline_cookie_id = "offline_#{@shop}"
         @offline_cookies = { ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME => @offline_cookie_id }
         @offline_session = ShopifyAPI::Auth::Session.new(id: @offline_cookie_id, shop: @shop, is_online: false)
-        ShopifyAPI::Context.session_storage.store_session(@offline_session)
+        T.must(ShopifyAPI::Context.session_storage).store_session(@offline_session)
 
         @offline_embedded_session = ShopifyAPI::Auth::Session.new(id: "offline_#{@shop}", shop: @shop, is_online: false)
-        ShopifyAPI::Context.session_storage.store_session(@offline_embedded_session)
+        T.must(ShopifyAPI::Context.session_storage).store_session(@offline_embedded_session)
       end
 
       def test_gets_the_current_session_from_cookies_for_non_embedded_apps
@@ -57,6 +57,37 @@ def test_returns_nil_if_there_is_no_session_for_non_embedded_apps
         ))
       end
 
+      def test_raise_error_when_session_storage_missing_when_using_deprecated_session_utils
+        ShopifyAPI::Context.stubs(:session_storage).returns(nil)
+
+        assert_raises(ShopifyAPI::Errors::SessionStorageError) do
+          ShopifyAPI::Utils::SessionUtils.load_current_session(
+            auth_header: @jwt_header,
+            is_online: true,
+          )
+        end
+
+        assert_raises(ShopifyAPI::Errors::SessionStorageError) do
+          ShopifyAPI::Utils::SessionUtils.delete_current_session(
+            auth_header: @jwt_header,
+            cookies: @cookies,
+            is_online: true,
+          )
+        end
+
+        assert_raises(ShopifyAPI::Errors::SessionStorageError) do
+          ShopifyAPI::Utils::SessionUtils.load_offline_session(
+            shop: @shop,
+          )
+        end
+
+        assert_raises(ShopifyAPI::Errors::SessionStorageError) do
+          ShopifyAPI::Utils::SessionUtils.delete_offline_session(
+            shop: @shop,
+          )
+        end
+      end
+
       def test_gets_the_current_session_from_auth_header_for_embedded_apps
         modify_context(is_embedded: true)
         add_session(is_online: true)
@@ -148,15 +179,15 @@ def test_loads_offline_session_by_shop
       def test_returns_nil_for_expired_offline_session
         offline_session = ShopifyAPI::Auth::Session.new(id: "offline_#{@shop}", shop: @shop, is_online: false,
           expires: Time.now - 60)
-        ShopifyAPI::Context.session_storage.store_session(offline_session)
+        T.must(ShopifyAPI::Context.session_storage).store_session(offline_session)
         loaded_session = ShopifyAPI::Utils::SessionUtils.load_offline_session(shop: @shop)
         assert_nil(loaded_session)
       end
 
       def test_loads_expired_offline_session_if_requested
         offline_session = ShopifyAPI::Auth::Session.new(id: "offline_#{@shop}", shop: @shop, is_online: false,
           expires: Time.now - 60)
-        ShopifyAPI::Context.session_storage.store_session(offline_session)
+        T.must(ShopifyAPI::Context.session_storage).store_session(offline_session)
         loaded_session = ShopifyAPI::Utils::SessionUtils.load_offline_session(shop: @shop, include_expired: true)
         assert_equal(offline_session, loaded_session)
       end
@@ -235,7 +266,7 @@ def test_load_private_session_no_private_shop
 
       def add_session(is_online:)
         another_session = ShopifyAPI::Auth::Session.new(shop: @shop, is_online: is_online)
-        ShopifyAPI::Context.session_storage.store_session(another_session)
+        T.must(ShopifyAPI::Context.session_storage).store_session(another_session)
       end
 
       def create_jwt_header(api_secret_key)