From 14711a682645a23506d9f8dbe0cca8392cb5da10 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 22 Aug 2017 14:55:36 +0200 Subject: [PATCH] Fixed issue #1351 - handling of incoming mails seems strange / customer is not set correctly. --- app/models/channel/filter/identify_sender.rb | 2 +- .../filter/sender_is_system_address.rb | 20 +++---- app/models/email_address.rb | 10 ++++ app/models/ticket/overviews.rb | 4 +- ...822000001_agend_based_sender_issue_1351.rb | 14 +++++ ..._sender_is_system_address_or_agent_test.rb | 53 ++++++++++++++++++- 6 files changed, 88 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20170822000001_agend_based_sender_issue_1351.rb diff --git a/app/models/channel/filter/identify_sender.rb b/app/models/channel/filter/identify_sender.rb index a5833fcf9..e408ab10e 100644 --- a/app/models/channel/filter/identify_sender.rb +++ b/app/models/channel/filter/identify_sender.rb @@ -35,7 +35,7 @@ module Channel::Filter::IdentifySender items.each { |item| # skip if recipient is system email - next if EmailAddress.find_by(email: item.address) + next if EmailAddress.find_by(email: item.address.downcase) customer_user = user_create( login: item.address, diff --git a/app/models/channel/filter/sender_is_system_address.rb b/app/models/channel/filter/sender_is_system_address.rb index 1321edd3f..8e58fe51a 100644 --- a/app/models/channel/filter/sender_is_system_address.rb +++ b/app/models/channel/filter/sender_is_system_address.rb @@ -5,13 +5,13 @@ module Channel::Filter::SenderIsSystemAddress def self.run(_channel, mail) # if attributes already set by header - return if mail[ 'x-zammad-ticket-create-article-sender'.to_sym ] - return if mail[ 'x-zammad-article-sender'.to_sym ] + return if mail['x-zammad-ticket-create-article-sender'.to_sym] + return if mail['x-zammad-article-sender'.to_sym] # check if sender address is system form = 'raw-from'.to_sym - return if !mail[form] - return if !mail[:to] + return if mail[form].blank? + return if mail[:to].blank? # in case, set sender begin @@ -19,8 +19,8 @@ module Channel::Filter::SenderIsSystemAddress items = mail[form].addrs items.each { |item| next if !EmailAddress.find_by(email: item.address.downcase) - mail[ 'x-zammad-ticket-create-article-sender'.to_sym ] = 'Agent' - mail[ 'x-zammad-article-sender'.to_sym ] = 'Agent' + mail['x-zammad-ticket-create-article-sender'.to_sym] = 'Agent' + mail['x-zammad-article-sender'.to_sym] = 'Agent' return true } rescue => e @@ -28,13 +28,13 @@ module Channel::Filter::SenderIsSystemAddress end # check if sender is agent - return if mail[:from_email].empty? + return if mail[:from_email].blank? begin - user = User.find_by(email: mail[:from_email]) + user = User.find_by(email: mail[:from_email].downcase) return if !user return if !user.permissions?('ticket.agent') - mail[ 'x-zammad-ticket-create-article-sender'.to_sym ] = 'Agent' - mail[ 'x-zammad-article-sender'.to_sym ] = 'Agent' + mail['x-zammad-ticket-create-article-sender'.to_sym] = 'Agent' + mail['x-zammad-article-sender'.to_sym] = 'Agent' return true rescue => e Rails.logger.error 'ERROR: SenderIsSystemAddress: ' + e.inspect diff --git a/app/models/email_address.rb b/app/models/email_address.rb index 6c36238e9..3c9985d30 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -8,6 +8,7 @@ class EmailAddress < ApplicationModel validates :realname, presence: true validates :email, presence: true + before_validation :check_email before_create :check_if_channel_exists_set_inactive before_update :check_if_channel_exists_set_inactive after_create :update_email_address_id @@ -41,6 +42,15 @@ check and if channel not exists reset configured channels for email addresses private + def check_email + return true if Setting.get('import_mode') + return true if email.blank? + self.email = email.downcase.strip + raise Exceptions::UnprocessableEntity, 'Invalid email' if email !~ /@/ + raise Exceptions::UnprocessableEntity, 'Invalid email' if email =~ /\s/ + true + end + # set email address to inactive/active if channel exists or not def check_if_channel_exists_set_inactive diff --git a/app/models/ticket/overviews.rb b/app/models/ticket/overviews.rb index bea92fced..644039a64 100644 --- a/app/models/ticket/overviews.rb +++ b/app/models/ticket/overviews.rb @@ -5,9 +5,7 @@ module Ticket::Overviews all overviews by user - result = Ticket::Overviews.all( - current_user: User.find(123), - ) + result = Ticket::Overviews.all(current_user: User.find(123)) returns diff --git a/db/migrate/20170822000001_agend_based_sender_issue_1351.rb b/db/migrate/20170822000001_agend_based_sender_issue_1351.rb new file mode 100644 index 000000000..8095b238e --- /dev/null +++ b/db/migrate/20170822000001_agend_based_sender_issue_1351.rb @@ -0,0 +1,14 @@ +class AgendBasedSenderIssue1351 < ActiveRecord::Migration + def up + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + EmailAddress.all.each { |email_address| + begin + email_address.save! + rescue => e + Rails.logger.error "Unable to update EmailAddress.find(#{email_address.id}) '#{email_address.inspect}': #{e.message}" + end + } + end +end diff --git a/test/unit/email_process_sender_is_system_address_or_agent_test.rb b/test/unit/email_process_sender_is_system_address_or_agent_test.rb index f4a62daba..47c0f62e4 100644 --- a/test/unit/email_process_sender_is_system_address_or_agent_test.rb +++ b/test/unit/email_process_sender_is_system_address_or_agent_test.rb @@ -7,7 +7,7 @@ class EmailProcessSenderIsSystemAddressOrAgent < ActiveSupport::TestCase EmailAddress.create_or_update( channel_id: 1, realname: 'My System', - email: 'myzammad@system.test', + email: 'Myzammad@system.TEST', active: true, updated_by_id: 1, created_by_id: 1, @@ -176,5 +176,56 @@ Some Text" assert_equal(agent1.id, ticket.created_by_id) assert_equal(agent1.id, article.created_by_id) + email_raw_string = "From: ticket-system-sender-AGENT1@example.com +To: MYZAMMAD@system.test, ticket-system-sender-CUSTOMER1@example.com +Subject: some subject #4 + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article = Ticket::Article.find(article_p.id) + assert_equal('some subject #4', ticket.title) + assert_equal('open', ticket.state.name) + assert_equal('Agent', ticket.create_article_sender.name) + assert_equal('Agent', article.sender.name) + assert_equal('ticket-system-sender-customer1@example.com', ticket.customer.email) + assert_equal(agent1.id, ticket.created_by_id) + assert_equal(agent1.id, article.created_by_id) + + email_raw_string = "From: ticket-system-sender-agent1@example.com +To: myzammad@system.test +Subject: some subject #5 + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article = Ticket::Article.find(article_p.id) + assert_equal('some subject #5', ticket.title) + assert_equal('open', ticket.state.name) + assert_equal('Agent', ticket.create_article_sender.name) + assert_equal('Agent', article.sender.name) + assert_equal('ticket-system-sender-agent1@example.com', ticket.customer.email) + assert_equal(agent1.id, ticket.created_by_id) + assert_equal(agent1.id, article.created_by_id) + + email_raw_string = "From: ticket-system-sender-agent1@example.com +To: myZammad@system.Test +Subject: some subject #6 + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article = Ticket::Article.find(article_p.id) + assert_equal('some subject #6', ticket.title) + assert_equal('open', ticket.state.name) + assert_equal('Agent', ticket.create_article_sender.name) + assert_equal('Agent', article.sender.name) + assert_equal('ticket-system-sender-agent1@example.com', ticket.customer.email) + assert_equal(agent1.id, ticket.created_by_id) + assert_equal(agent1.id, article.created_by_id) + end end