diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb index e4412c39..09af6ab8 100644 --- a/lib/devise/models/confirmable.rb +++ b/lib/devise/models/confirmable.rb @@ -93,7 +93,7 @@ module Devise self.confirmation_token = nil if reconfirmation_required? @reconfirmation_required = false - generate_confirmation_token! if self.confirmation_token.blank? + ensure_confirmation_token! opts = pending_reconfirmation? ? { :to => unconfirmed_email } : { } send_devise_notification(:confirmation_instructions, opts) @@ -106,6 +106,11 @@ module Devise send_confirmation_instructions end end + + # Generate a confirmation token unless already exists and save the record. + def ensure_confirmation_token! + generate_confirmation_token! if should_generate_confirmation_token? + end # Overwrites active_for_authentication? for confirmation # by verifying whether a user is active to sign in or not. If the user @@ -139,6 +144,9 @@ module Devise end protected + def should_generate_confirmation_token? + confirmation_token.nil? || confirmation_period_expired? + end # A callback method used to deliver confirmation # instructions on creation. This can be overriden diff --git a/lib/devise/models/recoverable.rb b/lib/devise/models/recoverable.rb index 494852f0..563dd962 100644 --- a/lib/devise/models/recoverable.rb +++ b/lib/devise/models/recoverable.rb @@ -44,10 +44,15 @@ module Devise # Resets reset password token and send reset password instructions by email def send_reset_password_instructions - generate_reset_password_token! if should_generate_reset_token? + ensure_reset_password_token! send_devise_notification(:reset_password_instructions) end - + + # Generate reset password token unless already exists and save the record. + def ensure_reset_password_token! + generate_reset_password_token! if should_generate_reset_token? + end + # Checks if the reset password token sent is within the limit time. # We do this by calculating if the difference between today and the # sending date does not exceed the confirm in time configured. diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb index 4c6c9f04..32c753c2 100644 --- a/test/models/confirmable_test.rb +++ b/test/models/confirmable_test.rb @@ -294,6 +294,24 @@ class ConfirmableTest < ActiveSupport::TestCase assert_not_equal user.confirmation_token, old end end + + test 'should generate a new token when a valid one does not exist' do + swap Devise, :confirm_within => 3.days do + user = create_user + user.update_attribute(:confirmation_sent_at, 4.days.ago) + old = user.confirmation_token + user.ensure_confirmation_token! + assert_not_equal user.confirmation_token, old + end + end + + test 'should not generate a new token when a valid one exists' do + user = create_user + assert_not_nil user.confirmation_token + old = user.confirmation_token + user.ensure_confirmation_token! + assert_equal user.confirmation_token, old + end end class ReconfirmableTest < ActiveSupport::TestCase diff --git a/test/models/recoverable_test.rb b/test/models/recoverable_test.rb index 9907fc26..be54444e 100644 --- a/test/models/recoverable_test.rb +++ b/test/models/recoverable_test.rb @@ -110,7 +110,7 @@ class RecoverableTest < ActiveSupport::TestCase test 'should find a user to reset his password based on reset_password_token' do user = create_user - user.send :generate_reset_password_token! + user.ensure_reset_password_token! reset_password_user = User.reset_password_by_token(:reset_password_token => user.reset_password_token) assert_equal reset_password_user, user @@ -130,7 +130,7 @@ class RecoverableTest < ActiveSupport::TestCase test 'should return a new record with errors if password is blank' do user = create_user - user.send :generate_reset_password_token! + user.ensure_reset_password_token! reset_password_user = User.reset_password_by_token(:reset_password_token => user.reset_password_token, :password => '') assert_not reset_password_user.errors.empty? @@ -140,7 +140,7 @@ class RecoverableTest < ActiveSupport::TestCase test 'should reset successfully user password given the new password and confirmation' do user = create_user old_password = user.password - user.send :generate_reset_password_token! + user.ensure_reset_password_token! User.reset_password_by_token( :reset_password_token => user.reset_password_token, @@ -179,7 +179,7 @@ class RecoverableTest < ActiveSupport::TestCase swap Devise, :reset_password_within => 1.hour do user = create_user old_password = user.password - user.send :generate_reset_password_token! + user.ensure_reset_password_token! user.reset_password_sent_at = 2.days.ago user.save! @@ -202,4 +202,21 @@ class RecoverableTest < ActiveSupport::TestCase :reset_password_token ] end + + test 'should generate a new token when a valid one does not exist' do + user = create_user + assert_nil user.reset_password_token + + user.ensure_reset_password_token! + assert_not_nil user.reset_password_token + end + + test 'should not generate a new token when a valid one exists' do + user = create_user + user.send :generate_reset_password_token! + assert_not_nil user.reset_password_token + old = user.reset_password_token + user.ensure_reset_password_token! + assert_equal user.reset_password_token, old + end end