Merge pull request #5369 from heartcombo/ca-lockable-reset-attempts

Create a model hook around the lockable warden hook to reset attempts

CHANGELOG.md

@@ -4,6 +4,8 @@
   * Devise now enables the upgrade of OmniAuth 2+. Previously Devise would raise an error if you'd try to upgrade. Please note that OmniAuth 2 is considered a security upgrade and recommended to everyone. You can read more about the details (and possible necessary changes to your app as part of the upgrade) in [their release notes](https://github.com/omniauth/omniauth/releases/tag/v2.0.0). [Devise's OmniAuth Overview wiki](https://github.com/heartcombo/devise/wiki/OmniAuth:-Overview) was also updated to cover OmniAuth 2.0 requirements.
     - Note that the upgrade required Devise shared links that initiate the OmniAuth flow to be changed to `method: :post`, which is now a requirement for OmniAuth, part of the security improvement. If you have copied and customized the Devise shared links partial to your app, or if you have other links in your app that initiate the OmniAuth flow, they will have to be updated to use `method: :post`, or changed to use buttons (e.g. `button_to`) to work with OmniAuth 2. (if you're using links with `method: :post`, make sure your app has `rails-ujs` or `jquery-ujs` included in order for these links to work properly.)
     - As part of the OmniAuth 2.0 upgrade you might also need to add the [`omniauth-rails_csrf_protection`](https://github.com/cookpad/omniauth-rails_csrf_protection) gem to your app if you don't have it already. (and you don't want to roll your own code to verify requests.) Check the OmniAuth v2 release notes for more info.
+  * Introduce `Lockable#reset_failed_attempts!` model method to reset failed attempts counter to 0 after the user signs in.
+    - This logic existed inside the lockable warden hook and is triggered automatically after the user signs in. The new model method is an extraction to allow you to override it in the application to implement things like switching to a write database if you're using the new multi-DB infrastructure from Rails for example, similar to how it's already possible with `Trackable#update_tracked_fields!`.
   * Add support for Ruby 3.
   * Add support for Rails 6.1.
   * Move CI to GitHub Actions.

lib/devise/hooks/lockable.rb

@@ -3,10 +3,7 @@
 # After each sign in, if resource responds to failed_attempts, sets it to 0
 # This is only triggered when the user is explicitly set (with set_user)
 Warden::Manager.after_set_user except: :fetch do |record, warden, options|
-  if record.respond_to?(:failed_attempts) && warden.authenticated?(options[:scope])
-    unless record.failed_attempts.to_i.zero?
-      record.failed_attempts = 0
-      record.save(validate: false)
-    end
+  if record.respond_to?(:reset_failed_attempts!) && warden.authenticated?(options[:scope])
+    record.reset_failed_attempts!
   end
 end

lib/devise/models/lockable.rb

@@ -57,6 +57,14 @@ def unlock_access!
         save(validate: false)
       end
 
+      # Resets failed attempts counter to 0.
+      def reset_failed_attempts!
+        if respond_to?(:failed_attempts) && !failed_attempts.to_i.zero?
+          self.failed_attempts = 0
+          save(validate: false)
+        end
+      end
+
       # Verifies whether a user is locked or not.
       def access_locked?
         !!locked_at && !lock_expired?
@@ -110,7 +118,7 @@ def valid_for_authentication?
           false
         end
       end
-      
+
       def increment_failed_attempts
         self.class.increment_counter(:failed_attempts, id)
         reload

test/models/lockable_test.rb

@@ -50,6 +50,32 @@ def setup
     assert_equal initial_failed_attempts + 2, user.reload.failed_attempts
   end
 
+  test "reset_failed_attempts! updates the failed attempts counter back to 0" do
+    user = create_user(failed_attempts: 3)
+    assert_equal 3, user.failed_attempts
+
+    user.reset_failed_attempts!
+    assert_equal 0, user.failed_attempts
+
+    user.reset_failed_attempts!
+    assert_equal 0, user.failed_attempts
+  end
+
+  test "reset_failed_attempts! does not run model validations" do
+    user = create_user(failed_attempts: 1)
+    user.expects(:after_validation_callback).never
+
+    assert user.reset_failed_attempts!
+    assert_equal 0, user.failed_attempts
+  end
+
+  test "reset_failed_attempts! does not try to reset if not using failed attempts strategy" do
+    admin = create_admin
+
+    refute_respond_to admin, :failed_attempts
+    refute admin.reset_failed_attempts!
+  end
+
   test 'should be valid for authentication with a unlocked user' do
     user = create_user
     user.lock_access!