Bypass MX validation for explicitly allowed domains (#15930)

* Bypass MX validation for explicitly allowed domains

This spares some lookups and prevent issues in some edge cases with
local domains.

* Add tests

* Fix test

diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb
index 9f70a1469aa288161cdc8c8fcd869a067f5d1965..dceef50290c58e72d3f316ee00bbadfdd8bfe120 100644 (file)
--- a/app/validators/email_mx_validator.rb
+++ b/app/validators/email_mx_validator.rb
@@ -10,7 +10,7 @@ class EmailMxValidator < ActiveModel::Validator
 
     if domain.blank?
       user.errors.add(:email, :invalid)
-    else
+    elsif !on_allowlist?(domain)
       ips, hostnames = resolve_mx(domain)
 
       if ips.empty?
@@ -33,6 +33,12 @@ class EmailMxValidator < ActiveModel::Validator
     nil
   end
 
+  def on_allowlist?(domain)
+    return false if Rails.configuration.x.email_domains_whitelist.blank?
+
+    Rails.configuration.x.email_domains_whitelist.include?(domain)
+  end
+
   def resolve_mx(domain)
     hostnames = []
     ips       = []

diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index cded4c99bdca2a7d602be7c2180b0b9536493adf..1dae43536087b8cdaf714c04eb26e2b7f3e7e516 100644 (file)
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -206,7 +206,7 @@ RSpec.describe User, type: :model do
 
   describe 'whitelist' do
     around(:each) do |example|
-      old_whitelist = Rails.configuration.x.email_whitelist
+      old_whitelist = Rails.configuration.x.email_domains_whitelist
 
       Rails.configuration.x.email_domains_whitelist = 'mastodon.space'
 

diff --git a/spec/validators/email_mx_validator_spec.rb b/spec/validators/email_mx_validator_spec.rb
index 48e17a4f1086efd18751bce520615a8f41264fc9..550e91996bb97b1b37a374f8fc35a3d6918e1c6d 100644 (file)
--- a/spec/validators/email_mx_validator_spec.rb
+++ b/spec/validators/email_mx_validator_spec.rb
@@ -6,6 +6,24 @@ describe EmailMxValidator do
   describe '#validate' do
     let(:user) { double(email: 'foo@example.com', errors: double(add: nil)) }
 
+    it 'does not add errors if there are no DNS records for an e-mail domain that is explicitly allowed' do
+      old_whitelist = Rails.configuration.x.email_domains_whitelist
+      Rails.configuration.x.email_domains_whitelist = 'example.com'
+
+      resolver = double
+
+      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
+      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
+      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
+      allow(resolver).to receive(:timeouts=).and_return(nil)
+      allow(Resolv::DNS).to receive(:open).and_yield(resolver)
+
+      subject.validate(user)
+      expect(user.errors).to_not have_received(:add)
+
+      Rails.configuration.x.email_domains_whitelist = old_whitelist
+    end
+
     it 'adds an error if there are no DNS records for the e-mail domain' do
       resolver = double