diff --git a/src/strass/strass_app/forms.py b/src/strass/strass_app/forms.py index 6e922d16e9155f222eda0e2219458d5bf8fd4e6f..5b2a9eca8a30dbf3d82271b2d6166308b2f2f907 100644 --- a/src/strass/strass_app/forms.py +++ b/src/strass/strass_app/forms.py @@ -18,7 +18,9 @@ import datetime import json +import logging import smtplib +import traceback from basetheme_bootstrap.templatetags.sstatic import get_absolut_url from crispy_forms import layout @@ -30,7 +32,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.models import Group from django.core.exceptions import ValidationError from django.core.files.uploadedfile import SimpleUploadedFile -from django.core.mail import EmailMultiAlternatives +from django.core.mail import EmailMultiAlternatives, mail_admins from django.core.validators import RegexValidator from django.db import transaction from django.db.models import Q, Case, When, Value, BooleanField @@ -42,6 +44,7 @@ from django.utils import timezone, translation from django.utils.regex_helper import _lazy_re_compile from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _, gettext, ngettext +from tempfile import NamedTemporaryFile from language_override.translation import gettext_lazy as ogettext from live_settings import live_settings @@ -50,6 +53,8 @@ from strass_app.custom_layout_object import Formset from strass_app.templatetags.strass_tags import markdown from strass_app.utils import get_email_backend, validate_multiple_email, safe_pdf +logger = logging.getLogger(__name__) + class BoostrapSelectMultiple(forms.SelectMultiple): def __init__(self, attrs=None, *args, **kwargs): @@ -373,6 +378,8 @@ class CandidateForm(ModelFormWithReadOnly): def clean(self): cleaned_data = super().clean() + if self.errors: + return cleaned_data if len(cleaned_data.get("profiles", [])) > live_settings.max_num_profile__int > 0: raise ValidationError( { @@ -385,11 +392,27 @@ class CandidateForm(ModelFormWithReadOnly): raise ValidationError({'email': _("The email cannot be used to apply")}) if live_settings.cv_enabled__bool: - cleaned_data['cv'] = SimpleUploadedFile( - "cv.pdf", - safe_pdf(cleaned_data['cv']).read(), - content_type="application/pdf", - ) + try: + cleaned_data['cv'] = SimpleUploadedFile( + "cv.pdf", + safe_pdf(cleaned_data['cv']).read(), + content_type="application/pdf", + ) + except Exception as e: + logger.error(f"Failed while cleaning pdf...", exc_info=True) + cv_ko = NamedTemporaryFile( + prefix=f'StrassCV-{datetime.datetime.now().strftime("%Y-%m-%d--%H-%M-%S-")}', + suffix='.pdf', + delete=False, + ) + cleaned_data['cv'].seek(0) + with open(cv_ko.name, 'wb+') as fh: + for chunk in cleaned_data['cv'].chunks(): + fh.write(chunk) + logger.error(f"Failed while cleaning pdf, dump saved to {cv_ko.name}") + self.add_error('cv', _('Error while importing CV')) + tb = traceback.format_exc() + mail_admins("PDF cleanup failure", f"PDF file saved to {cv_ko.name}\n{tb}") return cleaned_data diff --git a/src/strass/strass_app/locale/en/LC_MESSAGES/django.po b/src/strass/strass_app/locale/en/LC_MESSAGES/django.po index d728cb6481b18e6218fc3002f93d588577164cc1..360ad9b943afc3ac0e55dd5a30834679152aa54e 100644 --- a/src/strass/strass_app/locale/en/LC_MESSAGES/django.po +++ b/src/strass/strass_app/locale/en/LC_MESSAGES/django.po @@ -293,6 +293,9 @@ msgstr "Too much choices selected (%(c)i), at most %(m)i choices are allowed." msgid "The email cannot be used to apply" msgstr "" +msgid "Error while importing CV" +msgstr "" + msgid "AppSettingsForm.reviewer_can_see_all_candidates.label" msgstr "Reviewers can read all applications." diff --git a/src/strass/strass_app/locale/fr/LC_MESSAGES/django.po b/src/strass/strass_app/locale/fr/LC_MESSAGES/django.po index f9fdc50c16836cc985730bc1e9986f552b307618..f6e166e8174b471a238780165551e5e69befec9c 100644 --- a/src/strass/strass_app/locale/fr/LC_MESSAGES/django.po +++ b/src/strass/strass_app/locale/fr/LC_MESSAGES/django.po @@ -306,6 +306,9 @@ msgstr "" msgid "The email cannot be used to apply" msgstr "Ce courriel ne peut être utilisé pour soumettre une candidature." +msgid "Error while importing CV" +msgstr "Erreur durant l'import du CV" + msgid "AppSettingsForm.reviewer_can_see_all_candidates.label" msgstr "Les reviewers peuvent voir toutes les dossiers de candidature." diff --git a/src/strass/strass_app/tests/test_candidate_apply.py b/src/strass/strass_app/tests/test_candidate_apply.py index f32f596e5ac32a225b6546d31875a2b13c99901d..312280c5cd14b873443b4d58aae1b9c9d6c0b75f 100644 --- a/src/strass/strass_app/tests/test_candidate_apply.py +++ b/src/strass/strass_app/tests/test_candidate_apply.py @@ -1191,6 +1191,46 @@ class TestCandidateApply(TooledTestCase): "all question must have an answer, even empty", ) + def test_apply_with_wrong_email(self): + live_settings.show_email_as_message = False + live_settings.max_num_referee = 0 + live_settings.cv_enabled = False + load_demo.create_candidate_questions(load_demo.create_faker_instance(0)) + steps = list() + + ####################################################################### + # Apply + ####################################################################### + candidate_wizard = "candidate_wizard" + url = reverse('strass:candidate-apply') + # self.client.force_login(self.user) + response_home = self.client.get(url, follow=True) + target = response_home.redirect_chain[0][0] + step_name = target.split("/")[-2] + form_data = {f"{candidate_wizard}-current_step": target.split("/")[-2]} + response = self.client.post(target, form_data, follow=True) + self.assertEqual(response.status_code, 200) + steps.append(WizardStep(target=target, response=response, form_data=form_data, step_name=step_name)) + del target, response, form_data, step_name + del response_home, url + + target = steps[-1].response.redirect_chain[0][0] + + step_name = target.split("/")[-2] + cv = open(os.path.join(self.test_data, "cv.pdf"), "rb") + form_data = { + step_name + "-first_name": "Ada", + step_name + "-last_name": "Lovelace", + step_name + "-email": "ada.lovelace@pasteurfr", # dot is missing + step_name + "-profiles": "2", + step_name + "-motivation": "Yes I am !", + step_name + "-cv": SimpleUploadedFile(cv.name, cv.read(), content_type="application/pdf"), + step_name + "-lang": "en", + f"{candidate_wizard}-current_step": step_name, + } + response = self.client.post(target, form_data, follow=False) + self.assertEqual(response.status_code, 200) + def test_reviewers_notification(self): fake = Faker() fake.seed_instance(0) diff --git a/src/strass/strass_app/tests/test_forms.py b/src/strass/strass_app/tests/test_forms.py index 8a7c1217cb4036497887f094cebd986953e93bcf..69e5ab83254505f0f751e7ae041d7da5fbe3a0cb 100644 --- a/src/strass/strass_app/tests/test_forms.py +++ b/src/strass/strass_app/tests/test_forms.py @@ -17,11 +17,23 @@ # import logging +import os +import pathlib +import random +from tempfile import NamedTemporaryFile +import live_settings from crispy_forms import layout +from django.contrib.auth.models import AnonymousUser +from django.core import mail +from django.core.files.uploadedfile import SimpleUploadedFile from django.template import Template, Context +from django.test import RequestFactory +from freezegun import freeze_time +from strass_app import forms from strass_app.forms import EmptyForm +from strass_app.management.commands import load_demo from strass_app.tests.test_base_test_case import TooledTestCase logger = logging.getLogger(__name__) @@ -45,3 +57,41 @@ class TestMain(TooledTestCase): self.assertNotEqual(r1, r2) self.assertIn('form1', r1) self.assertIn('form2', r2) + + +class TestCandidateForm(TooledTestCase): + def test_pdf_safe_crashes_log_collected(self): + load_demo.create_profiles() + mail_count = len(mail.outbox) + m = random.randint(1, 12) + d = random.randint(1, 28) + with freeze_time(f"1999-{m}-{d}"): + request_an = RequestFactory().get('/blabla') + request_an.user = AnonymousUser() + form = forms.CandidateForm( + request=request_an, + initial=dict(), + data={ + "first_name": "Ada", + "last_name": "Lovelace", + "email": "ada.lovelace@pasteur.fr", + "profiles": [2], + "motivation": "Yes I am !", + "lang": "en", + }, + files={ + "cv": SimpleUploadedFile('cv.pdf', b'zeazeazeaze', content_type="application/pdf"), + }, + ) + form.is_valid() + + with NamedTemporaryFile() as f: + files = list(pathlib.Path(f.name).parent.glob(f"*1999-{m:02d}-{d:02d}*.pdf")) + self.assertEqual( + 1, + len(list(pathlib.Path(f.name).parent.glob(f"*1999-{m:02d}-{d:02d}*.pdf"))), + "We must have one and only one file that have been created following the crash of the validation", + ) + os.remove(files[0]) + + self.assertEqual(1 + mail_count, len(mail.outbox))