From 2ba8275dcc3bd4037f189949f293bddb6681d001 Mon Sep 17 00:00:00 2001
From: Tobin Juday <tobinibot@gmail.com>
Date: Mon, 6 Jan 2014 23:51:45 -0500
Subject: [PATCH] Fix off-by-one error in Lockable module

When using the maximum_attempts config, Devise actually let you fail n
+ 1 times, not n times.

See https://github.com/plataformatec/devise/issues/2825 for details.
---
 lib/devise/models/lockable.rb |  4 ++--
 test/models/lockable_test.rb  | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/devise/models/lockable.rb b/lib/devise/models/lockable.rb
index 8be25a89..5efd484e 100644
--- a/lib/devise/models/lockable.rb
+++ b/lib/devise/models/lockable.rb
@@ -127,11 +127,11 @@ module Devise
       protected
 
         def attempts_exceeded?
-          self.failed_attempts > self.class.maximum_attempts
+          self.failed_attempts >= self.class.maximum_attempts
         end
 
         def last_attempt?
-          self.failed_attempts == self.class.maximum_attempts
+          self.failed_attempts == self.class.maximum_attempts - 1
         end
 
         # Tells if the lock is expired if :time unlock strategy is active
diff --git a/test/models/lockable_test.rb b/test/models/lockable_test.rb
index 9e44a071..c371f9c8 100644
--- a/test/models/lockable_test.rb
+++ b/test/models/lockable_test.rb
@@ -9,7 +9,7 @@ class LockableTest < ActiveSupport::TestCase
     user = create_user
     user.confirm!
     swap Devise, :maximum_attempts => 2 do
-      3.times { user.valid_for_authentication?{ false } }
+      2.times { user.valid_for_authentication?{ false } }
       assert user.reload.access_locked?
     end
   end
@@ -19,12 +19,12 @@ class LockableTest < ActiveSupport::TestCase
     user.confirm!
 
     swap Devise, :maximum_attempts => 2 do
-      3.times { user.valid_for_authentication?{ false } }
+      2.times { user.valid_for_authentication?{ false } }
       assert user.reload.access_locked?
     end
 
     user.valid_for_authentication?{ true }
-    assert_equal 4, user.reload.failed_attempts
+    assert_equal 3, user.reload.failed_attempts
   end
 
   test "should not touch failed_attempts if lock_strategy is none" do
@@ -302,13 +302,13 @@ class LockableTest < ActiveSupport::TestCase
     swap Devise, :last_attempt_warning => :true do
       swap Devise, :lock_strategy => :failed_attempts do
         user = create_user
-        user.failed_attempts = Devise.maximum_attempts - 1
+        user.failed_attempts = Devise.maximum_attempts - 2
         assert_equal :invalid, user.unauthenticated_message
 
-        user.failed_attempts = Devise.maximum_attempts
+        user.failed_attempts = Devise.maximum_attempts - 1
         assert_equal :last_attempt, user.unauthenticated_message
 
-        user.failed_attempts = Devise.maximum_attempts + 1
+        user.failed_attempts = Devise.maximum_attempts
         assert_equal :locked, user.unauthenticated_message
       end
     end