From 5a8adbed3f463c8fadd9d1f2dd9a78dcc9465d08 Mon Sep 17 00:00:00 2001 From: Bola Ahmed Buari Date: Mon, 10 May 2021 17:30:12 +0000 Subject: [PATCH] Fixes #3467: Sending and forwarding internal email messages --- .../channel/filter/internal_article_check.rb | 43 ++++ ...0001_setting_add_internal_article_check.rb | 17 ++ db/seeds/settings.rb | 9 + .../filter/internal_article_check_spec.rb | 186 ++++++++++++++++++ 4 files changed, 255 insertions(+) create mode 100644 app/models/channel/filter/internal_article_check.rb create mode 100644 db/migrate/202104070000001_setting_add_internal_article_check.rb create mode 100644 spec/models/channel/filter/internal_article_check_spec.rb diff --git a/app/models/channel/filter/internal_article_check.rb b/app/models/channel/filter/internal_article_check.rb new file mode 100644 index 000000000..10e52145e --- /dev/null +++ b/app/models/channel/filter/internal_article_check.rb @@ -0,0 +1,43 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ + +module Channel::Filter::InternalArticleCheck + def self.run(_channel, mail, _transaction_params) + return if mail[ :'x-zammad-ticket-id' ].blank? + + ticket = Ticket.find_by(id: mail[ :'x-zammad-ticket-id' ]) + return if ticket.blank? + + return if !in_reply_to_is_internal?(mail, ticket) && + !last_outgoing_mail_is_internal?(mail, ticket) + + mail[ :'x-zammad-article-internal' ] = true + true + end + + def self.in_reply_to_is_internal?(mail, ticket) + return false if mail[:'in-reply-to'].blank? + + message_id_md5 = Digest::MD5.hexdigest(mail[:'in-reply-to']) + ticket.articles.exists?(message_id_md5: message_id_md5, internal: true) + end + + def self.last_outgoing_mail_is_internal?(mail, ticket) + return false if mail[:'in-reply-to'].present? + + from_email = parse_email(mail[:from_email]) + return false if from_email.blank? + + last_outgoing_mail = ticket.articles + .where("ticket_articles.to #{Rails.application.config.db_like} ?", "%#{from_email}%") + .order(created_at: :desc).first + + last_outgoing_mail&.internal.present? + end + + def self.parse_email(email) + Mail::AddressList.new(email)&.addresses&.first&.address + rescue + Rails.logger.error "Can not parse email: #{email}" + nil + end +end diff --git a/db/migrate/202104070000001_setting_add_internal_article_check.rb b/db/migrate/202104070000001_setting_add_internal_article_check.rb new file mode 100644 index 000000000..1b3f1a19b --- /dev/null +++ b/db/migrate/202104070000001_setting_add_internal_article_check.rb @@ -0,0 +1,17 @@ +class SettingAddInternalArticleCheck < ActiveRecord::Migration[5.2] + def change + + # return if it's a new setup + return if !Setting.exists?(name: 'system_init_done') + + Setting.create_if_not_exists( + title: 'Define postmaster filter.', + name: '5500_postmaster_internal_article_check', + area: 'Postmaster::PreFilter', + description: 'Defines the postmaster filter which set the article internal if a forwarded, replied or sent email also exists with the article internal received.', + options: {}, + state: 'Channel::Filter::InternalArticleCheck', + frontend: false + ) + end +end diff --git a/db/seeds/settings.rb b/db/seeds/settings.rb index c3d26f774..cc46e45bf 100644 --- a/db/seeds/settings.rb +++ b/db/seeds/settings.rb @@ -3480,6 +3480,15 @@ Setting.create_if_not_exists( state: 'Channel::Filter::ServiceNowCheck', frontend: false ) +Setting.create_if_not_exists( + title: 'Define postmaster filter.', + name: '5500_postmaster_internal_article_check', + area: 'Postmaster::PreFilter', + description: 'Defines the postmaster filter which set the article internal if a forwarded, replied or sent email also exists with the article internal received.', + options: {}, + state: 'Channel::Filter::InternalArticleCheck', + frontend: false +) Setting.create_if_not_exists( title: 'Icinga integration', name: 'icinga_integration', diff --git a/spec/models/channel/filter/internal_article_check_spec.rb b/spec/models/channel/filter/internal_article_check_spec.rb new file mode 100644 index 000000000..27181534b --- /dev/null +++ b/spec/models/channel/filter/internal_article_check_spec.rb @@ -0,0 +1,186 @@ +require 'rails_helper' + +RSpec.describe Channel::Filter::InternalArticleCheck do + let(:ticket) { create(:ticket) } + let(:vendor_email) { 'vendor@example.com' } + let(:article_to) { vendor_email } + let(:from) { "From: <#{vendor_email}>" } + let(:message_id) { 'some_message_id_999@example.com' } + let(:in_reply_to) { message_id } + let(:subject) { "Subject: #{ticket.subject_build('some subject')}" } + let(:ticket_article) { build(:ticket_article, ticket: ticket, to: article_to, internal: false, message_id: message_id) } + let(:inbound_email) { create(:ticket_article, :inbound_email, ticket: ticket) } + let(:outbound_email) { create(:ticket_article, :outbound_email, ticket: ticket) } + let(:internal_note) { create(:ticket_article, :outbound_note, ticket: ticket, internal: true) } + + let(:email_raw_string) do + email_file_path = Rails.root.join('test/data/mail/mail001.box') + File.read(email_file_path) + end + + let(:email_parse_mail_answer) do + channel_as_model = Channel.new(options: {}) + + email_raw_string.sub!(/^Subject: .+?$/, subject) + email_raw_string.sub!('From: ', from) + email_raw_string.sub!('Message-Id: <053EA3703574649ABDAF24D43A05604F327A130@MEMASFRK004.example.com>', "Message-Id: <053EA3703574649ABDAF24D43A05604F327A130@MEMASFRK004.example.com>\nIn-Reply-To: #{in_reply_to}") + Channel::EmailParser.new.process(channel_as_model, email_raw_string) + end + + shared_examples 'setting new article to internal' do + it 'sets new article to internal' do + _ticket_p, article_p, _user_p = email_parse_mail_answer + expect(article_p.internal).to be true + end + end + + shared_examples 'not setting new article to internal' do + it 'does not set new article to internal' do + _ticket_p, article_p, _user_p = email_parse_mail_answer + expect(article_p.internal).not_to be true + end + end + + shared_examples 'sets new article to internal' do + context 'when From has email only' do + it_behaves_like 'setting new article to internal' + end + + context 'when From have both name and email' do + let(:from) { "From: Some Vendor Name <#{vendor_email}>" } + + it_behaves_like 'setting new article to internal' + end + + context 'when From have name with brackets and email' do + let(:from) { "From: (Some Vendor Name) <#{vendor_email}>" } + + it_behaves_like 'setting new article to internal' + end + + context 'when From have name with brackets and uppercase email' do + let(:from) { "From: (Some Vendor Name) <#{vendor_email.upcase}>" } + + it_behaves_like 'setting new article to internal' + end + + context 'when From have name in quotes and email' do + let(:from) { "From: 'Günther John | Example GmbH' <#{vendor_email}>" } + + it_behaves_like 'setting new article to internal' + end + + context 'when From have email before name' do + let(:from) { "From: <#{vendor_email.upcase}> (Some Vendor Name)" } + + it_behaves_like 'setting new article to internal' + end + + context 'when article to have both name and email' do + let(:article_to) { "Some Vendor Name <#{vendor_email}>" } + + it_behaves_like 'setting new article to internal' + end + + context 'when article to have name with brackets and email' do + let(:article_to) { "(Some Vendor Name) <#{vendor_email}>" } + + it_behaves_like 'setting new article to internal' + end + + context 'when article to have name with brackets and uppercase email' do + let(:article_to) { "(Some Vendor Name) <#{vendor_email.upcase}>" } + + it_behaves_like 'setting new article to internal' + end + + context 'when article to have name in quotes and email' do + let(:article_to) { "'Günther John | Example GmbH' <#{vendor_email}>" } + + it_behaves_like 'setting new article to internal' + end + + context 'when article to have email before name' do + let(:article_to) { "<#{vendor_email}> (Some Vendor Name)" } + + it_behaves_like 'setting new article to internal' + end + end + + shared_examples 'checks in reply to header' do + + context 'when associated article is internal' do + before { ticket_article.update! internal: true } + + include_examples 'sets new article to internal' + end + + context 'when there is no associated article' do + let(:article_to) { 'me@example.com' } + + it_behaves_like 'not setting new article to internal' + end + + context 'when associated article is not internal' do + before { ticket_article.update! internal: false } + + it_behaves_like 'not setting new article to internal' + end + + end + + shared_examples 'checks last outgoing mail' do + + context 'when associated article is internal' do + before { ticket_article.update! internal: true } + + include_examples 'sets new article to internal' + end + + context 'when there is no associated article' do + let(:article_to) { nil } + + it_behaves_like 'not setting new article to internal' + end + + context 'when associated article is not internal' do + before { ticket_article.update! internal: false } + + it_behaves_like 'not setting new article to internal' + end + + context 'when From have wrong email format' do + let(:from) { "From: 'Günther John | Example GmbH' " } + + it_behaves_like 'not setting new article to internal' + end + + context 'when article to have wrong email format' do + let(:article_to) { "'Günther John | Example GmbH' " } + + it_behaves_like 'not setting new article to internal' + end + + context 'when there is no article to can not be parsed' do + let(:article_to) { "From: (Some Vendor Name) <#{vendor_email.upcase}>" } + + it_behaves_like 'not setting new article to internal' + end + + end + + describe '.run' do + before { inbound_email && outbound_email && internal_note } + + context 'when in reply to header is present' do + + include_examples 'checks in reply to header' + end + + context 'when in reply to header is blank' do + let(:in_reply_to) { '' } + + include_examples 'checks last outgoing mail' + end + end +end