Merge remote-tracking branch 'heimidal/updates' into reconfirm

Conflicts:
	lib/devise/models/confirmable.rb
	test/support/helpers.rb

app/controllers/devise/registrations_controller.rb

@@ -41,7 +41,8 @@ class Devise::RegistrationsController < ApplicationController
     self.resource = resource_class.to_adapter.get!(send(:"current_#{resource_name}").to_key)
 
     if resource.update_with_password(params[resource_name])
-      set_flash_message :notice, :updated if is_navigational_format?
+      flash_key = :update_needs_confirmation if Devise.reconfirmable && resource.unconfirmed_email?
+      set_flash_message :notice, flash_key || :updated if is_navigational_format?
       sign_in resource_name, resource, :bypass => true
       respond_with resource, :location => after_update_path_for(resource)
     else

app/views/devise/mailer/confirmation_instructions.html.erb

@@ -1,5 +1,5 @@
 <p>Welcome <%= @resource.email %>!</p>
 
-<p>You can confirm your account through the link below:</p>
+<p>You can confirm your account email through the link below:</p>
 
 <p><%= link_to 'Confirm my account', confirmation_url(@resource, :confirmation_token => @resource.confirmation_token) %></p>

config/locales/en.yml

@@ -37,6 +37,7 @@ en:
       signed_up: 'Welcome! You have signed up successfully.'
       inactive_signed_up: 'You have signed up successfully. However, we could not sign you in because your account is %{reason}.'
       updated: 'You updated your account successfully.'
+      update_needs_confirmation: "You updated your account successfully, but we need to verify your new email address. Please check your email and click on the confirm link to finalize your new email address."
       destroyed: 'Bye! Your account was successfully cancelled. We hope to see you again soon.'
       reasons:
         inactive: 'inactive'

lib/devise.rb

@@ -141,6 +141,9 @@ module Devise
   mattr_accessor :confirmation_keys
   @@confirmation_keys = [ :email ]
 
+  mattr_accessor :reconfirmable
+  @@reconfirmable = false
+
   # Time interval to timeout the user session without activity.
   mattr_accessor :timeout_in
   @@timeout_in = 30.minutes

lib/devise/models/confirmable.rb

@@ -14,6 +14,11 @@ module Devise
     #     use this to let your user access some features of your application without
     #     confirming the account, but blocking it after a certain period (ie 7 days).
     #     By default confirm_within is zero, it means users always have to confirm to sign in.
+    #   * +reconfirmable+: requires any email changes to be confirmed (exactly the same way as
+    #     initial account confirmation) to be applied. Requires additional unconfirmed_email
+    #     db field to be setup (t.reconfirmable in migrations). Until confirmed new email is
+    #     stored in unconfirmed email column, and copied to email column on successful
+    #     confirmation.
     #
     # == Examples
     #
@@ -24,18 +29,50 @@ 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)
+          count = record.class.where(:unconfirmed_email => record.email).count
+          expected_count = record.new_record? ? 0 : 1
+
+          count > expected_count
+        end
+      end
+
       included do
         before_create :generate_confirmation_token, :if => :confirmation_required?
         after_create  :send_confirmation_instructions, :if => :confirmation_required?
+        before_update :postpone_email_change_until_confirmation, :if => :postpone_email_change?
+        after_update :send_confirmation_instructions, :if => :email_change_confirmation_required?
       end
 
-      # Confirm a user by setting its confirmed_at to actual time. If the user
-      # is already confirmed, add en error to email field
+      # Confirm a user by setting it's confirmed_at to actual time. If the user
+      # is already confirmed, add en error to email field. If the user is invalid
+      # add errors
       def confirm!
         unless_confirmed do
           self.confirmation_token = nil
           self.confirmed_at = Time.now.utc
-          save(:validate => false)
+
+          if self.class.reconfirmable
+            @bypass_postpone = true
+            self.email = unconfirmed_email if unconfirmed_email.present?
+            self.unconfirmed_email = nil
+            save
+          else
+            save(:validate => false)
+          end
         end
       end
 
@@ -46,6 +83,7 @@ module Devise
 
       # Send confirmation instructions by email
       def send_confirmation_instructions
+        @email_change_confirmation_required = false
         generate_confirmation_token! if self.confirmation_token.nil?
         self.devise_mailer.confirmation_instructions(self).deliver
       end
@@ -74,6 +112,14 @@ module Devise
         self.confirmed_at = Time.now.utc
       end
 
+      def headers_for(action)
+        if action == :confirmation_instructions && respond_to?(:unconfirmed_email)
+          { :to => unconfirmed_email.present? ? unconfirmed_email : email }
+        else
+          {}
+        end
+      end
+
       protected
 
         # Callback to overwrite if confirmation is required or not.
@@ -104,10 +150,10 @@ module Devise
           confirmation_sent_at && confirmation_sent_at.utc >= self.class.confirm_within.ago
         end
 
-        # Checks whether the record is confirmed or not, yielding to the block
+        # Checks whether the record is confirmed or not or a new email has been added, yielding to the block
         # if it's already confirmed, otherwise adds an error to email.
         def unless_confirmed
-          unless confirmed?
+          unless confirmed? && (self.class.reconfirmable ? unconfirmed_email.blank? : true)
             yield
           else
             self.errors.add(:email, :already_confirmed)
@@ -118,7 +164,6 @@ module Devise
         # Generates a new random token for confirmation, and stores the time
         # this token is being generated
         def generate_confirmation_token
-          self.confirmed_at = nil
           self.confirmation_token = self.class.confirmation_token
           self.confirmation_sent_at = Time.now.utc
         end
@@ -132,13 +177,32 @@ module Devise
           confirm! unless confirmed?
         end
 
+        def postpone_email_change_until_confirmation
+          @email_change_confirmation_required = true
+          self.unconfirmed_email = self.email
+          self.email = self.email_was
+        end
+
+        def postpone_email_change?
+          postpone = self.class.reconfirmable && email_changed? && !@bypass_postpone
+          @bypass_postpone = nil
+          postpone
+        end
+
+        def email_change_confirmation_required?
+          self.class.reconfirmable && @email_change_confirmation_required
+        end
+
       module ClassMethods
         # Attempt to find a user by its email. If a record is found, send new
-        # confirmation instructions to it. If not user is found, returns a new user
-        # with an email not found error.
+        # confirmation instructions to it. If not, try searching for a user by unconfirmed_email
+        # 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)
+          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
@@ -158,7 +222,15 @@ module Devise
           generate_token(:confirmation_token)
         end
 
-        Devise::Models.config(self, :confirm_within, :confirmation_keys)
+        # Find a record for confirmation by unconfirmed email field
+        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)
       end
     end
   end

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?

lib/devise/schema.rb

@@ -40,6 +40,11 @@ module Devise
       apply_devise_schema :confirmation_sent_at, DateTime
     end
 
+    # Creates unconfirmed_email
+    def reconfirmable
+      apply_devise_schema :unconfirmed_email,    String
+    end
+
     # Creates reset_password_token and reset_password_sent_at.
     #
     # == Options

lib/generators/templates/devise.rb

@@ -79,6 +79,12 @@ Devise.setup do |config|
   # the user cannot access the website without confirming his account.
   # config.confirm_within = 2.days
 
+  # If true, requires any email changes to be confirmed (exctly the same way as
+  # initial account confirmation) to be applied. Requires additional unconfirmed_email
+  # db field (see migrations). Until confirmed new email is stored in
+  # unconfirmed email column, and copied to email column on successful confirmation.
+  # config.reconfirmable = false
+
   # Defines which key will be used when confirming an account
   # config.confirmation_keys = [ :email ]
 

test/integration/confirmable_test.rb

@@ -201,3 +201,64 @@ class ConfirmationTest < ActionController::IntegrationTest
     end
   end
 end
+
+class ConfirmationOnChangeTest < ConfirmationTest
+
+  def create_second_user(options={})
+    @user = nil
+    create_user(options)
+  end
+
+  def setup
+    add_unconfirmed_email_column
+    Devise.reconfirmable = true
+  end
+
+  def teardown
+    remove_unconfirmed_email_column
+    Devise.reconfirmable = false
+  end
+
+  test 'user should be able to request a new confirmation after email changed' do
+    user = create_user(:confirm => true)
+    user.update_attributes(:email => 'new_test@example.com')
+    ActionMailer::Base.deliveries.clear
+
+    visit new_user_session_path
+    click_link "Didn't receive confirmation instructions?"
+
+    fill_in 'email', :with => user.unconfirmed_email
+    click_button 'Resend confirmation instructions'
+
+    assert_current_url '/users/sign_in'
+    assert_contain 'You will receive an email with instructions about how to confirm your account in a few minutes'
+    assert_equal 1, ActionMailer::Base.deliveries.size
+  end
+
+  test 'user with valid confirmation token should be able to confirm email after email changed' do
+    user = create_user(:confirm => true)
+    user.update_attributes(:email => 'new_test@example.com')
+    assert 'new_test@example.com', user.unconfirmed_email
+    visit_user_confirmation_with_token(user.confirmation_token)
+
+    assert_contain 'Your account was successfully confirmed.'
+    assert_current_url '/'
+    assert user.reload.confirmed?
+  end
+
+  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
+
+    get new_user_registration_path
+
+    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

test/integration/registerable_test.rb

@@ -291,3 +291,45 @@ class RegistrationTest < ActionController::IntegrationTest
     assert_equal User.count, 0
   end
 end
+
+class ReconfirmableRegistrationTest < ActionController::IntegrationTest
+  def setup
+    add_unconfirmed_email_column
+    Devise.reconfirmable = true
+  end
+
+  def teardown
+    remove_unconfirmed_email_column
+    Devise.reconfirmable = false
+  end
+
+  test 'a signed in user should see a more appropriate flash message when editing his account if reconfirmable is enabled' do
+    sign_in_as_user
+    get edit_user_registration_path
+
+    fill_in 'email', :with => 'user.new@example.com'
+    fill_in 'current password', :with => '123456'
+    click_button 'Update'
+
+    assert_current_url '/'
+    assert_contain 'You updated your account successfully, but we need to verify your new email address. Please check your email and click on the confirm link to finalize your new email address.'
+
+    assert_equal "user.new@example.com", User.first.unconfirmed_email
+  end
+
+  test 'A signed in user should not see a reconfirmation message if they did not change their password' do
+    sign_in_as_user
+    get edit_user_registration_path
+
+    fill_in 'password', :with => 'pas123'
+    fill_in 'password confirmation', :with => 'pas123'
+    fill_in 'current password', :with => '123456'
+    click_button 'Update'
+
+    assert_current_url '/'
+    assert_contain 'You updated your account successfully.'
+
+    assert User.first.valid_password?('pas123')
+  end
+end
+

test/models/confirmable_test.rb

@@ -80,8 +80,8 @@ class ConfirmableTest < ActiveSupport::TestCase
   end
 
   test 'should send confirmation instructions by email' do
-    assert_email_sent do
-      create_user
+    assert_email_sent "mynewuser@example.com" do
+      create_user :email => "mynewuser@example.com"
     end
   end
 
@@ -123,7 +123,7 @@ class ConfirmableTest < ActiveSupport::TestCase
 
   test 'should send email instructions for the user confirm its email' do
     user = create_user
-    assert_email_sent do
+    assert_email_sent user.email do
       User.send_confirmation_instructions(:email => user.email)
     end
   end
@@ -236,3 +236,99 @@ class ConfirmableTest < ActiveSupport::TestCase
     end
   end
 end
+
+class ReconfirmableTest < ConfirmableTest
+  def setup
+    add_unconfirmed_email_column
+    Devise.reconfirmable = true
+  end
+
+  def teardown
+    remove_unconfirmed_email_column
+    Devise.reconfirmable = false
+  end
+
+  def test_should_not_resend_email_instructions_if_the_user_change_his_email
+    #behaves differently
+  end
+
+  def test_should_not_reset_confirmation_status_or_token_when_updating_email
+    #behaves differently
+  end
+
+  test 'should generate confirmation token after changing email' do
+    user = create_user
+    assert user.confirm!
+    assert_nil user.confirmation_token
+    assert user.update_attributes(:email => 'new_test@example.com')
+    assert_not_nil user.confirmation_token
+  end
+
+  test 'should send confirmation instructions by email after changing email' do
+    user = create_user
+    assert user.confirm!
+    assert_email_sent "new_test@example.com" do
+      assert user.update_attributes(:email => 'new_test@example.com')
+    end
+  end
+
+  test 'should not send confirmation by email after changing password' do
+    user = create_user
+    assert user.confirm!
+    assert_email_not_sent do
+      assert user.update_attributes(:password => 'newpass', :password_confirmation => 'newpass')
+    end
+  end
+
+  test 'should stay confirmed when email is changed' do
+    user = create_user
+    assert user.confirm!
+    assert user.update_attributes(:email => 'new_test@example.com')
+    assert user.confirmed?
+  end
+
+  test 'should update email only when it is confirmed' do
+    user = create_user
+    assert user.confirm!
+    assert user.update_attributes(:email => 'new_test@example.com')
+    assert_not_equal 'new_test@example.com', user.email
+    assert user.confirm!
+    assert_equal 'new_test@example.com', user.email
+  end
+
+  test 'should not allow user to get past confirmation email by resubmitting their new address' do
+    user = create_user
+    assert user.confirm!
+    assert user.update_attributes(:email => 'new_test@example.com')
+    assert_not_equal 'new_test@example.com', user.email
+    assert user.update_attributes(:email => 'new_test@example.com')
+    assert_not_equal 'new_test@example.com', user.email
+  end
+
+  test 'should find a user by send confirmation instructions with unconfirmed_email' do
+    user = create_user
+    assert user.confirm!
+    assert user.update_attributes(:email => 'new_test@example.com')
+    confirmation_user = User.send_confirmation_instructions(:email => user.unconfirmed_email)
+    assert_equal confirmation_user, user
+  end
+
+  test 'should return a new user if no email or unconfirmed_email was found' do
+    confirmation_user = User.send_confirmation_instructions(:email => "invalid@email.com")
+    assert_not confirmation_user.persisted?
+  end
+
+  test 'should add error to new user email if no email or unconfirmed_email was found' do
+    confirmation_user = User.send_confirmation_instructions(:email => "invalid@email.com")
+    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

test/models/serializable_test.rb

@@ -16,7 +16,7 @@ class SerializableTest < ActiveSupport::TestCase
   end
 
   test 'should include unsafe keys on XML if a force_except is provided' do
-    assert_no_match /email/, @user.to_xml(:force_except => :email)
+    assert_no_match /<email/, @user.to_xml(:force_except => :email)
     assert_match /confirmation-token/, @user.to_xml(:force_except => :email)
   end
 

test/models/validatable_test.rb

@@ -105,6 +105,22 @@ 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
+    add_unconfirmed_email_column
+
+    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
+
+    remove_unconfirmed_email_column
+  end
+
   test 'shuold not be included in objects with invalid API' do
     assert_raise RuntimeError do
       Class.new.send :include, Devise::Models::Validatable

test/support/assertions.rb

@@ -14,8 +14,11 @@ class ActiveSupport::TestCase
   end
   alias :assert_present :assert_not_blank
 
-  def assert_email_sent(&block)
+  def assert_email_sent(address = nil, &block)
     assert_difference('ActionMailer::Base.deliveries.size') { yield }
+    if address.present?
+      assert_equal address, ActionMailer::Base.deliveries.last['to'].to_s
+    end
   end
 
   def assert_email_not_sent(&block)

test/support/helpers.rb

@@ -84,4 +84,26 @@ class ActiveSupport::TestCase
       end
     end
   end
+
+  # TODO: Get rid of this in favor of a real model with unconfirmed
+  def add_unconfirmed_email_column
+    if DEVISE_ORM == :active_record
+      ActiveRecord::Base.connection.add_column(:users, :unconfirmed_email, :string)
+      User.reset_column_information
+    elsif DEVISE_ORM == :mongoid
+      User.field(:unconfirmed_email, :type => String)
+    end
+  end
+
+  # TODO: Get rid of this in favor of a real model with unconfirmed
+  def remove_unconfirmed_email_column
+    if DEVISE_ORM == :active_record
+      ActiveRecord::Base.connection.remove_column(:users, :unconfirmed_email)
+      User.reset_column_information
+    elsif DEVISE_ORM == :mongoid
+      User.fields.delete(:unconfirmed_email)
+      User.send(:undefine_attribute_methods)
+      User.send(:define_attribute_methods, User.fields.keys)
+    end
+  end
 end