Make #increment_failed_attempts concurrency safe

Backported from heartcombo#4996
maestrano · Mar 14, 2019 · 36690f3 · 36690f3
1 parent bddf051
commit 36690f3
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 2 deletions.
diff --git a/lib/devise/models/lockable.rb b/lib/devise/models/lockable.rb
@@ -99,8 +99,8 @@ def valid_for_authentication?
         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

diff --git a/test/models/lockable_test.rb b/test/models/lockable_test.rb
@@ -37,6 +37,17 @@ def setup
     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!