diff --git a/helpdesk/email.py b/helpdesk/email.py index 85dd07d4..428ffa70 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -478,12 +478,16 @@ def create_object_from_email_message(message, ticket_id, payload, files, logger) logger.info("[%s-%s] %s" % (ticket.queue.slug, ticket.id, ticket.title,)) - attached = process_attachments(f, files) - for att_file in attached: - logger.info( - "Attachment '%s' (with size %s) successfully added to ticket from email.", - att_file[0], att_file[1].size - ) + try: + attached = process_attachments(f, files) + except ValidationError as e: + logger.error(str(e)) + else: + for att_file in attached: + logger.info( + "Attachment '%s' (with size %s) successfully added to ticket from email.", + att_file[0], att_file[1].size + ) context = safe_template_context(ticket) diff --git a/helpdesk/forms.py b/helpdesk/forms.py index 11ec89ed..8a6781d2 100644 --- a/helpdesk/forms.py +++ b/helpdesk/forms.py @@ -7,6 +7,7 @@ forms.py - Definitions of newforms-based forms for creating and maintaining tickets. """ + from datetime import datetime from django import forms from django.conf import settings @@ -33,6 +34,7 @@ from helpdesk.settings import ( CUSTOMFIELD_TIME_FORMAT, CUSTOMFIELD_TO_FIELD_DICT ) +from helpdesk.validators import validate_file_extension import logging @@ -243,6 +245,7 @@ class AbstractTicketForm(CustomFieldMixin, forms.Form): 'Only file types such as plain text (.txt), ' 'a document (.pdf, .docx, or .odt), ' 'or screenshot (.png or .jpg) may be uploaded.'), + validators=[validate_file_extension] ) class Media: diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index a3ae7aee..0c298fa1 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -3,6 +3,7 @@ from django.contrib.auth.hashers import make_password from django.contrib.auth.models import User +from django.core import mail from django.core.files.uploadedfile import SimpleUploadedFile from django.core.management import call_command from django.shortcuts import get_object_or_404 @@ -36,7 +37,7 @@ unused_port = "49151" class GetEmailCommonTests(TestCase): def setUp(self): - self.queue_public = Queue.objects.create() + self.queue_public = Queue.objects.create(title='Test', slug='test') self.logger = logging.getLogger('helpdesk') # tests correct syntax for command line option @@ -147,7 +148,7 @@ class GetEmailCommonTests(TestCase): ignore = IgnoreEmail(name="Test Ignore", email_address=from_meta[1], keep_in_mailbox=False) ignore.save() with self.assertRaises(DeleteIgnoredTicketException): - object_from_message(message.as_string(), self.queue_public, self.logger); + object_from_message(message.as_string(), self.queue_public, self.logger) def test_will_not_delete_ignored_email(self): """ @@ -158,7 +159,7 @@ class GetEmailCommonTests(TestCase): ignore = IgnoreEmail(name="Test Ignore", email_address=from_meta[1], keep_in_mailbox=True) ignore.save() with self.assertRaises(IgnoreTicketException): - object_from_message(message.as_string(), self.queue_public, self.logger); + object_from_message(message.as_string(), self.queue_public, self.logger) def test_utf8_filename_attachment(self): """ @@ -172,6 +173,26 @@ class GetEmailCommonTests(TestCase): # The extractor prepends a part identifier so compare the ending self.assertTrue(sent_file.name.endswith(filename), f"Filename extracted does not match: {sent_file.name}") + @override_settings(VALID_EXTENSIONS=['.png']) + def test_wrong_extension_attachment(self): + """ + Tests if an attachment with a wrong extension doesn't stop the email process + """ + message, _, _ = utils.generate_multipart_email(type_list=['plain', 'image']) + + self.assertEqual(len(mail.outbox), 0) + + with self.assertLogs(logger='helpdesk', level='ERROR') as cm: + object_from_message(message.as_string(), self.queue_public, self.logger) + + self.assertIn( + "ERROR:helpdesk:{'file': ['Unsupported file extension: .jpg']}", + cm.output + ) + + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(f'[test-1] {message.get("subject")} (Opened)', mail.outbox[0].subject) + class GetEmailParametricTemplate(object): """TestCase that checks basic email functionality across methods and socks configs.""" diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py index 00b7e6a5..88f0f990 100644 --- a/helpdesk/tests/utils.py +++ b/helpdesk/tests/utils.py @@ -161,7 +161,7 @@ def generate_image_mime_part(locale: str="en_US",imagename: str = None) -> Messa :param locale: change this to generate locale specific file name and attachment content :param filename: pass a file name if you want to specify a specific name otherwise a random name will be generated """ - part = MIMEImage(generate_random_image(image_format="JPEG")) + part = MIMEImage(generate_random_image(image_format="JPEG", array_dims=(200, 200))) part.set_payload(get_fake("text", locale=locale, min_length=1024)) encoders.encode_base64(part) if not imagename: @@ -206,7 +206,7 @@ def add_simple_email_headers(message: Message, locale: str="en_US", def generate_mime_part(locale: str="en_US", part_type: str="plain", use_short_email: bool=False - ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: + ) -> typing.Optional[Message]: """ Generates amime part of the sepecified type @@ -222,10 +222,8 @@ def generate_mime_part(locale: str="en_US", msg = MIMEText(body) elif "file" == part_type: msg = generate_file_mime_part(locale=locale) - msg = MIMEText(body) elif "image" == part_type: msg = generate_image_mime_part(locale=locale) - msg = MIMEText(body) else: raise Exception("Mime part not implemented: " + part_type) return msg @@ -243,7 +241,7 @@ def generate_multipart_email(locale: str="en_US", """ msg = MIMEMultipart() for part_type in type_list: - msg.append(generate_mime_part(locale=locale, part_type=part_type, use_short_email=use_short_email)) + msg.attach(generate_mime_part(locale=locale, part_type=part_type, use_short_email=use_short_email)) from_meta, to_meta = add_simple_email_headers(msg, locale=locale, use_short_email=use_short_email) return msg, from_meta, to_meta diff --git a/helpdesk/validators.py b/helpdesk/validators.py index 08086e1f..5fb80e39 100644 --- a/helpdesk/validators.py +++ b/helpdesk/validators.py @@ -4,6 +4,7 @@ from django.conf import settings +from django.utils.translation import gettext as _ # TODO: can we use the builtin Django validator instead? @@ -32,4 +33,5 @@ def validate_file_extension(value): # should always allow that? if not (ext.lower() == '' or ext.lower() == '.'): raise ValidationError( - 'Unsupported file extension: %s.' % ext.lower()) + _('Unsupported file extension: ') + ext.lower() + )