Don't delete periods when validating username uniqueness (#11392) (#11400)

* Check to make sure usernames with '.' cannot be created

* Add test for instance actor account name conflicts

This makes sure that migration 20190715164535_add_instance_actor
won't fail if there's already an account that is named the same
as the domain (minus the .)

* Put the test into the correct context...

* Add another test to split this into two validations

* Don't delete periods when validating username uniqueness (#11392)

The 20190715164535_add_instance_actor migration fails if there's
already a username similar to the domain name, e.g. if you are
'vulpine.club' and have a user named 'vulpineclub', validation
fails.

Upon further review, usernames with periods are dropped by the
regular expression in the Account class, so we don't need to
worry about it here.

Fixes #11392
This commit is contained in:
Rey Tucker 2019-07-24 08:19:17 -04:00 committed by Eugen Rochko
parent fada60cbe7
commit 94f5c714f1
2 changed files with 20 additions and 1 deletions

View File

@ -1,10 +1,12 @@
# frozen_string_literal: true # frozen_string_literal: true
# See also: USERNAME_RE in the Account class
class UniqueUsernameValidator < ActiveModel::Validator class UniqueUsernameValidator < ActiveModel::Validator
def validate(account) def validate(account)
return if account.username.nil? return if account.username.nil?
normalized_username = account.username.downcase.delete('.') normalized_username = account.username.downcase
scope = Account.where(domain: nil).where('lower(username) = ?', normalized_username) scope = Account.where(domain: nil).where('lower(username) = ?', normalized_username)
scope = scope.where.not(id: account.id) if account.persisted? scope = scope.where.not(id: account.id) if account.persisted?

View File

@ -583,12 +583,29 @@ RSpec.describe Account, type: :model do
expect(account.valid?).to be true expect(account.valid?).to be true
end end
it 'is valid if we are creating an instance actor account with a period' do
account = Fabricate.build(:account, id: -99, actor_type: 'Application', locked: true, username: 'example.com')
expect(account.valid?).to be true
end
it 'is valid if we are creating a possibly-conflicting instance actor account' do
account_1 = Fabricate(:account, username: 'examplecom')
account_2 = Fabricate.build(:account, id: -99, actor_type: 'Application', locked: true, username: 'example.com')
expect(account_2.valid?).to be true
end
it 'is invalid if the username doesn\'t only contains letters, numbers and underscores' do it 'is invalid if the username doesn\'t only contains letters, numbers and underscores' do
account = Fabricate.build(:account, username: 'the-doctor') account = Fabricate.build(:account, username: 'the-doctor')
account.valid? account.valid?
expect(account).to model_have_error_on_field(:username) expect(account).to model_have_error_on_field(:username)
end end
it 'is invalid if the username contains a period' do
account = Fabricate.build(:account, username: 'the.doctor')
account.valid?
expect(account).to model_have_error_on_field(:username)
end
it 'is invalid if the username is longer then 30 characters' do it 'is invalid if the username is longer then 30 characters' do
account = Fabricate.build(:account, username: Faker::Lorem.characters(31)) account = Fabricate.build(:account, username: Faker::Lorem.characters(31))
account.valid? account.valid?