FIX: Add server side uniqueness validations for `Group#name` and `User#username`.

https://meta.discourse.org/t/groups-can-be-given-same-name-as-existing-username/74010

app/models/group.rb

@@ -43,7 +43,7 @@ class Group < ActiveRecord::Base
   end
 
   validate :name_format_validator
-  validates :name, presence: true, uniqueness: { case_sensitive: false }
+  validates :name, presence: true
   validate :automatic_membership_email_domains_format_validator
   validate :incoming_email_validator
   validate :can_allow_membership_requests, if: :allow_membership_requests
@@ -597,7 +597,19 @@ class Group < ActiveRecord::Base
 
     def name_format_validator
       self.name.strip!
-      UsernameValidator.perform_validation(self, 'name')
+      self.name.downcase!
+
+      UsernameValidator.perform_validation(self, 'name') || begin
+        if will_save_change_to_name?
+          existing = Group.exec_sql(
+            User::USERNAME_EXISTS_SQL, username: self.name
+          ).values.present?
+
+          if existing
+            errors.add(:name, I18n.t("activerecord.errors.messages.taken"))
+          end
+        end
+      end
     end
 
     def automatic_membership_email_domains_format_validator

app/models/user.rb

@@ -1121,12 +1121,28 @@ class User < ActiveRecord::Base
     self.username_lower = username.downcase
   end
 
+  USERNAME_EXISTS_SQL = <<~SQL
+  (SELECT 1 FROM users
+  WHERE users.username_lower = :username)
+
+  UNION ALL
+
+  (SELECT 1 FROM groups
+  WHERE lower(groups.name) = :username)
+  SQL
+
   def username_validator
     username_format_validator || begin
-      lower = username.downcase
-      existing = User.find_by(username_lower: lower)
-      if will_save_change_to_username? && existing && existing.id != self.id
-        errors.add(:username, I18n.t(:'user.username.unique'))
+      if will_save_change_to_username?
+        lower = username.downcase
+
+        existing = User.exec_sql(
+          USERNAME_EXISTS_SQL, username: lower
+        ).values.present?
+
+        if existing
+          errors.add(:username, I18n.t(:'user.username.unique'))
+        end
       end
     end
   end

spec/models/group_spec.rb

@@ -5,6 +5,32 @@ describe Group do
   let(:user) { Fabricate(:user) }
   let(:group) { Fabricate(:group) }
 
+  context 'validations' do
+    describe '#username' do
+      context 'when a user with a similar name exists' do
+        it 'should not be valid' do
+          new_group = Fabricate.build(:group, name: admin.username.upcase)
+
+          expect(new_group).to_not be_valid
+
+          expect(new_group.errors.full_messages.first)
+            .to include(I18n.t("activerecord.errors.messages.taken"))
+        end
+      end
+
+      context 'when a group with a similar name exists' do
+        it 'should not be valid' do
+          new_group = Fabricate.build(:group, name: group.name.upcase)
+
+          expect(new_group).to_not be_valid
+
+          expect(new_group.errors.full_messages.first)
+            .to include(I18n.t("activerecord.errors.messages.taken"))
+        end
+      end
+    end
+  end
+
   describe "#posts_for" do
     it "returns the post in the group" do
       p = Fabricate(:post)

spec/models/user_spec.rb

@@ -5,10 +5,36 @@ describe User do
   let(:user) { Fabricate(:user) }
 
   context 'validations' do
-    it { is_expected.to validate_presence_of :username }
-    it { is_expected.to validate_presence_of :primary_email }
+    describe '#username' do
+      it { is_expected.to validate_presence_of :username }
+
+      describe 'when username already exists' do
+        it 'should not be valid' do
+          new_user = Fabricate.build(:user, username: user.username.upcase)
+
+          expect(new_user).to_not be_valid
+
+          expect(new_user.errors.full_messages.first)
+            .to include(I18n.t(:'user.username.unique'))
+        end
+      end
+
+      describe 'when group with a same name already exists' do
+        it 'should not be valid' do
+          group = Fabricate(:group)
+          new_user = Fabricate.build(:user, username: group.name.upcase)
+
+          expect(new_user).to_not be_valid
+
+          expect(new_user.errors.full_messages.first)
+            .to include(I18n.t(:'user.username.unique'))
+        end
+      end
+    end
 
     describe 'emails' do
+      it { is_expected.to validate_presence_of :primary_email }
+
       let(:user) { Fabricate.build(:user) }
 
       describe 'when record has a valid email' do