[36746] Unset plain password accessor after saving (#9157)

* [36746] Unset plain password accessor after saving

Otherwise, this password remains and is validated again due to
"user.register!" which then fails with an exception

https://community.openproject.org/wp/36746

* Don't try to use user.password after saving

it now gets cleared

* Remove user.password usage in specs

The user password attribute is an accessor that now gets deleted after
saving so tests can no longer depend on it being present after created
from a Factory

* Also retain user password in mail handler

app/controllers/users_controller.rb

@@ -136,7 +136,7 @@ class UsersController < ApplicationController
                                 self_notified: params[:self_notified] == '1',
                                 notified_project_ids: params[:notified_project_ids])
 
-      if !@user.password.blank? && @user.change_password_allowed?
+      if update_params[:password].present? && @user.change_password_allowed?
         send_information = params[:send_information]
 
         if @user.invited?
@@ -152,7 +152,7 @@ class UsersController < ApplicationController
         end
 
         if @user.active? && send_information
-          UserMailer.account_information(@user, @user.password).deliver_later
+          UserMailer.account_information(@user, update_params[:password]).deliver_later
         end
       end
 

app/models/mail_handler.rb

@@ -98,10 +98,10 @@ class MailHandler < ActionMailer::Base
       when 'accept'
         @user = User.anonymous
       when 'create'
-        @user = MailHandler.create_user_from_email(email)
+        @user, password = MailHandler.create_user_from_email(email)
         if @user
           log "[#{@user.login}] account created"
-          UserMailer.account_information(@user, @user.password).deliver_later
+          UserMailer.account_information(@user, password).deliver_later
         else
           log "could not create account for [#{sender_email}]", :error
           return false
@@ -442,8 +442,9 @@ class MailHandler < ActionMailer::Base
     end
     if addr.present?
       user = new_user_from_attributes(addr, name)
+      password = user.password
       if user.save
-        user
+        [user, password]
       else
         log "failed to create User: #{user.errors.full_messages}", :error
         nil

app/models/user.rb

@@ -166,6 +166,7 @@ class User < Principal
       passwords.reload
 
       clean_up_former_passwords
+      clean_up_password_attribute
     end
   end
 
@@ -731,6 +732,10 @@ class User < Principal
     (passwords[keep_count..-1] || []).each(&:destroy)
   end
 
+  def clean_up_password_attribute
+    self.password = self.password_confirmation = nil
+  end
+
   ##
   # Brute force prevention - class methods
   #

modules/two_factor_authentication/spec/features/remember_cookie/login_with_remember_cookie_spec.rb

@@ -6,12 +6,11 @@ describe 'Login with 2FA remember cookie',
          with_2fa_ee: true,
          with_config: { '2fa': { active_strategies: [:developer], allow_remember_for_days: 30 } },
          js: true do
-  let(:user) do
-    FactoryBot.create(:user)
-  end
   let(:user_password) do
-    # Works because the user is not reloaded
-    user.password
+    "user!user!"
+  end
+  let(:user) do
+    FactoryBot.create(:user, password: user_password, password_confirmation: user_password)
   end
   let!(:device) { FactoryBot.create :two_factor_authentication_device_sms, user: user, active: true, default: true }
 

spec/features/auth/login_spec.rb

@@ -68,7 +68,7 @@ describe 'Login', type: :feature do
     context 'with leading and trailing space in login' do
       it 'can still login' do
         # first login
-        login_with(" #{user.login} ", user.password)
+        login_with(" #{user.login} ", user_password)
 
         # on the my page
         expect(page)
@@ -83,7 +83,7 @@ describe 'Login', type: :feature do
       it 'redirects to homescreen after forced password change
          (with validation error) and first login' do
         # first login
-        login_with(user.login, user.password)
+        login_with(user.login, user_password)
         expect(current_path).to eql signin_path
 
         # change password page (giving an invalid password)

spec/features/users/self_registration_spec.rb

@@ -29,7 +29,8 @@
 require 'spec_helper'
 
 describe 'user self registration', type: :feature, js: true do
-  let(:admin) { FactoryBot.create :admin, password: 'Test123Test123', password_confirmation: 'Test123Test123' }
+  let(:admin_password) { 'Test123Test123' }
+  let(:admin) { FactoryBot.create :admin, password: admin_password, password_confirmation: admin_password }
   let(:home_page) { Pages::Home.new }
 
   context 'with "manual account activation"',
@@ -104,7 +105,7 @@ describe 'user self registration', type: :feature, js: true do
         .to have_content I18n.t(:'account.error_inactive_manual_activation')
 
       # activation as admin
-      login_with admin.login, admin.password
+      login_with admin.login, admin_password
 
       user_page = Pages::Admin::Users::Edit.new(registered_user.id)