Fix bugs in extend_remember_period

lib/devise/hooks/rememberable.rb

@@ -1,9 +1,22 @@
 # frozen_string_literal: true
 
+# This hook runs when a user logs in, if they set the `remember_me` param (eg. from a checkbox in the UI).
 Warden::Manager.after_set_user except: :fetch do |record, warden, options|
   scope = options[:scope]
   if record.respond_to?(:remember_me) && options[:store] != false &&
      record.remember_me && warden.authenticated?(scope)
+
     Devise::Hooks::Proxy.new(warden).remember_me(record)
   end
 end
+
+# This hook runs when we retrieve a user from the session. If the user's remember session should be extended
+# we do it here.
+Warden::Manager.after_set_user only: :fetch do |record, warden, options|
+  if record.respond_to?(:extend_remember_me?) && record.extend_remember_me? &&
+      options[:store] != false && warden.authenticated?(options[:scope])
+
+    proxy = Devise::Hooks::Proxy.new(warden)
+    proxy.remember_me(record) if proxy.remember_me_is_active?(record)
+  end
+end

lib/devise/models/rememberable.rb

@@ -70,6 +70,10 @@ def extend_remember_period
         self.class.extend_remember_period
       end
 
+      def extend_remember_me?
+        !!self.class.extend_remember_period
+      end
+
       def rememberable_value
         if respond_to?(:remember_token)
           remember_token

lib/generators/templates/devise.rb

@@ -169,7 +169,12 @@
   # Invalidates all the remember me tokens when the user signs out.
   config.expire_all_remember_me_on_sign_out = true
 
-  # If true, extends the user's remember period when remembered via cookie.
+  # If true, extends the user's remember period every time the user makes a request.
+  # As long as the user accesses the site once every `config.remember_for`, they can
+  # stay logged in forever.
+  # If false, how long the user will be remembered for is set on initial login,
+  # and only when the user starts a new session. So users will need to log in
+  # again every `config.remember_for`.
   # config.extend_remember_period = false
 
   # Options to be passed to the created cookie. For instance, you can set

test/integration/rememberable_test.rb

@@ -111,7 +111,7 @@ def cookie_expires(key)
     assert_redirected_to root_path
   end
 
-  test 'does not extend remember period through sign in' do
+  test 'logging in does not extend remember_created_at if it is already set' do
     swap Devise, extend_remember_period: true, remember_for: 1.year do
       user = create_user
       user.remember_me!
@@ -127,37 +127,226 @@ def cookie_expires(key)
     end
   end
 
-  test 'extends remember period when extend remember period config is true' do
+  test 'logging in sets remember_created_at if it is blank' do
     swap Devise, extend_remember_period: true, remember_for: 1.year do
-      create_user_and_remember
-      old_remember_token = nil
+      user = create_user
+      user.forget_me!
+
+      assert_nil user.remember_created_at
+
+      sign_in_as_user remember_me: true
+      user.reload
+
+      assert warden.user(:user) == user
+      assert_not_nil user.remember_created_at
+    end
+  end
+
+  test 'extends remember period on every authenticated request when extend remember period config is true (session expires)' do
+    swap Devise, extend_remember_period: true, remember_for: 1.year, timeout_in: 6.hours do
+      user = create_user_and_remember
+
+      get root_path
+      assert_response :success
+
+      travel_to 1.day.from_now do
+        # tomorrow, still logged in (by remember me), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 6.months.from_now do
+        # 6 months later, still logged in (by remember me), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 13.months.from_now do
+        # 13 months after remember_created_at was first set, we are still logged in because period was extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 20.months.from_now do
+        # 20 months after remember_created_at was first set, we are still logged in because period was extended
+        get root_path
+        assert_response :success
+      end
 
-      travel_to 1.day.ago do
+      travel_to 33.months.from_now do
+        # don't log in for over a year, we get logged out
         get root_path
-        old_remember_token = request.cookies['remember_user_token']
+        assert_response :redirect
       end
+    end
+  end
+
+  test 'does not extend remember period when extend period config is false (session expires)' do
+    swap Devise, extend_remember_period: false, remember_for: 1.year, timeout_in: 6.hours do
+      user = create_user_and_remember
 
       get root_path
-      current_remember_token = request.cookies['remember_user_token']
+      assert_response :success
 
-      refute_equal old_remember_token, current_remember_token
+      travel_to 1.day.from_now do
+        # tomorrow, still logged in (by remember me), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 6.months.from_now do
+        # 6 months later, still logged in (by remember me), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 13.months.from_now do
+        # 13 months after remember_created_at was first set, we are no longer logged in because period was not extended
+        get root_path
+        assert_response :redirect
+      end
     end
   end
 
-  test 'does not extend remember period when extend period config is false' do
-    swap Devise, extend_remember_period: false, remember_for: 1.year do
-      create_user_and_remember
-      old_remember_token = nil
+  test 'extends remember period on every authenticated request when extend remember period config is true (session still active; only session expires)' do
+    swap Devise, extend_remember_period: true, remember_for: 1.year, timeout_in: 8.months do
+      user = create_user_and_remember
 
-      travel_to 1.day.ago do
+      get root_path
+      assert_response :success
+
+      travel_to 1.day.from_now do
+        # tomorrow, still logged in (by session), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 6.months.from_now do
+        # 6 months later, still logged in (by session), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 13.months.from_now do
+        # 13 months later, still logged in (by session), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 20.months.from_now do
+        # 20 months later, still logged in (by session), remember period is extended
         get root_path
-        old_remember_token = request.cookies['remember_user_token']
+        assert_response :success
       end
 
+      travel_to 29.months.from_now do
+        # don't access for over 8 months, session is now expired, still logged in (by remember me)
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 42.months.from_now do
+        # don't access for over a year, session and remember me are now expired, we get logged out
+        get root_path
+        assert_response :redirect
+      end
+    end
+  end
+
+  test 'extends remember period on every authenticated request when extend remember period config is true (session still active; both expire at the same time)' do
+    swap Devise, extend_remember_period: true, remember_for: 1.year, timeout_in: 8.months do
+      user = create_user_and_remember
+
       get root_path
-      current_remember_token = request.cookies['remember_user_token']
+      assert_response :success
+
+      travel_to 1.day.from_now do
+        # tomorrow, still logged in (by session), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 6.months.from_now do
+        # 6 months later, still logged in (by session), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 13.months.from_now do
+        # 13 months later, still logged in (by session), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 20.months.from_now do
+        # 20 months later, still logged in (by session), remember period is extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 33.months.from_now do
+        # don't access for over a year, session and remember me are now expired, we get logged out
+        get root_path
+        assert_response :redirect
+      end
+    end
+  end
+
+  test 'does not extend remember period when extend period config is false (session still active)' do
+    swap Devise, extend_remember_period: false, remember_for: 1.year, timeout_in: 8.months do
+      user = create_user_and_remember
 
-      assert_equal old_remember_token, current_remember_token
+      get root_path
+      assert_response :success
+
+      travel_to 1.day.from_now do
+        # tomorrow, still logged in (by session), remember period is not extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 6.months.from_now do
+        # 6 months later, still logged in (by session), remember period is not extended
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 13.months.from_now do
+        # 13 months after remember_created_at was first set, we are no longer remembered
+        # because the period was not extended but still logged in by the session
+        get root_path
+        assert_response :success
+      end
+
+      travel_to 22.months.from_now do
+        # don't access for over a year, session is now expired, we get logged out
+        get root_path
+        assert_response :redirect
+      end
+    end
+  end
+
+  test 'do not start remember period when remember me is not used' do
+    swap Devise, extend_remember_period: true, remember_for: 1.year, timeout_in: 6.hours do
+      sign_in_as_user
+      assert_nil request.cookies["remember_user_cookie"]
+
+      get root_path
+      assert_response :success
+
+      travel_to 1.hour.from_now do
+        # 1 hour later, still logged in (by session), remember me is not set
+        get root_path
+        assert_response :success
+        assert_nil request.cookies["remember_user_cookie"]
+      end
+
+      travel_to 8.hours.from_now do
+        # 8 hours later, session has expired and remember me is not set
+        get root_path
+        assert_response :redirect
+        assert_nil request.cookies["remember_user_cookie"]
+      end
     end
   end