Added postmaster filter validation. Change behaviour of using regular expressions - use explizit regular expressions by prefixing with "regex:your_regexp".

app/assets/javascripts/app/controllers/_channel/email.coffee

@@ -108,7 +108,7 @@ class App.ChannelEmailFilterEdit extends App.ControllerModal
     # show errors in form
     if errors
       @log 'error', errors
-      @formValidate( form: e.target, errors: errors )
+      @formValidate(form: e.target, errors: errors)
       return false
 
     # disable form
@@ -118,8 +118,10 @@ class App.ChannelEmailFilterEdit extends App.ControllerModal
     object.save(
       done: =>
         @close()
-      fail: =>
-        @close()
+      fail: (settings, details) =>
+        @log 'errors', details
+        @formEnable(e)
+        @form.showAlert(details.error_human || details.error || 'Unable to create object!')
     )
 
 class App.ChannelEmailSignature extends App.Controller
@@ -201,7 +203,7 @@ class App.ChannelEmailSignatureEdit extends App.ControllerModal
     # show errors in form
     if errors
       @log 'error', errors
-      @formValidate( form: e.target, errors: errors )
+      @formValidate(form: e.target, errors: errors)
       return false
 
     # disable form
@@ -211,8 +213,10 @@ class App.ChannelEmailSignatureEdit extends App.ControllerModal
     object.save(
       done: =>
         @close()
-      fail: =>
+      fail: (settings, details) =>
+        @log 'errors', details
         @formEnable(e)
+        @form.showAlert(details.error_human || details.error || 'Unable to create object!')
     )
 
 class App.ChannelEmailAccountOverview extends App.Controller

app/assets/javascripts/app/models/postmaster_filter.coffee

@@ -6,7 +6,7 @@ class App.PostmasterFilter extends App.Model
   @configure_attributes = [
     { name: 'name',           display: 'Name',              tag: 'input', type: 'text', limit: 250, 'null': false },
     { name: 'channel',        display: 'Channel',           type: 'input', readonly: 1 },
-    { name: 'match',          display: 'Match all of the following',      tag: 'postmaster_match' },
+    { name: 'match',          display: 'Match all of the following',      tag: 'postmaster_match', note: 'You can use regular expression by using "regex:your_reg_exp".' },
     { name: 'perform',        display: 'Perform action of the following', tag: 'postmaster_set' },
     { name: 'note',           display: 'Note',              tag: 'textarea', limit: 250, null: true },
     { name: 'updated_at',     display: 'Updated',           tag: 'datetime', readonly: 1 },

app/models/channel/filter/database.rb

@@ -14,26 +14,27 @@ module Channel::Filter::Database
       filter[:match].each { |key, meta|
         begin
           next if meta.blank? || meta['value'].blank?
+          value = mail[ key.downcase.to_sym ]
+          match_rule = meta['value']
           min_one_rule_exists = true
-          has_matched = false
-          if mail[ key.downcase.to_sym ].present? && mail[ key.downcase.to_sym ] =~ /#{meta['value']}/i
-            has_matched = true
-          end
-          if has_matched
-            if meta[:operator] == 'contains not'
+          if meta[:operator] == 'contains not'
+            if value.present? && match(value, match_rule, false)
               all_matches_ok = false
+              Rails.logger.info "  matching #{key.downcase}:'#{value}' on #{match_rule}, but shoud not"
             end
-            Rails.logger.info "  matching #{key.downcase}:'#{mail[ key.downcase.to_sym ]}' on #{meta['value']}"
-          else
-            if meta[:operator] == 'contains'
+          elsif meta[:operator] == 'contains'
+            if value.blank? || !match(value, match_rule, true)
               all_matches_ok = false
+              Rails.logger.info "  not matching #{key.downcase}:'#{value}' on #{match_rule}, but should"
             end
-            Rails.logger.info "  not matching #{key.downcase}:'#{mail[ key.downcase.to_sym ]}' on #{meta['value']}"
+          else
+            all_matches_ok = false
+            Rails.logger.info "  Invalid operator in match #{meta.inspect}"
           end
           break if !all_matches_ok
         rescue => e
           all_matches_ok = false
-          Rails.logger.error "can't use match rule #{meta['value']} on #{mail[ key.to_sym ]}"
+          Rails.logger.error "can't use match rule #{match_rule} on #{value}"
           Rails.logger.error e.inspect
         end
       }
@@ -49,4 +50,31 @@ module Channel::Filter::Database
     }
 
   end
+
+  def self.match(value, match_rule, _should_match, check_mode = false)
+
+    regexp = false
+    if match_rule =~ /^(regex:)(.+?)$/
+      regexp = true
+      match_rule = $2
+    end
+
+    if regexp == false
+      match_rule_quoted = Regexp.quote(match_rule).gsub(/\\\*/, '.*')
+      return true if value =~ /#{match_rule_quoted}/i
+      return false
+    end
+
+    begin
+      return true if value =~ /#{match_rule}/i
+      return false
+    rescue => e
+      message = "Can't use regex '#{match_rule}' on '#{value}': #{e.message}"
+      Rails.logger.error message
+      raise message if check_mode == true
+    end
+
+    false
+  end
+
 end

app/models/postmaster_filter.rb

@@ -4,4 +4,26 @@ class PostmasterFilter < ApplicationModel
   store     :perform
   store     :match
   validates :name, presence: true
+
+  before_create :validate_condition
+  before_update :validate_condition
+
+  def validate_condition
+    raise Exceptions::UnprocessableEntity, 'Min. one match rule needed!' if match.blank?
+    match.each { |_key, meta|
+      raise Exceptions::UnprocessableEntity, 'operator invalid, ony "contains" and "contains not" is supported' if meta['operator'].blank? || meta['operator'] !~ /^(contains|contains not)$/
+      raise Exceptions::UnprocessableEntity, 'value invalid/empty' if meta['value'].blank?
+      begin
+        if meta['operator'] == 'contains not'
+          Channel::Filter::Database.match('test content', meta['value'], false, true)
+        else
+          Channel::Filter::Database.match('test content', meta['value'], true, true)
+        end
+      rescue => e
+        raise Exceptions::UnprocessableEntity, e.message
+      end
+    }
+    true
+  end
+
 end

test/unit/email_postmaster_test.rb

@@ -3,7 +3,141 @@
 require 'test_helper'
 
 class EmailPostmasterTest < ActiveSupport::TestCase
-  test 'process with postmaster filter' do
+  test 'valid/invalid postmaster filter' do
+    PostmasterFilter.create!(
+      name: 'not used',
+      match: {
+        from: {
+          operator: 'contains',
+          value: 'nobody@example.com',
+        },
+      },
+      perform: {
+        'X-Zammad-Ticket-priority' => {
+          value: '3 high',
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+    assert_raises(Exceptions::UnprocessableEntity) {
+      PostmasterFilter.create!(
+        name: 'empty filter should not work',
+        match: {},
+        perform: {
+          'X-Zammad-Ticket-priority' => {
+            value: '3 high',
+          },
+        },
+        channel: 'email',
+        active: true,
+        created_by_id: 1,
+        updated_by_id: 1,
+      )
+    }
+    assert_raises(Exceptions::UnprocessableEntity) {
+      PostmasterFilter.create!(
+        name: 'empty filter should not work',
+        match: {
+          from: {
+            operator: 'contains',
+            value: '',
+          },
+        },
+        perform: {
+          'X-Zammad-Ticket-priority' => {
+            value: '3 high',
+          },
+        },
+        channel: 'email',
+        active: true,
+        created_by_id: 1,
+        updated_by_id: 1,
+      )
+    }
+    assert_raises(Exceptions::UnprocessableEntity) {
+      PostmasterFilter.create!(
+        name: 'invalid regex',
+        match: {
+          from: {
+            operator: 'contains',
+            value: 'regex:[]',
+          },
+        },
+        perform: {
+          'X-Zammad-Ticket-priority' => {
+            value: '3 high',
+          },
+        },
+        channel: 'email',
+        active: true,
+        created_by_id: 1,
+        updated_by_id: 1,
+      )
+    }
+    assert_raises(Exceptions::UnprocessableEntity) {
+      PostmasterFilter.create!(
+        name: 'invalid regex',
+        match: {
+          from: {
+            operator: 'contains',
+            value: 'regex:??',
+          },
+        },
+        perform: {
+          'X-Zammad-Ticket-priority' => {
+            value: '3 high',
+          },
+        },
+        channel: 'email',
+        active: true,
+        created_by_id: 1,
+        updated_by_id: 1,
+      )
+    }
+    assert_raises(Exceptions::UnprocessableEntity) {
+      PostmasterFilter.create!(
+        name: 'invalid regex',
+        match: {
+          from: {
+            operator: 'contains',
+            value: 'regex:*',
+          },
+        },
+        perform: {
+          'X-Zammad-Ticket-priority' => {
+            value: '3 high',
+          },
+        },
+        channel: 'email',
+        active: true,
+        created_by_id: 1,
+        updated_by_id: 1,
+      )
+    }
+    PostmasterFilter.create!(
+      name: 'use .*',
+      match: {
+        from: {
+          operator: 'contains',
+          value: '*',
+        },
+      },
+      perform: {
+        'X-Zammad-Ticket-priority' => {
+          value: '3 high',
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+  end
+
+  test 'process with postmaster filter with regex' do
     group_default = Group.lookup(name: 'Users')
     group1 = Group.create_if_not_exists(
       name: 'Test Group1',
@@ -15,18 +149,25 @@ class EmailPostmasterTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+
     PostmasterFilter.destroy_all
     PostmasterFilter.create!(
-      name: 'not used',
+      name: 'used - empty selector',
       match: {
         from: {
           operator: 'contains',
-          value: 'nobody@example.com',
+          value: 'regex:.*',
         },
       },
       perform: {
-        'X-Zammad-Ticket-priority' => {
-          value: '3 high',
+        'X-Zammad-Ticket-group_id' => {
+          value: group2.id,
+        },
+        'X-Zammad-Ticket-priority_id' => {
+          value: '1',
+        },
+        'x-Zammad-Article-Internal' => {
+          value: true,
         },
       },
       channel: 'email',
@@ -35,17 +176,39 @@ class EmailPostmasterTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
 
+    data = 'From: Some Body <somebody@example.com>
+To: Bob <bod@example.com>
+Cc: any@example.com
+Subject: some subject - no selector
+
+Some Text'
+
+    parser = Channel::EmailParser.new
+    ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
+
+    assert_equal('Test Group2', ticket.group.name)
+    assert_equal('1 low', ticket.priority.name)
+    assert_equal('some subject - no selector', ticket.title)
+
+    assert_equal('Customer', article.sender.name)
+    assert_equal('email', article.type.name)
+    assert_equal(true, article.internal)
+
+    PostmasterFilter.destroy_all
     PostmasterFilter.create!(
-      name: 'used',
+      name: 'used - empty selector',
       match: {
         from: {
           operator: 'contains',
-          value: 'me@example.com',
+          value: '*',
         },
       },
       perform: {
         'X-Zammad-Ticket-group_id' => {
-          value: group1.id,
+          value: group2.id,
+        },
+        'X-Zammad-Ticket-priority_id' => {
+          value: '1',
         },
         'x-Zammad-Article-Internal' => {
           value: true,
@@ -57,18 +220,40 @@ class EmailPostmasterTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
 
+    data = 'From: Some Body <somebody@example.com>
+To: Bob <bod@example.com>
+Cc: any@example.com
+Subject: some subject - no selector
+
+Some Text'
+
+    parser = Channel::EmailParser.new
+    ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
+
+    assert_equal('Test Group2', ticket.group.name)
+    assert_equal('1 low', ticket.priority.name)
+    assert_equal('some subject - no selector', ticket.title)
+
+    assert_equal('Customer', article.sender.name)
+    assert_equal('email', article.type.name)
+    assert_equal(true, article.internal)
+
+    PostmasterFilter.destroy_all
     PostmasterFilter.create!(
-      name: 'used x-any-recipient',
+      name: 'used - empty selector',
       match: {
-        'x-any-recipient' => {
+        subject: {
           operator: 'contains',
-          value: 'any@example.com',
+          value: '*me*',
         },
       },
       perform: {
         'X-Zammad-Ticket-group_id' => {
           value: group2.id,
         },
+        'X-Zammad-Ticket-priority_id' => {
+          value: '1',
+        },
         'x-Zammad-Article-Internal' => {
           value: true,
         },
@@ -79,27 +264,54 @@ class EmailPostmasterTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
 
-
-    data = 'From: me@example.com
-To: customer@example.com
-Subject: some subject
+    data = 'From: Some Body <somebody@example.com>
+To: Bob <bod@example.com>
+Cc: any@example.com
+Subject: *me*
 
 Some Text'
 
     parser = Channel::EmailParser.new
     ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
-    assert_equal('Test Group1', ticket.group.name)
-    assert_equal('2 normal', ticket.priority.name)
-    assert_equal('some subject', ticket.title)
+
+    assert_equal('Test Group2', ticket.group.name)
+    assert_equal('1 low', ticket.priority.name)
+    assert_equal('*me*', ticket.title)
 
     assert_equal('Customer', article.sender.name)
     assert_equal('email', article.type.name)
     assert_equal(true, article.internal)
 
+    PostmasterFilter.destroy_all
+    PostmasterFilter.create!(
+      name: 'used - empty selector',
+      match: {
+        subject: {
+          operator: 'contains not',
+          value: '*me*',
+        },
+      },
+      perform: {
+        'X-Zammad-Ticket-group_id' => {
+          value: group2.id,
+        },
+        'X-Zammad-Ticket-priority_id' => {
+          value: '1',
+        },
+        'x-Zammad-Article-Internal' => {
+          value: true,
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
     data = 'From: Some Body <somebody@example.com>
 To: Bob <bod@example.com>
 Cc: any@example.com
-Subject: some subject
+Subject: *mo*
 
 Some Text'
 
@@ -107,19 +319,20 @@ Some Text'
     ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
 
     assert_equal('Test Group2', ticket.group.name)
-    assert_equal('2 normal', ticket.priority.name)
-    assert_equal('some subject', ticket.title)
+    assert_equal('1 low', ticket.priority.name)
+    assert_equal('*mo*', ticket.title)
 
     assert_equal('Customer', article.sender.name)
     assert_equal('email', article.type.name)
     assert_equal(true, article.internal)
 
+    PostmasterFilter.destroy_all
     PostmasterFilter.create!(
-      name: 'used x-any-recipient 2',
+      name: 'used - empty selector',
       match: {
-        'x-any-recipient' => {
+        subject: {
           operator: 'contains not',
-          value: 'any_not@example.com',
+          value: '*me*',
         },
       },
       perform: {
@@ -130,7 +343,7 @@ Some Text'
           value: '1',
         },
         'x-Zammad-Article-Internal' => {
-          value: 'false',
+          value: true,
         },
       },
       channel: 'email',
@@ -142,29 +355,141 @@ Some Text'
     data = 'From: Some Body <somebody@example.com>
 To: Bob <bod@example.com>
 Cc: any@example.com
-Subject: some subject2
+Subject: *me*
 
 Some Text'
 
     parser = Channel::EmailParser.new
     ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
 
-    assert_equal('Test Group2', ticket.group.name)
-    assert_equal('1 low', ticket.priority.name)
-    assert_equal('some subject2', ticket.title)
+    assert_equal('Users', ticket.group.name)
+    assert_equal('2 normal', ticket.priority.name)
+    assert_equal('*me*', ticket.title)
 
     assert_equal('Customer', article.sender.name)
     assert_equal('email', article.type.name)
     assert_equal(false, article.internal)
 
     PostmasterFilter.destroy_all
+  end
 
+  test 'process with postmaster filter' do
+    group_default = Group.lookup(name: 'Users')
+    group1 = Group.create_if_not_exists(
+      name: 'Test Group1',
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+    group2 = Group.create_if_not_exists(
+      name: 'Test Group2',
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+    PostmasterFilter.destroy_all
     PostmasterFilter.create!(
-      name: 'used - empty selector',
+      name: 'not used',
       match: {
         from: {
           operator: 'contains',
-          value: '',
+          value: 'nobody@example.com',
+        },
+      },
+      perform: {
+        'X-Zammad-Ticket-priority' => {
+          value: '3 high',
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    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,
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    PostmasterFilter.create!(
+      name: 'used x-any-recipient',
+      match: {
+        'x-any-recipient' => {
+          operator: 'contains',
+          value: 'any@example.com',
+        },
+      },
+      perform: {
+        'X-Zammad-Ticket-group_id' => {
+          value: group2.id,
+        },
+        'x-Zammad-Article-Internal' => {
+          value: true,
+        },
+      },
+      channel: 'email',
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+
+    data = 'From: 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('Test Group1', ticket.group.name)
+    assert_equal('2 normal', ticket.priority.name)
+    assert_equal('some subject', ticket.title)
+
+    assert_equal('Customer', article.sender.name)
+    assert_equal('email', article.type.name)
+    assert_equal(true, article.internal)
+
+    data = 'From: Some Body <somebody@example.com>
+To: Bob <bod@example.com>
+Cc: any@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('Test Group2', ticket.group.name)
+    assert_equal('2 normal', ticket.priority.name)
+    assert_equal('some subject', ticket.title)
+
+    assert_equal('Customer', article.sender.name)
+    assert_equal('email', article.type.name)
+    assert_equal(true, article.internal)
+
+    PostmasterFilter.create!(
+      name: 'used x-any-recipient 2',
+      match: {
+        'x-any-recipient' => {
+          operator: 'contains not',
+          value: 'any_not@example.com',
         },
       },
       perform: {
@@ -175,7 +500,7 @@ Some Text'
           value: '1',
         },
         'x-Zammad-Article-Internal' => {
-          value: true,
+          value: 'false',
         },
       },
       channel: 'email',
@@ -187,16 +512,16 @@ Some Text'
     data = 'From: Some Body <somebody@example.com>
 To: Bob <bod@example.com>
 Cc: any@example.com
-Subject: some subject - no selector
+Subject: some subject2
 
 Some Text'
 
     parser = Channel::EmailParser.new
     ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data)
 
-    assert_equal('Users', ticket.group.name)
-    assert_equal('2 normal', ticket.priority.name)
-    assert_equal('some subject - no selector', ticket.title)
+    assert_equal('Test Group2', ticket.group.name)
+    assert_equal('1 low', ticket.priority.name)
+    assert_equal('some subject2', ticket.title)
 
     assert_equal('Customer', article.sender.name)
     assert_equal('email', article.type.name)
@@ -477,6 +802,110 @@ Some Text'
 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('Users', 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)
+
+    assert_equal('Customer', article.sender.name)
+    assert_equal('email', article.type.name)
+    assert_equal(false, article.internal)
+
+    PostmasterFilter.destroy_all
+    PostmasterFilter.create!(
+      name: 'Autoresponder',
+      match: {
+        'auto-submitted' => {
+          'operator' => 'contains not',
+          'value' => 'auto-generated',
+        },
+        'to' => {
+          'operator' => 'contains',
+          'value' => 'customer@example.com',
+        },
+        'from' => {
+          'operator' => 'contains',
+          'value' => '@example.com',
+        }
+      },
+      perform: {
+        'x-zammad-article-internal' => {
+          'value' => 'true',
+        },
+        'x-zammad-article-type_id' => {
+          'value' => Ticket::Article::Type.find_by(name: 'note').id.to_s,
+        },
+        'x-zammad-ignore' => {
+          'value' => 'false',
+        },
+      },
+      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('Users', 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)
+
+    assert_equal('Customer', article.sender.name)
+    assert_equal('note', article.type.name)
+    assert_equal(true, article.internal)
+
+    PostmasterFilter.destroy_all
+    PostmasterFilter.create!(
+      name: 'Autoresponder',
+      match: {
+        'auto-submitted' => {
+          'operator' => 'contains',
+          'value' => 'auto-generated',
+        },
+        'to' => {
+          'operator' => 'contains',
+          'value' => 'customer1@example.com',
+        },
+        'from' => {
+          'operator' => 'contains',
+          'value' => '@example.com',
+        }
+      },
+      perform: {
+        'x-zammad-article-internal' => {
+          'value' => 'true',
+        },
+        'x-zammad-article-type_id' => {
+          'value' => Ticket::Article::Type.find_by(name: 'note').id.to_s,
+        },
+        'x-zammad-ignore' => {
+          'value' => 'false',
+        },
+      },
+      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