Don't delete periods when validating username uniqueness (mastodon#1…

…1392) (mastodon#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 (mastodon#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 mastodon#11392
hachyderm · Jul 24, 2019 · 94f5c71 · 94f5c71
1 parent fada60c
commit 94f5c71
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 1 deletion.
diff --git a/app/validators/unique_username_validator.rb b/app/validators/unique_username_validator.rb
@@ -1,10 +1,12 @@
 # frozen_string_literal: true
 
+# See also: USERNAME_RE in the Account class
+
 class UniqueUsernameValidator < ActiveModel::Validator
   def validate(account)
     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 = scope.where.not(id: account.id) if account.persisted?

diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb
@@ -583,12 +583,29 @@
         expect(account.valid?).to be true
       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
         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 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
         account = Fabricate.build(:account, username: Faker::Lorem.characters(31))
         account.valid?