Skip to content

Commit

Permalink
Create a model hook around the lockable warden hook to reset attempts
Browse files Browse the repository at this point in the history
Resetting failed attempts after sign in happened inside a warden hook
specific for the lockable module, but that was hidden inside the hook
implementation and didn't allow any user customization.

One such customization needed for example is to direct these updates to
a write DB when using a multi-DB setup. With the logic hidden in the
warden hook this wasn't possible, now that it's exposed in a model
method much like trackable, we can override the model method to wrap it
in a connection switch block for example, point to a write DB, and
simply call `super`.

Closes #5310
Related to #5264 and #5133
  • Loading branch information
carlosantoniodasilva committed Apr 2, 2021
1 parent e8e0c27 commit a3ae35e
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 2 additions & 5 deletions lib/devise/hooks/lockable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 9 additions & 1 deletion lib/devise/models/lockable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -110,7 +118,7 @@ def valid_for_authentication?
false
end
end

def increment_failed_attempts
self.class.increment_counter(:failed_attempts, id)
reload
Expand Down
26 changes: 26 additions & 0 deletions test/models/lockable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down

0 comments on commit a3ae35e

Please sign in to comment.