diff --git a/app/models/user.rb b/app/models/user.rb index 7db7e2639e3..7abdae8bf5b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -106,6 +106,7 @@ class User < ActiveRecord::Base validates_presence_of :username validate :username_validator, if: :will_save_change_to_username? validate :password_validator + validate :name_validator, if: :will_save_change_to_name? validates :name, user_full_name: true, if: :will_save_change_to_name?, length: { maximum: 255 } validates :ip_address, allowed_ip_address: { on: :create, message: :signup_not_allowed } validates :primary_email, presence: true @@ -1333,20 +1334,34 @@ class User < ActiveRecord::Base def username_validator username_format_validator || begin - existing = DB.query( - USERNAME_EXISTS_SQL, - username: self.class.normalize_username(username) - ) + if will_save_change_to_username? + existing = DB.query( + USERNAME_EXISTS_SQL, + username: self.class.normalize_username(username) + ) - user_id = existing.select { |u| u.is_user }.first&.id - same_user = user_id && user_id == self.id + user_id = existing.select { |u| u.is_user }.first&.id + same_user = user_id && user_id == self.id - if will_save_change_to_username? && existing.present? && !same_user - errors.add(:username, I18n.t(:'user.username.unique')) + if existing.present? && !same_user + errors.add(:username, I18n.t(:'user.username.unique')) + end + + if confirm_password?(username) || confirm_password?(username.downcase) + errors.add(:username, :same_as_password) + end end end end + def name_validator + if name.present? && + (confirm_password?(name) || confirm_password?(name&.downcase)) + + errors.add(:name, :same_as_password) + end + end + def set_default_categories_preferences return if self.staged? diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 05bb8a5a0f9..dca75522f0e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -491,7 +491,12 @@ en: same_as_username: "is the same as your username. Please use a more secure password." same_as_email: "is the same as your email. Please use a more secure password." same_as_current: "is the same as your current password." + same_as_name: "is the same as your name." unique_characters: "has too many repeated characters. Please use a more secure password." + username: + same_as_password: "is the same as your password." + name: + same_as_password: "is the same as your password." ip_address: signup_not_allowed: "Signup is not allowed from this account." user_email: diff --git a/lib/validators/password_validator.rb b/lib/validators/password_validator.rb index 45a4dab2563..1b0f9374a9e 100644 --- a/lib/validators/password_validator.rb +++ b/lib/validators/password_validator.rb @@ -15,6 +15,8 @@ class PasswordValidator < ActiveModel::EachValidator record.errors.add(attribute, :too_short, count: SiteSetting.min_password_length) elsif record.username.present? && value == record.username record.errors.add(attribute, :same_as_username) + elsif record.name.present? && value == record.name + record.errors.add(attribute, :same_as_name) elsif record.email.present? && value == record.email record.errors.add(attribute, :same_as_email) elsif record.confirm_password?(value) diff --git a/spec/components/validators/password_validator_spec.rb b/spec/components/validators/password_validator_spec.rb index 571a6e17fb8..c4cf64216b6 100644 --- a/spec/components/validators/password_validator_spec.rb +++ b/spec/components/validators/password_validator_spec.rb @@ -121,6 +121,13 @@ describe PasswordValidator do expect(record.errors[:password]).to include(password_error_message(:same_as_username)) end + it "adds an error when password is the same as the name" do + @password = "myawesomepassword" + record.name = @password + validate + expect(record.errors[:password]).to include(password_error_message(:same_as_name)) + end + it "adds an error when password is the same as the email" do @password = "pork@chops.com" record.email = @password diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 19a4bb0f3a5..08a736b002a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -6,6 +6,10 @@ require_dependency 'user' describe User do let(:user) { Fabricate(:user) } + def user_error_message(*keys) + I18n.t(:"activerecord.errors.models.user.attributes.#{keys.join('.')}") + end + context 'validations' do describe '#username' do it { is_expected.to validate_presence_of :username } @@ -32,6 +36,36 @@ describe User do .to include(I18n.t(:'user.username.unique')) end end + + it 'is not valid if username changes to be same as password' do + user.username = 'myawesomepassword' + expect(user).to_not be_valid + expect(user.errors.full_messages.first) + .to include(user_error_message(:username, :same_as_password)) + end + + it 'is not valid if username lowercase changes to be same as password' do + user.username = 'MyAwesomePassword' + expect(user).to_not be_valid + expect(user.errors.full_messages.first) + .to include(user_error_message(:username, :same_as_password)) + end + end + + describe 'name' do + it 'is not valid if it changes to be the same as the password' do + user.name = 'myawesomepassword' + expect(user).to_not be_valid + expect(user.errors.full_messages.first) + .to include(user_error_message(:name, :same_as_password)) + end + + it 'is not valid if name lowercase changes to be the same as the password' do + user.name = 'MyAwesomePassword' + expect(user).to_not be_valid + expect(user.errors.full_messages.first) + .to include(user_error_message(:name, :same_as_password)) + end end describe 'emails' do