Merge pull request #5043 from maestrano/increment-failed-attempts-concurency

Backport CVE-2019-5421 fix to 3.x

lib/devise/models/lockable.rb

@@ -99,8 +99,8 @@ module Devise
         if super && !access_locked?
           true
         else
-          self.failed_attempts ||= 0
-          self.failed_attempts += 1
+          self.class.increment_counter(:failed_attempts, id)
+          reload
           if attempts_exceeded?
             lock_access! unless access_locked?
           else

test/models/lockable_test.rb

@@ -37,6 +37,17 @@ class LockableTest < ActiveSupport::TestCase
     end
   end
 
+  test "should read failed_attempts from database when incrementing" do
+    user = create_user
+    initial_failed_attempts = user.failed_attempts
+    same_user = User.find(user.id)
+
+    user.valid_for_authentication?{ false }
+    same_user.valid_for_authentication?{ false }
+
+    assert_equal initial_failed_attempts + 2, user.reload.failed_attempts
+  end
+
   test 'should be valid for authentication with a unlocked user' do
     user = create_user
     user.lock_access!