From e8d7f172b950257149ce615728480d9894a7b3cb Mon Sep 17 00:00:00 2001
From: Hauke Klement <dev@hklement.de>
Date: Mon, 23 Feb 2015 16:19:06 +0100
Subject: [PATCH] fixed broken password confirmation on password reset

---
 app/models/internal_user.rb       |  7 ++++-
 spec/models/internal_user_spec.rb | 48 ++++++++++++++++++++++---------
 2 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/app/models/internal_user.rb b/app/models/internal_user.rb
index 3f88d46d..26227522 100644
--- a/app/models/internal_user.rb
+++ b/app/models/internal_user.rb
@@ -6,10 +6,15 @@ class InternalUser < ActiveRecord::Base
   has_and_belongs_to_many :teams
 
   validates :email, presence: true, uniqueness: true
-  validates :password, confirmation: true, on: :update, presence: true, unless: :activated?
+  validates :password, confirmation: true, if: :password_void?, on: :update, presence: true
   validates :role, inclusion: {in: ROLES}
 
   def activated?
     activation_state == 'active'
   end
+
+  def password_void?
+    activation_token? || reset_password_token?
+  end
+  private :password_void?
 end
diff --git a/spec/models/internal_user_spec.rb b/spec/models/internal_user_spec.rb
index 28b313dd..9a41d7b6 100644
--- a/spec/models/internal_user_spec.rb
+++ b/spec/models/internal_user_spec.rb
@@ -13,21 +13,13 @@ describe InternalUser do
     expect(user.errors[:email]).to be_present
   end
 
-  context 'when activated' do
-    let(:user) { FactoryGirl.create(:teacher, activation_state: 'active') }
-
-    it 'does not validate the confirmation of the password' do
-      user.update(password: password, password_confirmation: '')
-      expect(user.errors[:password_confirmation]).not_to be_present
-    end
-
-    it 'does not validate the presence of a password' do
-      expect(user.errors[:password]).not_to be_present
-    end
-  end
-
   context 'when not activated' do
-    let(:user) { described_class.create(FactoryGirl.attributes_for(:teacher, activation_state: 'pending', password: nil)) }
+    let(:user) { FactoryGirl.create(:teacher) }
+
+    before(:each) do
+      user.send(:setup_activation)
+      user.send(:send_activation_needed_email!)
+    end
 
     it 'validates the confirmation of the password' do
       user.update(password: password, password_confirmation: '')
@@ -40,6 +32,34 @@ describe InternalUser do
     end
   end
 
+  context 'with a pending password reset' do
+    let(:user) { FactoryGirl.create(:teacher) }
+    before(:each) { user.deliver_reset_password_instructions! }
+
+    it 'validates the confirmation of the password' do
+      user.update(password: password, password_confirmation: '')
+      expect(user.errors[:password_confirmation]).to be_present
+    end
+
+    it 'validates the presence of a password' do
+      user.update(name: Forgery(:name).full_name)
+      expect(user.errors[:password]).to be_present
+    end
+  end
+
+  context 'when complete' do
+    let(:user) { FactoryGirl.create(:teacher, activation_state: 'active') }
+
+    it 'does not validate the confirmation of the password' do
+      user.update(password: password, password_confirmation: '')
+      expect(user.errors[:password_confirmation]).not_to be_present
+    end
+
+    it 'does not validate the presence of a password' do
+      expect(user.errors[:password]).not_to be_present
+    end
+  end
+
   it 'validates the domain of the role' do
     user.update(role: 'Foo')
     expect(user.errors[:role]).to be_present