From 08e3d9f84efc5c17cb4df926a1f631aca2871375 Mon Sep 17 00:00:00 2001 From: Evan Date: Sun, 22 Oct 2023 22:35:20 +0000 Subject: [PATCH] Address CRs --- dmoj/settings.py | 2 +- judge/forms.py | 6 ++---- judge/utils/mail.py | 9 ++++++--- judge/views/register.py | 6 ++---- judge/views/user.py | 11 ++++++----- templates/registration/email_change_notify_email.html | 4 ++-- templates/registration/email_change_notify_email.txt | 4 ++-- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/dmoj/settings.py b/dmoj/settings.py index 2e2e2e7636..d9dbfcdbda 100644 --- a/dmoj/settings.py +++ b/dmoj/settings.py @@ -115,7 +115,7 @@ DMOJ_EMAIL_CHANGE_LIMIT_WINDOW = 3600 DMOJ_EMAIL_CHANGE_LIMIT_COUNT = 10 # Number of minutes before an email change request activation key expires -DMOJ_EMAIL_CHANGE_EXPIRY_MINUTES = 10 +DMOJ_EMAIL_CHANGE_EXPIRY_MINUTES = 60 # At the bare minimum, dark and light theme CSS file locations must be declared DMOJ_THEME_CSS = { diff --git a/judge/forms.py b/judge/forms.py index dbfbf80eee..87003e56f2 100644 --- a/judge/forms.py +++ b/judge/forms.py @@ -18,7 +18,7 @@ from django_ace import AceWidget from judge.models import Contest, Language, Organization, Problem, ProblemPointsVote, Profile, Submission, \ WebAuthnCredential -from judge.utils.mail import is_email_address_bad +from judge.utils.mail import validate_email_domain from judge.utils.subscription import newsletter_id from judge.widgets import HeavyPreviewPageDownWidget, Select2MultipleWidget, Select2Widget @@ -108,9 +108,7 @@ def __init__(self, *args, user, **kwargs): def clean_email(self): if User.objects.filter(email=self.cleaned_data['email']).exists(): raise ValidationError(_('This email address is already taken.')) - if is_email_address_bad(self.cleaned_data['email']): - raise ValidationError(_('Your email provider is not allowed due to history of abuse. ' - 'Please use a reputable email provider.')) + validate_email_domain(self.cleaned_data['email']) return self.cleaned_data['email'] def clean_password(self): diff --git a/judge/utils/mail.py b/judge/utils/mail.py index afc4414fa3..44a857a878 100644 --- a/judge/utils/mail.py +++ b/judge/utils/mail.py @@ -2,17 +2,20 @@ from django.conf import settings from django.core.mail import EmailMultiAlternatives +from django.forms import ValidationError from django.template import loader +from django.utils.translation import gettext bad_mail_regex = list(map(re.compile, settings.BAD_MAIL_PROVIDER_REGEX)) -def is_email_address_bad(email): +def validate_email_domain(email): if '@' in email: domain = email.split('@')[-1].lower() - return domain in settings.BAD_MAIL_PROVIDERS or any(regex.match(domain) for regex in bad_mail_regex) - return False + if domain in settings.BAD_MAIL_PROVIDERS or any(regex.match(domain) for regex in bad_mail_regex): + raise ValidationError(gettext('Your email provider is not allowed due to history of abuse. ' + 'Please use a reputable email provider.')) def send_mail( diff --git a/judge/views/register.py b/judge/views/register.py index 6eb08a4bca..f5cdd64029 100644 --- a/judge/views/register.py +++ b/judge/views/register.py @@ -12,7 +12,7 @@ from sortedm2m.forms import SortedMultipleChoiceField from judge.models import Language, Organization, Profile, TIMEZONE -from judge.utils.mail import is_email_address_bad +from judge.utils.mail import validate_email_domain from judge.utils.recaptcha import ReCaptchaField, ReCaptchaWidget from judge.utils.subscription import Subscription, newsletter_id from judge.widgets import Select2MultipleWidget, Select2Widget @@ -40,9 +40,7 @@ def clean_email(self): if User.objects.filter(email=self.cleaned_data['email']).exists(): raise forms.ValidationError(gettext('The email address "%s" is already taken. Only one registration ' 'is allowed per address.') % self.cleaned_data['email']) - if is_email_address_bad(self.cleaned_data['email']): - raise forms.ValidationError(gettext('Your email provider is not allowed due to history of abuse. ' - 'Please use a reputable email provider.')) + validate_email_domain(self.cleaned_data['email']) return self.cleaned_data['email'] def clean_organizations(self): diff --git a/judge/views/user.py b/judge/views/user.py index 20277f57a2..7d006169d8 100644 --- a/judge/views/user.py +++ b/judge/views/user.py @@ -545,13 +545,14 @@ def form_valid(self, form): 'activation_key': activation_key, 'new_email': new_email, } + # When from_email is none, we use the DEFAULT_FROM_EMAIL setting send_mail( - self.notify_subject_template_name, self.notify_email_template_name, context, None, self.request.user.email, - self.notify_html_email_template_name, + self.notify_subject_template_name, self.notify_email_template_name, context, from_email=None, + to_email=self.request.user.email, html_email_template_name=self.notify_html_email_template_name, ) send_mail( - self.activate_subject_template_name, self.activate_email_template_name, context, None, new_email, - self.activate_html_email_template_name, + self.activate_subject_template_name, self.activate_email_template_name, context, from_email=None, + to_email=new_email, html_email_template_name=self.activate_html_email_template_name, ) return generic_message( @@ -587,7 +588,7 @@ def get(self, request, *args, **kwargs): except (binascii.Error, signing.BadSignature): raise ValueError(_('Invalid activation key. Please try again.')) except signing.SignatureExpired: - raise ValueError(_('This request is expired. Please try again.')) + raise ValueError(_('This request has expired. Please try again.')) if data['id'] != request.user.id: raise ValueError( _('Please try again from the account this email change was originally requested from.'), diff --git a/templates/registration/email_change_notify_email.html b/templates/registration/email_change_notify_email.html index e8b78ec445..bd7dea62ef 100644 --- a/templates/registration/email_change_notify_email.html +++ b/templates/registration/email_change_notify_email.html @@ -14,10 +14,10 @@ {% if site_admin_email %} {% with link='%(email)s'|safe|format(email=site_admin_email) %} -{{ _('If this was not you, please email us immediately at %(email)s.', email=link) }} +{{ _('If this was not you, please change your password and email us immediately at %(email)s.', email=link) }} {% endwith %} {% else %} -{{ _('If this was not you, please reply to this email immediately.') }} +{{ _('If this was not you, please change your password and reply to this email immediately.') }} {% endif %} diff --git a/templates/registration/email_change_notify_email.txt b/templates/registration/email_change_notify_email.txt index 132eddb70e..103df5a5db 100644 --- a/templates/registration/email_change_notify_email.txt +++ b/templates/registration/email_change_notify_email.txt @@ -3,7 +3,7 @@ {{ _('If this was you, no further action is required.') }} {% if site_admin_email %} -{{ _('If this was not you, please email us immediately at %(email)s.', email=site_admin_email) }} +{{ _('If this was not you, please change your password and email us immediately at %(email)s.', email=site_admin_email) }} {% else %} -{{ _('If this was not you, please reply to this email immediately.') }} +{{ _('If this was not you, please change your password and reply to this email immediately.') }} {% endif %}