From 051efed5edd544b4f88c63a1038274ae2db30038 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 19 Mar 2021 23:48:47 +0100 Subject: [PATCH] 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 --- app/validators/email_mx_validator.rb | 8 +++++++- spec/models/user_spec.rb | 2 +- spec/validators/email_mx_validator_spec.rb | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb index 9f70a1469a..dceef50290 100644 --- 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 cded4c99bd..1dae435360 100644 --- 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 48e17a4f10..550e91996b 100644 --- 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