Improve email address validation (#14565)

* Increase DNS timeout from 1 second to 5 seconds for MX check

1 seconds is rather short when using a recursive DNS resolver which
hasn't got a cached result already available. Use 5 seconds instead,
which is the timeout value we use for outgoing HTTP queries.

* Add more precise error messages for invalid e-mail addresses

diff --git a/app/controllers/admin/email_domain_blocks_controller.rb b/app/controllers/admin/email_domain_blocks_controller.rb
index c2591972624e3076ae5ad5328aa43daa0a780a0f..f7bdfb0c5f6f8b3aa4a23bdca00d9fd07622cedc 100644 (file)
--- a/app/controllers/admin/email_domain_blocks_controller.rb
+++ b/app/controllers/admin/email_domain_blocks_controller.rb
@@ -27,7 +27,7 @@ module Admin
           ips       = []
 
           Resolv::DNS.open do |dns|
-            dns.timeouts = 1
+            dns.timeouts = 5
 
             hostnames = dns.getresources(@email_domain_block.domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }
 

diff --git a/app/validators/blacklisted_email_validator.rb b/app/validators/blacklisted_email_validator.rb
index 0d01a1c47fb7fb842542a5d0981fcb547dd1d06b..16e3abf1298ed080f586e486524a7f89673a68d3 100644 (file)
--- a/app/validators/blacklisted_email_validator.rb
+++ b/app/validators/blacklisted_email_validator.rb
@@ -6,7 +6,7 @@ class BlacklistedEmailValidator < ActiveModel::Validator
 
     @email = user.email
 
-    user.errors.add(:email, I18n.t('users.invalid_email')) if blocked_email?
+    user.errors.add(:email, I18n.t('users.blocked_email_provider')) if blocked_email?
   end
 
   private

diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb
index 9b5009966833ac391ce42e631e836aefd7e43525..ef1554494cccbe535e2b69496d509bdb395adc90 100644 (file)
--- a/app/validators/email_mx_validator.rb
+++ b/app/validators/email_mx_validator.rb
@@ -4,22 +4,38 @@ require 'resolv'
 
 class EmailMxValidator < ActiveModel::Validator
   def validate(user)
-    user.errors.add(:email, I18n.t('users.invalid_email')) if invalid_mx?(user.email)
+    domain = get_domain(user.email)
+
+    if domain.nil?
+      user.errors.add(:email, I18n.t('users.invalid_email'))
+    else
+      ips, hostnames = resolve_mx(domain)
+      if ips.empty?
+        user.errors.add(:email, I18n.t('users.invalid_email_mx'))
+      elsif on_blacklist?(hostnames + ips)
+        user.errors.add(:email, I18n.t('users.blocked_email_provider'))
+      end
+    end
   end
 
   private
 
-  def invalid_mx?(value)
+  def get_domain(value)
     _, domain = value.split('@', 2)
 
-    return true if domain.nil?
+    return nil if domain.nil?
+
+    TagManager.instance.normalize_domain(domain)
+  rescue Addressable::URI::InvalidURIError
+    nil
+  end
 
-    domain    = TagManager.instance.normalize_domain(domain)
+  def resolve_mx(domain)
     hostnames = []
     ips       = []
 
     Resolv::DNS.open do |dns|
-      dns.timeouts = 1
+      dns.timeouts = 5
 
       hostnames = dns.getresources(domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }
 
@@ -29,9 +45,7 @@ class EmailMxValidator < ActiveModel::Validator
       end
     end
 
-    ips.empty? || on_blacklist?(hostnames + ips)
-  rescue Addressable::URI::InvalidURIError
-    true
+    [ips, hostnames]
   end
 
   def on_blacklist?(values)

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 2cae0a3e3bad355d72efaf046b3fec03e5973721..40adfc21e4d1768518c43ad7aacbb94706948da7 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1325,9 +1325,11 @@ en:
       tips: Tips
       title: Welcome aboard, %{name}!
   users:
+    blocked_email_provider: This e-mail provider isn't allowed
     follow_limit_reached: You cannot follow more than %{limit} people
     generic_access_help_html: Trouble accessing your account? You may get in touch with %{email} for assistance
     invalid_email: The e-mail address is invalid
+    invalid_email_mx: The e-mail address does not seem to exist
     invalid_otp_token: Invalid two-factor code
     invalid_sign_in_token: Invalid security code
     otp_lost_help_html: If you lost access to both, you may get in touch with %{email}

diff --git a/lib/mastodon/email_domain_blocks_cli.rb b/lib/mastodon/email_domain_blocks_cli.rb
index 7fe1efaaa5c13043f8f2f8ccb413e1b822fdb20f..55a637d68281c230f1b391dfd18126fb95ad9b33 100644 (file)
--- a/lib/mastodon/email_domain_blocks_cli.rb
+++ b/lib/mastodon/email_domain_blocks_cli.rb
@@ -63,7 +63,7 @@ module Mastodon
         ips       = []
 
         Resolv::DNS.open do |dns|
-          dns.timeouts = 1
+          dns.timeouts = 5
           hostnames = dns.getresources(email_domain_block.domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }
 
           ([email_domain_block.domain] + hostnames).uniq.each do |hostname|

diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/blacklisted_email_validator_spec.rb
index ccc5dc0f485050fb017d3df4b9f8270250950bad..f0708dc4623724aa6b7c09dc4a3d38442ebbcfe6 100644 (file)
--- a/spec/validators/blacklisted_email_validator_spec.rb
+++ b/spec/validators/blacklisted_email_validator_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do
       let(:blocked_email) { true }
 
       it 'calls errors.add' do
-        expect(errors).to have_received(:add).with(:email, I18n.t('users.invalid_email'))
+        expect(errors).to have_received(:add).with(:email, I18n.t('users.blocked_email_provider'))
       end
     end
 
@@ -25,7 +25,7 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do
       let(:blocked_email) { false }
 
       it 'not calls errors.add' do
-        expect(errors).not_to have_received(:add).with(:email, I18n.t('users.invalid_email'))
+        expect(errors).not_to have_received(:add).with(:email, I18n.t('users.blocked_email_provider'))
       end
     end
   end