Merge pull request #1055 from Benbb96/fix-1054

Fix fail email check when an attachment has a wrong extension

helpdesk/email.py

@@ -478,7 +478,11 @@ def create_object_from_email_message(message, ticket_id, payload, files, logger)
 
     logger.info("[%s-%s] %s" % (ticket.queue.slug, ticket.id, ticket.title,))
 
+    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.",

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:

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."""

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
 

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()
+            )