diff --git a/app/models/email_subscription.rb b/app/models/email_subscription.rb index a965f06e31..fc3a0f125d 100644 --- a/app/models/email_subscription.rb +++ b/app/models/email_subscription.rb @@ -20,6 +20,7 @@ class EmailSubscription < ApplicationRecord normalizes :email, with: ->(str) { str.squish.downcase } validates :email, presence: true, email_address: true, uniqueness: { scope: :account_id } + validates :email, email_mx: true, if: -> { email_changed? && !Rails.env.local? } scope :confirmed, -> { where.not(confirmed_at: nil) } scope :unconfirmed, -> { where(confirmed_at: nil) } diff --git a/app/models/user.rb b/app/models/user.rb index b4b9f9a9bf..ac33324ddd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -93,9 +93,9 @@ class User < ApplicationRecord validates :invite_request, presence: true, on: :create, if: :invite_text_required? validates :email, presence: true, email_address: true + validates :email, email_mx: { attempt_ip: :sign_up_ip }, if: :validate_email_dns? validates_with UserEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? } - validates_with EmailMxValidator, if: :validate_email_dns? validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create # Honeypot/anti-spam fields diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb index f78b98d7dd..d24b101a6e 100644 --- a/app/validators/email_mx_validator.rb +++ b/app/validators/email_mx_validator.rb @@ -2,21 +2,21 @@ require 'resolv' -class EmailMxValidator < ActiveModel::Validator - def validate(user) - return if user.email.blank? +class EmailMxValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + return if value.blank? - domain = get_domain(user.email) + domain = get_domain(value) if domain.blank? || domain.include?('..') - user.errors.add(:email, :invalid) + record.errors.add(attribute, :invalid) elsif !on_allowlist?(domain) resolved_ips, resolved_domains = resolve_mx(domain) if resolved_ips.empty? - user.errors.add(:email, :unreachable) - elsif email_domain_blocked?(resolved_domains, user.sign_up_ip) - user.errors.add(:email, :blocked) + record.errors.add(attribute, :unreachable) + elsif email_domain_blocked?([domain, *resolved_domains], options[:attempt_ip].is_a?(Symbol) ? record.public_send(options[:attempt_ip]) : nil) + record.errors.add(attribute, :blocked) end end end diff --git a/spec/validators/email_mx_validator_spec.rb b/spec/validators/email_mx_validator_spec.rb index 0ffbf7fb8b..e71794b808 100644 --- a/spec/validators/email_mx_validator_spec.rb +++ b/spec/validators/email_mx_validator_spec.rb @@ -3,109 +3,101 @@ require 'rails_helper' RSpec.describe EmailMxValidator do - let(:user) { Fabricate.build :user, email: } - let(:email) { 'foo@example.com' } - let(:resolv_dns_double) { instance_double(Resolv::DNS) } + subject { record_class.new } - context 'with an e-mail domain that is explicitly allowed' do - around do |example| - original = Rails.configuration.x.email_domains_allowlist - Rails.configuration.x.email_domains_allowlist = 'example.com' - example.run - Rails.configuration.x.email_domains_allowlist = original - end + context 'with no options' do + let(:record_class) do + Class.new do + include ActiveModel::Validations - context 'when there are not DNS records' do - before { configure_resolver('example.com') } + def self.name = 'Record' - it 'does not add errors to record' do - subject.validate(user) - expect(user.errors).to be_empty + attr_accessor :email + + validates :email, email_mx: true end end - end - context 'when there are DNS records for the domain' do - before { configure_resolver('example.com', a: resolv_double_a('192.0.2.42')) } + let(:user) { Fabricate.build :user, email: } + let(:email) { 'foo@example.com' } + let(:resolv_dns_double) { instance_double(Resolv::DNS) } - it 'does not add errors to record' do - subject.validate(user) - expect(user.errors).to be_empty - end - end + context 'with an e-mail domain that is explicitly allowed' do + around do |example| + original = Rails.configuration.x.email_domains_allowlist + Rails.configuration.x.email_domains_allowlist = 'example.com' + example.run + Rails.configuration.x.email_domains_allowlist = original + end - context 'when the TagManager fails to normalize the domain' do - before do - allow(TagManager).to receive(:instance).and_return(tag_manage_double) - allow(tag_manage_double).to receive(:normalize_domain).with('example.com').and_raise(Addressable::URI::InvalidURIError) + context 'when there are not DNS records' do + before { configure_resolver('example.com') } + + it { is_expected.to allow_value(email).for(:email) } + end end - let(:tag_manage_double) { instance_double(TagManager) } + context 'when there are DNS records for the domain' do + before { configure_resolver('example.com', a: resolv_double_a('192.0.2.42')) } - it 'adds errors to record' do - subject.validate(user) - expect(user.errors).to be_present - end - end - - context 'when the email portion is blank' do - let(:email) { 'foo@' } - - it 'adds errors to record' do - subject.validate(user) - expect(user.errors).to be_present - end - end - - context 'when the email domain contains empty labels' do - let(:email) { 'foo@example..com' } - - before { configure_resolver('example..com', a: resolv_double_a('192.0.2.42')) } - - it 'adds errors to record' do - subject.validate(user) - expect(user.errors).to be_present - end - end - - context 'when there are no DNS records for the email domain' do - before { configure_resolver('example.com') } - - it 'adds errors to record' do - subject.validate(user) - expect(user.errors).to be_present - end - end - - context 'when MX record does not lead to an IP' do - before do - configure_resolver('example.com', mx: resolv_double_mx('mail.example.com')) - configure_resolver('mail.example.com') + it { is_expected.to allow_value(email).for(:email) } end - it 'adds errors to record' do - subject.validate(user) - expect(user.errors).to be_present - end - end + context 'when the TagManager fails to normalize the domain' do + before do + allow(TagManager).to receive(:instance).and_return(tag_manage_double) + allow(tag_manage_double).to receive(:normalize_domain).with('example.com').and_raise(Addressable::URI::InvalidURIError) + end - context 'when the MX record has an email domain block' do - before do - Fabricate :email_domain_block, domain: 'mail.example.com' - configure_resolver( - 'example.com', - mx: resolv_double_mx('mail.example.com') - ) - configure_resolver( - 'mail.example.com', - a: instance_double(Resolv::DNS::Resource::IN::A, address: '2.3.4.5'), - aaaa: instance_double(Resolv::DNS::Resource::IN::AAAA, address: 'fd00::2') - ) + let(:tag_manage_double) { instance_double(TagManager) } + + it { is_expected.to_not allow_value(email).for(:email) } end - it 'adds errors to record' do - subject.validate(user) - expect(user.errors).to be_present + context 'when the email portion is blank' do + let(:email) { 'foo@' } + + it { is_expected.to_not allow_value(email).for(:email) } + end + + context 'when the email domain contains empty labels' do + let(:email) { 'foo@example..com' } + + before { configure_resolver('example..com', a: resolv_double_a('192.0.2.42')) } + + it { is_expected.to_not allow_value(email).for(:email) } + end + + context 'when there are no DNS records for the email domain' do + before { configure_resolver('example.com') } + + it { is_expected.to_not allow_value(email).for(:email) } + end + + context 'when MX record does not lead to an IP' do + before do + configure_resolver('example.com', mx: resolv_double_mx('mail.example.com')) + configure_resolver('mail.example.com') + end + + it { is_expected.to_not allow_value(email).for(:email) } + end + + context 'when the MX record has an email domain block' do + before do + Fabricate :email_domain_block, domain: 'mail.example.com' + configure_resolver( + 'example.com', + mx: resolv_double_mx('mail.example.com') + ) + configure_resolver( + 'mail.example.com', + a: instance_double(Resolv::DNS::Resource::IN::A, address: '2.3.4.5'), + aaaa: instance_double(Resolv::DNS::Resource::IN::AAAA, address: 'fd00::2') + ) + end + + it { is_expected.to_not allow_value(email).for(:email) } end end