From 5910fe30249b580887ab1df39913518eed734ffe Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Fri, 5 Apr 2024 09:48:45 +0200
Subject: [PATCH] Improve email address validation (#29838)

---
 Gemfile                                   |  2 ++
 Gemfile.lock                              |  3 ++-
 app/models/user.rb                        |  5 ++++-
 app/validators/email_address_validator.rb | 18 ++++++++++++++++++
 spec/models/user_spec.rb                  |  6 ++++++
 5 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 app/validators/email_address_validator.rb

diff --git a/Gemfile b/Gemfile
index 54c2f6232e5b1d..fd641009d3a46d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -160,3 +160,5 @@ gem 'concurrent-ruby', require: false
 gem 'connection_pool', require: false
 
 gem 'xorcist', '~> 1.1'
+gem 'cocoon', '~> 1.2'
+gem 'mail', '~> 2.8'
diff --git a/Gemfile.lock b/Gemfile.lock
index f260b2c6a7865e..463c2eaa892fc6 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -766,7 +766,8 @@ DEPENDENCIES
   letter_opener (~> 1.7)
   letter_opener_web (~> 1.4)
   link_header (~> 0.0)
-  lograge (~> 0.11)
+  lograge (~> 0.12)
+  mail (~> 2.8)
   makara (~> 0.5)
   mario-redis-lock (~> 1.2)
   memory_profiler
diff --git a/app/models/user.rb b/app/models/user.rb
index f35764fde4286d..c18c3ff3d03ea6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -88,7 +88,10 @@ class User < ApplicationRecord
   validates :invite_request, presence: true, on: :create, if: :invite_text_required?
 
   validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale?
-  validates_with BlacklistedEmailValidator, if: -> { !confirmed? }
+
+  validates :email, presence: true, email_address: true
+
+  validates_with BlacklistedEmailValidator, 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
   validates :setting_max_frequently_used_emojis, numericality: { greater_than_or_equal_to: 1, less_than_or_equal_to: CustomEmoji::FREQUENTLY_USED_EMOJIS_LIMIT }
diff --git a/app/validators/email_address_validator.rb b/app/validators/email_address_validator.rb
new file mode 100644
index 00000000000000..ed0bb116524aec
--- /dev/null
+++ b/app/validators/email_address_validator.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+# NOTE: I initially wrote this as `EmailValidator` but it ended up clashing
+# with an indirect dependency of ours, `validate_email`, which, turns out,
+# has the same approach as we do, but with an extra check disallowing
+# single-label domains. Decided to not switch to `validate_email` because
+# we do want to allow at least `localhost`.
+
+class EmailAddressValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    value = value.strip
+
+    address = Mail::Address.new(value)
+    record.errors.add(attribute, :invalid) if address.address != value
+  rescue Mail::Field::FieldError
+    record.errors.add(attribute, :invalid)
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 9acc81026ae82c..8392d7c1466043 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -40,6 +40,12 @@
       expect(user.valid?).to be true
     end
 
+    it 'is valid with a localhost e-mail address' do
+      user = Fabricate.build(:user, email: 'admin@localhost')
+      user.valid?
+      expect(user.valid?).to be true
+    end
+
     it 'cleans out empty string from languages' do
       user = Fabricate.build(:user, chosen_languages: [''])
       user.valid?