diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8ef94d4a3..a0d00a002 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -711,7 +711,7 @@ curl http://localhost/api/v1/users/password_reset_verify.json -v -u #{login}:#{p end else - user = User.password_reset_check(params[:token]) + user = User.by_reset_token(params[:token]) end if user render json: { message: 'ok', user_login: user.login }, status: :ok diff --git a/app/models/user.rb b/app/models/user.rb index 925afcb69..92d46d715 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -39,7 +39,7 @@ class User < ApplicationModel before_validation :check_name, :check_email, :check_login, :ensure_password before_create :check_preferences_default, :validate_roles, :domain_based_assignment, :set_locale - before_update :check_preferences_default, :validate_roles + before_update :check_preferences_default, :validate_roles, :reset_login_failed after_create :avatar_for_email_check after_update :avatar_for_email_check after_destroy :avatar_destroy @@ -490,9 +490,9 @@ returns =begin -check reset password token +returns the User instance for a given password token if found - result = User.password_reset_check(token) + result = User.by_reset_token(token) returns @@ -500,15 +500,8 @@ returns =end - def self.password_reset_check(token) - user = Token.check(action: 'PasswordReset', name: token) - - # reset login failed if token is valid - if user - user.login_failed = 0 - user.save - end - user + def self.by_reset_token(token) + Token.check(action: 'PasswordReset', name: token) end =begin @@ -526,7 +519,7 @@ returns def self.password_reset_via_token(token, password) # check token - user = Token.check(action: 'PasswordReset', name: token) + user = by_reset_token(token) return if !user # reset password @@ -938,4 +931,11 @@ returns self.password = password_was true end + + # reset login_failed if password is changed + def reset_login_failed + return if !changes + return if !changes['password'] + self.login_failed = 0 + end end diff --git a/spec/factories/token.rb b/spec/factories/token.rb new file mode 100644 index 000000000..bdd77e6f3 --- /dev/null +++ b/spec/factories/token.rb @@ -0,0 +1,9 @@ +FactoryGirl.define do + factory :token do + user_id { FactoryGirl.create(:user).id } + end + + factory :token_password_reset, parent: :token do + action 'PasswordReset' + end +end diff --git a/spec/factories/user.rb b/spec/factories/user.rb index bae6b1953..c27cbc040 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -13,10 +13,15 @@ FactoryGirl.define do email { generate(:email) } password 'zammad' active true + login_failed 0 updated_by_id 1 created_by_id 1 end + factory :user_login_failed, parent: :user do + login_failed { (Setting.get('password_max_login_failed').to_i || 10) + 1 } + end + factory :user_legacy_password_sha2, parent: :user do after(:build) { |user| user.class.skip_callback(:validation, :before, :ensure_password) } password '{sha2}dd9c764fa7ea18cd992c8600006d3dc3ac983d1ba22e9ba2d71f6207456be0ba' # zammad diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb new file mode 100644 index 000000000..ca76a7fb0 --- /dev/null +++ b/spec/models/user_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe User do + + let(:new_password) { 'N3W54V3PW!' } + + context 'password' do + + it 'resets login_failed on password change' do + user = create(:user_login_failed) + expect { + user.password = new_password + user.save + }.to change { user.login_failed }.to(0) + end + end + + context '#by_reset_token' do + + it 'returns a User instance for existing tokens' do + token = create(:token_password_reset) + expect(described_class.by_reset_token(token.name)).to be_instance_of(described_class) + end + + it 'returns nil for not existing tokens' do + expect(described_class.by_reset_token('not-existing')).to be nil + end + end + + context '#password_reset_via_token' do + + it 'changes the password of the token user and destroys the token' do + token = create(:token_password_reset) + user = User.find(token.user_id) + + expect { + described_class.password_reset_via_token(token.name, new_password) + user.reload + }.to change { + user.password + }.and change { + Token.count + }.by(-1) + end + end + +end