Added content validation to x-zammad mail headers and postmaster filters.

app/controllers/ticket_articles_controller.rb

@@ -254,15 +254,11 @@ class TicketArticlesController < ApplicationController
     article = Ticket::Article.find(params[:id])
     article_permission(article)
 
-    list = Store.list(
+    file = article.as_raw
-      object: 'Ticket::Article::Mail',
-      o_id: params[:id],
-    )
 
     # find file
-    return if !list
+    return if !file
 
-    file = Store.find(list.first)
     send_data(
       file.content,
       filename: file.filename,

app/models/application_model.rb

@@ -418,6 +418,42 @@ returns
 
 =begin
 
+reference if association id check
+
+  model = Model.find(123)
+  attributes = model.association_id_check('attribute_id', value)
+
+returns
+
+  true | false
+
+=end
+
+  def association_id_check(attribute_id, value)
+    return true if value.nil?
+
+    attributes.each { |key, _value|
+      next if key != attribute_id
+
+      # check if id is assigned
+      key_short = key[ key.length - 3, key.length ]
+      next if key_short != '_id'
+      key_short = key[ 0, key.length - 3 ]
+
+      self.class.reflect_on_all_associations.map { |assoc|
+        next if assoc.name.to_s != key_short
+        item = assoc.class_name.constantize
+        return false if !item.respond_to?(:find_by)
+        ref_object = item.find_by(id: value)
+        return false if !ref_object
+        return true
+      }
+    }
+    true
+  end
+
+=begin
+
 set created_by_id & updated_by_id if not given based on UserInfo (current session)
 
 Used as before_create callback, no own use needed

app/models/channel/email_parser.rb

@@ -538,13 +538,7 @@ returns
         article.save!
 
         # store mail plain
-        Store.add(
+        article.save_as_raw(msg)
-          object: 'Ticket::Article::Mail',
-          o_id: article.id,
-          data: msg,
-          filename: "ticket-#{ticket.number}-#{article.id}.eml",
-          preferences: {}
-        )
 
         # store attachments
         if mail[:attachments]
@@ -580,6 +574,29 @@ returns
     [ticket, article, session_user, mail]
   end
 
+  def self.check_attributes_by_x_headers(header_name, value)
+    class_name = nil
+    attribute = nil
+    if header_name =~ /^x-zammad-(.+?)-(followup-|)(.*)$/i
+      class_name = $1
+      attribute = $3
+    end
+    return true if !class_name
+    if class_name.downcase == 'article'
+      class_name = 'Ticket::Article'
+    end
+    return true if !attribute
+    key_short = attribute[ attribute.length - 3, attribute.length ]
+    return true if key_short != '_id'
+
+    class_object = Object.const_get(class_name.to_classname)
+    return if !class_object
+    class_instance = class_object.new
+
+    return false if !class_instance.association_id_check(attribute, value)
+    true
+  end
+
   def set_attributes_by_x_headers(item_object, header_name, mail, suffix = false)
 
     # loop all x-zammad-hedaer-* headers
@@ -597,6 +614,8 @@ returns
         if suffix
           header = "x-zammad-#{header_name}-#{suffix}-#{key_short}"
         end
+
+        # only set value on _id if value/reference lookup exists
         if mail[ header.to_sym ]
           Rails.logger.info "header #{header} found #{mail[ header.to_sym ]}"
           item_object.class.reflect_on_all_associations.map { |assoc|
@@ -606,15 +625,29 @@ returns
             Rails.logger.info "ASSOC found #{assoc.class_name} lookup #{mail[ header.to_sym ]}"
             item = assoc.class_name.constantize
 
+            assoc_object = nil
+            assoc_has_object = false
             if item.respond_to?(:name)
+              assoc_has_object = true
               if item.lookup(name: mail[ header.to_sym ])
-                item_object[key] = item.lookup(name: mail[ header.to_sym ]).id
+                assoc_object = item.lookup(name: mail[ header.to_sym ])
               end
             elsif item.respond_to?(:login)
+              assoc_has_object = true
               if item.lookup(login: mail[ header.to_sym ])
-                item_object[key] = item.lookup(login: mail[ header.to_sym ]).id
+                assoc_object = item.lookup(login: mail[ header.to_sym ])
               end
             end
+
+            next if assoc_has_object == false
+
+            if assoc_object
+              item_object[key] = assoc_object.id
+              next
+            end
+
+            # no assoc exists, remove header
+            mail.delete(header.to_sym)
           }
         end
       end

app/models/channel/filter/database.rb

@@ -42,6 +42,7 @@ module Channel::Filter::Database
       next if !all_matches_ok
 
       filter[:perform].each { |key, meta|
+        next if !Channel::EmailParser.check_attributes_by_x_headers(key, meta['value'])
         Rails.logger.info "  perform '#{key.downcase}' = '#{meta.inspect}'"
         mail[ key.downcase.to_sym ] = meta['value']
       }

app/models/channel/filter/identify_sender.rb

@@ -6,7 +6,7 @@ module Channel::Filter::IdentifySender
 
     customer_user_id = mail[ 'x-zammad-ticket-customer_id'.to_sym ]
     customer_user = nil
-    if !customer_user_id.empty?
+    if customer_user_id.present?
       customer_user = User.lookup(id: customer_user_id)
       if customer_user
         Rails.logger.debug "Took customer form x-zammad-ticket-customer_id header '#{customer_user_id}'."
@@ -16,10 +16,10 @@ module Channel::Filter::IdentifySender
     end
 
     # check if sender exists in database
-    if !customer_user && !mail[ 'x-zammad-customer-login'.to_sym ].empty?
+    if !customer_user && mail[ 'x-zammad-customer-login'.to_sym ].present?
       customer_user = User.find_by(login: mail[ 'x-zammad-customer-login'.to_sym ])
     end
-    if !customer_user && !mail[ 'x-zammad-customer-email'.to_sym ].empty?
+    if !customer_user && mail[ 'x-zammad-customer-email'.to_sym ].present?
       customer_user = User.find_by(email: mail[ 'x-zammad-customer-email'.to_sym ])
     end
     if !customer_user
@@ -64,7 +64,7 @@ module Channel::Filter::IdentifySender
     # find session user
     session_user_id = mail[ 'x-zammad-session-user-id'.to_sym ]
     session_user = nil
-    if !session_user_id.empty?
+    if session_user_id.present?
       session_user = User.lookup(id: session_user_id)
       if session_user
         Rails.logger.debug "Took session form x-zammad-session-user-id header '#{session_user_id}'."

app/models/channel/filter/trusted.rb

@@ -11,7 +11,17 @@ module Channel::Filter::Trusted
         next if key !~ /^x-zammad/i
         mail.delete(key)
       }
+      return
     end
 
+    # verify values
+    mail.each { |key, value|
+      next if key !~ /^x-zammad/i
+
+      # no assoc exists, remove header
+      next if Channel::EmailParser.check_attributes_by_x_headers(key, value)
+      mail.delete(key.to_sym)
+    }
+
   end
 end

app/models/observer/ticket/article/communicate_email/background_job.rb

@@ -69,14 +69,7 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob
     record.save!
 
     # store mail plain
-    Store.add(
+    record.save_as_raw(message.to_s)
-      object: 'Ticket::Article::Mail',
-      o_id: record.id,
-      data: message.to_s,
-      filename: "ticket-#{ticket.number}-#{record.id}.eml",
-      preferences: {},
-      created_by_id: record.created_by_id,
-    )
 
     # add history record
     recipient_list = ''

app/models/ticket.rb

@@ -49,6 +49,7 @@ class Ticket < ApplicationModel
       last_contact_at: true,
       last_contact_agent_at: true,
       last_contact_customer_at: true,
+      preferences: true,
     }
   )
 
@@ -414,7 +415,7 @@ get count of tickets and tickets which match on selector
 
 generate condition query to search for tickets based on condition
 
-  query_condition, bind_condition = selector2sql(params[:condition], current_user)
+  query_condition, bind_condition, tables = selector2sql(params[:condition], current_user)
 
 condition example
 
@@ -879,7 +880,8 @@ result
 
     return if !customer_id
 
-    customer = User.find(customer_id)
+    customer = User.find_by(id: customer_id)
+    return if !customer
     return if organization_id == customer.organization_id
 
     self.organization_id = customer.organization_id

app/models/ticket/article.rb

@@ -164,6 +164,54 @@ get body as text with quote sign "> " at the beginning of each line
     body_as_text.word_wrap.message_quote
   end
 
+=begin
+
+get article as raw (e. g. if it's a email, the raw email)
+
+  article = Ticket::Article.find(123)
+  article.as_raw
+
+returns:
+
+  file # Store
+
+=end
+
+  def as_raw
+    list = Store.list(
+      object: 'Ticket::Article::Mail',
+      o_id: id,
+    )
+    return if !list
+    return if list.empty?
+    return if !list[0]
+    list[0]
+  end
+
+=begin
+
+save article as raw (e. g. if it's a email, the raw email)
+
+  article = Ticket::Article.find(123)
+  article.save_as_raw(msg)
+
+returns:
+
+  file # Store
+
+=end
+
+  def save_as_raw(msg)
+    Store.add(
+      object: 'Ticket::Article::Mail',
+      o_id: id,
+      data: msg,
+      filename: "ticket-#{ticket.number}-#{id}.eml",
+      preferences: {},
+      created_by_id: created_by_id,
+    )
+  end
+
   private
 
   # strip not wanted chars

test/unit/email_postmaster_test.rb

@@ -270,6 +270,130 @@ Some Text"
 
     PostmasterFilter.destroy_all
 
+    PostmasterFilter.create(
+      name: 'used',
+      match: {
+        from: {
+          operator: 'contains',
+          value: 'me@example.com',
+        },
+      },
+      perform: {
+        'X-Zammad-Ticket-group_id' => {
+          value: group1.id,
+        },
+        'x-Zammad-Article-Internal' => {
+          value: true,
+        },
+        'x-Zammad-Ticket-customer_id' => {
+          value: '',
+          value_completion: '',
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    data = 'From: ME Bob <me@example.com>
+To: customer@example.com
+Subject: some subject
+
+Some Text'
+
+    parser = Channel::EmailParser.new
+    ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
+    assert_equal(group1.name, ticket.group.name)
+    assert_equal('2 normal', ticket.priority.name)
+    assert_equal('some subject', ticket.title)
+    assert_equal('me@example.com', ticket.customer.email)
+
+    PostmasterFilter.destroy_all
+    PostmasterFilter.create(
+      name: 'used',
+      match: {
+        from: {
+          operator: 'contains',
+          value: 'me@example.com',
+        },
+      },
+      perform: {
+        'X-Zammad-Ticket-group_id' => {
+          value: group1.id,
+        },
+        'x-Zammad-Article-Internal' => {
+          value: true,
+        },
+        'x-Zammad-Ticket-customer_id' => {
+          value: 999_999,
+          value_completion: 'xxx',
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    data = 'From: ME Bob <me@example.com>
+To: customer@example.com
+Subject: some subject
+
+Some Text'
+
+    parser = Channel::EmailParser.new
+    ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
+    assert_equal(group1.name, ticket.group.name)
+    assert_equal('2 normal', ticket.priority.name)
+    assert_equal('some subject', ticket.title)
+    assert_equal('me@example.com', ticket.customer.email)
+
+    PostmasterFilter.destroy_all
+    PostmasterFilter.create(
+      name: 'used',
+      match: {
+        from: {
+          operator: 'contains',
+          value: 'me@example.com',
+        },
+      },
+      perform: {
+        'X-Zammad-Ticket-group_id' => {
+          value: group1.id,
+        },
+        'X-Zammad-Ticket-priority_id' => {
+          value: 888_888,
+        },
+        'x-Zammad-Article-Internal' => {
+          value: true,
+        },
+        'x-Zammad-Ticket-customer_id' => {
+          value: 999_999,
+          value_completion: 'xxx',
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    data = 'From: ME Bob <me@example.com>
+To: customer@example.com
+Subject: some subject
+
+Some Text'
+
+    parser = Channel::EmailParser.new
+    ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
+    assert_equal(group1.name, ticket.group.name)
+    assert_equal('2 normal', ticket.priority.name)
+    assert_equal('some subject', ticket.title)
+    assert_equal('me@example.com', ticket.customer.email)
+    assert_equal('2 normal', ticket.priority.name)
+
+    PostmasterFilter.destroy_all
   end
 
 end

test/unit/email_process_test.rb

@@ -5,7 +5,7 @@ require 'test_helper'
 class EmailProcessTest < ActiveSupport::TestCase
   test 'process simple' do
     Ticket.destroy_all
-    
+
     files = [
       {
         data: 'From: me@example.com
@@ -2018,8 +2018,9 @@ Some Text',
         success: false,
       },
     ]
-    process(files)
+    assert_process(files)
   end
+
   test 'process trusted' do
     files = [
       {
@@ -2062,8 +2063,36 @@ Some Text',
           },
         },
       },
+      {
+        data: 'From: me@example.com
+To: customer@example.com
+Subject: some subject
+X-Zammad-Ticket-Followup-State: closed
+X-Zammad-Ticket-priority_id: 777777
+X-Zammad-Article-sender_id: 999999
+x-Zammad-Article-type: phone
+x-Zammad-Article-Internal: true
+
+Some Text',
+        channel: {
+          trusted: true,
+        },
+        success: true,
+        result: {
+          0 => {
+            state: 'new',
+            priority: '2 normal',
+            title: 'some subject',
+          },
+          1 => {
+            sender: 'Customer',
+            type: 'phone',
+            internal: true,
+          },
+        },
+      },
     ]
-    process(files)
+    assert_process(files)
   end
 
   test 'process not trusted' do
@@ -2097,7 +2126,7 @@ Some Text',
         },
       },
     ]
-    process(files)
+    assert_process(files)
   end
 
   test 'process inactive group - a' do
@@ -2133,7 +2162,7 @@ Some Text',
         },
       },
     ]
-    process(files)
+    assert_process(files)
   end
 
   test 'process inactive group - b' do
@@ -2167,7 +2196,7 @@ Some Text',
         },
       },
     ]
-    process(files)
+    assert_process(files)
 
     Group.all.each {|group|
       next if !group_active_map.key?(group.id)
@@ -2176,7 +2205,7 @@ Some Text',
     }
   end
 
-  def process(files)
+  def assert_process(files)
     files.each { |file|
       result = Channel::EmailParser.new.process(file[:channel]||{}, file[:data], false)
       if file[:success]
@@ -2215,7 +2244,7 @@ Some Text',
             end
           end
         else
-          assert(false, 'ticket not created', file)
+          assert(false, 'ticket not created')
         end
       elsif !file[:success]
         if result && result.class == Array && result[1]