diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb index 7907cf76..d87d3791 100644 --- a/lib/devise/models/confirmable.rb +++ b/lib/devise/models/confirmable.rb @@ -28,6 +28,33 @@ module Devise module Confirmable extend ActiveSupport::Concern + # email uniqueness validation in unconfirmed_email column, works only if unconfirmed_email is defined on record + class ConfirmableValidator < ActiveModel::Validator + def validate(record) + if unconfirmed_email_defined?(record) && email_exists_in_unconfirmed_emails?(record) + record.errors.add(:email, :taken) + end + end + + protected + def unconfirmed_email_defined?(record) + record.respond_to?(:unconfirmed_email) + end + + def email_exists_in_unconfirmed_emails?(record) + query = record.class + unless record.new_record? + if record.respond_to?(:_id) + query = query.where(:_id => {'$ne' => record._id}) + else + query = query.where('id <> ?', record.id) + end + end + query = query.where(:unconfirmed_email => record.email) + query.exists? + end + end + included do before_create :generate_confirmation_token, :if => :confirmation_required? after_create :send_confirmation_instructions, :if => :confirmation_required? @@ -161,9 +188,10 @@ module Devise # field. If no user is found, returns a new user with an email not found error. # Options must contain the user email def send_confirmation_instructions(attributes={}) - confirmable = find_or_initialize_with_errors(confirmation_keys, attributes, :not_found) - temp = find_by_unconfirmed_email(confirmation_keys, attributes, :not_found) - confirmable = temp if temp.persisted? + confirmable = find_by_unconfirmed_email_with_errors(attributes) if reconfirmable + unless confirmable.try(:persisted?) + confirmable = find_or_initialize_with_errors(confirmation_keys, attributes, :not_found) + end confirmable.resend_confirmation_token if confirmable.persisted? confirmable end @@ -184,10 +212,11 @@ module Devise end # Find a record for confirmation by unconfirmed email field - def find_by_unconfirmed_email(required_attributes, attributes, error=:invalid) - confirmation_keys_with_replaced_email = required_attributes.map{ |k| k == :email ? :unconfirmed_email : k } - attributes[:unconfirmed_email] = attributes.delete(:email) - find_or_initialize_with_errors(confirmation_keys_with_replaced_email, attributes, :not_found) + def find_by_unconfirmed_email_with_errors(attributes = {}) + unconfirmed_required_attributes = confirmation_keys.map{ |k| k == :email ? :unconfirmed_email : k } + unconfirmed_attributes = attributes.symbolize_keys + unconfirmed_attributes[:unconfirmed_email] = unconfirmed_attributes.delete(:email) + find_or_initialize_with_errors(unconfirmed_required_attributes, unconfirmed_attributes, :not_found) end Devise::Models.config(self, :confirm_within, :confirmation_keys, :reconfirmable) diff --git a/lib/devise/models/validatable.rb b/lib/devise/models/validatable.rb index b4440441..7ccf4150 100644 --- a/lib/devise/models/validatable.rb +++ b/lib/devise/models/validatable.rb @@ -25,6 +25,7 @@ module Devise validates_presence_of :email, :if => :email_required? validates_uniqueness_of :email, :case_sensitive => (case_insensitive_keys != false), :allow_blank => true, :if => :email_changed? validates_format_of :email, :with => email_regexp, :allow_blank => true, :if => :email_changed? + validates_with Devise::Models::Confirmable::ConfirmableValidator validates_presence_of :password, :if => :password_required? validates_confirmation_of :password, :if => :password_required? diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index 4b5f2f07..eb9302c7 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -179,6 +179,12 @@ class ConfirmationTest < ActionController::IntegrationTest end class ConfirmationOnChangeTest < ConfirmationTest + + def create_second_user(options={}) + @user = nil + create_user(options) + end + def setup Devise.reconfirmable = true end @@ -214,16 +220,19 @@ class ConfirmationOnChangeTest < ConfirmationTest assert user.reload.confirmed? end - test 'user who changed email should get a detailed message about email being not unique' do + test 'user email should be unique also within unconfirmed_email' do user = create_user(:confirm => true) user.update_attributes(:email => 'new_test@example.com') assert 'new_test@example.com', user.unconfirmed_email - @user = nil - create_user(:email => 'new_test@example.com', :confirm => true) + get new_user_registration_path - visit_user_confirmation_with_token(user.confirmation_token) + fill_in 'email', :with => 'new_test@example.com' + fill_in 'password', :with => 'new_user123' + fill_in 'password confirmation', :with => 'new_user123' + click_button 'Sign up' + assert_have_selector '#error_explanation' assert_contain /Email.*already.*taken/ end end diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb index e4b1c831..1734dbed 100644 --- a/test/models/confirmable_test.rb +++ b/test/models/confirmable_test.rb @@ -312,4 +312,12 @@ class ConfirmableOnChangeTest < ConfirmableTest assert confirmation_user.errors[:email] assert_equal "not found", confirmation_user.errors[:email].join end + + test 'should find user with email in unconfirmed_emails' do + user = create_user + user.unconfirmed_email = "new_test@email.com" + assert user.save + user = User.find_by_unconfirmed_email_with_errors(:email => "new_test@email.com") + assert user.persisted? + end end diff --git a/test/models/validatable_test.rb b/test/models/validatable_test.rb index 1633917e..16206881 100644 --- a/test/models/validatable_test.rb +++ b/test/models/validatable_test.rb @@ -105,6 +105,18 @@ class ValidatableTest < ActiveSupport::TestCase assert_equal 'is too long (maximum is 128 characters)', user.errors[:password].join end + test 'should check if email is unique in unconfirmed_email column' do + swap Devise, :reconfirmable => [:username, :email] do + user = create_user + user.update_attributes({:email => 'new_test@email.com'}) + assert 'new_test@email.com', user.unconfirmed_email + + @user = nil + user = new_user(:email => 'new_test@email.com') + assert user.invalid? + end + end + test 'shuold not be included in objects with invalid API' do assert_raise RuntimeError do Class.new.send :include, Devise::Models::Validatable