FIX: Password required flag should be cleared whenever clearing the raw password (#5384)

app/models/user.rb

@@ -452,6 +452,10 @@ class User < ActiveRecord::Base
     !!@password_required
   end
 
+  def password_validation_required?
+    password_required? || @raw_password.present?
+  end
+
   def has_password?
     password_hash.present?
   end
@@ -1029,6 +1033,7 @@ class User < ActiveRecord::Base
       UserAuthToken.where(user_id: id).destroy_all
       # We should not carry this around after save
       @raw_password = nil
+      @password_required = false
     end
   end
 

lib/validators/password_validator.rb

@@ -3,7 +3,8 @@ require_dependency "common_passwords/common_passwords"
 class PasswordValidator < ActiveModel::EachValidator
 
   def validate_each(record, attribute, value)
-    return unless record.password_required?
+    return unless record.password_validation_required?
+
     if value.nil?
       record.errors.add(attribute, :blank)
     elsif value.length < SiteSetting.min_admin_password_length && (record.admin? || is_developer?(record.email))

spec/components/validators/password_validator_spec.rb

@@ -134,6 +134,19 @@ describe PasswordValidator do
       validate
       expect(record.errors[:password]).to include(password_error_message(:same_as_current))
     end
+
+    it "validation required if password is required" do
+      expect(record.password_validation_required?).to eq(true)
+    end
+
+    it "validation not required after save until a new password is set" do
+      @password = "myoldpassword"
+      record.save!
+      record.reload
+      expect(record.password_validation_required?).to eq(false)
+      record.password = "mynewpassword"
+      expect(record.password_validation_required?).to eq(true)
+    end
   end
 
   context "password not required" do
@@ -144,6 +157,17 @@ describe PasswordValidator do
       validate
       expect(record.errors[:password]).not_to be_present
     end
+
+    it "validation required if a password is set" do
+      @password = "mygameshow"
+      expect(record.password_validation_required?).to eq(true)
+    end
+
+    it "adds an error even password not required" do
+      @password = "p"
+      validate
+      expect(record.errors[:password]).to be_present
+    end
   end
 
 end

spec/controllers/session_controller_spec.rb

@@ -302,7 +302,7 @@ describe SessionController do
         @sso.sso_secret = SiteSetting.sso_secret
         @sso.return_sso_url = "http://somewhere.over.rainbow/sso"
 
-        @user = Fabricate(:user, password: "frogs", active: true, admin: true)
+        @user = Fabricate(:user, password: "myfrogs123ADMIN", active: true, admin: true)
         group = Fabricate(:group)
         group.add(@user)
         @user.reload
@@ -314,7 +314,7 @@ describe SessionController do
         expect(response).to redirect_to("/login")
 
         post :create,
-          params: { login: @user.username, password: "frogs" },
+          params: { login: @user.username, password: "myfrogs123ADMIN" },
           format: :json,
           xhr: true
 
@@ -422,7 +422,7 @@ describe SessionController do
       @sso.sso_secret = SiteSetting.sso_secret
       @sso.return_sso_url = "http://somewhere.over.rainbow/sso"
 
-      @user = Fabricate(:user, password: "frogs", active: true, admin: true)
+      @user = Fabricate(:user, password: "myfrogs123ADMIN", active: true, admin: true)
       EmailToken.update_all(confirmed: true)
     end
 
@@ -431,7 +431,7 @@ describe SessionController do
       expect(response).to redirect_to("/login")
 
       post :create,
-        params: { login: @user.username, password: "frogs" },
+        params: { login: @user.username, password: "myfrogs123ADMIN" },
         format: :json,
         xhr: true
 

spec/fabricators/user_fabricator.rb

@@ -26,14 +26,14 @@ Fabricator(:evil_trout, from: :user) do
   name 'Evil Trout'
   username 'eviltrout'
   email 'eviltrout@somewhere.com'
-  password 'imafish'
+  password 'imafish123'
 end
 
 Fabricator(:walter_white, from: :user) do
   name 'Walter White'
   username 'heisenberg'
   email 'wwhite@bluemeth.com'
-  password 'letscook'
+  password 'letscook123'
 end
 
 Fabricator(:inactive_user, from: :user) do

spec/models/user_spec.rb

@@ -644,7 +644,7 @@ describe User do
 
       UserAuthToken.generate!(user_id: @user.id)
 
-      @user.password = "passwordT"
+      @user.password = "passwordT0"
       @user.save!
 
       # must expire old token on password change

spec/requests/groups_controller_spec.rb

@@ -89,10 +89,6 @@ describe GroupsController do
       )
     end
 
-    before do
-      sign_in(user)
-    end
-
     context "when user is group owner" do
       before do
         group.add_owner(user)