diff --git a/rdmo/core/exceptions.py b/rdmo/core/exceptions.py index cde361b78b..84fedabe76 100644 --- a/rdmo/core/exceptions.py +++ b/rdmo/core/exceptions.py @@ -1,2 +1,9 @@ class RDMOException(Exception): pass + + +class MailSendError(RDMOException): + def __init__(self, reason, original_exception=None): + super().__init__(reason) + self.reason = reason + self.original_exception = original_exception diff --git a/rdmo/core/mail.py b/rdmo/core/mail.py index 641935923a..07cfa12b40 100644 --- a/rdmo/core/mail.py +++ b/rdmo/core/mail.py @@ -1,7 +1,29 @@ +import smtplib + from django.conf import settings from django.contrib.sites.models import Site from django.core.mail import EmailMessage +from rdmo.core.exceptions import MailSendError + + +def _format_smtp_exception(exception): + if isinstance(exception, smtplib.SMTPRecipientsRefused): + details = [] + for _recipient, (_code, response) in exception.recipients.items(): + if isinstance(response, bytes): + response = response.decode(errors='replace') + details.append(response) + return '; '.join(details) if details else str(exception) + + if isinstance(exception, smtplib.SMTPResponseException): + response = exception.smtp_error + if isinstance(response, bytes): + response = response.decode(errors='replace') + return f'{response}' if response else str(exception) + + return str(exception) + def send_mail(subject, message, from_email=None, to=None, cc=None, bcc=None, reply_to=None, attachments=None): site = Site.objects.get_current() @@ -11,4 +33,7 @@ def send_mail(subject, message, from_email=None, to=None, cc=None, bcc=None, rep from_email = settings.DEFAULT_FROM_EMAIL mail = EmailMessage(subject, message, from_email, to=to, cc=cc, bcc=bcc, reply_to=reply_to, attachments=attachments) - mail.send() + try: + mail.send() + except smtplib.SMTPException as e: + raise MailSendError(_format_smtp_exception(e), original_exception=e) from e diff --git a/rdmo/core/tests/test_mail.py b/rdmo/core/tests/test_mail.py index 939a4d12aa..ad92b6c535 100644 --- a/rdmo/core/tests/test_mail.py +++ b/rdmo/core/tests/test_mail.py @@ -1,6 +1,11 @@ +import smtplib + +import pytest + from django.conf import settings from django.core import mail +from rdmo.core.exceptions import MailSendError from rdmo.core.mail import send_mail @@ -71,3 +76,23 @@ def test_send_mail_from_attachments(db): assert mail.outbox[0].attachments == [ ('Attachment', b'attachment', 'plain/text') ] + + +def test_send_mail_recipient_refused(db, mocker): + mocker.patch( + 'rdmo.core.mail.EmailMessage.send', + side_effect=smtplib.SMTPRecipientsRefused({ + 'name@non-existent-domain.abc': ( + 550, + b'1.2.3 : Recipient address rejected: Domain not found' + ) + }) + ) + + with pytest.raises(MailSendError) as e: + send_mail('Subject', 'Message', to=['name@non-existent-domain.abc']) + + assert e.value.reason == ( + '1.2.3 : Recipient address rejected: Domain not found' + ) + assert isinstance(e.value.original_exception, smtplib.SMTPRecipientsRefused) diff --git a/rdmo/projects/tests/test_view_membership.py b/rdmo/projects/tests/test_view_membership.py index 4eef8c5c98..46d6a40b09 100644 --- a/rdmo/projects/tests/test_view_membership.py +++ b/rdmo/projects/tests/test_view_membership.py @@ -3,6 +3,7 @@ from django.core import mail from django.urls import reverse +from rdmo.core.exceptions import MailSendError from rdmo.projects.models import Invite, Membership users = ( @@ -164,6 +165,33 @@ def test_membership_create_post_mail_error(db, client, username, password, membe assert response.status_code == 302 +def test_membership_create_post_mail_send_error(db, client, settings, mocker): + settings.PROJECT_SEND_INVITE = True + client.login(username='owner', password='owner') + + mocker.patch( + 'rdmo.projects.utils.send_mail', + side_effect=MailSendError( + '1.2.3 : Recipient address rejected: Domain not found' + ) + ) + + url = reverse('membership_create', args=[1]) + response = client.post(url, { + 'username_or_email': 'someuser@example.com', + 'role': 'guest' + }) + + assert response.status_code == 200 + assert not Invite.objects.filter(project_id=1, user=None, + email='someuser@example.com').exists() + assert len(mail.outbox) == 0 + assert response.context_data['form'].non_field_errors()[0] == ( + 'Could not send e-mail: 1.2.3 : ' + 'Recipient address rejected: Domain not found' + ) + + @pytest.mark.parametrize('username,password', users) @pytest.mark.parametrize('membership_role', membership_roles) def test_membership_create_post_silent(db, client, username, password, membership_role): diff --git a/rdmo/projects/tests/test_viewset_project_contact.py b/rdmo/projects/tests/test_viewset_project_contact.py index 06851c21cf..e82985d8cd 100644 --- a/rdmo/projects/tests/test_viewset_project_contact.py +++ b/rdmo/projects/tests/test_viewset_project_contact.py @@ -3,6 +3,7 @@ from django.core import mail from django.urls import reverse +from rdmo.core.exceptions import MailSendError from rdmo.projects.models import Project, Value from rdmo.questions.models import Page, Question @@ -191,3 +192,27 @@ def test_contact_post_error(db, client, username, password, project_id): assert response.status_code == 401 assert len(mail.outbox) == 0 + + +def test_contact_post_mail_send_error(db, client, mocker): + client.login(username='owner', password='owner') + + mocker.patch( + 'rdmo.projects.utils.send_mail', + side_effect=MailSendError( + '1.2.3 : Recipient address rejected: Domain not found' + ) + ) + + url = reverse(urlnames['contact'], args=[1]) + response = client.post(url, { + 'subject': 'Test subject', + 'message': 'Test message' + }) + + assert response.status_code == 400 + assert response.json()['non_field_errors'][0] == ( + 'Could not send e-mail: 1.2.3 : ' + 'Recipient address rejected: Domain not found' + ) + assert len(mail.outbox) == 0 diff --git a/rdmo/projects/tests/test_viewset_project_invite.py b/rdmo/projects/tests/test_viewset_project_invite.py index a99fc9fd97..26b3cf020d 100644 --- a/rdmo/projects/tests/test_viewset_project_invite.py +++ b/rdmo/projects/tests/test_viewset_project_invite.py @@ -3,6 +3,8 @@ from django.contrib.auth import get_user_model from django.urls import reverse +from rdmo.core.exceptions import MailSendError + from ..models import Invite users = ( @@ -38,6 +40,7 @@ projects = [1, 11] invites = [1, 2] +excluded_invite_ids = [1, 2] @pytest.mark.parametrize('username,password', users) @@ -121,6 +124,31 @@ def test_create_email(db, client, username, password, project_id): assert response.status_code == 404 +def test_create_email_mail_send_error(db, client, settings, mocker): + settings.PROJECT_SEND_INVITE = True + client.login(username='owner', password='owner') + + mocker.patch( + 'rdmo.projects.utils.send_mail', + side_effect=MailSendError( + '1.2.3 : Recipient address rejected: Domain not found' + ) + ) + + url = reverse(urlnames['list'], args=[1]) + response = client.post(url, { + 'email': 'name@non-existent-domain.abc', + 'role': 'guest' + }) + + assert response.status_code == 400 + assert response.json()['non_field_errors'][0] == ( + 'Could not send e-mail: 1.2.3 : ' + 'Recipient address rejected: Domain not found' + ) + assert not Invite.objects.exclude(id__in=excluded_invite_ids).exists() + + def test_create_error(db, client): client.login(username='owner', password='owner') diff --git a/rdmo/projects/views/membership.py b/rdmo/projects/views/membership.py index 54242b57e9..3d0d0b254a 100644 --- a/rdmo/projects/views/membership.py +++ b/rdmo/projects/views/membership.py @@ -1,13 +1,16 @@ import logging from django.conf import settings +from django.db import transaction from django.http import HttpResponseBadRequest, HttpResponseForbidden, HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.urls import reverse +from django.utils.translation import gettext_lazy as _ from django.views.generic import DeleteView, UpdateView from django.views.generic.edit import FormView from rdmo.accounts.utils import is_site_manager +from rdmo.core.exceptions import MailSendError from rdmo.core.views import ObjectPermissionMixin, RedirectViewMixin from ..forms import MembershipCreateForm @@ -43,9 +46,14 @@ def get_success_url(self): return self.project.get_absolute_url() def form_valid(self, form): - invite = form.save() - if invite is not None and settings.PROJECT_SEND_INVITE: - send_invite_email(self.request, invite) + try: + with transaction.atomic(): + invite = form.save() + if invite is not None and settings.PROJECT_SEND_INVITE: + send_invite_email(self.request, invite) + except MailSendError as e: + form.add_error(None, _('Could not send e-mail: %(reason)s') % {'reason': e.reason}) + return self.form_invalid(form) return super().form_valid(form) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index 84782db09f..60651b56b8 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -3,6 +3,7 @@ from django.conf import settings from django.contrib.sites.shortcuts import get_current_site from django.core.exceptions import ObjectDoesNotExist +from django.db import transaction from django.db.models import OuterRef, Prefetch, Q, Subquery from django.db.models.functions import Coalesce, Greatest from django.http import Http404, HttpResponseRedirect @@ -23,6 +24,7 @@ from rest_framework_extensions.mixins import NestedViewSetMixin from rdmo.conditions.models import Condition +from rdmo.core.exceptions import MailSendError from rdmo.core.permissions import HasModelPermission from rdmo.core.utils import human2bytes, is_truthy, return_file_response from rdmo.options.models import OptionSet @@ -445,7 +447,12 @@ def contact(self, request, pk): message = request.data.get('message') if subject and message: - send_contact_message(request, subject, message) + try: + send_contact_message(request, subject, message) + except MailSendError as e: + raise ValidationError({'non_field_errors': [ + _('Could not send e-mail: %(reason)s') % {'reason': e.reason} + ]}) from e return Response(status=status.HTTP_204_NO_CONTENT) else: raise ValidationError({ @@ -562,9 +569,15 @@ def get_serializer_class(self): return ProjectInviteSerializer def perform_create(self, serializer): - super().perform_create(serializer) - if settings.PROJECT_SEND_INVITE: - send_invite_email(self.request, serializer.instance) + try: + with transaction.atomic(): + super().perform_create(serializer) + if settings.PROJECT_SEND_INVITE: + send_invite_email(self.request, serializer.instance) + except MailSendError as e: + raise ValidationError({'non_field_errors': [ + _('Could not send e-mail: %(reason)s') % {'reason': e.reason} + ]}) from e class ProjectIssueViewSet(ProjectNestedViewSetMixin, ListModelMixin, RetrieveModelMixin,