Skip to content
Browse files

Fix some unlockable bugs.

  • Loading branch information...
1 parent 97b7ba8 commit 2a082f3e4ce558dcab2625f7509f92c5b44d74f9 @josevalim josevalim committed Mar 28, 2010
View
7 app/controllers/devise/unlocks_controller.rb
@@ -1,4 +1,5 @@
class Devise::UnlocksController < ApplicationController
+ prepend_before_filter :ensure_email_as_unlock_strategy
prepend_before_filter :require_no_authentication
include Devise::Controllers::InternalHelpers
@@ -31,4 +32,10 @@ def show
render_with_scope :new
end
end
+
+ protected
+
+ def ensure_email_as_unlock_strategy
+ raise ActionController::UnknownAction unless resource_class.unlock_strategy_enabled?(:email)
+ end
end
View
10 lib/devise/models/confirmable.rb
@@ -57,15 +57,9 @@ def send_confirmation_instructions
::Devise::Mailer.confirmation_instructions(self).deliver
end
- # Remove confirmation date and send confirmation instructions, to ensure
- # after sending these instructions the user won't be able to sign in without
- # confirming it's account
+ # Resend confirmation token. This method does not need to generate a new token.
def resend_confirmation_token
- unless_confirmed do
- generate_confirmation_token
- save(:validate => false)
- send_confirmation_instructions
- end
+ unless_confirmed { send_confirmation_instructions }
end
# Overwrites active? from Devise::Models::Activatable for confirmation
View
26 lib/devise/models/lockable.rb
@@ -23,9 +23,10 @@ module Lockable
# Lock an user setting it's locked_at to actual time.
def lock_access!
+ return true if access_locked?
self.locked_at = Time.now
- if unlock_strategy_enabled?(:email)
+ if self.class.unlock_strategy_enabled?(:email)
generate_unlock_token
send_unlock_instructions
end
@@ -55,11 +56,7 @@ def send_unlock_instructions
# Resend the unlock instructions if the user is locked.
def resend_unlock_token
- if_access_locked do
- generate_unlock_token unless unlock_token.present?
- save(:validate => false)
- send_unlock_instructions
- end
+ if_access_locked { send_unlock_instructions }
end
# Overwrites active? from Devise::Models::Activatable for locking purposes
@@ -82,10 +79,7 @@ def valid_for_authentication?(attributes)
self.failed_attempts = 0
else
self.failed_attempts += 1
- if failed_attempts > self.class.maximum_attempts
- lock_access!
- return false
- end
+ lock_access! if failed_attempts > self.class.maximum_attempts
end
save(:validate => false) if changed?
result
@@ -100,7 +94,7 @@ def generate_unlock_token
# Tells if the lock is expired if :time unlock strategy is active
def lock_expired?
- if unlock_strategy_enabled?(:time)
+ if self.class.unlock_strategy_enabled?(:time)
locked_at && locked_at < self.class.unlock_in.ago
else
false
@@ -118,11 +112,6 @@ def if_access_locked
end
end
- # Is the unlock enabled for the given unlock strategy?
- def unlock_strategy_enabled?(strategy)
- [:both, strategy].include?(self.class.unlock_strategy)
- end
-
module ClassMethods
# Attempt to find a user by it's email. If a record is found, send new
# unlock instructions to it. If not user is found, returns a new user
@@ -144,6 +133,11 @@ def unlock_access_by_token(unlock_token)
lockable
end
+ # Is the unlock enabled for the given unlock strategy?
+ def unlock_strategy_enabled?(strategy)
+ [:both, strategy].include?(self.unlock_strategy)
+ end
+
Devise::Models.config(self, :maximum_attempts, :unlock_strategy, :unlock_in)
end
end
View
20 test/integration/lockable_test.rb
@@ -36,6 +36,15 @@ def visit_user_unlock_with_token(unlock_token)
assert_equal 0, ActionMailer::Base.deliveries.size
end
+ test 'unlocked pages should not be available if email strategy is disabled' do
+ visit new_user_unlock_path
+ swap Devise, :unlock_strategy => :time do
+ assert_raise AbstractController::ActionNotFound do
+ visit new_user_unlock_path
+ end
+ end
+ end
+
test 'user with invalid unlock token should not be able to unlock an account' do
visit_user_unlock_with_token('invalid_token')
@@ -60,7 +69,6 @@ def visit_user_unlock_with_token(unlock_token)
test "sign in user automatically after unlocking it's account" do
user = create_user(:locked => true)
visit_user_unlock_with_token(user.unlock_token)
-
assert warden.authenticated?(:user)
end
@@ -71,6 +79,16 @@ def visit_user_unlock_with_token(unlock_token)
assert_not warden.authenticated?(:user)
end
+ test "user should not send a new e-mail if already locked" do
+ user = create_user(:locked => true)
+ user.update_attribute(:failed_attempts, User.maximum_attempts + 1)
+ ActionMailer::Base.deliveries.clear
+
+ sign_in_as_user(:password => "invalid")
+ assert_contain 'Invalid email or password.'
+ assert ActionMailer::Base.deliveries.empty?
+ end
+
test 'error message is configurable by resource name' do
store_translations :en, :devise => {
:sessions => { :admin => { :locked => "You are locked!" } }
View
16 test/models/confirmable_test.rb
@@ -11,15 +11,6 @@ def setup
assert_not_nil create_user.confirmation_token
end
- test 'should regenerate confirmation token each time' do
- user = create_user
- 3.times do
- token = user.confirmation_token
- user.resend_confirmation_token
- assert_not_equal token, user.confirmation_token
- end
- end
-
test 'should never generate the same confirmation token for different users' do
confirmation_tokens = []
3.times do
@@ -137,13 +128,6 @@ def setup
assert_equal "not found", confirmation_user.errors[:email].join
end
- test 'should generate a confirmation token before send the confirmation instructions email' do
- user = create_user
- token = user.confirmation_token
- confirmation_user = User.send_confirmation_instructions(:email => user.email)
- assert_not_equal token, user.reload.confirmation_token
- end
-
test 'should send email instructions for the user confirm it\'s email' do
user = create_user
assert_email_sent do
View
20 test/models/lockable_test.rb
@@ -37,7 +37,7 @@ def setup
assert_equal 0, user.reload.failed_attempts
end
- test "should verify wheter a user is locked or not" do
+ test "should verify whether a user is locked or not" do
user = create_user
assert_not user.access_locked?
user.lock_access!
@@ -64,6 +64,14 @@ def setup
assert 0, user.reload.failed_attempts
end
+ test "should not lock a locked account" do
+ user = create_user
+ user.lock_access!
+ assert_no_difference "ActionMailer::Base.deliveries.size" do
+ user.lock_access!
+ end
+ end
+
test 'should not unlock an unlocked user' do
user = create_user
assert_not user.unlock_access!
@@ -101,16 +109,6 @@ def setup
assert_not_nil user.unlock_token
end
- test 'should not regenerate unlock token if it already exists' do
- user = create_user
- user.lock!
- 3.times do
- token = user.unlock_token
- user.resend_unlock_token
- assert_equal token, user.unlock_token
- end
- end
-
test "should never generate the same unlock token for different users" do
unlock_tokens = []
3.times do
View
2 test/support/integration.rb
@@ -33,7 +33,7 @@ def sign_in_as_user(options={}, &block)
user = create_user(options)
get new_user_session_path unless options[:visit] == false
fill_in 'email', :with => 'user@test.com'
- fill_in 'password', :with => '123456'
+ fill_in 'password', :with => options[:password] || '123456'
check 'remember me' if options[:remember_me] == true
yield if block_given?
click_button 'Sign In'

0 comments on commit 2a082f3

Please sign in to comment.
Something went wrong with that request. Please try again.