From ffe7501d545d447f878b2f9ea16f7290db1a6139 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 5 Mar 2020 15:41:53 +0100 Subject: [PATCH 2/2] misc: use email domain validation (#40200) --- src/authentic2/forms/fields.py | 9 +++++++- src/authentic2/forms/passwords.py | 4 ++-- src/authentic2/forms/profile.py | 13 ++++++++--- src/authentic2/forms/registration.py | 5 +++-- src/authentic2/manager/forms.py | 4 ++-- tests/settings.py | 2 ++ tests/test_manager.py | 21 ++++++++++++++++++ tests/test_registration.py | 32 ++++++++++++++++------------ tests/utils.py | 9 -------- 9 files changed, 66 insertions(+), 33 deletions(-) diff --git a/src/authentic2/forms/fields.py b/src/authentic2/forms/fields.py index 57ad4507..b72c8f1b 100644 --- a/src/authentic2/forms/fields.py +++ b/src/authentic2/forms/fields.py @@ -17,7 +17,7 @@ import warnings import io -from django.forms import CharField, FileField, ValidationError +from django.forms import CharField, FileField, ValidationError, EmailField from django.forms.fields import FILE_INPUT_CONTRADICTION from django.utils.translation import ugettext_lazy as _ from django.core.files import File @@ -26,6 +26,7 @@ from authentic2 import app_settings from authentic2.passwords import password_help_text, validate_password from authentic2.forms.widgets import (PasswordInput, NewPasswordInput, CheckPasswordInput, ProfileImageInput) +from authentic2.validators import email_validator import PIL.Image @@ -111,3 +112,9 @@ class ProfileImageField(FileField): image = image.crop(box) return image.resize([width, height], PIL.Image.ANTIALIAS) + + +class ValidatedEmailField(EmailField): + def validate(self, value): + super(ValidatedEmailField, self).validate(value) + email_validator(value) diff --git a/src/authentic2/forms/passwords.py b/src/authentic2/forms/passwords.py index bb04b5a4..62b769b9 100644 --- a/src/authentic2/forms/passwords.py +++ b/src/authentic2/forms/passwords.py @@ -25,7 +25,7 @@ from django.utils.translation import ugettext_lazy as _ from .. import models, hooks, app_settings, utils from ..backends import get_user_queryset -from .fields import PasswordField, NewPasswordField, CheckPasswordField +from .fields import PasswordField, NewPasswordField, CheckPasswordField, ValidatedEmailField from .utils import NextUrlFormMixin @@ -35,7 +35,7 @@ logger = logging.getLogger(__name__) class PasswordResetForm(forms.Form): next_url = forms.CharField(widget=forms.HiddenInput, required=False) - email = forms.EmailField( + email = ValidatedEmailField( label=_("Email"), max_length=254) def save(self): diff --git a/src/authentic2/forms/profile.py b/src/authentic2/forms/profile.py index 3d3e9f86..3054ff3a 100644 --- a/src/authentic2/forms/profile.py +++ b/src/authentic2/forms/profile.py @@ -20,10 +20,12 @@ from django.forms.models import modelform_factory as dj_modelform_factory from django import forms from django.utils.translation import ugettext_lazy as _, ugettext -from ..custom_user.models import User -from .. import app_settings, models +from authentic2 import app_settings, models +from authentic2.custom_user.models import User +from authentic2.validators import email_validator from .utils import NextUrlFormMixin from .mixins import LockedFieldFormMixin +from .fields import ValidatedEmailField class DeleteAccountForm(forms.Form): @@ -41,7 +43,7 @@ class DeleteAccountForm(forms.Form): class EmailChangeFormNoPassword(forms.Form): - email = forms.EmailField(label=_('New email')) + email = ValidatedEmailField(label=_('New email')) def __init__(self, user, *args, **kwargs): self.user = user @@ -122,6 +124,11 @@ class BaseUserForm(LockedFieldFormMixin, forms.ModelForm): self.save_m2m = save_m2m return result + def clean_email(self): + email = self.cleaned_data['email'] + email_validator(email) + return email + class EditProfileForm(NextUrlFormMixin, BaseUserForm): pass diff --git a/src/authentic2/forms/registration.py b/src/authentic2/forms/registration.py index 61d645a9..60283ede 100644 --- a/src/authentic2/forms/registration.py +++ b/src/authentic2/forms/registration.py @@ -19,7 +19,7 @@ import re from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from django.utils.translation import ugettext_lazy as _, ugettext -from django.forms import Form, EmailField +from django.forms import Form from django.contrib.auth.models import BaseUserManager, Group @@ -28,6 +28,7 @@ from authentic2.a2_rbac.models import OrganizationalUnit from .. import app_settings, models from . import profile as profile_forms +from .fields import ValidatedEmailField User = get_user_model() @@ -36,7 +37,7 @@ class RegistrationForm(Form): error_css_class = 'form-field-error' required_css_class = 'form-field-required' - email = EmailField(label=_('Email')) + email = ValidatedEmailField(label=_('Email')) def __init__(self, *args, **kwargs): super(RegistrationForm, self).__init__(*args, **kwargs) diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py index 7c3ca5fd..aa348ce9 100644 --- a/src/authentic2/manager/forms.py +++ b/src/authentic2/manager/forms.py @@ -29,7 +29,7 @@ from django.core.exceptions import ValidationError from authentic2.passwords import generate_password from authentic2.utils import send_templated_mail -from authentic2.forms.fields import NewPasswordField, CheckPasswordField +from authentic2.forms.fields import NewPasswordField, CheckPasswordField, ValidatedEmailField from django_rbac.models import Operation from django_rbac.utils import get_ou_model, get_role_model, get_permission_model @@ -678,7 +678,7 @@ def get_role_form_class(): # we need a model form so that we can use a BaseEditView, a simple Form # would not work class UserChangeEmailForm(CssClass, FormWithRequest, forms.ModelForm): - new_email = forms.EmailField(label=_('Email')) + new_email = ValidatedEmailField(label=_('Email')) def __init__(self, *args, **kwargs): initial = kwargs.setdefault('initial', {}) diff --git a/tests/settings.py b/tests/settings.py index b18fb702..3161bc75 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -42,3 +42,5 @@ if 'postgres' in DATABASES['default']['ENGINE']: ALLOWED_HOSTS = ALLOWED_HOSTS + ['example.net', 'cache1.example.com', 'cache2.example.com'] A2_AUTH_KERBEROS_ENABLED = False + +A2_VALIDATE_EMAIL_DOMAIN = False diff --git a/tests/test_manager.py b/tests/test_manager.py index cba6179e..1a89c1b2 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -24,6 +24,7 @@ from django.core import mail from webtest import Upload from authentic2.a2_rbac.utils import get_default_ou +from authentic2.validators import EmailValidator from django_rbac.utils import get_ou_model, get_role_model from django.contrib.auth import get_user_model @@ -293,6 +294,26 @@ def test_manager_create_user(superuser_or_admin, app, settings): assert urlparse(resp['Location']).path == url2 +def test_manager_create_user_email_validation(superuser_or_admin, app, settings, monkeypatch): + settings.A2_VALIDATE_EMAIL_DOMAIN = True + monkeypatch.setattr(EmailValidator, 'check_mxs', lambda x, y: []) + ou1 = OU.objects.create(name='OU1', slug='ou1') + + url = reverse('a2-manager-user-add', kwargs={'ou_pk': ou1.pk}) + resp = login(app, superuser_or_admin, url) + resp.form.set('first_name', 'John') + resp.form.set('last_name', 'Doe') + resp.form.set('email', 'john.doe@entrouvert.com') + resp.form.set('password1', 'ABcd1234') + resp.form.set('password2', 'ABcd1234') + resp = resp.form.submit() + assert 'domain is invalid' in resp.text + + monkeypatch.setattr(EmailValidator, 'check_mxs', lambda x, y: ['mx1.entrouvert.org']) + resp.form.submit() + assert User.objects.filter(email='john.doe@entrouvert.com').count() == 1 + + def test_app_setting_login_url(app, settings): settings.A2_MANAGER_LOGIN_URL = '/other_login/' response = app.get('/manage/') diff --git a/tests/test_registration.py b/tests/test_registration.py index 81c09293..fd6f8edb 100644 --- a/tests/test_registration.py +++ b/tests/test_registration.py @@ -23,15 +23,15 @@ from django.utils.http import urlquote from django.utils.six.moves.urllib.parse import urlparse from authentic2 import utils, models +from authentic2.validators import EmailValidator -from utils import can_resolve_dns, get_link_from_mail +from utils import get_link_from_mail def test_registration(app, db, settings, mailoutbox, external_redirect): next_url, good_next_url = external_redirect settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.DEFAULT_FROM_EMAIL = 'show only addr ' # disable existing attributes @@ -102,7 +102,6 @@ def test_registration(app, db, settings, mailoutbox, external_redirect): def test_registration_realm(app, db, settings, mailoutbox): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.A2_REGISTRATION_REALM = 'realm' settings.A2_REDIRECT_WHITELIST = ['http://relying-party.org/'] settings.A2_REQUIRED_FIELDS = ['username'] @@ -156,9 +155,24 @@ def test_registration_realm(app, db, settings, mailoutbox): assert urlparse(response['Location']).path == reverse('auth_homepage') +def test_registration_email_validation(app, db, monkeypatch, settings): + settings.A2_VALIDATE_EMAIL_DOMAIN = True + monkeypatch.setattr(EmailValidator, 'check_mxs', lambda x, y: ['mx1.entrouvert.org']) + + resp = app.get(reverse('registration_register')) + resp.form.set('email', 'testbot@entrouvert.com') + resp = resp.form.submit().follow() + assert 'Follow the instructions' in resp.text + + monkeypatch.setattr(EmailValidator, 'check_mxs', lambda x, y: []) + resp = app.get(reverse('registration_register')) + resp.form.set('email', 'testbot@entrouvert.com') + resp = resp.form.submit() + assert 'domain is invalid' in resp.text + + def test_username_settings(app, db, settings, mailoutbox): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.A2_REGISTRATION_FORM_USERNAME_REGEX = r'^(ab)+$' settings.A2_REGISTRATION_FORM_USERNAME_LABEL = 'Identifiant' settings.A2_REGISTRATION_FORM_USERNAME_HELP_TEXT = 'Bien remplir' @@ -213,7 +227,6 @@ def test_username_settings(app, db, settings, mailoutbox): def test_username_is_unique(app, db, settings, mailoutbox): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.A2_REGISTRATION_FIELDS = ['username'] settings.A2_REQUIRED_FIELDS = ['username'] settings.A2_USERNAME_IS_UNIQUE = True @@ -261,7 +274,6 @@ def test_username_is_unique(app, db, settings, mailoutbox): def test_email_is_unique(app, db, settings, mailoutbox): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.A2_EMAIL_IS_UNIQUE = True # disable existing attributes @@ -307,7 +319,6 @@ def test_email_is_unique(app, db, settings, mailoutbox): def test_attribute_model(app, db, settings, mailoutbox): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() # disable existing attributes models.Attribute.objects.update(disabled=True) @@ -400,7 +411,6 @@ def test_registration_email_blacklist(app, settings, db): def test_registration_bad_email(app, db, settings): - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.LANGUAGE_CODE = 'en-us' response = app.post(reverse('registration_register'), params={'email': 'fred@0d..be'}, @@ -455,7 +465,6 @@ def test_registration_confirm_data(app, settings, db, rf): def test_revalidate_email(app, rf, db, settings, mailoutbox): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() # disable existing attributes models.Attribute.objects.update(disabled=True) @@ -482,7 +491,6 @@ def test_revalidate_email(app, rf, db, settings, mailoutbox): def test_email_is_unique_multiple_objects_returned(app, db, settings, mailoutbox, rf): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.A2_REGISTRATION_EMAIL_IS_UNIQUE = True # Create two user objects @@ -506,7 +514,6 @@ def test_email_is_unique_multiple_objects_returned(app, db, settings, mailoutbox def test_username_is_unique_multiple_objects_returned(app, db, settings, mailoutbox, rf): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.A2_REGISTRATION_USERNAME_IS_UNIQUE = True settings.A2_REQUIRED_FIELDS = ['username', 'first_name', 'last_name'] @@ -535,7 +542,6 @@ def test_registration_redirect(app, db, settings, mailoutbox, external_redirect) settings.A2_REGISTRATION_REDIRECT = 'http://cms/welcome/' settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() new_next_url = settings.A2_REGISTRATION_REDIRECT if good_next_url: @@ -590,7 +596,6 @@ def test_registration_redirect_tuple(app, db, settings, mailoutbox, external_red next_url, good_next_url = external_redirect settings.A2_REGISTRATION_REDIRECT = 'http://cms/welcome/', 'target' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() new_next_url = settings.A2_REGISTRATION_REDIRECT[0] if good_next_url: @@ -614,7 +619,6 @@ def test_registration_redirect_tuple(app, db, settings, mailoutbox, external_red def test_registration_activate_passwords_not_equal(app, db, settings, mailoutbox): settings.LANGUAGE_CODE = 'en-us' - settings.A2_VALIDATE_EMAIL_DOMAIN = can_resolve_dns() settings.A2_EMAIL_IS_UNIQUE = True response = app.get(reverse('registration_register')) diff --git a/tests/utils.py b/tests/utils.py index f19972c6..aa25631b 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -162,15 +162,6 @@ def check_log(caplog, message, levelname=None): '%r not found in log records' % message -def can_resolve_dns(): - '''Verify that DNS resolving is available''' - import socket - try: - return isinstance(socket.gethostbyname('entrouvert.com'), str) - except: - return False - - def get_links_from_mail(mail): '''Extract links from mail sent by Django''' return re.findall('https?://[^ \n]*', mail.body) -- 2.20.1