DEV: pull email address validation out to a new EmailAddressValidator

We validate the *format* of email addresses in many places with a match against
a regex, often with very slightly different syntax.

Adding a separate EmailAddressValidator simplifies the code in a few spots and
feels cleaner.

Deprecated the old location in case someone is using it in a plugin.

No functionality change is in this commit.

Note: the regex used at the moment does not support using address literals, e.g.:
* localpart@[192.168.0.1]
* localpart@[2001:db8::1]

app/controllers/session_controller.rb

@@ -546,7 +546,7 @@ class SessionController < ApplicationController
     RateLimiter.new(nil, "forgot-password-min-#{request.remote_ip}", 3, 1.minute).performed!
 
     user = if SiteSetting.hide_email_address_taken && !current_user&.staff?
-      raise Discourse::InvalidParameters.new(:login) if EmailValidator.email_regex !~ normalized_login_param
+      raise Discourse::InvalidParameters.new(:login) if !EmailAddressValidator.valid_value?(normalized_login_param)
       User.real.where(staged: false).find_by_email(Email.downcase(normalized_login_param))
     else
       User.real.where(staged: false).find_by_username_or_email(normalized_login_param)

app/controllers/users_controller.rb

@@ -600,7 +600,7 @@ class UsersController < ApplicationController
       return render json: success_json
     end
 
-    if !(email =~ EmailValidator.email_regex)
+    if !EmailAddressValidator.valid_value?(email)
       error = User.new.errors.full_message(:email, I18n.t(:'user.email.invalid'))
       return render json: failed_json.merge(errors: [error])
     end

app/jobs/regular/bulk_invite.rb

@@ -37,7 +37,7 @@ module Jobs
 
     def process_invites(invites)
       invites.each do |invite|
-        if (EmailValidator.email_regex =~ invite[:email])
+        if EmailAddressValidator.valid_value?(invite[:email])
           # email is valid
           send_invite(invite)
           @sent += 1

app/jobs/regular/group_smtp_email.rb

@@ -42,9 +42,9 @@ module Jobs
         return skip(email, post, recipient_user, :group_smtp_topic_deleted)
       end
 
-      cc_addresses = args[:cc_emails].map do |cc|
-        cc.match(EmailValidator.email_regex) ? cc : nil
-      end.compact
+      cc_addresses = args[:cc_emails].filter do |address|
+        EmailAddressValidator.valid_value?(address)
+      end
 
       # There is a rare race condition causing the Imap::Sync class to create
       # an incoming email and associated post/topic, which then kicks off

app/models/email_change_request.rb

@@ -6,7 +6,7 @@ class EmailChangeRequest < ActiveRecord::Base
   belongs_to :new_email_token, class_name: 'EmailToken', dependent: :destroy
   belongs_to :requested_by, class_name: "User", foreign_key: :requested_by_user_id
 
-  validates :new_email, presence: true, format: { with: EmailValidator.email_regex }
+  validates :new_email, presence: true, format: { with: EmailAddressValidator.email_regex }
 
   def self.states
     @states ||= Enum.new(authorizing_old: 1, authorizing_new: 2, complete: 3)

app/models/user.rb

@@ -1151,7 +1151,7 @@ class User < ActiveRecord::Base
   end
 
   def find_email
-    last_sent_email_address.present? && EmailValidator.email_regex =~ last_sent_email_address ? last_sent_email_address : email
+    last_sent_email_address.present? && EmailAddressValidator.valid_value?(last_sent_email_address) ? last_sent_email_address : email
   end
 
   def tl3_requirements

lib/email.rb

@@ -10,7 +10,7 @@ module Email
 
   def self.is_valid?(email)
     return false unless String === email
-    !!(EmailValidator.email_regex =~ email)
+    EmailAddressValidator.valid_value?(email)
   end
 
   def self.downcase(email)

lib/validators/email_address_validator.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class EmailAddressValidator
+  def self.valid_value?(email)
+    email.match? email_regex
+  end
+
+  def self.email_regex
+    /\A[a-zA-Z0-9!#\$%&'*+\/=?\^_`{|}~\-]+(?:\.[a-zA-Z0-9!#\$%&'\*+\/=?\^_`{|}~\-]+)*@(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?\.)+[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?$\z/
+  end
+end

lib/validators/email_setting_validator.rb

@@ -6,7 +6,8 @@ class EmailSettingValidator
   end
 
   def valid_value?(val)
-    !val.present? || !!(EmailValidator.email_regex =~ val)
+    return true if val.blank?
+    EmailAddressValidator.valid_value?(val)
   end
 
   def error_message

lib/validators/email_validator.rb

@@ -3,7 +3,7 @@
 class EmailValidator < ActiveModel::EachValidator
 
   def validate_each(record, attribute, value)
-    unless value =~ EmailValidator.email_regex
+    if !EmailAddressValidator.valid_value?(value)
       if Invite === record && attribute == :email
         record.errors.add(:base, I18n.t(:'invite.invalid_email', email: CGI.escapeHTML(value)))
       else
@@ -12,7 +12,7 @@ class EmailValidator < ActiveModel::EachValidator
       invalid = true
     end
 
-    unless EmailValidator.allowed?(value)
+    if !EmailValidator.allowed?(value)
       record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
       invalid = true
     end
@@ -51,7 +51,11 @@ class EmailValidator < ActiveModel::EachValidator
   end
 
   def self.email_regex
-    /\A[a-zA-Z0-9!#\$%&'*+\/=?\^_`{|}~\-]+(?:\.[a-zA-Z0-9!#\$%&'\*+\/=?\^_`{|}~\-]+)*@(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?\.)+[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?$\z/
+    Discourse.deprecate(
+      "EmailValidator.email_regex is deprecated. Please use EmailAddressValidator instead.",
+      output_in_test: true,
+      drop_from: '2.9.0',
+    )
+    EmailAddressValidator.email_regex
   end
-
 end

script/bulk_import/base.rb

@@ -452,7 +452,7 @@ class BulkImport::Base
     user_email[:email] ||= random_email
     user_email[:email].downcase!
     # unique email
-    user_email[:email] = random_email until user_email[:email] =~ EmailValidator.email_regex && !@emails.has_key?(user_email[:email])
+    user_email[:email] = random_email until EmailAddressValidator.valid_value?(user_email[:email]) && !@emails.has_key?(user_email[:email])
 
     user_email
   end

script/import_scripts/base.rb

@@ -320,7 +320,7 @@ class ImportScripts::Base
       opts[:username] = UserNameSuggester.suggest(opts[:username].presence || opts[:name].presence || opts[:email])
     end
 
-    unless opts[:email][EmailValidator.email_regex]
+    if !EmailAddressValidator.valid_value?(opts[:email])
       opts[:email] = fake_email
       puts "Invalid email '#{original_email}' for '#{opts[:username]}'. Using '#{opts[:email]}'"
     end

script/import_scripts/drupal.rb

@@ -76,7 +76,7 @@ class ImportScripts::Drupal < ImportScripts::Base
 
       create_users(users, total: user_count, offset: offset) do |user|
         email = user["email"].presence || fake_email
-        email = fake_email unless email[EmailValidator.email_regex]
+        email = fake_email if !EmailAddressValidator.valid_value?(email)
 
         username = @htmlentities.decode(user["username"]).strip
 

script/import_scripts/vbulletin.rb

@@ -151,7 +151,7 @@ EOM
 
       create_users(users, total: user_count, offset: offset) do |user|
         email = user["email"].presence || fake_email
-        email = fake_email unless email[EmailValidator.email_regex]
+        email = fake_email if !EmailAddressValidator.valid_value?(email)
 
         password = [user["password"].presence, user["salt"].presence].compact.join(":")
 

spec/components/validators/email_address_validator_spec.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe EmailAddressValidator do
+  it 'should match valid emails' do
+    ['test@discourse.org', 'good_user@discourse.org', 'incoming+%{reply_key}@discourse.org'].each do |email|
+      expect(EmailAddressValidator.valid_value?(email)).to eq(true)
+    end
+  end
+
+  it 'should not match invalid emails' do
+    ['testdiscourse.org', 'frank@invalid_host.contoso.com', 'frank@invalid_host.com', 'test@discourse.org; a@discourse.org', 'random', ''].each do |email|
+      expect(EmailAddressValidator.valid_value?(email)).to eq(false)
+    end
+  end
+end

spec/components/validators/email_validator_spec.rb

@@ -57,17 +57,4 @@ describe EmailValidator do
       expect(EmailValidator.can_auto_approve_user?("foobar@googlemail.com")).to eq(true)
     end
   end
-
-  context '.email_regex' do
-    it 'should match valid emails' do
-      expect(!!('test@discourse.org' =~ EmailValidator.email_regex)).to eq(true)
-    end
-
-    it 'should not match invalid emails' do
-      ['testdiscourse.org', 'test@discourse.org; a@discourse.org', 'random'].each do |email|
-        expect(!!(email =~ EmailValidator.email_regex)).to eq(false)
-      end
-    end
-  end
-
 end