From cb9ae34418ba60a95f46a5fa6db92f62a5d4aca7 Mon Sep 17 00:00:00 2001 From: Jean-Romain Garnier Date: Sun, 19 Apr 2020 20:06:34 +0200 Subject: [PATCH] Make emails throw timeout errors, and gracefully handle them --- cotisations/models.py | 11 ++++++++--- cotisations/utils.py | 27 +++++++++++++++++++++++---- cotisations/views.py | 2 +- re2o/settings.py | 3 +++ re2o/utils.py | 18 ++++++++++++++++++ tickets/models.py | 9 ++++++--- tickets/views.py | 2 ++ users/models.py | 32 +++++++++++++++++++++++--------- users/views.py | 8 ++++++++ 9 files changed, 92 insertions(+), 20 deletions(-) diff --git a/cotisations/models.py b/cotisations/models.py index 215fcdb5..503ad244 100644 --- a/cotisations/models.py +++ b/cotisations/models.py @@ -330,9 +330,14 @@ class Facture(BaseInvoice): def save(self, *args, **kwargs): super(Facture, self).save(*args, **kwargs) + + request = None + if "request" in kwargs: + request = kwargs["request"] + if not self.__original_valid and self.valid: self.reorder_purchases() - send_mail_invoice(self) + send_mail_invoice(self, request) if ( self.is_subscription() and not self.__original_control @@ -340,7 +345,7 @@ class Facture(BaseInvoice): and CotisationsOption.get_cached_value("send_voucher_mail") and self.user.is_adherent() ): - send_mail_voucher(self) + send_mail_voucher(self, request) def __str__(self): return str(self.user) + " " + str(self.date) @@ -870,7 +875,7 @@ class Paiement(RevMixin, AclMixin, models.Model): # So make this invoice valid, trigger send mail invoice.valid = True - invoice.save() + invoice.save(request) # In case a cotisation was bought, inform the user, the # cotisation time has been extended too diff --git a/cotisations/utils.py b/cotisations/utils.py index 95672dd4..c370fe59 100644 --- a/cotisations/utils.py +++ b/cotisations/utils.py @@ -23,6 +23,9 @@ import os from django.template.loader import get_template from django.core.mail import EmailMessage +from django.utils.translation import ugettext_lazy as _ +from django.contrib import messages +from smtplib import SMTPException from .tex import create_pdf from preferences.models import AssoOption, GeneralOption, CotisationsOption, Mandate @@ -43,7 +46,21 @@ def find_payment_method(payment): return None -def send_mail_invoice(invoice): +def send_mail(mail, request): + """Wrapper for Django's EmailMessage.send which handles errors""" + try: + mail.send() + except SMTPException as e: + if request: + messages.error( + request, + _("Failed to send email: %(error)s.") % { + "error": e, + }, + ) + + +def send_mail_invoice(invoice, request=None): """Creates the pdf of the invoice and sends it by email to the client""" purchases_info = [] for purchase in invoice.vente_set.all(): @@ -90,10 +107,11 @@ def send_mail_invoice(invoice): [invoice.user.get_mail], attachments=[("invoice.pdf", pdf, "application/pdf")], ) - mail.send() + + send_mail(mail, request) -def send_mail_voucher(invoice): +def send_mail_voucher(invoice, request=None): """Creates a voucher from an invoice and sends it by email to the client""" president = Mandate.get_mandate(invoice.date).president ctx = { @@ -126,4 +144,5 @@ def send_mail_voucher(invoice): [invoice.user.get_mail], attachments=[("voucher.pdf", pdf, "application/pdf")], ) - mail.send() + + send_mail(mail, request) diff --git a/cotisations/views.py b/cotisations/views.py index cc80e22d..def26cda 100644 --- a/cotisations/views.py +++ b/cotisations/views.py @@ -1008,7 +1008,7 @@ def credit_solde(request, user, **_kwargs): else: price_ok = True if price_ok: - invoice.save() + invoice.save(request) Vente.objects.create( facture=invoice, name="solde", diff --git a/re2o/settings.py b/re2o/settings.py index 3d883615..c6df1a69 100644 --- a/re2o/settings.py +++ b/re2o/settings.py @@ -181,6 +181,9 @@ MEDIA_URL = os.path.join(BASE_DIR, "/media/") # Models to use for graphs GRAPH_MODELS = {"all_applications": True, "group_models": True} +# Timeout when sending emails through Django (in seconds) +EMAIL_TIMEOUT = 10 + # Activate API if "api" in INSTALLED_APPS: from api.settings import * diff --git a/re2o/utils.py b/re2o/utils.py index 7c49ff0a..0e735720 100644 --- a/re2o/utils.py +++ b/re2o/utils.py @@ -39,6 +39,10 @@ from __future__ import unicode_literals from django.utils import timezone from django.db.models import Q from django.contrib.auth.models import Permission +from django.utils.translation import ugettext_lazy as _ +from django.core.mail import send_mail as django_send_mail +from django.contrib import messages +from smtplib import SMTPException from cotisations.models import Cotisation, Facture, Vente from machines.models import Interface, Machine @@ -213,3 +217,17 @@ def remove_user_room(room, force=True): if force or not user.has_access(): user.room = None user.save() + + +def send_mail(request, *args, **kwargs): + """Wrapper for Django's send_mail which handles errors""" + try: + kwargs["fail_silently"] = request is None + django_send_mail(*args, **kwargs) + except SMTPException as e: + messages.error( + request, + _("Failed to send email: %(error)s.") % { + "error": e, + }, + ) diff --git a/tickets/models.py b/tickets/models.py index 55827097..5b74248c 100644 --- a/tickets/models.py +++ b/tickets/models.py @@ -1,11 +1,11 @@ from django.db import models from django.utils.translation import ugettext_lazy as _ -from django.core.mail import send_mail from django.template import loader from django.db.models.signals import post_save from django.dispatch import receiver from re2o.mixins import AclMixin +from re2o.utils import send_mail from preferences.models import GeneralOption @@ -38,6 +38,7 @@ class Ticket(AclMixin, models.Model): help_text=_("An email address to get back to you."), max_length=100, null=True ) solved = models.BooleanField(default=False) + request = None class Meta: permissions = (("view_tickets", _("Can view a ticket object")),) @@ -50,7 +51,7 @@ class Ticket(AclMixin, models.Model): else: return _("Anonymous ticket. Date: %s.") % (self.date) - def publish_mail(self): + def publish_mail(self, request=None): site_url = GeneralOption.objects.first().main_site_url to_addr = Preferences.objects.first().publish_address context = {"ticket": self, "site_url": site_url} @@ -62,7 +63,9 @@ class Ticket(AclMixin, models.Model): else: obj = "New ticket opened" template = loader.get_template("tickets/publication_mail_en") + send_mail( + request, obj, template.render(context), GeneralOption.get_cached_value("email_from"), @@ -108,4 +111,4 @@ def ticket_post_save(**kwargs): if kwargs["created"]: if Preferences.objects.first().publish_address: ticket = kwargs["instance"] - ticket.publish_mail() + ticket.publish_mail(ticket.request) diff --git a/tickets/views.py b/tickets/views.py index d330719a..03cc535c 100644 --- a/tickets/views.py +++ b/tickets/views.py @@ -60,6 +60,8 @@ def new_ticket(request): if ticketform.is_valid(): email = ticketform.cleaned_data.get("email") ticket = ticketform.save(commit=False) + ticket.request = request + if request.user.is_authenticated: ticket.user = request.user ticket.save() diff --git a/users/models.py b/users/models.py index 6b260170..55acd051 100755 --- a/users/models.py +++ b/users/models.py @@ -60,7 +60,6 @@ from django.db.models.signals import post_save, post_delete, m2m_changed from django.dispatch import receiver from django.utils.functional import cached_property from django.template import loader -from django.core.mail import send_mail from django.core.urlresolvers import reverse from django.db import transaction from django.utils import timezone @@ -84,6 +83,7 @@ from re2o.settings import LDAP, GID_RANGES, UID_RANGES from re2o.field_permissions import FieldPermissionModelMixin from re2o.mixins import AclMixin, RevMixin from re2o.base import smtp_check +from re2o.utils import send_mail from cotisations.models import Cotisation, Facture, Paiement, Vente from machines.models import Domain, Interface, Machine, regen @@ -244,6 +244,7 @@ class User( REQUIRED_FIELDS = ["surname", "email"] objects = UserManager() + request = None class Meta: permissions = ( @@ -749,7 +750,7 @@ class User( name__in=list(queryset_users.values_list("pseudo", flat=True)) ) - def notif_inscription(self): + def notif_inscription(self, request=None): """ Prend en argument un objet user, envoie un mail de bienvenue """ template = loader.get_template("users/email_welcome") mailmessageoptions, _created = MailMessageOption.objects.get_or_create() @@ -761,7 +762,9 @@ class User( "welcome_mail_en": mailmessageoptions.welcome_mail_en, "pseudo": self.pseudo, } + send_mail( + request, "Bienvenue au %(name)s / Welcome to %(name)s" % {"name": AssoOption.get_cached_value("name")}, "", @@ -769,7 +772,6 @@ class User( [self.email], html_message=template.render(context), ) - return def reset_passwd_mail(self, request): """ Prend en argument un request, envoie un mail de @@ -789,7 +791,9 @@ class User( ), "expire_in": str(GeneralOption.get_cached_value("req_expire_hrs")), } + send_mail( + request, "Changement de mot de passe de %(name)s / Password change for " "%(name)s" % {"name": AssoOption.get_cached_value("name")}, template.render(context), @@ -797,7 +801,6 @@ class User( [req.user.email], fail_silently=False, ) - return def send_confirm_email_if_necessary(self, request): """Update the user's email state: @@ -873,7 +876,9 @@ class User( "confirm_before_fr": self.confirm_email_before_date().strftime("%d/%m/%Y"), "confirm_before_en": self.confirm_email_before_date().strftime("%Y-%m-%d"), } + send_mail( + request, "Confirmation du mail de %(name)s / Email confirmation for " "%(name)s" % {"name": AssoOption.get_cached_value("name")}, template.render(context), @@ -883,7 +888,7 @@ class User( ) return - def autoregister_machine(self, mac_address, nas_type): + def autoregister_machine(self, mac_address, nas_type, request=None): """ Fonction appellée par freeradius. Enregistre la mac pour une machine inconnue sur le compte de l'user""" allowed, _message, _rights = Machine.can_create(self, self.id) @@ -927,7 +932,9 @@ class User( "asso_email": AssoOption.get_cached_value("contact"), "pseudo": self.pseudo, } + send_mail( + None, "Ajout automatique d'une machine / New machine autoregistered", "", GeneralOption.get_cached_value("email_from"), @@ -936,7 +943,7 @@ class User( ) return - def notif_disable(self): + def notif_disable(self, request=None): """Envoi un mail de notification informant que l'adresse mail n'a pas été confirmée""" template = loader.get_template("users/email_disable_notif") context = { @@ -945,7 +952,9 @@ class User( "asso_email": AssoOption.get_cached_value("contact"), "site_name": GeneralOption.get_cached_value("site_name"), } + send_mail( + request, "Suspension automatique / Automatic suspension", template.render(context), GeneralOption.get_cached_value("email_from"), @@ -1509,8 +1518,10 @@ def user_post_save(**kwargs): is_created = kwargs["created"] user = kwargs["instance"] EMailAddress.objects.get_or_create(local_part=user.pseudo.lower(), user=user) + if is_created: - user.notif_inscription() + user.notif_inscription(user.request) + user.state_sync() user.ldap_sync( base=True, access_refresh=True, mac_refresh=False, group_refresh=True @@ -1737,13 +1748,14 @@ class Ban(RevMixin, AclMixin, models.Model): date_start = models.DateTimeField(auto_now_add=True) date_end = models.DateTimeField() state = models.IntegerField(choices=STATES, default=STATE_HARD) + request = None class Meta: permissions = (("view_ban", _("Can view a ban object")),) verbose_name = _("ban") verbose_name_plural = _("bans") - def notif_ban(self): + def notif_ban(self, request=None): """ Prend en argument un objet ban, envoie un mail de notification """ template = loader.get_template("users/email_ban_notif") context = { @@ -1752,7 +1764,9 @@ class Ban(RevMixin, AclMixin, models.Model): "date_end": self.date_end, "asso_name": AssoOption.get_cached_value("name"), } + send_mail( + request, "Déconnexion disciplinaire / Disciplinary disconnection", template.render(context), GeneralOption.get_cached_value("email_from"), @@ -1795,7 +1809,7 @@ def ban_post_save(**kwargs): user.ldap_sync(base=False, access_refresh=True, mac_refresh=False) regen("mailing") if is_created: - ban.notif_ban() + ban.notif_ban(ban.request) regen("dhcp") regen("mac_ip_list") if user.has_access(): diff --git a/users/views.py b/users/views.py index 28fb107c..1c544ba6 100644 --- a/users/views.py +++ b/users/views.py @@ -119,6 +119,8 @@ def new_user(request): """ Vue de création d'un nouvel utilisateur, envoie un mail pour le mot de passe""" user = AdherentCreationForm(request.POST or None, user=request.user) + user.request = request + GTU_sum_up = GeneralOption.get_cached_value("GTU_sum_up") GTU = GeneralOption.get_cached_value("GTU") is_set_password_allowed = OptionalUser.get_cached_value("allow_set_password_during_user_creation") @@ -165,6 +167,8 @@ def new_club(request): """ Vue de création d'un nouveau club, envoie un mail pour le mot de passe""" club = ClubForm(request.POST or None, user=request.user) + club.request = request + if club.is_valid(): club = club.save(commit=False) club.save() @@ -368,6 +372,8 @@ def add_ban(request, user, userid): Syntaxe : JJ/MM/AAAA , heure optionnelle, prend effet immédiatement""" ban_instance = Ban(user=user) ban = BanForm(request.POST or None, instance=ban_instance) + ban.request = request + if ban.is_valid(): ban.save() messages.success(request, _("The ban was added.")) @@ -386,6 +392,8 @@ def edit_ban(request, ban_instance, **_kwargs): (a fortiori bureau) Syntaxe : JJ/MM/AAAA , heure optionnelle, prend effet immédiatement""" ban = BanForm(request.POST or None, instance=ban_instance) + ban.request = request + if ban.is_valid(): if ban.changed_data: ban.save()