From 2b4c82fd1bc4dcc8bf640f6941d400cfc20d504f Mon Sep 17 00:00:00 2001 From: Arkadiy Korotaev Date: Tue, 13 Apr 2021 14:04:08 +0200 Subject: [PATCH] fix(email): Add ability to attach full first email text to avoid losing forwards, and to save .eml files for any incoming mesages, plus fix tests and some minor bugs --- docs/settings.rst | 4 + helpdesk/.flake8 | 3 + helpdesk/email.py | 89 +++++++++++++------ helpdesk/settings.py | 9 ++ helpdesk/templates/helpdesk/ticket.html | 2 +- .../templates/helpdesk/ticket_desc_table.html | 4 +- .../tests/test_files/forwarded-message.eml | 27 ++++++ helpdesk/tests/test_get_email.py | 20 ++++- 8 files changed, 128 insertions(+), 30 deletions(-) create mode 100644 helpdesk/.flake8 create mode 100644 helpdesk/tests/test_files/forwarded-message.eml diff --git a/docs/settings.rst b/docs/settings.rst index 5a9b5088..405e8f8d 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -223,3 +223,7 @@ The following settings were defined in previous versions and are no longer suppo - **HELPDESK_FOOTER_SHOW_CHANGE_LANGUAGE_LINK** Is never shown. Use your own template if required. - **HELPDESK_ENABLE_PER_QUEUE_MEMBERSHIP** Discontinued in favor of HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION. + +- **HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL** Do not ignore fowarded and replied text from the email messages which create a new ticket; useful for cases when customer forwards some email (error from service or something) and wants support to see that + +- **HELPDESK_ALWAYS_SAVE_INCOMING_EMAIL_MESSAGE** Any incoming .eml message is saved and available, helps when customer spent some time doing fancy markup which has been corrupted during the email-to-ticket-comment translate process diff --git a/helpdesk/.flake8 b/helpdesk/.flake8 new file mode 100644 index 00000000..f119cca1 --- /dev/null +++ b/helpdesk/.flake8 @@ -0,0 +1,3 @@ +[flake8] +max-line-length = 120 +import-order-style = pep8 diff --git a/helpdesk/email.py b/helpdesk/email.py index becc90d0..b1647e31 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -21,6 +21,7 @@ from os.path import isfile, join from time import ctime from bs4 import BeautifulSoup +from django.conf import settings as django_settings from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from django.core.files.uploadedfile import SimpleUploadedFile @@ -394,7 +395,7 @@ def create_object_from_email_message(message, ticket_id, payload, files, logger) title=_('E-Mail Received from %(sender_email)s' % {'sender_email': sender_email}), date=now, public=True, - comment=payload['body'], + comment=payload.get('full_body', payload['body']) or "", message_id=message_id ) @@ -505,6 +506,7 @@ def object_from_message(message, queue, logger): ticket = None body = None + full_body = None counter = 0 files = [] @@ -523,7 +525,19 @@ def object_from_message(message, queue, logger): if part['Content-Transfer-Encoding'] == '8bit' and part.get_content_charset() == 'utf-8': body = body.decode('unicode_escape') body = decodeUnknown(part.get_content_charset(), body) - body = EmailReplyParser.parse_reply(body) + # have to use django_settings here so overwritting it works in tests + # the default value is False anyway + if ticket is None and getattr(django_settings, 'HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL', False): + # first message in thread, we save full body to avoid losing forwards and things like that + body_parts = [] + for f in EmailReplyParser.read(body).fragments: + body_parts.append(f.content) + full_body = '\n\n'.join(body_parts) + body = EmailReplyParser.parse_reply(body) + else: + # second and other reply, save only first part of the message + body = EmailReplyParser.parse_reply(body) + full_body = body # workaround to get unicode text out rather than escaped text try: body = body.encode('ascii').decode('unicode_escape') @@ -536,13 +550,23 @@ def object_from_message(message, queue, logger): except UnicodeDecodeError: email_body = encoding.smart_text(part.get_payload(decode=False)) - payload = """ - - - - -%s -""" % email_body + if not body and not full_body: + # no text has been parsed so far - try such deep parsing for some messages + altered_body = email_body.replace("

", "

\n").replace("{email_body}" + + payload = ( + '' + '' + '' + '' + '%s' + '' + ) % email_body files.append( SimpleUploadedFile(_("email_html_body.html"), payload.encode("utf-8"), 'text/html') ) @@ -554,22 +578,22 @@ def object_from_message(message, queue, logger): else: name = ("part-%i_" % counter) + name - # FIXME: this code gets the paylods, then does something with it and then completely ignores it - # writing the part.get_payload(decode=True) instead; and then the payload variable is - # replaced by some dict later. - # the `payloadToWrite` has been also ignored so was commented - payload = part.get_payload() - if isinstance(payload, list): - payload = payload.pop().as_string() - # payloadToWrite = payload - # check version of python to ensure use of only the correct error type - non_b64_err = TypeError - try: - logger.debug("Try to base64 decode the attachment payload") - # payloadToWrite = base64.decodebytes(payload) - except non_b64_err: - logger.debug("Payload was not base64 encoded, using raw bytes") - # payloadToWrite = payload + # # FIXME: this code gets the paylods, then does something with it and then completely ignores it + # # writing the part.get_payload(decode=True) instead; and then the payload variable is + # # replaced by some dict later. + # # the `payloadToWrite` has been also ignored so was commented + # payload = part.get_payload() + # if isinstance(payload, list): + # payload = payload.pop().as_string() + # # payloadToWrite = payload + # # check version of python to ensure use of only the correct error type + # non_b64_err = TypeError + # try: + # logger.debug("Try to base64 decode the attachment payload") + # # payloadToWrite = base64.decodebytes(payload) + # except non_b64_err: + # logger.debug("Payload was not base64 encoded, using raw bytes") + # # payloadToWrite = payload files.append(SimpleUploadedFile(name, part.get_payload(decode=True), mimetypes.guess_type(name)[0])) logger.debug("Found MIME attachment %s" % name) @@ -581,11 +605,25 @@ def object_from_message(message, queue, logger): if beautiful_body: try: body = beautiful_body.text + full_body = body except AttributeError: pass if not body: body = "" + if getattr(django_settings, 'HELPDESK_ALWAYS_SAVE_INCOMING_EMAIL_MESSAGE', False): + # save message as attachment in case of some complex markup renders wrong + files.append( + SimpleUploadedFile( + _("original_message.eml").replace( + ".eml", + timezone.localtime().strftime("_%d-%m-%Y_%H:%M") + ".eml" + ), + str(message).encode("utf-8"), + 'text/plain' + ) + ) + smtp_priority = message.get('priority', '') smtp_importance = message.get('importance', '') high_priority_types = {'high', 'important', '1', 'urgent'} @@ -593,6 +631,7 @@ def object_from_message(message, queue, logger): payload = { 'body': body, + 'full_body': full_body or body, 'subject': subject, 'queue': queue, 'sender_email': sender_email, diff --git a/helpdesk/settings.py b/helpdesk/settings.py index 705b38bf..6b4c6967 100644 --- a/helpdesk/settings.py +++ b/helpdesk/settings.py @@ -162,3 +162,12 @@ HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = getattr( # use https in the email links HELPDESK_USE_HTTPS_IN_EMAIL_LINK = getattr(settings, 'HELPDESK_USE_HTTPS_IN_EMAIL_LINK', False) + +# Include all signatures and forwards in the first ticket message if set +# Useful if you get forwards dropped from them while they are useful part of request +HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL = getattr(settings, 'HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL', False) + +# If set then we always save incoming emails as .eml attachments +# which is quite noisy but very helpful for complicated markup, forwards and so on +# (which gets stripped/corrupted otherwise) +HELPDESK_ALWAYS_SAVE_INCOMING_EMAIL_MESSAGE = getattr(settings, "HELPDESK_ALWAYS_SAVE_INCOMING_EMAIL_MESSAGE", False) diff --git a/helpdesk/templates/helpdesk/ticket.html b/helpdesk/templates/helpdesk/ticket.html index 20f55bc7..caf4bb3d 100644 --- a/helpdesk/templates/helpdesk/ticket.html +++ b/helpdesk/templates/helpdesk/ticket.html @@ -50,7 +50,7 @@

{% if followup.comment %} -

{{ followup.get_markdown|urlizetrunc:50|num_to_link|linebreaksbr }}

+

{{ followup.get_markdown|urlizetrunc:50|num_to_link }}

{% endif %} {% for change in followup.ticketchange_set.all %} {% if forloop.first %}
    {% endif %} diff --git a/helpdesk/templates/helpdesk/ticket_desc_table.html b/helpdesk/templates/helpdesk/ticket_desc_table.html index eb339936..2bf4c922 100644 --- a/helpdesk/templates/helpdesk/ticket_desc_table.html +++ b/helpdesk/templates/helpdesk/ticket_desc_table.html @@ -56,7 +56,7 @@ {{ ticket.get_priority_display }} {% trans "Copies To" %} - {{ ticketcc_string }} {% if SHOW_SUBSCRIBE %}, {% endif %} + {{ ticketcc_string }} {% if SHOW_SUBSCRIBE %} {% endif %} @@ -99,7 +99,7 @@

    {% trans "Description" %}

    - {{ ticket.get_markdown|urlizetrunc:50|num_to_link|linebreaksbr }} + {{ ticket.get_markdown|urlizetrunc:50|num_to_link }} {% if ticket.resolution %} diff --git a/helpdesk/tests/test_files/forwarded-message.eml b/helpdesk/tests/test_files/forwarded-message.eml new file mode 100644 index 00000000..f699b148 --- /dev/null +++ b/helpdesk/tests/test_files/forwarded-message.eml @@ -0,0 +1,27 @@ +Return-Path: sender@domain.tld +To: recipient@domain.tld +From: The Sender +Subject: Test with original message from GitHub +Message-ID: <0d3a17d5-7136-75c0-c8ba-a7698c57ac42@gmail.com> +Date: Tue, 13 Apr 2021 12:56:39 +0200 +User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 + Thunderbird/78.8.1 +MIME-Version: 1.0 +Content-Type: text/plain; charset=utf-8; format=flowed +Content-Transfer-Encoding: 7bit +Content-Language: en-US + +This is email body + +-----Original Message----- +From: GitHub +Sent: Tuesday, 6 April 2021 9:09 AM +Subject: [GitHub API] Update notice for access token [SEC=UNOFFICIAL] + +Hello there! + +We noticed that, at April 5th, 2021 at 23:09 (UTC) your application accessed the GitHub API with a token that has an outdated format. + +Thanks, +The GitHub Team + diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index b44ca8d2..fe8750cd 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from django.test import TestCase +from django.test import TestCase, override_settings from django.core.management import call_command from django.shortcuts import get_object_or_404 from django.contrib.auth.models import User @@ -84,9 +84,11 @@ class GetEmailCommonTests(TestCase): self.assertEqual(ticket.title, "Testovácí email") self.assertEqual(ticket.description, "íářčšáíéřášč") + @override_settings(HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL=True) def test_email_with_utf_8_non_decodable_sequences(self): """ Tests that emails with utf-8 non-decodable sequences are parsed correctly + The message is fowarded as well """ with open(os.path.join(THIS_DIR, "test_files/utf-nondecodable.eml")) as fd: test_email = fd.read() @@ -99,6 +101,20 @@ class GetEmailCommonTests(TestCase): attachment = attachments[0] self.assertIn('prosazuje lepší', attachment.file.read().decode("utf-8")) + @override_settings(HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL=True) + def test_email_with_forwarded_message(self): + """ + Forwarded message of that format must be still attached correctly + """ + with open(os.path.join(THIS_DIR, "test_files/forwarded-message.eml")) as fd: + test_email = fd.read() + ticket = helpdesk.email.object_from_message(test_email, self.queue_public, self.logger) + self.assertEqual(ticket.title, "Test with original message from GitHub") + self.assertIn("This is email body", ticket.description) + assert "Hello there!" not in ticket.description, ticket.description + assert FollowUp.objects.filter(ticket=ticket).count() == 1 + assert "Hello there!" in FollowUp.objects.filter(ticket=ticket).first().comment + class GetEmailParametricTemplate(object): """TestCase that checks basic email functionality across methods and socks configs.""" @@ -555,7 +571,7 @@ class GetEmailParametricTemplate(object): self.assertEqual(followup1.ticket.id, 1) attach1 = get_object_or_404(FollowUpAttachment, pk=1) self.assertEqual(attach1.followup.id, 1) - self.assertEqual(attach1.filename, 'signature.asc') + self.assertEqual(attach1.filename, 'part-1_signature.asc') self.assertEqual(attach1.file.read(), b"""-----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJaA3dnAAoJELBLc7QPITnLN54P/3Zsu7+AIQWDFTvziJfCqswG