diff options
| -rw-r--r-- | app/models/user.rb | 6 | ||||
| -rw-r--r-- | app/validators/email_mx_validator.rb | 19 | ||||
| -rw-r--r-- | spec/validators/email_mx_validator_spec.rb | 75 |
3 files changed, 94 insertions, 6 deletions
diff --git a/app/models/user.rb b/app/models/user.rb index f4130d7b1..44e0d1113 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -73,7 +73,7 @@ class User < ApplicationRecord validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale? validates_with BlacklistedEmailValidator, if: :email_changed? - validates_with EmailMxValidator, if: :email_changed? + validates_with EmailMxValidator, if: :validate_email_dns? scope :recent, -> { order(id: :desc) } scope :admins, -> { where(admin: true) } @@ -360,4 +360,8 @@ class User < ApplicationRecord def needs_feed_update? last_sign_in_at < ACTIVE_DURATION.ago end + + def validate_email_dns? + email_changed? && !(Rails.env.test? || Rails.env.development?) + end end diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb index 8d1e58b38..5b4c684b2 100644 --- a/app/validators/email_mx_validator.rb +++ b/app/validators/email_mx_validator.rb @@ -4,7 +4,6 @@ require 'resolv' class EmailMxValidator < ActiveModel::Validator def validate(user) - return if Rails.env.test? || Rails.env.development? user.errors.add(:email, I18n.t('users.invalid_email')) if invalid_mx?(user.email) end @@ -15,13 +14,23 @@ class EmailMxValidator < ActiveModel::Validator return true if domain.nil? - records = Resolv::DNS.new.getresources(domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s } - records = Resolv::DNS.new.getresources(domain, Resolv::DNS::Resource::IN::A).to_a.map { |e| e.address.to_s } if records.empty? + hostnames = [] + ips = [] - records.empty? || on_blacklist?(records) + Resolv::DNS.open do |dns| + dns.timeouts = 1 + + hostnames = dns.getresources(domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s } + + ([domain] + hostnames).uniq.each do |hostname| + ips.concat(dns.getresources(hostname, Resolv::DNS::Resource::IN::A).to_a.map { |e| e.address.to_s }) + end + end + + ips.empty? || on_blacklist?(hostnames + ips) end def on_blacklist?(values) - EmailDomainBlock.where(domain: values).any? + EmailDomainBlock.where(domain: values.uniq).any? end end diff --git a/spec/validators/email_mx_validator_spec.rb b/spec/validators/email_mx_validator_spec.rb new file mode 100644 index 000000000..bc68f63cf --- /dev/null +++ b/spec/validators/email_mx_validator_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe EmailMxValidator do + describe '#validate' do + let(:user) { double(email: 'foo@example.com', errors: double(add: nil)) } + + it 'adds an error if there are no DNS records for the e-mail domain' do + 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(:timeouts=).and_return(nil) + allow(Resolv::DNS).to receive(:open).and_yield(resolver) + + subject.validate(user) + expect(user.errors).to have_received(:add) + end + + it 'adds an error if a MX record exists but does not lead to an IP' do + resolver = double + + allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')]) + allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([]) + allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).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 have_received(:add) + end + + it 'adds an error if the A record is blacklisted' do + EmailDomainBlock.create!(domain: '1.2.3.4') + 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([double(address: '1.2.3.4')]) + allow(resolver).to receive(:timeouts=).and_return(nil) + allow(Resolv::DNS).to receive(:open).and_yield(resolver) + + subject.validate(user) + expect(user.errors).to have_received(:add) + end + + it 'adds an error if the MX record is blacklisted' do + EmailDomainBlock.create!(domain: '2.3.4.5') + resolver = double + + allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')]) + allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([]) + allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([double(address: '2.3.4.5')]) + allow(resolver).to receive(:timeouts=).and_return(nil) + allow(Resolv::DNS).to receive(:open).and_yield(resolver) + + subject.validate(user) + expect(user.errors).to have_received(:add) + end + + it 'adds an error if the MX hostname is blacklisted' do + EmailDomainBlock.create!(domain: 'mail.example.com') + resolver = double + + allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')]) + allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([]) + allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([double(address: '2.3.4.5')]) + allow(resolver).to receive(:timeouts=).and_return(nil) + allow(Resolv::DNS).to receive(:open).and_yield(resolver) + + subject.validate(user) + expect(user.errors).to have_received(:add) + end + end +end |