Maintenance: Improved handling of long passwords.

app/assets/javascripts/app/controllers/_profile/password.coffee

@@ -56,26 +56,32 @@ class ProfilePassword extends App.ControllerSubContent
       data:        JSON.stringify(params)
       processData: true
       success:     @success
+      error:       @error
     )
 
-  success: (data) =>
-    if data.message is 'ok'
-      @render()
-      @notify(
-        type: 'success'
-        msg:  App.i18n.translateContent( 'Password changed successfully!' )
-      )
-    else
-      if data.notice
-        @notify
-          type:      'error'
-          msg:       App.i18n.translateContent( data.notice[0], data.notice[1] )
-          removeAll: true
-      else
-        @notify
-          type:      'error'
-          msg:       __('The password could not be set. Please contact your administrator.')
-          removeAll: true
-      @formEnable( @$('form') )
+  success: =>
+    @render()
+
+    @notify(
+      type: 'success'
+      msg:  App.i18n.translateContent( 'Password changed successfully!' )
+    )
+
+  error: (xhr, status, error) =>
+    return if xhr.status != 422
+
+    data = xhr.responseJSON
+
+    message = if data.notice
+                App.i18n.translateContent( data.notice[0], data.notice[1] )
+              else
+                __('The password could not be set. Please contact your administrator.')
+
+    @notify
+      type:      'error'
+      msg:       message
+      removeAll: true
+
+    @formEnable( @$('form') )
 
 App.Config.set('Password', { prio: 2000, name: __('Password'), parent: '#profile', target: '#profile/password', controller: ProfilePassword, permission: ['user_preferences.password'] }, 'NavBarProfile')

app/controllers/users_controller.rb

@@ -557,26 +557,26 @@ curl http://localhost/api/v1/users/password_change -v -u #{login}:#{password} -H
   def password_change
 
     # check old password
-    if !params[:password_old]
-      render json: { message: 'failed', notice: [__('Current password needed!')] }, status: :ok
+    if !params[:password_old] || !PasswordPolicy::MaxLength.valid?(params[:password_old])
+      render json: { message: 'failed', notice: [__('Current password needed!')] }, status: :unprocessable_entity
       return
     end
 
     current_password_verified = PasswordHash.verified?(current_user.password, params[:password_old])
     if !current_password_verified
-      render json: { message: 'failed', notice: [__('Current password is wrong!')] }, status: :ok
+      render json: { message: 'failed', notice: [__('Current password is wrong!')] }, status: :unprocessable_entity
       return
     end
 
     # set new password
     if !params[:password_new]
-      render json: { message: 'failed', notice: [__('Please supply your new password!')] }, status: :ok
+      render json: { message: 'failed', notice: [__('Please supply your new password!')] }, status: :unprocessable_entity
       return
     end
 
     result = PasswordPolicy.new(params[:password_new])
     if !result.valid?
-      render json: { message: 'failed', notice: result.error }, status: :ok
+      render json: { message: 'failed', notice: result.error }, status: :unprocessable_entity
       return
     end
 

app/models/user.rb

@@ -1169,6 +1169,11 @@ raise 'At least one user need to have admin permissions'
     # don't re-hash passwords
     return password if PasswordHash.crypted?(password)
 
+    if !PasswordPolicy::MaxLength.valid? password
+      errors.add :base, PasswordPolicy::MaxLength.error
+      return nil
+    end
+
     # hash the plaintext password
     PasswordHash.crypt(password)
   end

db/migrate/20220330092945_object_manager_update_user_password.rb

@@ -0,0 +1,19 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+class ObjectManagerUpdateUserPassword < ActiveRecord::Migration[6.1]
+  def up
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    UserInfo.current_user_id = 1
+
+    object_type = ObjectLookup.find_by(name: 'User')
+    attr        = ObjectManager::Attribute.find_by object_lookup_id: object_type.id, name: 'password'
+
+    # password length is capped at 1000 in PasswordPolicy::MaxLength::MAX_LENGTH
+    # if user copy-pastes a very long string
+    # this ensures that max length check is triggered preventing saving of truncated password
+    attr.data_option[:maxlength] = 1001
+    attr.save!
+  end
+end

db/seeds/object_manager_attributes.rb

@@ -1166,7 +1166,10 @@ ObjectManager::Attribute.add(
   data_type:   'input',
   data_option: {
     type:         'password',
-    maxlength:    100,
+    # password length is capped at 1000 in PasswordPolicy::MaxLength::MAX_LENGTH
+    # if user copy-pastes a very long string
+    # this ensures that max length check is triggered preventing saving of truncated password
+    maxlength:    1001,
     null:         true,
     autocomplete: 'new-password',
     item_class:   'formGroup--halfSize',

i18n/zammad.pot

@@ -5176,10 +5176,14 @@ msgstr ""
 msgid "Invalid oauth_token given!"
 msgstr ""
 
-#: lib/password_policy/length.rb
+#: lib/password_policy/min_length.rb
 msgid "Invalid password, it must be at least %s characters long!"
 msgstr ""
 
+#: lib/password_policy/max_length.rb
+msgid "Invalid password, it must be shorter than %s characters!"
+msgstr ""
+
 #: lib/password_policy/digit.rb
 msgid "Invalid password, it must contain at least 1 digit!"
 msgstr ""

lib/auth/backend/internal.rb

@@ -27,6 +27,12 @@ class Auth
       end
 
       def hash_matches?
+        # makes sure that very long strings supplied as password
+        # rejected early and not even tried to match to password
+        if !PasswordPolicy::MaxLength.valid? password
+          return false
+        end
+
         # Because of legacy reason a special check exists and afterwards the
         # password will be saved in the current format.
         if PasswordHash.legacy?(user.password, password)

lib/password_policy/max_length.rb

@@ -0,0 +1,27 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+class PasswordPolicy
+  class MaxLength < PasswordPolicy::Backend
+    MAX_LENGTH = 1_000
+
+    def valid?
+      self.class.valid? @password
+    end
+
+    def error
+      self.class.error
+    end
+
+    def self.applicable?
+      true
+    end
+
+    def self.valid?(input)
+      input.length <= MAX_LENGTH
+    end
+
+    def self.error
+      [__('Invalid password, it must be shorter than %s characters!'), MAX_LENGTH]
+    end
+  end
+end

lib/password_policy/length.rb → lib/password_policy/min_length.rb

@@ -1,7 +1,7 @@
 # Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
 
 class PasswordPolicy
-  class Length < PasswordPolicy::Backend
+  class MinLength < PasswordPolicy::Backend
 
     def valid?
       Setting.get('password_min_size').to_i <= @password.length

spec/db/migrate/object_manager_update_user_password_spec.rb

@@ -0,0 +1,19 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe ObjectManagerUpdateUserPassword, type: :db_migration do
+  let(:attr) do
+    object_type = ObjectLookup.find_by(name: 'User')
+    ObjectManager::Attribute.find_by object_lookup_id: object_type.id, name: 'password'
+  end
+
+  before do
+    attr.data_option['maxlength'] = 123
+    attr.save!
+  end
+
+  it 'changes maxlength' do
+    expect { migrate }.to change { attr.reload.data_option[:maxlength] }.from(123).to(1001)
+  end
+end