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

This commit is contained in:
Arkadiy Korotaev 2021-04-13 14:04:08 +02:00
parent 012ba4f42d
commit 2b4c82fd1b
No known key found for this signature in database
GPG Key ID: 4BD092DD84540FAB
8 changed files with 128 additions and 30 deletions

View File

@ -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_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_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

3
helpdesk/.flake8 Normal file
View File

@ -0,0 +1,3 @@
[flake8]
max-line-length = 120
import-order-style = pep8

View File

@ -21,6 +21,7 @@ from os.path import isfile, join
from time import ctime from time import ctime
from bs4 import BeautifulSoup from bs4 import BeautifulSoup
from django.conf import settings as django_settings
from django.contrib.auth import get_user_model from django.contrib.auth import get_user_model
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.core.files.uploadedfile import SimpleUploadedFile 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}), title=_('E-Mail Received from %(sender_email)s' % {'sender_email': sender_email}),
date=now, date=now,
public=True, public=True,
comment=payload['body'], comment=payload.get('full_body', payload['body']) or "",
message_id=message_id message_id=message_id
) )
@ -505,6 +506,7 @@ def object_from_message(message, queue, logger):
ticket = None ticket = None
body = None body = None
full_body = None
counter = 0 counter = 0
files = [] 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': if part['Content-Transfer-Encoding'] == '8bit' and part.get_content_charset() == 'utf-8':
body = body.decode('unicode_escape') body = body.decode('unicode_escape')
body = decodeUnknown(part.get_content_charset(), body) 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 # workaround to get unicode text out rather than escaped text
try: try:
body = body.encode('ascii').decode('unicode_escape') body = body.encode('ascii').decode('unicode_escape')
@ -536,13 +550,23 @@ def object_from_message(message, queue, logger):
except UnicodeDecodeError: except UnicodeDecodeError:
email_body = encoding.smart_text(part.get_payload(decode=False)) email_body = encoding.smart_text(part.get_payload(decode=False))
payload = """ if not body and not full_body:
<html> # no text has been parsed so far - try such deep parsing for some messages
<head> altered_body = email_body.replace("</p>", "</p>\n").replace("<br", "\n<br")
<meta charset="utf-8"/> mail = BeautifulSoup(str(altered_body), "html.parser")
</head> full_body = mail.get_text()
%s
</html>""" % email_body if "<body" not in email_body:
email_body = f"<body>{email_body}</body>"
payload = (
'<html>'
'<head>'
'<meta charset="utf-8" />'
'</head>'
'%s'
'</html>'
) % email_body
files.append( files.append(
SimpleUploadedFile(_("email_html_body.html"), payload.encode("utf-8"), 'text/html') SimpleUploadedFile(_("email_html_body.html"), payload.encode("utf-8"), 'text/html')
) )
@ -554,22 +578,22 @@ def object_from_message(message, queue, logger):
else: else:
name = ("part-%i_" % counter) + name name = ("part-%i_" % counter) + name
# FIXME: this code gets the paylods, then does something with it and then completely ignores it # # 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 # # writing the part.get_payload(decode=True) instead; and then the payload variable is
# replaced by some dict later. # # replaced by some dict later.
# the `payloadToWrite` has been also ignored so was commented # # the `payloadToWrite` has been also ignored so was commented
payload = part.get_payload() # payload = part.get_payload()
if isinstance(payload, list): # if isinstance(payload, list):
payload = payload.pop().as_string() # payload = payload.pop().as_string()
# payloadToWrite = payload # # payloadToWrite = payload
# check version of python to ensure use of only the correct error type # # check version of python to ensure use of only the correct error type
non_b64_err = TypeError # non_b64_err = TypeError
try: # try:
logger.debug("Try to base64 decode the attachment payload") # logger.debug("Try to base64 decode the attachment payload")
# payloadToWrite = base64.decodebytes(payload) # # payloadToWrite = base64.decodebytes(payload)
except non_b64_err: # except non_b64_err:
logger.debug("Payload was not base64 encoded, using raw bytes") # logger.debug("Payload was not base64 encoded, using raw bytes")
# payloadToWrite = payload # # payloadToWrite = payload
files.append(SimpleUploadedFile(name, part.get_payload(decode=True), mimetypes.guess_type(name)[0])) files.append(SimpleUploadedFile(name, part.get_payload(decode=True), mimetypes.guess_type(name)[0]))
logger.debug("Found MIME attachment %s" % name) logger.debug("Found MIME attachment %s" % name)
@ -581,11 +605,25 @@ def object_from_message(message, queue, logger):
if beautiful_body: if beautiful_body:
try: try:
body = beautiful_body.text body = beautiful_body.text
full_body = body
except AttributeError: except AttributeError:
pass pass
if not body: if not body:
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_priority = message.get('priority', '')
smtp_importance = message.get('importance', '') smtp_importance = message.get('importance', '')
high_priority_types = {'high', 'important', '1', 'urgent'} high_priority_types = {'high', 'important', '1', 'urgent'}
@ -593,6 +631,7 @@ def object_from_message(message, queue, logger):
payload = { payload = {
'body': body, 'body': body,
'full_body': full_body or body,
'subject': subject, 'subject': subject,
'queue': queue, 'queue': queue,
'sender_email': sender_email, 'sender_email': sender_email,

View File

@ -162,3 +162,12 @@ HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = getattr(
# use https in the email links # use https in the email links
HELPDESK_USE_HTTPS_IN_EMAIL_LINK = getattr(settings, 'HELPDESK_USE_HTTPS_IN_EMAIL_LINK', False) 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)

View File

@ -50,7 +50,7 @@
</div> </div>
<p class="mb-1"> <p class="mb-1">
{% if followup.comment %} {% if followup.comment %}
<p>{{ followup.get_markdown|urlizetrunc:50|num_to_link|linebreaksbr }}</p> <p>{{ followup.get_markdown|urlizetrunc:50|num_to_link }}</p>
{% endif %} {% endif %}
{% for change in followup.ticketchange_set.all %} {% for change in followup.ticketchange_set.all %}
{% if forloop.first %}<div class='changes'><ul>{% endif %} {% if forloop.first %}<div class='changes'><ul>{% endif %}

View File

@ -56,7 +56,7 @@
<td class="{% if ticket.priority < 3 %}table-warning{% endif %}">{{ ticket.get_priority_display }} <td class="{% if ticket.priority < 3 %}table-warning{% endif %}">{{ ticket.get_priority_display }}
</td> </td>
<th class="table-active">{% trans "Copies To" %}</th> <th class="table-active">{% trans "Copies To" %}</th>
<td>{{ ticketcc_string }} <a data-toggle='tooltip' href='{% url 'helpdesk:ticket_cc' ticket.id %}' title='{% trans "Click here to add / remove people who should receive an e-mail whenever this ticket is updated." %}'><strong><button type="button" class="btn btn-warning btn-sm float-right"><i class="fa fa-share"></i></button></strong></a>{% if SHOW_SUBSCRIBE %}, <strong><a data-toggle='tooltip' href='?subscribe' title='{% trans "Click here to subscribe yourself to this ticket, if you want to receive an e-mail whenever this ticket is updated." %}'><button type="button" class="btn btn-warning btn-sm float-right"><i class="fas fa-rss-square"></i></button></a></strong>{% endif %}</td> <td>{{ ticketcc_string }} <a data-toggle='tooltip' href='{% url 'helpdesk:ticket_cc' ticket.id %}' title='{% trans "Click here to add / remove people who should receive an e-mail whenever this ticket is updated." %}'><strong><button type="button" class="btn btn-warning btn-sm float-right"><i class="fa fa-share"></i></button></strong></a>{% if SHOW_SUBSCRIBE %} <strong><a data-toggle='tooltip' href='?subscribe' title='{% trans "Click here to subscribe yourself to this ticket, if you want to receive an e-mail whenever this ticket is updated." %}'><button type="button" class="btn btn-warning btn-sm float-right"><i class="fas fa-rss-square"></i></button></a></strong>{% endif %}</td>
</tr> </tr>
<tr> <tr>
@ -99,7 +99,7 @@
<tr> <tr>
<td id="ticket-description" colspan='4'> <td id="ticket-description" colspan='4'>
<h4>{% trans "Description" %}</h4> <h4>{% trans "Description" %}</h4>
{{ ticket.get_markdown|urlizetrunc:50|num_to_link|linebreaksbr }}</td> {{ ticket.get_markdown|urlizetrunc:50|num_to_link }}</td>
</tr> </tr>
{% if ticket.resolution %}<tr> {% if ticket.resolution %}<tr>

View File

@ -0,0 +1,27 @@
Return-Path: sender@domain.tld
To: recipient@domain.tld
From: The Sender <sender@domain.tld>
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 <noreply@github.com>
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

View File

@ -1,5 +1,5 @@
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
from django.test import TestCase from django.test import TestCase, override_settings
from django.core.management import call_command from django.core.management import call_command
from django.shortcuts import get_object_or_404 from django.shortcuts import get_object_or_404
from django.contrib.auth.models import User from django.contrib.auth.models import User
@ -84,9 +84,11 @@ class GetEmailCommonTests(TestCase):
self.assertEqual(ticket.title, "Testovácí email") self.assertEqual(ticket.title, "Testovácí email")
self.assertEqual(ticket.description, "íářčšáíéřášč") self.assertEqual(ticket.description, "íářčšáíéřášč")
@override_settings(HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL=True)
def test_email_with_utf_8_non_decodable_sequences(self): def test_email_with_utf_8_non_decodable_sequences(self):
""" """
Tests that emails with utf-8 non-decodable sequences are parsed correctly 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: with open(os.path.join(THIS_DIR, "test_files/utf-nondecodable.eml")) as fd:
test_email = fd.read() test_email = fd.read()
@ -99,6 +101,20 @@ class GetEmailCommonTests(TestCase):
attachment = attachments[0] attachment = attachments[0]
self.assertIn('prosazuje lepší', attachment.file.read().decode("utf-8")) 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): class GetEmailParametricTemplate(object):
"""TestCase that checks basic email functionality across methods and socks configs.""" """TestCase that checks basic email functionality across methods and socks configs."""
@ -555,7 +571,7 @@ class GetEmailParametricTemplate(object):
self.assertEqual(followup1.ticket.id, 1) self.assertEqual(followup1.ticket.id, 1)
attach1 = get_object_or_404(FollowUpAttachment, pk=1) attach1 = get_object_or_404(FollowUpAttachment, pk=1)
self.assertEqual(attach1.followup.id, 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----- self.assertEqual(attach1.file.read(), b"""-----BEGIN PGP SIGNATURE-----
iQIcBAEBCAAGBQJaA3dnAAoJELBLc7QPITnLN54P/3Zsu7+AIQWDFTvziJfCqswG iQIcBAEBCAAGBQJaA3dnAAoJELBLc7QPITnLN54P/3Zsu7+AIQWDFTvziJfCqswG