Refactoring: Added consistent email address validation to backend.

app/controllers/form_controller.rb

@@ -41,13 +41,6 @@ class FormController < ApplicationController
     if params[:name].blank?
       errors['name'] = 'required'
     end
-    if params[:email].blank?
-      errors['email'] = 'required'
-    elsif !/@/.match?(params[:email])
-      errors['email'] = 'invalid'
-    elsif params[:email].match?(/(>|<|\||\!|"|§|'|\$|%|&|\(|\)|\?|\s|\.\.)/)
-      errors['email'] = 'invalid'
-    end
     if params[:title].blank?
       errors['title'] = 'required'
     end
@@ -55,11 +48,12 @@ class FormController < ApplicationController
       errors['body'] = 'required'
     end
 
-    # realtime verify
+    if params[:email].blank?
-    if errors['email'].blank?
+      errors['email'] = 'required'
+    else
       begin
-        address = ValidEmail2::Address.new(params[:email])
+        email_address_validation = EmailAddressValidation.new(params[:email])
-        if !address || !address.valid? || !address.valid_mx?
+        if !email_address_validation.valid_format? || !email_address_validation.valid_mx?
           errors['email'] = 'invalid'
         end
       rescue => e

app/controllers/tickets_controller.rb

@@ -97,18 +97,19 @@ class TicketsController < ApplicationController
 
     # try to create customer if needed
     if clean_params[:customer_id].present? && clean_params[:customer_id] =~ /^guess:(.+?)$/
-      email = $1
+      email_address = $1
-      if email !~ /@/ || email =~ /(>|<|\||\!|"|§|'|\$|%|&|\(|\)|\?|\s)/
+      email_address_validation = EmailAddressValidation.new(email_address)
+      if !email_address_validation.valid_format?
         render json: { error: 'Invalid email of customer' }, status: :unprocessable_entity
         return
       end
-      local_customer = User.find_by(email: email.downcase)
+      local_customer = User.find_by(email: email_address.downcase)
       if !local_customer
         role_ids = Role.signup_role_ids
         local_customer = User.create(
           firstname: '',
           lastname:  '',
-          email:     email,
+          email:     email_address,
           password:  '',
           active:    true,
           role_ids:  role_ids,

app/models/channel/driver/sendmail.rb

@@ -8,7 +8,8 @@ class Channel::Driver::Sendmail
 
     # set system_bcc of config if defined
     system_bcc = Setting.get('system_bcc')
-    if system_bcc.present? && system_bcc =~ /@/
+    email_address_validation = EmailAddressValidation.new(system_bcc)
+    if system_bcc.present? && email_address_validation.valid_format?
       attr[:bcc] ||= ''
       attr[:bcc] += ', ' if attr[:bcc].present?
       attr[:bcc] += system_bcc

app/models/channel/driver/smtp.rb

@@ -55,7 +55,8 @@ class Channel::Driver::Smtp
 
     # set system_bcc of config if defined
     system_bcc = Setting.get('system_bcc')
-    if system_bcc.present? && system_bcc =~ /@/
+    email_address_validation = EmailAddressValidation.new(system_bcc)
+    if system_bcc.present? && email_address_validation.valid_format?
       attr[:bcc] ||= ''
       attr[:bcc] += ', ' if attr[:bcc].present?
       attr[:bcc] += system_bcc

app/models/channel/filter/identify_sender.rb

@@ -102,10 +102,11 @@ module Channel::Filter::IdentifySender
         next if items.blank?
 
         items.each do |address_data|
-          email_address = address_data.address
+          email_address = sanitize_email(address_data.address)
           next if email_address.blank?
-          next if !email_address.match?(/@/)
+
-          next if email_address.match?(/\s/)
+          email_address_validation = EmailAddressValidation.new(email_address)
+          next if !email_address_validation.valid_format?
 
           user_create(
             firstname: address_data.display_name,
@@ -131,9 +132,13 @@ module Channel::Filter::IdentifySender
           if recipient =~ /^(.+?)<(.+?)>/
             display_name = $1
           end
+
           next if address.blank?
-          next if !address.match?(/@/)
+
-          next if address.match?(/\s/)
+          address = sanitize_email(address)
+
+          email_address_validation = EmailAddressValidation.new(address)
+          next if !email_address_validation.valid_format?
 
           user_create(
             firstname: display_name,
@@ -194,10 +199,12 @@ module Channel::Filter::IdentifySender
     string.downcase
           .strip
           .delete('"')
+          .delete("'")
           .delete(' ')             # see https://github.com/zammad/zammad/issues/2254
           .sub(/^<|>$/, '')        # see https://github.com/zammad/zammad/issues/2254
           .sub(/\A'(.*)'\z/, '\1') # see https://github.com/zammad/zammad/issues/2154
           .gsub(/\s/, '')          # see https://github.com/zammad/zammad/issues/2198
+          .gsub(/\.\z/, '')
   end
 
 end

app/models/email_address.rb

@@ -48,8 +48,10 @@ check and if channel not exists reset configured channels for email addresses
     return true if email.blank?
 
     self.email = email.downcase.strip
-    raise Exceptions::UnprocessableEntity, 'Invalid email' if !email.match?(/@/)
+    email_address_validation = EmailAddressValidation.new(email)
-    raise Exceptions::UnprocessableEntity, 'Invalid email' if email.match?(/\s/)
+    if !email_address_validation.valid_format?
+      raise Exceptions::UnprocessableEntity, 'Invalid email'
+    end
 
     true
   end

app/models/ticket.rb

@@ -1352,13 +1352,13 @@ result
 
       # send notifications only to email addresses
       next if recipient_email.blank?
-      next if !recipient_email.match?(/@/)
 
       # check if address is valid
       begin
         Mail::AddressList.new(recipient_email).addresses.each do |address|
           recipient_email = address.address
-          break if recipient_email.present? && recipient_email =~ /@/ && !recipient_email.match?(/\s/)
+          email_address_validation = EmailAddressValidation.new(recipient_email)
+          break if recipient_email.present? && email_address_validation.valid_format?
         end
       rescue
         if recipient_email.present?
@@ -1368,11 +1368,11 @@ result
 
           recipient_email = "#{$2}@#{$3}"
         end
-        next if recipient_email.blank?
-        next if !recipient_email.match?(/@/)
-        next if recipient_email.match?(/\s/)
       end
 
+      email_address_validation = EmailAddressValidation.new(recipient_email)
+      next if !email_address_validation.valid_format?
+
       # do not sent notifications to this recipients
       send_no_auto_response_reg_exp = Setting.get('send_no_auto_response_reg_exp')
       begin

app/models/user.rb

@@ -959,8 +959,11 @@ try to find correct name
 
     self.email = email.downcase.strip
     return true if id == 1
-    raise Exceptions::UnprocessableEntity, 'Invalid email' if !email.match?(/@/)
+
-    raise Exceptions::UnprocessableEntity, 'Invalid email' if email.match?(/\s/)
+    email_address_validation = EmailAddressValidation.new(email)
+    if !email_address_validation.valid_format?
+      raise Exceptions::UnprocessableEntity, 'Invalid email'
+    end
 
     true
   end
@@ -1182,7 +1185,10 @@ raise 'Minimum one user need to have admin permissions'
   def avatar_for_email_check
     return true if Setting.get('import_mode')
     return true if email.blank?
-    return true if !email.match?(/@/)
+
+    email_address_validation = EmailAddressValidation.new(email)
+    return true if !email_address_validation.valid_format?
+
     return true if !saved_change_to_attribute?('email') && updated_at > Time.zone.now - 10.days
 
     # save/update avatar

lib/email_address_validation.rb

@@ -0,0 +1,38 @@
+# Validation for email addresses
+
+class EmailAddressValidation
+
+  attr_reader :email_address
+
+  # @param [String] email_address Email address to be validated
+  def initialize(email_address)
+    @email_address = email_address
+  end
+
+  def to_s
+    email_address
+  end
+
+  # Checks if the email address has a valid format.
+  # Reports email addresses without dot in domain as valid (zammad@localhost).
+  #
+  # @return [true]  if email address has valid format
+  # @return [false] if email address has no valid format
+  def valid_format?
+    # Note: Don't use ValidEmail2::Address.valid? here because it requires the
+    # email address to have a dot in its domain.
+    @valid_format ||= email_address.match?(URI::MailTo::EMAIL_REGEXP)
+  end
+
+  # Checks if the domain of the email address has a valid MX record.
+  #
+  # @return [true]  if email address domain has an MX record
+  # @return [false] if email address domain has no MX record
+  def valid_mx?
+    return @valid_mx if @valid_mx
+
+    validated_email_address = ValidEmail2::Address.new(email_address)
+    @valid_mx               = validated_email_address&.valid_mx?
+  end
+
+end

spec/lib/email_address_validation_spec.rb

@@ -0,0 +1,125 @@
+require 'rails_helper'
+
+RSpec.describe EmailAddressValidation do
+
+  describe 'Valid email address' do
+
+    describe 'with dot in domain' do
+
+      describe 'with MX record' do
+        let(:email_address) { 'support@zammad.com' }
+        let(:email_address_validation) { described_class.new(email_address) }
+
+        it 'reports given email address' do
+          expect(email_address_validation.email_address).to eq(email_address)
+          expect(email_address_validation.to_s).to eq(email_address)
+        end
+
+        it 'reports email address as valid' do
+          expect(email_address_validation.valid_format?).to be(true)
+        end
+
+        it 'reports email address to have domain with MX record' do
+          expect(email_address_validation.valid_mx?).to be(true)
+        end
+      end
+
+      describe 'without MX record' do
+        let(:email_address) { 'someone@this-is-probably-a-non-existent-domain.com.example' }
+        let(:email_address_validation) { described_class.new(email_address) }
+
+        it 'reports given email address' do
+          expect(email_address_validation.email_address).to eq(email_address)
+          expect(email_address_validation.to_s).to eq(email_address)
+        end
+
+        it 'reports email address as valid' do
+          expect(email_address_validation.valid_format?).to be(true)
+        end
+
+        it 'reports email address to have domain without MX record' do
+          expect(email_address_validation.valid_mx?).to be(false)
+        end
+      end
+
+    end
+
+    describe 'without dot in domain' do
+      let(:email_address) { 'zammad@localhost' }
+      let(:email_address_validation) { described_class.new(email_address) }
+
+      it 'reports given email address' do
+        expect(email_address_validation.email_address).to eq(email_address)
+        expect(email_address_validation.to_s).to eq(email_address)
+      end
+
+      it 'reports email address as valid' do
+        expect(email_address_validation.valid_format?).to be(true)
+      end
+
+      it 'reports email address to have domain without MX record' do
+        expect(email_address_validation.valid_mx?).to be(false)
+      end
+    end
+
+  end
+
+  describe 'Email address' do
+
+    describe 'without domain' do
+      let(:email_address) { 'zammad' }
+      let(:email_address_validation) { described_class.new(email_address) }
+
+      it 'reports given email address' do
+        expect(email_address_validation.email_address).to eq(email_address)
+        expect(email_address_validation.to_s).to eq(email_address)
+      end
+
+      it 'reports email address as invalid' do
+        expect(email_address_validation.valid_format?).to be(false)
+      end
+
+      it 'reports email address to have domain without MX record' do
+        expect(email_address_validation.valid_mx?).to be(false)
+      end
+    end
+
+    describe 'with invalid domain format' do
+      let(:email_address) { 'support@zammad..com' }
+      let(:email_address_validation) { described_class.new(email_address) }
+
+      it 'reports given email address' do
+        expect(email_address_validation.email_address).to eq(email_address)
+        expect(email_address_validation.to_s).to eq(email_address)
+      end
+
+      it 'reports email address as invalid' do
+        expect(email_address_validation.valid_format?).to be(false)
+      end
+
+      it 'reports email address to have domain without MX record' do
+        expect(email_address_validation.valid_mx?).to be(false)
+      end
+    end
+
+    describe 'which is empty' do
+      let(:email_address) { '' }
+      let(:email_address_validation) { described_class.new(email_address) }
+
+      it 'reports given email address' do
+        expect(email_address_validation.email_address).to eq(email_address)
+        expect(email_address_validation.to_s).to eq(email_address)
+      end
+
+      it 'reports email address as invalid' do
+        expect(email_address_validation.valid_format?).to be(false)
+      end
+
+      it 'reports email address to have domain without MX record' do
+        expect(email_address_validation.valid_mx?).to be(false)
+      end
+    end
+
+  end
+
+end

test/unit/email_process_test.rb

@@ -95,14 +95,14 @@ Some Textäöü",
             {
               firstname: '',
               lastname: '',
-              fullname: 'me@exampl\'e.com',
+              fullname: 'me@example.com',
-              email: 'me@exampl\'e.com',
+              email: 'me@example.com',
             },
             {
               firstname: '',
               lastname: '',
-              fullname: 'customer@exampl\'e.com',
+              fullname: 'customer@example.com',
-              email: 'customer@exampl\'e.com',
+              email: 'customer@example.com',
             },
           ],
         },
@@ -2777,7 +2777,7 @@ Some Text',
               firstname: 'Clement.Si',
               lastname: '',
               fullname: 'Clement.Si',
-              email: 'claudia.shu@yahoo.com.',
+              email: 'claudia.shu@yahoo.com',
             },
             {
               firstname: '',
@@ -3266,7 +3266,7 @@ To: customer@example.com
 Subject: some subject
 X-Zammad-Ticket-Followup-State: closed
 X-Zammad-Ticket-priority: 3 high
-X-Zammad-Ticket-owner: agent1@example.com 
+X-Zammad-Ticket-owner: agent1@example.com
 X-Zammad-Article-sender: System
 x-Zammad-Article-type: phone
 x-Zammad-Article-Internal: true
@@ -3296,7 +3296,7 @@ To: customer@example.com
 Subject: some subject
 X-Zammad-Ticket-Followup-State: closed
 X-Zammad-Ticket-priority_id: 777777
-X-Zammad-Ticket-owner: not_existing@example.com 
+X-Zammad-Ticket-owner: not_existing@example.com
 X-Zammad-Article-sender_id: 999999
 x-Zammad-Article-type: phone
 x-Zammad-Article-Internal: true
@@ -3324,7 +3324,7 @@ Some Text',
         data: 'From: me@example.com
 To: customer@example.com
 Subject: some subject / with customer as agent - customer can not be owner
-X-Zammad-Ticket-owner: customer1@example.com 
+X-Zammad-Ticket-owner: customer1@example.com
 
 Some Text',
         channel: {