From 1961de6b5deb7c1799a265d506221fef9d7bb6a9 Mon Sep 17 00:00:00 2001
From: mandaryn <mandaryyyn@gmail.com>
Date: Sun, 3 Apr 2011 21:26:14 +0200
Subject: [PATCH 1/8] Add email confirmation when it is changed by a user

---
 .../mailer/confirmation_instructions.html.erb |  2 +-
 lib/devise.rb                                 |  3 +
 lib/devise/models/confirmable.rb              | 46 ++++++++---
 lib/devise/schema.rb                          |  1 +
 test/integration/confirmable_test.rb          | 50 ++++++++++++
 test/models/confirmable_test.rb               | 77 +++++++++++++++++++
 6 files changed, 169 insertions(+), 10 deletions(-)

diff --git a/app/views/devise/mailer/confirmation_instructions.html.erb b/app/views/devise/mailer/confirmation_instructions.html.erb
index a6ea8ca1..a5c4585e 100644
--- a/app/views/devise/mailer/confirmation_instructions.html.erb
+++ b/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>
diff --git a/lib/devise.rb b/lib/devise.rb
index d05dba07..e147d98f 100644
--- a/lib/devise.rb
+++ b/lib/devise.rb
@@ -139,6 +139,9 @@ module Devise
   mattr_accessor :confirmation_keys
   @@confirmation_keys = [ :email ]
 
+  mattr_accessor :confirmation_on_email_change
+  @@confirmation_on_email_change = false
+
   # Time interval to timeout the user session without activity.
   mattr_accessor :timeout_in
   @@timeout_in = 30.minutes
diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb
index 1fdfe5d1..139078e6 100644
--- a/lib/devise/models/confirmable.rb
+++ b/lib/devise/models/confirmable.rb
@@ -27,15 +27,20 @@ module Devise
       included do
         before_create :generate_confirmation_token, :if => :confirmation_required?
         after_create  :send_confirmation_instructions, :if => :confirmation_required?
+        before_update :prevent_email_change, :if => :prevent_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
-          save(:validate => false)
+          self.email = unconfirmed_email if unconfirmed_email.present?
+          self.unconfirmed_email = nil
+          save
         end
       end
 
@@ -46,6 +51,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?
         ::Devise.mailer.confirmation_instructions(self).deliver
       end
@@ -104,10 +110,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? && unconfirmed_email.blank?
             yield
           else
             self.errors.add(:email, :already_confirmed)
@@ -118,7 +124,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 +137,29 @@ module Devise
           confirm! unless confirmed?
         end
 
+        def prevent_email_change
+          @email_change_confirmation_required = true
+          self.unconfirmed_email = self.email
+          self.email = self.email_was
+        end
+
+        def prevent_email_change?
+          self.class.confirmation_on_email_change && email_changed? && email != unconfirmed_email_was
+        end
+
+        def email_change_confirmation_required?
+          self.class.confirmation_on_email_change && @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)
+          temp = find_by_unconfirmed_email(confirmation_keys, attributes, :not_found)
+          confirmable = temp if temp.persisted?
           confirmable.resend_confirmation_token if confirmable.persisted?
           confirmable
         end
@@ -158,7 +179,14 @@ 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(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)
+        end
+
+        Devise::Models.config(self, :confirm_within, :confirmation_keys, :confirmation_on_email_change)
       end
     end
   end
diff --git a/lib/devise/schema.rb b/lib/devise/schema.rb
index 222a6e33..bfa69cfb 100644
--- a/lib/devise/schema.rb
+++ b/lib/devise/schema.rb
@@ -38,6 +38,7 @@ module Devise
       apply_devise_schema :confirmation_token,   String
       apply_devise_schema :confirmed_at,         DateTime
       apply_devise_schema :confirmation_sent_at, DateTime
+      apply_devise_schema :unconfirmed_email,   String
     end
 
     # Creates reset_password_token and reset_password_sent_at.
diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb
index 6add9177..c460c9a5 100644
--- a/test/integration/confirmable_test.rb
+++ b/test/integration/confirmable_test.rb
@@ -177,3 +177,53 @@ class ConfirmationTest < ActionController::IntegrationTest
     end
   end
 end
+
+class ConfirmationOnChangeTest < ConfirmationTest
+  def setup
+    Devise.confirmation_on_email_change = true
+  end
+
+  def teardown
+    Devise.confirmation_on_email_change = 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 who changed email should get a detailed message about email being not unique' 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)
+
+    visit_user_confirmation_with_token(user.confirmation_token)
+
+    assert_contain /Email.*already.*taken/
+  end
+end
diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb
index c5136612..19b073d4 100644
--- a/test/models/confirmable_test.rb
+++ b/test/models/confirmable_test.rb
@@ -236,3 +236,80 @@ class ConfirmableTest < ActiveSupport::TestCase
     end
   end
 end
+
+class ConfirmableOnChangeTest < ConfirmableTest
+  def setup
+    Devise.confirmation_on_email_change = true
+  end
+
+  def teardown
+    Devise.confirmation_on_email_change = 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 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 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
+end

From 6469cbc62a72cfaa2aa93b45a78fe7b824b70382 Mon Sep 17 00:00:00 2001
From: mandaryn <mandaryyyn@gmail.com>
Date: Sun, 17 Apr 2011 12:15:35 +0200
Subject: [PATCH 2/8] renamed confirmation_on_email_change property to
 reconfirmable and added reconfirmable explanations

---
 lib/devise.rb                        |  4 ++--
 lib/devise/models/confirmable.rb     | 10 +++++++---
 lib/devise/schema.rb                 |  2 +-
 lib/generators/templates/devise.rb   |  6 ++++++
 test/integration/confirmable_test.rb |  4 ++--
 test/models/confirmable_test.rb      |  4 ++--
 6 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/lib/devise.rb b/lib/devise.rb
index e147d98f..8b8c4687 100644
--- a/lib/devise.rb
+++ b/lib/devise.rb
@@ -139,8 +139,8 @@ module Devise
   mattr_accessor :confirmation_keys
   @@confirmation_keys = [ :email ]
 
-  mattr_accessor :confirmation_on_email_change
-  @@confirmation_on_email_change = false
+  mattr_accessor :reconfirmable
+  @@reconfirmable = false
 
   # Time interval to timeout the user session without activity.
   mattr_accessor :timeout_in
diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb
index 139078e6..7907cf76 100644
--- a/lib/devise/models/confirmable.rb
+++ b/lib/devise/models/confirmable.rb
@@ -14,6 +14,10 @@ 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 (exctly the same way as
+    #     initial account confirmation) to be applied. Requires additional unconfirmed_email
+    #     db field to be setup (see migrations). Until confirmed new email is stored in
+    #     unconfirmed email column, and copied to email column on successful confirmation.
     #
     # == Examples
     #
@@ -144,11 +148,11 @@ module Devise
         end
 
         def prevent_email_change?
-          self.class.confirmation_on_email_change && email_changed? && email != unconfirmed_email_was
+          self.class.reconfirmable && email_changed? && email != unconfirmed_email_was
         end
 
         def email_change_confirmation_required?
-          self.class.confirmation_on_email_change && @email_change_confirmation_required
+          self.class.reconfirmable && @email_change_confirmation_required
         end
 
       module ClassMethods
@@ -186,7 +190,7 @@ module Devise
           find_or_initialize_with_errors(confirmation_keys_with_replaced_email, attributes, :not_found)
         end
 
-        Devise::Models.config(self, :confirm_within, :confirmation_keys, :confirmation_on_email_change)
+        Devise::Models.config(self, :confirm_within, :confirmation_keys, :reconfirmable)
       end
     end
   end
diff --git a/lib/devise/schema.rb b/lib/devise/schema.rb
index bfa69cfb..55a53a2a 100644
--- a/lib/devise/schema.rb
+++ b/lib/devise/schema.rb
@@ -38,7 +38,7 @@ module Devise
       apply_devise_schema :confirmation_token,   String
       apply_devise_schema :confirmed_at,         DateTime
       apply_devise_schema :confirmation_sent_at, DateTime
-      apply_devise_schema :unconfirmed_email,   String
+      apply_devise_schema :unconfirmed_email,    String
     end
 
     # Creates reset_password_token and reset_password_sent_at.
diff --git a/lib/generators/templates/devise.rb b/lib/generators/templates/devise.rb
index 292dc949..2afecc24 100644
--- a/lib/generators/templates/devise.rb
+++ b/lib/generators/templates/devise.rb
@@ -80,6 +80,12 @@ Devise.setup do |config|
   # (ie 2 days).
   # 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 ]
 
diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb
index c460c9a5..4b5f2f07 100644
--- a/test/integration/confirmable_test.rb
+++ b/test/integration/confirmable_test.rb
@@ -180,11 +180,11 @@ end
 
 class ConfirmationOnChangeTest < ConfirmationTest
   def setup
-    Devise.confirmation_on_email_change = true
+    Devise.reconfirmable = true
   end
 
   def teardown
-    Devise.confirmation_on_email_change = false
+    Devise.reconfirmable = false
   end
 
   test 'user should be able to request a new confirmation after email changed' do
diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb
index 19b073d4..e4b1c831 100644
--- a/test/models/confirmable_test.rb
+++ b/test/models/confirmable_test.rb
@@ -239,11 +239,11 @@ end
 
 class ConfirmableOnChangeTest < ConfirmableTest
   def setup
-    Devise.confirmation_on_email_change = true
+    Devise.reconfirmable = true
   end
 
   def teardown
-    Devise.confirmation_on_email_change = false
+    Devise.reconfirmable = false
   end
 
   def test_should_not_resend_email_instructions_if_the_user_change_his_email

From 10ac4dbc35fca4c7bc7b569a4f3ffd76c6e646e9 Mon Sep 17 00:00:00 2001
From: mandaryn <mandaryyyn@gmail.com>
Date: Sun, 17 Apr 2011 23:37:47 +0200
Subject: [PATCH 3/8] reconfirmable uniqueness validations

---
 lib/devise/models/confirmable.rb     | 43 +++++++++++++++++++++++-----
 lib/devise/models/validatable.rb     |  1 +
 test/integration/confirmable_test.rb | 17 ++++++++---
 test/models/confirmable_test.rb      |  8 ++++++
 test/models/validatable_test.rb      | 12 ++++++++
 5 files changed, 70 insertions(+), 11 deletions(-)

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

From a7c5a2e65dc01b511d3f09cdf5e3e9550808f604 Mon Sep 17 00:00:00 2001
From: Brian Rose <brian@heimidal.net>
Date: Fri, 12 Aug 2011 10:27:57 -0600
Subject: [PATCH 4/8] Fix up implementation of reconfirmable.

---
 lib/devise/models/confirmable.rb | 28 +++++++++++++++-------------
 test/models/confirmable_test.rb  |  8 ++++----
 test/models/serializable_test.rb |  2 +-
 test/support/assertions.rb       |  5 ++++-
 4 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb
index d87d3791..6206c4ba 100644
--- a/lib/devise/models/confirmable.rb
+++ b/lib/devise/models/confirmable.rb
@@ -42,23 +42,17 @@ module Devise
         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?
+          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 :prevent_email_change, :if => :prevent_email_change?
+        before_update :postpone_email_change_until_confirmation, :if => :postpone_email_change?
         after_update :send_confirmation_instructions, :if => :email_change_confirmation_required?
       end
 
@@ -111,6 +105,14 @@ module Devise
         self.confirmed_at = Time.now
       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.
@@ -168,13 +170,13 @@ module Devise
           confirm! unless confirmed?
         end
 
-        def prevent_email_change
+        def postpone_email_change_until_confirmation
           @email_change_confirmation_required = true
           self.unconfirmed_email = self.email
           self.email = self.email_was
         end
 
-        def prevent_email_change?
+        def postpone_email_change?
           self.class.reconfirmable && email_changed? && email != unconfirmed_email_was
         end
 
diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb
index 1734dbed..986190f5 100644
--- a/test/models/confirmable_test.rb
+++ b/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
@@ -265,7 +265,7 @@ class ConfirmableOnChangeTest < ConfirmableTest
   test 'should send confirmation instructions by email after changing email' do
     user = create_user
     assert user.confirm!
-    assert_email_sent do
+    assert_email_sent "new_test@example.com" do
       assert user.update_attributes(:email => 'new_test@example.com')
     end
   end
diff --git a/test/models/serializable_test.rb b/test/models/serializable_test.rb
index ead1db75..2d41fa24 100644
--- a/test/models/serializable_test.rb
+++ b/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
 
diff --git a/test/support/assertions.rb b/test/support/assertions.rb
index d1637f1b..a80d36cf 100644
--- a/test/support/assertions.rb
+++ b/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)

From a1407565c84160c4df650039697d5ab8c08217a0 Mon Sep 17 00:00:00 2001
From: Brian Rose <brian@heimidal.net>
Date: Fri, 12 Aug 2011 12:13:31 -0600
Subject: [PATCH 5/8] Selectively add reconfirmable field in tests when
 necessary.

---
 lib/devise/models/confirmable.rb     | 10 +++++++---
 lib/devise/schema.rb                 |  4 ++++
 test/integration/confirmable_test.rb |  2 ++
 test/models/confirmable_test.rb      |  4 +++-
 test/models/validatable_test.rb      |  4 ++++
 test/support/helpers.rb              | 20 ++++++++++++++++++++
 6 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb
index 6206c4ba..7c0cf00e 100644
--- a/lib/devise/models/confirmable.rb
+++ b/lib/devise/models/confirmable.rb
@@ -63,8 +63,12 @@ module Devise
         unless_confirmed do
           self.confirmation_token = nil
           self.confirmed_at = Time.now
-          self.email = unconfirmed_email if unconfirmed_email.present?
-          self.unconfirmed_email = nil
+
+          if Devise.reconfirmable
+            self.email = unconfirmed_email if unconfirmed_email.present?
+            self.unconfirmed_email = nil
+          end
+
           save
         end
       end
@@ -146,7 +150,7 @@ module Devise
         # 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? && unconfirmed_email.blank?
+          unless confirmed? && (Devise.reconfirmable ? unconfirmed_email.blank? : true)
             yield
           else
             self.errors.add(:email, :already_confirmed)
diff --git a/lib/devise/schema.rb b/lib/devise/schema.rb
index 55a53a2a..560efde5 100644
--- a/lib/devise/schema.rb
+++ b/lib/devise/schema.rb
@@ -38,6 +38,10 @@ module Devise
       apply_devise_schema :confirmation_token,   String
       apply_devise_schema :confirmed_at,         DateTime
       apply_devise_schema :confirmation_sent_at, DateTime
+    end
+
+    # Creates unconfirmed_email
+    def reconfirmable
       apply_devise_schema :unconfirmed_email,    String
     end
 
diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb
index eb9302c7..1692d208 100644
--- a/test/integration/confirmable_test.rb
+++ b/test/integration/confirmable_test.rb
@@ -186,10 +186,12 @@ class ConfirmationOnChangeTest < ConfirmationTest
   end
 
   def setup
+    add_unconfirmed_email_column
     Devise.reconfirmable = true
   end
 
   def teardown
+    remove_unconfirmed_email_column
     Devise.reconfirmable = false
   end
 
diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb
index 986190f5..549f8645 100644
--- a/test/models/confirmable_test.rb
+++ b/test/models/confirmable_test.rb
@@ -237,12 +237,14 @@ class ConfirmableTest < ActiveSupport::TestCase
   end
 end
 
-class ConfirmableOnChangeTest < ConfirmableTest
+class ReconfirmableTest < ConfirmableTest
   def setup
+    add_unconfirmed_email_column
     Devise.reconfirmable = true
   end
 
   def teardown
+    remove_unconfirmed_email_column
     Devise.reconfirmable = false
   end
 
diff --git a/test/models/validatable_test.rb b/test/models/validatable_test.rb
index 16206881..e9a903b3 100644
--- a/test/models/validatable_test.rb
+++ b/test/models/validatable_test.rb
@@ -106,6 +106,8 @@ class ValidatableTest < ActiveSupport::TestCase
   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'})
@@ -115,6 +117,8 @@ class ValidatableTest < ActiveSupport::TestCase
       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
diff --git a/test/support/helpers.rb b/test/support/helpers.rb
index 3e63a331..cc4a8e5e 100644
--- a/test/support/helpers.rb
+++ b/test/support/helpers.rb
@@ -57,4 +57,24 @@ class ActiveSupport::TestCase
       object.send :"#{key}=", value
     end
   end
+
+  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
+
+  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

From 390645699330d0e40938762a2517019b731d0e2c Mon Sep 17 00:00:00 2001
From: Brian Rose <brian@heimidal.net>
Date: Fri, 12 Aug 2011 12:24:49 -0600
Subject: [PATCH 6/8] Clean up copy and use the Devise API a bit better.

---
 lib/devise/models/confirmable.rb | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb
index 7c0cf00e..e39fadba 100644
--- a/lib/devise/models/confirmable.rb
+++ b/lib/devise/models/confirmable.rb
@@ -14,10 +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 (exctly the same way as
+    #   * +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 (see migrations). Until confirmed new email is stored in
-    #     unconfirmed email column, and copied to email column on successful confirmation.
+    #     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
     #
@@ -64,12 +65,13 @@ module Devise
           self.confirmation_token = nil
           self.confirmed_at = Time.now
 
-          if Devise.reconfirmable
+          if self.class.reconfirmable
             self.email = unconfirmed_email if unconfirmed_email.present?
             self.unconfirmed_email = nil
+            save
+          else
+            save(:validate => false)
           end
-
-          save
         end
       end
 
@@ -150,7 +152,7 @@ module Devise
         # 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? && (Devise.reconfirmable ? unconfirmed_email.blank? : true)
+          unless confirmed? && (self.class.reconfirmable ? unconfirmed_email.blank? : true)
             yield
           else
             self.errors.add(:email, :already_confirmed)

From 5a820262f9af41515e5bbaf0858ab3e143381ea4 Mon Sep 17 00:00:00 2001
From: Brian Rose <brian@heimidal.net>
Date: Fri, 12 Aug 2011 14:51:04 -0600
Subject: [PATCH 7/8] Fix double-submit reconfirmation bug

Previously, if a user submitted their new email twice, they would bypass
the reconfirmation requirement and wind up auto-confirmed.
---
 lib/devise/models/confirmable.rb | 5 ++++-
 test/models/confirmable_test.rb  | 9 +++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb
index e39fadba..afcba832 100644
--- a/lib/devise/models/confirmable.rb
+++ b/lib/devise/models/confirmable.rb
@@ -66,6 +66,7 @@ module Devise
           self.confirmed_at = Time.now
 
           if self.class.reconfirmable
+            @bypass_postpone = true
             self.email = unconfirmed_email if unconfirmed_email.present?
             self.unconfirmed_email = nil
             save
@@ -183,7 +184,9 @@ module Devise
         end
 
         def postpone_email_change?
-          self.class.reconfirmable && email_changed? && email != unconfirmed_email_was
+          postpone = self.class.reconfirmable && email_changed? && !@bypass_postpone
+          @bypass_postpone = nil
+          postpone
         end
 
         def email_change_confirmation_required?
diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb
index 549f8645..9469233a 100644
--- a/test/models/confirmable_test.rb
+++ b/test/models/confirmable_test.rb
@@ -296,6 +296,15 @@ class ReconfirmableTest < ConfirmableTest
     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!

From 8c0f74f036e5ae0174ab1c1c8df01edcd6ba0193 Mon Sep 17 00:00:00 2001
From: Brian Rose <brian@heimidal.net>
Date: Sat, 13 Aug 2011 13:34:29 -0600
Subject: [PATCH 8/8] Add better  message when user updates while reconfirmable
 is enabled.

---
 .../devise/registrations_controller.rb        |  3 +-
 config/locales/en.yml                         |  1 +
 test/integration/registerable_test.rb         | 42 +++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/app/controllers/devise/registrations_controller.rb b/app/controllers/devise/registrations_controller.rb
index 6eae97ad..1c7346e5 100644
--- a/app/controllers/devise/registrations_controller.rb
+++ b/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
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 80c84bba..ae1eebdf 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -36,6 +36,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'
diff --git a/test/integration/registerable_test.rb b/test/integration/registerable_test.rb
index ff14b757..d064b098 100644
--- a/test/integration/registerable_test.rb
+++ b/test/integration/registerable_test.rb
@@ -274,3 +274,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
+