Merge pull request #1757 from Shopify/store-session-expiry

Store session expiry and check it to trigger a re-auth

CHANGELOG.md

@@ -1,6 +1,7 @@
 Unreleased
 ----------
 * Fix session deletion for users with customized session storage[#1773](https://github.com/Shopify/shopify_app/pull/1773)
+* Add configuration flag `check_session_expiry_date` to trigger a re-auth when the (user) session is expired. The session expiry date must be stored and retrieved for this flag to be effective. When the `UserSessionStorageWithScopes` concern is used, a DB migration can be generated with `rails generate shopify_app:user_model --skip` and should be applied before enabling that flag[#1757](https://github.com/Shopify/shopify_app/pull/1757)
 
 21.9.0 (January 16, 2023)
 ----------

docs/shopify_app/sessions.md

@@ -159,7 +159,7 @@ end
 ```
 
 ##### User Sessions - `EnsureHasSession`
-- [EnsureHasSession](https://github.com/Shopify/shopify_app/blob/main/app/controllers/concerns/shopify_app/ensure_has_session.rb) controller concern will load a user session via `current_shopify_session`. As part of loading this session, this concern will also ensure that the user session has the appropriate scopes needed for the application. If the user isn't found or has fewer permitted scopes than are required, they will be prompted to authorize the application.
+- [EnsureHasSession](https://github.com/Shopify/shopify_app/blob/main/app/controllers/concerns/shopify_app/ensure_has_session.rb) controller concern will load a user session via `current_shopify_session`. As part of loading this session, this concern will also ensure that the user session has the appropriate scopes needed for the application and that it is not expired (when `check_session_expiry_date` is enabled). If the user isn't found or has fewer permitted scopes than are required, they will be prompted to authorize the application.
 - This controller concern should be used if you don't need your app to make calls on behalf of a user. With that in mind, there are a few other embedded concerns that are mixed in to ensure that embedding, CSRF, localization, and billing allow the action for the user.
 - Example
 ```ruby
@@ -234,6 +234,9 @@ class User < ActiveRecord::Base
 end
 ```
 
+## Expiry date
+When the configuration flag `check_session_expiry_date` is set to true, the user session expiry date will be checked to trigger a re-auth and get a fresh user token when it is expired. This requires the `ShopifyAPI::Auth::Session` `expires` attribute to be stored. When the `User` model includes the `UserSessionStorageWithScopes` concern, a DB migration can be generated with `rails generate shopify_app:user_model --skip` to add the `expires_at` attribute to the model.
+
 ## Migrating from shop-based to user-based token strategy
 
 1. Run the `user_model` generator as [mentioned above](#user-online-token-storage).

lib/generators/shopify_app/user_model/templates/db/migrate/add_user_expires_at_column.erb

@@ -0,0 +1,5 @@
+class AddUserExpiresAtColumn < ActiveRecord::Migration[<%= rails_migration_version %>]
+  def change
+    add_column :users, :expires_at, :datetime
+  end
+end

lib/generators/shopify_app/user_model/user_model_generator.rb

@@ -40,6 +40,26 @@ def create_scopes_storage_in_user_model
         end
       end
 
+      def create_expires_at_storage_in_user_model
+        expires_at_column_prompt = <<~PROMPT
+          It is highly recommended that apps record the User session expiry date. \
+          This will allow to check if the session has expired and re-authenticate \
+          without a first call to Shopify.
+
+          After running the migration, the `check_session_expiry_date` configuration can be enabled.
+
+          The following migration will add an `expires_at` column to the User model. \
+          Do you want to include this migration? [y/n]
+        PROMPT
+
+        if new_shopify_cli_app? || Rails.env.test? || yes?(expires_at_column_prompt)
+          migration_template(
+            "db/migrate/add_user_expires_at_column.erb",
+            "db/migrate/add_user_expires_at_column.rb",
+          )
+        end
+      end
+
       def update_shopify_app_initializer
         gsub_file("config/initializers/shopify_app.rb", "ShopifyApp::InMemoryUserSessionStore", "User")
       end

lib/shopify_app/configuration.rb

@@ -20,6 +20,7 @@ class Configuration
     attr_accessor :api_version
 
     attr_accessor :reauth_on_access_scope_changes
+    attr_accessor :check_session_expiry_date
     attr_accessor :log_level
 
     # customise urls

lib/shopify_app/controller_concerns/login_protection.rb

@@ -30,6 +30,12 @@ def activate_shopify_session
         return redirect_to_login
       end
 
+      if ShopifyApp.configuration.check_session_expiry_date && current_shopify_session.expired?
+        ShopifyApp::Logger.debug("Session expired, redirecting to login")
+        clear_shopify_session
+        return redirect_to_login
+      end
+
       if ShopifyApp.configuration.reauth_on_access_scope_changes &&
           !ShopifyApp.configuration.user_access_scopes_strategy.covers_scopes?(current_shopify_session)
         clear_shopify_session

lib/shopify_app/session/user_session_storage_with_scopes.rb

@@ -15,6 +15,7 @@ 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
@@ -56,6 +57,7 @@ def construct_session(user)
           scope: user.access_scopes,
           associated_user_scope: user.access_scopes,
           associated_user: associated_user,
+          expires: user.expires_at,
         )
       end
     end
@@ -71,5 +73,24 @@ 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
+    rescue NotImplementedError, NoMethodError
+      if ShopifyApp.configuration.check_session_expiry_date
+        raise NotImplementedError,
+          "#expires_at= must be defined to handle storing the session expiry date"
+      end
+    end
+
+    def expires_at
+      super
+    rescue NotImplementedError, NoMethodError
+      if ShopifyApp.configuration.check_session_expiry_date
+        raise NotImplementedError, "#expires_at must be defined to check the session expiry date"
+      end
+
+      nil
+    end
   end
 end

test/generators/user_model_generator_test.rb

@@ -35,7 +35,14 @@ class UserModelGeneratorTest < Rails::Generators::TestCase
     end
   end
 
-  test "create User with access_scopes migration with --new-shopify-cli-app flag provided" do
+  test "create expires_at migration for User model" do
+    run_generator
+    assert_migration "db/migrate/add_user_expires_at_column.rb" do |migration|
+      assert_match "add_column :users, :expires_at, :datetime", migration
+    end
+  end
+
+  test "create User with all migrations with --new-shopify-cli-app flag provided" do
     Rails.env = "mock_environment"
 
     run_generator ["--new-shopify-cli-app"]
@@ -44,6 +51,9 @@ class UserModelGeneratorTest < Rails::Generators::TestCase
     assert_migration "db/migrate/add_user_access_scopes_column.rb" do |migration|
       assert_match "add_column :users, :access_scopes, :string", migration
     end
+    assert_migration "db/migrate/add_user_expires_at_column.rb" do |migration|
+      assert_match "add_column :users, :expires_at, :datetime", migration
+    end
   end
 
   test "updates the shopify_app initializer to use User to store session" do

test/shopify_app/controller_concerns/login_protection_test.rb

@@ -426,6 +426,53 @@ class LoginProtectionControllerTest < ActionController::TestCase
     end
   end
 
+  test "#activate_shopify_session with an expired Shopify session redirects to the login url when check_session_expiry_date enabled" do
+    ShopifyApp.configuration.check_session_expiry_date = true
+
+    with_application_test_routes do
+      cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] = "cookie"
+      ShopifyApp::SessionRepository.expects(:load_session)
+        .returns(ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com", expires: 1.minute.ago))
+
+      get :index, params: { shop: "foobar" }
+
+      assert_redirected_to "/login?shop=foobar.myshopify.com"
+      assert_nil cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME]
+    end
+  end
+
+  test "#activate_shopify_session with an expired Shopify session, when the request is an XHR, returns an HTTP 401 when check_session_expiry_date enabled" do
+    ShopifyApp.configuration.check_session_expiry_date = true
+
+    with_application_test_routes do
+      cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] = "cookie"
+      ShopifyApp::SessionRepository.expects(:load_session)
+        .returns(ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com", expires: 1.minute.ago))
+
+      get :index, params: { shop: "foobar" }, xhr: true
+
+      assert_equal 401, response.status
+      assert_match "1", response.headers["X-Shopify-API-Request-Failure-Reauthorize"]
+      assert_match "/login?shop=foobar", response.headers["X-Shopify-API-Request-Failure-Reauthorize-Url"]
+      assert_nil cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME]
+    end
+  end
+
+  test "#activate_shopify_session with an expired Shopify session does not redirect to the login url when check_session_expiry_date disabled" do
+    ShopifyApp.configuration.check_session_expiry_date = false
+
+    with_application_test_routes do
+      cookies.encrypted[ShopifyAPI::Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] = "cookie"
+      ShopifyApp::SessionRepository.expects(:load_session)
+        .returns(ShopifyAPI::Auth::Session.new(shop: "shop.myshopify.com", expires: 1.minute.ago))
+      ::ShopifyAPI::Context.expects(:activate_session)
+
+      get :index, params: { shop: "foobar" }
+
+      assert_response :ok
+    end
+  end
+
   test "#fullpage_redirect_to sends a post message to that shop in the shop param" do
     with_application_test_routes do
       example_shop = "shop.myshopify.com"

test/shopify_app/session/user_session_storage_with_scopes_test.rb

@@ -12,20 +12,23 @@ 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
@@ -35,20 +38,23 @@ 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 ".destroy_by_shopify_user_id destroys user session by shopify_user_id" do

test/support/session_store_strategy_test_helpers.rb

@@ -21,23 +21,25 @@ def initialize(
   end
 
   class MockUserInstance
-    attr_reader :id, :shopify_user_id, :shopify_domain, :shopify_token, :api_version, :access_scopes
-    attr_writer :shopify_token, :shopify_domain, :access_scopes
+    attr_reader :id, :shopify_user_id, :shopify_domain, :shopify_token, :api_version, :access_scopes, :expires_at
+    attr_writer :shopify_token, :shopify_domain, :access_scopes, :expires_at
 
     def initialize(
       id: 1,
       shopify_user_id: 1,
       shopify_domain: "example.myshopify.com",
       shopify_token: "1234-user-token",
       api_version: ShopifyApp.configuration.api_version,
-      scopes: "read_products"
+      scopes: "read_products",
+      expires_at: nil
     )
       @id = id
       @shopify_user_id = shopify_user_id
       @shopify_domain = shopify_domain
       @shopify_token = shopify_token
       @api_version = api_version
       @access_scopes = scopes
+      @expires_at = expires_at
     end
   end
 end

test/test_helper.rb

@@ -57,6 +57,8 @@ def mock_session(shop: "my-shop.myshopify.com", scope: ShopifyApp.configuration.
     mock_session.stubs(:access_token).returns("a-new-user_token!")
     mock_session.stubs(:scope).returns(ShopifyAPI::Auth::AuthScopes.new(scope))
     mock_session.stubs(:shopify_session_id).returns(1)
+    mock_session.stubs(:expires).returns(nil)
+    mock_session.stubs(:expired?).returns(false)
 
     mock_session
   end