From 236ef88d93df41f90cec8d83a6e3996fd71c881e Mon Sep 17 00:00:00 2001
From: Mantas Masalskis <mm@zammad.com>
Date: Mon, 13 Jul 2020 13:27:44 +0200
Subject: [PATCH] Fixes #3106 - improve weak password error on initial user
 creation

---
 .../app/controllers/getting_started.coffee    | 18 ++++++++++-----
 .../javascripts/app/controllers/signup.coffee | 10 +++++++-
 app/controllers/users_controller.rb           |  5 +---
 spec/system/setup/system_spec.rb              | 23 +++++++++++++++++--
 4 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/app/assets/javascripts/app/controllers/getting_started.coffee b/app/assets/javascripts/app/controllers/getting_started.coffee
index f3817117b..6fb2cd5cd 100644
--- a/app/assets/javascripts/app/controllers/getting_started.coffee
+++ b/app/assets/javascripts/app/controllers/getting_started.coffee
@@ -187,7 +187,7 @@ class Admin extends App.WizardFullScreen
 
     @html App.view('getting_started/admin')()
 
-    new App.ControllerForm(
+    @form = new App.ControllerForm(
       el:        @$('.js-admin-form')
       model:     App.User
       screen:    'signup'
@@ -208,9 +208,16 @@ class Admin extends App.WizardFullScreen
     )
     if errors
       @log 'error new', errors
+
+      # Only highlight, but don't add message. Error text breaks layout.
+      Object.keys(errors).forEach (key) ->
+        errors[key] = null
+
       @formValidate(form: e.target, errors: errors)
       @formEnable(e)
       return false
+    else
+      @formValidate(form: e.target, errors: errors)
 
     # save user
     user.save(
@@ -231,11 +238,10 @@ class Admin extends App.WizardFullScreen
 
       fail: (settings, details) =>
         @formEnable(e)
-        App.Event.trigger 'notify', {
-          type:    'error'
-          msg:     App.i18n.translateContent(details.error_human || 'Can\'t create user!')
-          timeout: 2500
-        }
+        if _.isArray(details.error)
+          @form.showAlert( App.i18n.translateInline( details.error[0], details.error[1] ) )
+        else
+          @form.showAlert(details.error_human || details.error || 'Unable to create user!')
     )
 
   relogin: (data, status, xhr) =>
diff --git a/app/assets/javascripts/app/controllers/signup.coffee b/app/assets/javascripts/app/controllers/signup.coffee
index ff3374c58..b4e8ad5b1 100644
--- a/app/assets/javascripts/app/controllers/signup.coffee
+++ b/app/assets/javascripts/app/controllers/signup.coffee
@@ -53,11 +53,19 @@ class Index extends App.ControllerContent
     errors = user.validate(
       screen: 'signup'
     )
+
     if errors
       @log 'error new', errors
+
+      # Only highlight, but don't add message. Error text breaks layout.
+      Object.keys(errors).forEach (key) ->
+        errors[key] = null
+
       @formValidate(form: e.target, errors: errors)
       @formEnable(e)
       return false
+    else
+      @formValidate(form: e.target, errors: errors)
 
     # save user
     user.save(
@@ -70,7 +78,7 @@ class Index extends App.ControllerContent
         if _.isArray(details.error)
           @form.showAlert( App.i18n.translateInline( details.error[0], details.error[1] ) )
         else
-          @form.showAlert(details.error_human || details.error || 'Unable to update object!')
+          @form.showAlert(details.error_human || details.error || 'Unable to create user!')
     )
 
   resend: (e) =>
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index d8cd88313..e85a19dff 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -143,10 +143,7 @@ class UsersController < ApplicationController
       # check password policy
       if clean_params[:password].present?
         result = password_policy(clean_params[:password])
-        if result != true
-          render json: { error: result }, status: :unprocessable_entity
-          return
-        end
+        raise Exceptions::UnprocessableEntity, result if result != true
       end
 
       user = User.new(clean_params)
diff --git a/spec/system/setup/system_spec.rb b/spec/system/setup/system_spec.rb
index 782dc8764..97c33751c 100644
--- a/spec/system/setup/system_spec.rb
+++ b/spec/system/setup/system_spec.rb
@@ -1,6 +1,6 @@
 require 'rails_helper'
 
-RSpec.describe 'System setup process', type: :system, set_up: false do
+RSpec.describe 'System setup process', type: :system, set_up: false, authenticated_as: false do
 
   def fqdn
     match_data = %r{://(.+?)(:.+?|/.+?|)$}.match(app_host)
@@ -9,7 +9,7 @@ RSpec.describe 'System setup process', type: :system, set_up: false do
     raise "Unable to get fqdn based on #{app_host}"
   end
 
-  it 'Setting up a new system', authenticated_as: false do
+  it 'Setting up a new system' do
 
     if !ENV['MAILBOX_INIT']
       skip("NOTICE: Need MAILBOX_INIT as ENV variable like export MAILBOX_INIT='unittest01@znuny.com:somepass'")
@@ -110,4 +110,23 @@ RSpec.describe 'System setup process', type: :system, set_up: false do
       expect(page).to have_field('fqdn', with: fqdn)
     end
   end
+
+  # https://github.com/zammad/zammad/issues/3106
+  it 'Shows an error message if too weak password is filled in' do
+    visit '/'
+
+    click_on('Setup new System')
+
+    within('.js-admin') do
+      fill_in 'firstname',        with: 'Test Master'
+      fill_in 'lastname',         with: 'Agent'
+      fill_in 'email',            with: 'master@example.com'
+      fill_in 'password',         with: 'asd'
+      fill_in 'password_confirm', with: 'asd'
+
+      click_on('Create')
+
+      expect(page).to have_text 'Invalid password,'
+    end
+  end
 end