Fixes #5308 - Ticket creation with Send Email turns into Net::SMTPSyntaxError if customer has no email address.

Co-authored-by: Dominik Klein <dk@zammad.com>
Co-authored-by: Dusan Vuckovic <dv@zammad.com>
Co-authored-by: Florian Liebe <fl@zammad.com>

app/controllers/application_controller/handles_errors.rb

@@ -16,6 +16,7 @@ module ApplicationController::HandlesErrors
     rescue_from Exceptions::Forbidden, with: :forbidden
     rescue_from Pundit::NotAuthorizedError, with: :pundit_not_authorized_error
     rescue_from Store::Provider::S3::Error, with: :unprocessable_entity
+    rescue_from Exceptions::MissingAttribute, Exceptions::InvalidAttribute, with: :unprocessable_entity
   end
 
   def not_found(e)
@@ -104,7 +105,7 @@ module ApplicationController::HandlesErrors
     elsif e.message == 'Exceptions::NotAuthorized'
       data[:error]       = __('Authorization failed')
       data[:error_human] = data[:error]
-    elsif [ActionController::RoutingError, ActiveRecord::RecordNotFound, Exceptions::UnprocessableEntity, Exceptions::NotAuthorized, Exceptions::Forbidden, Store::Provider::S3::Error, Authorization::Provider::AccountError].include?(e.class)
+    elsif [ActionController::RoutingError, ActiveRecord::RecordNotFound, Exceptions::UnprocessableEntity, Exceptions::NotAuthorized, Exceptions::Forbidden, Store::Provider::S3::Error, Authorization::Provider::AccountError, Exceptions::MissingAttribute, Exceptions::InvalidAttribute].include?(e.class)
       data[:error_human] = data[:error]
     end
 

app/controllers/concerns/creates_ticket_articles.rb

@@ -54,6 +54,7 @@ module CreatesTicketArticles # rubocop:disable Metrics/ModuleLength
     article = Ticket::Article.new(clean_params)
     article.ticket_id = ticket.id
     article.check_mentions_raises_error = true
+    article.check_email_recipient_raises_error = true
 
     # store dataurl images to store
     attachments_inline = []

app/frontend/apps/desktop/form/theme/global/getCoreDesktopClasses.ts

@@ -82,6 +82,8 @@ export const getCoreDesktopClasses: FormThemeExtension = (
     treeselect: selectInputClasses(classes.treeselect),
     autocomplete: selectInputClasses(classes.autocomplete),
     agent: selectInputClasses(classes.agent),
+    customer: selectInputClasses(classes.customer),
+    recipient: selectInputClasses(classes.recipient),
     toggle: extendClasses(classes.toggle, {
       wrapper: 'flex h-10 flex-row-reverse items-center gap-1.5',
       label: '!mb-0 grow',

app/graphql/gql/mutations/ticket/create.rb

@@ -22,6 +22,10 @@ module Gql::Mutations
           .new(current_user: context.current_user)
           .execute(ticket_data: input)
       }
+    rescue Exceptions::InvalidAttribute => e
+      field = e.attribute == 'email_recipient' ? 'customer_id' : e.attribute
+
+      error_response({ field:, message: e.message })
     end
   end
 end

app/graphql/gql/mutations/ticket/update.rb

@@ -24,6 +24,10 @@ module Gql::Mutations
           .new(current_user: context.current_user)
           .execute(ticket: ticket, ticket_data: input, skip_validators: meta&.dig(:skip_validators), macro: meta&.dig(:macro))
       }
+    rescue Exceptions::InvalidAttribute => e
+      field = e.attribute == 'email_recipient' ? 'article.to' : e.attribute
+
+      error_response({ field:, message: e.message })
     rescue => e
       raise e if !e.class.name.starts_with?('Service::Ticket::Update::Validator')
 

app/models/ticket/article.rb

@@ -38,6 +38,7 @@ class Ticket::Article < ApplicationModel
   belongs_to :origin_by,  class_name: 'User', optional: true
 
   before_validation :check_mentions, on: :create
+  before_validation :check_email_recipient_validity, if: :check_email_recipient_raises_error
   before_create :check_subject, :check_body, :check_message_id_md5
   before_update :check_subject, :check_body, :check_message_id_md5
   after_destroy :store_delete, :update_time_units
@@ -67,7 +68,7 @@ class Ticket::Article < ApplicationModel
                              :to,
                              :cc
 
-  attr_accessor :should_clone_inline_attachments, :check_mentions_raises_error
+  attr_accessor :should_clone_inline_attachments, :check_mentions_raises_error, :check_email_recipient_raises_error
 
   alias should_clone_inline_attachments? should_clone_inline_attachments
 
@@ -384,6 +385,24 @@ returns
     end
   end
 
+  def check_email_recipient_validity
+    return if Setting.get('import_mode')
+
+    # Check if article type is email
+    email_article_type = Ticket::Article::Type.lookup(name: 'email')
+    return if type_id != email_article_type.id
+
+    # ... and if recipient is valid.
+    recipient = begin
+      Mail::Address.new(to).address
+    rescue Mail::Field::FieldError
+      # no-op
+    end
+    return if EmailAddressValidation.new(recipient).valid?
+
+    raise Exceptions::InvalidAttribute.new('email_recipient', __('Sending an email without a valid recipient is not possible.'))
+  end
+
   def history_log_attributes
     {
       related_o_id:           self['ticket_id'],

app/services/service/ticket/article/create.rb

@@ -13,6 +13,7 @@ class Service::Ticket::Article::Create < Service::BaseWithCurrentUser
 
     ticket.articles.new(article_data).tap do |article|
       article.check_mentions_raises_error = true
+      article.check_email_recipient_raises_error = true
 
       transform_article(article, attachments_raw, subtype)
 

i18n/zammad.pot

@@ -1910,7 +1910,7 @@ msgstr ""
 msgid "Author"
 msgstr ""
 
-#: app/controllers/application_controller/handles_errors.rb:105
+#: app/controllers/application_controller/handles_errors.rb:106
 msgid "Authorization failed"
 msgstr ""
 
@@ -9939,8 +9939,8 @@ msgstr ""
 #: app/assets/javascripts/app/controllers/_profile/out_of_office.coffee:158
 #: app/assets/javascripts/app/controllers/ticket_zoom.coffee:115
 #: app/assets/javascripts/app/views/ticket_zoom/sidebar_checklist_show_row.jst.eco:25
-#: app/controllers/application_controller/handles_errors.rb:60
-#: app/services/service/ticket/article/create.rb:121
+#: app/controllers/application_controller/handles_errors.rb:61
+#: app/services/service/ticket/article/create.rb:122
 msgid "Not authorized"
 msgstr ""
 
@@ -12594,6 +12594,10 @@ msgstr ""
 msgid "Sender of last article"
 msgstr ""
 
+#: app/models/ticket/article.rb:305
+msgid "Sending an email without a valid recipient is not possible."
+msgstr ""
+
 #: app/assets/javascripts/app/views/widget/invite_user.jst.eco:35
 msgid "Sending…"
 msgstr ""
@@ -15296,7 +15300,7 @@ msgstr ""
 msgid "This might take some time during which the system can't be used."
 msgstr ""
 
-#: app/controllers/application_controller/handles_errors.rb:98
+#: app/controllers/application_controller/handles_errors.rb:99
 #: app/graphql/gql/zammad_schema.rb:83
 msgid "This object already exists."
 msgstr ""

spec/graphql/gql/mutations/ticket/create_spec.rb

@@ -394,6 +394,7 @@ RSpec.describe Gql::Mutations::Ticket::Create, :aggregate_failures, type: :graph
             {
               body: 'dummy',
               type: Ticket::Article::Type.first.name,
+              to:   'dummy@example.org',
             }
           end
 

spec/graphql/gql/queries/ticket/articles_spec.rb

@@ -113,7 +113,7 @@ RSpec.describe Gql::Queries::Ticket::Articles, type: :graphql do
     let(:customer)             { create(:customer) }
     let(:ticket)               { create(:ticket, customer: customer) }
     let(:cc)                   { 'Zammad CI <ci@zammad.org>' }
-    let(:to)                   { '@unparseable_address' }
+    let(:to)                   { Faker::Internet.unique.email }
     let(:cid)                  { "#{SecureRandom.uuid}@zammad.example.com" }
     let!(:articles) do
       create_list(:ticket_article, 2, :outbound_email, ticket: ticket, to: to, cc: cc, content_type: 'text/html', body: "<img src=\"cid:#{cid}\"> some text") do |article, _i|
@@ -173,7 +173,13 @@ RSpec.describe Gql::Queries::Ticket::Articles, type: :graphql do
               'raw'    => cc,
             },
             'to'                       => {
-              'parsed' => nil,
+              'parsed' => [
+                {
+                  'emailAddress'    => to,
+                  'name'            => nil,
+                  'isSystemAddress' => false,
+                }
+              ],
               'raw'    => to,
             },
             'type'                     => {