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
This commit is contained in:
ThibG 2020-08-12 12:40:25 +02:00 committed by GitHub
parent 7dc4c74265
commit 8d217d7231
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 29 additions and 13 deletions

View file

@ -27,7 +27,7 @@ module Admin
ips = [] ips = []
Resolv::DNS.open do |dns| 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 } hostnames = dns.getresources(@email_domain_block.domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }

View file

@ -6,7 +6,7 @@ class BlacklistedEmailValidator < ActiveModel::Validator
@email = user.email @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 end
private private

View file

@ -4,22 +4,38 @@ require 'resolv'
class EmailMxValidator < ActiveModel::Validator class EmailMxValidator < ActiveModel::Validator
def validate(user) 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 end
private private
def invalid_mx?(value) def get_domain(value)
_, domain = value.split('@', 2) _, domain = value.split('@', 2)
return true if domain.nil? return nil if domain.nil?
domain = TagManager.instance.normalize_domain(domain) TagManager.instance.normalize_domain(domain)
rescue Addressable::URI::InvalidURIError
nil
end
def resolve_mx(domain)
hostnames = [] hostnames = []
ips = [] ips = []
Resolv::DNS.open do |dns| 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 } 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
end end
ips.empty? || on_blacklist?(hostnames + ips) [ips, hostnames]
rescue Addressable::URI::InvalidURIError
true
end end
def on_blacklist?(values) def on_blacklist?(values)

View file

@ -1325,9 +1325,11 @@ en:
tips: Tips tips: Tips
title: Welcome aboard, %{name}! title: Welcome aboard, %{name}!
users: users:
blocked_email_provider: This e-mail provider isn't allowed
follow_limit_reached: You cannot follow more than %{limit} people 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 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: 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_otp_token: Invalid two-factor code
invalid_sign_in_token: Invalid security 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} otp_lost_help_html: If you lost access to both, you may get in touch with %{email}

View file

@ -63,7 +63,7 @@ module Mastodon
ips = [] ips = []
Resolv::DNS.open do |dns| 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 } 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| ([email_domain_block.domain] + hostnames).uniq.each do |hostname|

View file

@ -17,7 +17,7 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do
let(:blocked_email) { true } let(:blocked_email) { true }
it 'calls errors.add' do 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
end end
@ -25,7 +25,7 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do
let(:blocked_email) { false } let(:blocked_email) { false }
it 'not calls errors.add' do 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 end
end end