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

1 files changed, 22 insertions, 8 deletions

diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb
index 9b5009966..ef1554494 100644
--- 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)