DEV: Make group auto e-mail domain limit configurable (#31873)

We currently limit the number of characters in the bar-separated list of auto-membership e-mail domains. We want to make this configurable through site settings.

After this change, we limit the length of each individual domain, and enable the number of domains to be configured through a hidden site setting.

The original limit is there to prevent DoS, since a TEXT column can take up to 1Gb. With this new limit we're still at a maximum of around 10kb.

app/models/group.rb

@@ -3,6 +3,10 @@
 require "net/imap"
 
 class Group < ActiveRecord::Base
+  # Maximum 255 characters including terminator.
+  # https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.4
+  MAX_EMAIL_DOMAIN_LENGTH = 253
+
   # TODO: Remove flair_url when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy
   # TODO: Remove smtp_ssl when db/post_migrate/20240717053710_drop_groups_smtp_ssl has been promoted to pre-deploy
   self.ignored_columns = %w[flair_url smtp_ssl]
@@ -80,11 +84,10 @@ class Group < ActiveRecord::Base
 
   validate :name_format_validator
   validates :name, presence: true
-  validate :automatic_membership_email_domains_format_validator
+  validate :automatic_membership_email_domains_validator
   validate :incoming_email_validator
   validate :can_allow_membership_requests, if: :allow_membership_requests
   validate :validate_grant_trust_level, if: :will_save_change_to_grant_trust_level?
-  validates :automatic_membership_email_domains, length: { maximum: 1000 }
   validates :bio_raw, length: { maximum: 3000 }
   validates :membership_request_template, length: { maximum: 5000 }
   validates :full_name, length: { maximum: 100 }
@@ -1141,7 +1144,7 @@ class Group < ActiveRecord::Base
       end
   end
 
-  def automatic_membership_email_domains_format_validator
+  def automatic_membership_email_domains_validator
     return if self.automatic_membership_email_domains.blank?
 
     domains =
@@ -1149,6 +1152,18 @@ class Group < ActiveRecord::Base
         self.errors.add :base, (I18n.t("groups.errors.invalid_domain", domain: domain))
       end
 
+    max_domains = SiteSetting.max_automatic_membership_email_domains
+
+    if domains.size > max_domains
+      self.errors.add :base, I18n.t("groups.errors.too_many_domains", max: max_domains)
+    end
+
+    domains.each do |domain|
+      if domain.length > MAX_EMAIL_DOMAIN_LENGTH
+        self.errors.add :base, I18n.t("groups.errors.invalid_domain", domain: domain)
+      end
+    end
+
     self.automatic_membership_email_domains = domains.join("|")
   end
 

config/locales/server.en.yml

@@ -630,6 +630,7 @@ en:
         one: "'%{username}' is already a member of this group."
         other: "The following users are already members of this group: %{username}"
       invalid_domain: "'%{domain}' is not a valid domain."
+      too_many_domains: "Too many domains. Maximum of %{max}."
       invalid_incoming_email: "'%{email}' is not a valid email address."
       email_already_used_in_group: "'%{email}' is already used by the group '%{group_name}'."
       email_already_used_in_category: "'%{email}' is already used by the category '%{category_name}'."

config/site_settings.yml

@@ -912,6 +912,9 @@ groups:
   enable_category_group_moderation:
     client: true
     default: false
+  max_automatic_membership_email_domains:
+    default: 50
+    hidden: true
 
 posting:
   min_post_length:

spec/models/group_spec.rb

@@ -8,10 +8,6 @@ RSpec.describe Group do
   it_behaves_like "it has custom fields"
 
   describe "Validations" do
-    it { is_expected.to allow_value("#{"a" * 996}.com").for(:automatic_membership_email_domains) }
-    it do
-      is_expected.not_to allow_value("#{"a" * 997}.com").for(:automatic_membership_email_domains)
-    end
     it { is_expected.to validate_length_of(:bio_raw).is_at_most(3000) }
     it { is_expected.to validate_length_of(:membership_request_template).is_at_most(5000) }
     it { is_expected.to validate_length_of(:full_name).is_at_most(100) }
@@ -176,6 +172,17 @@ RSpec.describe Group do
       expect(group.valid?).to eq true
     end
 
+    it "is invalid for too many domains" do
+      SiteSetting.max_automatic_membership_email_domains = 1
+      group.automatic_membership_email_domains = "discourse.org|wikipedia.org"
+      expect(group).not_to be_valid
+    end
+
+    it "is invalid for too abnormally long domains" do
+      group.automatic_membership_email_domains = "#{"d" * 253}.org"
+      expect(group).not_to be_valid
+    end
+
     it "is valid for newer TLDs" do
       group.automatic_membership_email_domains = "discourse.institute"
       expect(group.valid?).to eq true