diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cdbdd6c22..5370462fa6 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/lib/devise/hooks/lockable.rb b/lib/devise/hooks/lockable.rb index a73a1752e2..b11db1e879 100644 --- a/lib/devise/hooks/lockable.rb +++ b/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 diff --git a/lib/devise/models/lockable.rb b/lib/devise/models/lockable.rb index 578f52949d..ce9e3e57af 100644 --- a/lib/devise/models/lockable.rb +++ b/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 diff --git a/test/models/lockable_test.rb b/test/models/lockable_test.rb index 8b12d55040..4190de929f 100644 --- a/test/models/lockable_test.rb +++ b/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!