From 754bf5a79b86c74c797b61876d61fb07f704355e Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Fri, 22 Dec 2017 17:36:34 +0100 Subject: [PATCH] manager: send new email in the email change verification mail (fixes #20564) Use of a ModelForm keeping the original email field for the UserChangeEmailForm makes keeping the original email value after clean() is called impossible, as clean() is also responsible of transfering value from the form into the model instance. We keep using a ModelForm but we use a new field not present in the model to get the new email and we override the save() method so that the behaviour of sending the validation mail is kept inside the form and not in the view. Only the call to the manager's hook manager-change-email-request is kept in the view. --- src/authentic2/manager/forms.py | 23 ++++++++++++++++++++--- src/authentic2/manager/user_views.py | 20 +++++++++----------- tests/test_user_manager.py | 12 ++++++++++-- 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py index c09be8b9..870fb3db 100644 --- a/src/authentic2/manager/forms.py +++ b/src/authentic2/manager/forms.py @@ -21,7 +21,7 @@ from authentic2.forms import BaseUserForm from authentic2.models import PasswordReset from authentic2.utils import import_module_or_class from authentic2.a2_rbac.utils import get_default_ou -from authentic2.utils import send_password_reset_mail +from authentic2.utils import send_password_reset_mail, send_email_change_email from authentic2 import app_settings as a2_app_settings from . import fields, app_settings, utils @@ -651,9 +651,26 @@ def get_role_form_class(): return RoleEditForm -class UserChangeEmailForm(CssClass, forms.ModelForm): +# 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')) + + def __init__(self, *args, **kwargs): + initial = kwargs.setdefault('initial', {}) + instance = kwargs.get('instance') + if instance: + initial['new_email'] = instance.email + super(UserChangeEmailForm, self).__init__(*args, **kwargs) + def save(self, *args, **kwargs): + new_email = self.cleaned_data['new_email'] + send_email_change_email( + self.instance, + new_email, + request=self.request, + template_names=['authentic2/manager/user_change_email_notification']) return self.instance class Meta: - fields = ('email',) + fields = () diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py index 43b49220..a5d60ec8 100644 --- a/src/authentic2/manager/user_views.py +++ b/src/authentic2/manager/user_views.py @@ -353,20 +353,18 @@ class UserChangeEmailView(BaseEditView): title = _('Change user email') def get_success_message(self, cleaned_data): - return ugettext('A mail was sent to %s to verify it.') % cleaned_data['email'] - - def get_form_kwargs(self): - kwargs = super(UserChangeEmailView, self).get_form_kwargs() - kwargs.setdefault('initial', {})['email'] = self.object.email - return kwargs + return ugettext('A mail was sent to %s to verify it.') % cleaned_data['new_email'] def form_valid(self, form): response = super(UserChangeEmailView, self).form_valid(form) - email = form.cleaned_data['email'] - hooks.call_hooks('event', name='manager-change-email-request', user=self.request.user, - instance=form.instance, form=form, email=email) - send_email_change_email(self.object, email, request=self.request, - template_names=['authentic2/manager/user_change_email_notification']) + new_email = form.cleaned_data['new_email'] + hooks.call_hooks( + 'event', + name='manager-change-email-request', + user=self.request.user, + instance=form.instance, + form=form, + email=new_email) return response user_change_email = UserChangeEmailView.as_view() diff --git a/tests/test_user_manager.py b/tests/test_user_manager.py index 718c45b9..bde588e0 100644 --- a/tests/test_user_manager.py +++ b/tests/test_user_manager.py @@ -14,6 +14,10 @@ def test_manager_user_change_email(app, superuser_or_admin, simple_user, mailout ou.validate_emails = True ou.save() + NEW_EMAIL = 'john.doe@example.com' + + assert NEW_EMAIL != simple_user.email + response = login(app, superuser_or_admin, reverse('a2-manager-user-by-uuid-detail', kwargs={'slug': unicode(simple_user.uuid)})) @@ -21,13 +25,17 @@ def test_manager_user_change_email(app, superuser_or_admin, simple_user, mailout # cannot click it's a submit button :/ response = app.get(reverse('a2-manager-user-by-uuid-change-email', kwargs={'slug': unicode(simple_user.uuid)})) - response.form.set('email', 'john.doe@example.com') + assert response.form['new_email'].value == simple_user.email + response.form.set('new_email', NEW_EMAIL) assert len(mailoutbox) == 0 response = response.form.submit().follow() assert 'A mail was sent to john.doe@example.com to verify it.' in response.content assert 'Change user email' in response.content # cannot click it's a submit button :/ assert len(mailoutbox) == 1 + assert simple_user.email in mailoutbox[0].body + assert NEW_EMAIL in mailoutbox[0].body + # logout app.session.flush() @@ -37,7 +45,7 @@ def test_manager_user_change_email(app, superuser_or_admin, simple_user, mailout 'your request for changing your email for john.doe@example.com is successful' in response.content) simple_user.refresh_from_db() - assert simple_user.email == 'john.doe@example.com' + assert simple_user.email == NEW_EMAIL def test_search_by_attribute(app, simple_user, admin): -- 2.11.0