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.
This commit is contained in:
Tobin Juday 2014-01-06 23:51:45 -05:00
parent 72a0d9e350
commit 2ba8275dcc
2 changed files with 8 additions and 8 deletions

View File

@ -127,11 +127,11 @@ module Devise
protected protected
def attempts_exceeded? def attempts_exceeded?
self.failed_attempts > self.class.maximum_attempts self.failed_attempts >= self.class.maximum_attempts
end end
def last_attempt? def last_attempt?
self.failed_attempts == self.class.maximum_attempts self.failed_attempts == self.class.maximum_attempts - 1
end end
# Tells if the lock is expired if :time unlock strategy is active # Tells if the lock is expired if :time unlock strategy is active

View File

@ -9,7 +9,7 @@ class LockableTest < ActiveSupport::TestCase
user = create_user user = create_user
user.confirm! user.confirm!
swap Devise, :maximum_attempts => 2 do 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? assert user.reload.access_locked?
end end
end end
@ -19,12 +19,12 @@ class LockableTest < ActiveSupport::TestCase
user.confirm! user.confirm!
swap Devise, :maximum_attempts => 2 do 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? assert user.reload.access_locked?
end end
user.valid_for_authentication?{ true } user.valid_for_authentication?{ true }
assert_equal 4, user.reload.failed_attempts assert_equal 3, user.reload.failed_attempts
end end
test "should not touch failed_attempts if lock_strategy is none" do 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, :last_attempt_warning => :true do
swap Devise, :lock_strategy => :failed_attempts do swap Devise, :lock_strategy => :failed_attempts do
user = create_user user = create_user
user.failed_attempts = Devise.maximum_attempts - 1 user.failed_attempts = Devise.maximum_attempts - 2
assert_equal :invalid, user.unauthenticated_message 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 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 assert_equal :locked, user.unauthenticated_message
end end
end end