From 77a18fa223d64e3b303b707c7c0a9c2da7ae83e0 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Mon, 4 Dec 2017 15:12:01 +0100 Subject: [PATCH 3/3] manager: add a change email action on users (fixes #19716) It's only visible for OU with the validate_emails flag. --- src/authentic2/a2_rbac/models.py | 1 + src/authentic2/api_views.py | 3 +- src/authentic2/manager/forms.py | 10 ++++- .../authentic2/manager/user_change_email.html | 11 ++++++ .../user_change_email_notification_body.txt | 35 ++++++++++++++++++ .../user_change_email_notification_subject.txt | 1 + src/authentic2/manager/urls.py | 6 +++ src/authentic2/manager/user_views.py | 43 ++++++++++++++++++++-- src/authentic2/settings.py | 3 +- src/authentic2/utils.py | 2 +- src/authentic2/views.py | 2 +- tests/test_user_manager.py | 35 ++++++++++++++++++ 12 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 src/authentic2/manager/templates/authentic2/manager/user_change_email.html create mode 100644 src/authentic2/manager/templates/authentic2/manager/user_change_email_notification_body.txt create mode 100644 src/authentic2/manager/templates/authentic2/manager/user_change_email_notification_subject.txt create mode 100644 tests/test_user_manager.py diff --git a/src/authentic2/a2_rbac/models.py b/src/authentic2/a2_rbac/models.py index 5acd33f..034fb7a 100644 --- a/src/authentic2/a2_rbac/models.py +++ b/src/authentic2/a2_rbac/models.py @@ -247,3 +247,4 @@ GenericRelation(Permission, CHANGE_PASSWORD_OP = Operation(name=_('Change password'), slug='change_password') RESET_PASSWORD_OP = Operation(name=_('Reset password'), slug='reset_password') ACTIVATE_OP = Operation(name=_('Activate'), slug='activate') +CHANGE_EMAIL_OP = Operation(name=_('Change email'), slug='change_email') diff --git a/src/authentic2/api_views.py b/src/authentic2/api_views.py index a2f59aa..928cb8e 100644 --- a/src/authentic2/api_views.py +++ b/src/authentic2/api_views.py @@ -564,7 +564,6 @@ class UsersAPI(HookMixin, ExceptionHandlerMixin, ModelViewSet): @detail_route(methods=['post'], permission_classes=(DjangoPermission('custom_user.change_user'),)) def email(self, request, uuid): - from authentic2.views import EmailChangeView user = self.get_object() serializer = ChangeEmailSerializer(data=request.data) if not serializer.is_valid(): @@ -573,7 +572,7 @@ class UsersAPI(HookMixin, ExceptionHandlerMixin, ModelViewSet): 'errors': serializer.errors } return Response(response, status.HTTP_400_BAD_REQUEST) - EmailChangeView.send_email_change_email(request, user, serializer.validated_data['email']) + utils.send_email_change_email(user, serializer.validated_data['email'], request=request) return Response({'result': 1}) diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py index 93aacc5..bc5f69a 100644 --- a/src/authentic2/manager/forms.py +++ b/src/authentic2/manager/forms.py @@ -622,10 +622,18 @@ class OUEditForm(SlugMixin, CssClass, forms.ModelForm): class Meta: model = get_ou_model() - fields = ('name', 'default', 'username_is_unique', 'email_is_unique') + fields = ('name', 'default', 'username_is_unique', 'email_is_unique', 'validate_emails') def get_role_form_class(): if app_settings.ROLE_FORM_CLASS: return import_module_or_class(app_settings.ROLE_FORM_CLASS) return RoleEditForm + + +class UserChangeEmailForm(CssClass, forms.ModelForm): + def save(self, *args, **kwargs): + return self.instance + + class Meta: + fields = ('email',) diff --git a/src/authentic2/manager/templates/authentic2/manager/user_change_email.html b/src/authentic2/manager/templates/authentic2/manager/user_change_email.html new file mode 100644 index 0000000..0eec0f2 --- /dev/null +++ b/src/authentic2/manager/templates/authentic2/manager/user_change_email.html @@ -0,0 +1,11 @@ +{% extends "authentic2/manager/form.html" %} +{% load i18n %} + +{% block beforeform %} +

+ {% blocktrans %}User's email will not be changed immediately. First an email will be sent to this + new email address containing a link on which the user's will have to click to verify that it owns + the email address, then it will be changed.{% endblocktrans %} +

+ {{ block.super }} +{% endblock %} diff --git a/src/authentic2/manager/templates/authentic2/manager/user_change_email_notification_body.txt b/src/authentic2/manager/templates/authentic2/manager/user_change_email_notification_body.txt new file mode 100644 index 0000000..8613504 --- /dev/null +++ b/src/authentic2/manager/templates/authentic2/manager/user_change_email_notification_body.txt @@ -0,0 +1,35 @@ +{% load i18n %}{% autoescape off %}{% if email_is_not_unique%}{% blocktrans with name=user.get_short_name old_email=user.email %}Hi {{ name }} ! + +An administrator requested for changing your email on {{ domain }} from: + + {{ old_email }} + +to: + + {{ email }} + +But this email is already linked to another account. + +You can recover this account password using the password reset form: + + {{ password_reset_url }} + +-- +{{ domain }}{% endblocktrans %}{% else %}{% blocktrans with name=user.get_short_name old_email=user.email %}Hi {{ name }} ! + +And administrator requested for changing your email on {{ domain }} from: + + {{ old_email }} + +to: + + {{ email }} + +To validate this change please click on the following link: + + {{ link }} + +This link will be valid for {{ token_lifetime }}. + +-- +{{ domain }}{% endblocktrans %}{% endif %}{% endautoescape %} diff --git a/src/authentic2/manager/templates/authentic2/manager/user_change_email_notification_subject.txt b/src/authentic2/manager/templates/authentic2/manager/user_change_email_notification_subject.txt new file mode 100644 index 0000000..6fab6fa --- /dev/null +++ b/src/authentic2/manager/templates/authentic2/manager/user_change_email_notification_subject.txt @@ -0,0 +1 @@ +{% load i18n %}{% autoescape off %}{% blocktrans %}Change email on {{ domain }} requested by an administrator{% endblocktrans %}{% endautoescape %} diff --git a/src/authentic2/manager/urls.py b/src/authentic2/manager/urls.py index 1558976..bac85ac 100644 --- a/src/authentic2/manager/urls.py +++ b/src/authentic2/manager/urls.py @@ -36,6 +36,9 @@ urlpatterns = required( url(r'^users/(?P\d+)/change-password/$', user_views.user_change_password, name='a2-manager-user-change-password'), + url(r'^users/(?P\d+)/change-email/$', + user_views.user_change_email, + name='a2-manager-user-change-email'), # by uuid url(r'^users/uuid:(?P[a-z0-9]+)/$', user_views.user_detail, name='a2-manager-user-by-uuid-detail'), @@ -47,6 +50,9 @@ urlpatterns = required( url(r'^users/uuid:(?P[a-z0-9]+)/change-password/$', user_views.user_change_password, name='a2-manager-user-by-uuid-change-password'), + url(r'^users/uuid:(?P[a-z0-9]+)/change-email/$', + user_views.user_change_email, + name='a2-manager-user-by-uuid-change-email'), # Authentic2 roles url(r'^roles/$', role_views.listing, diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py index fc35175..0e6c24b 100644 --- a/src/authentic2/manager/user_views.py +++ b/src/authentic2/manager/user_views.py @@ -16,7 +16,7 @@ from django.views.generic import View from authentic2.constants import SWITCH_USER_SESSION_KEY from authentic2.models import Attribute, PasswordReset -from authentic2.utils import switch_user, send_password_reset_mail, redirect +from authentic2.utils import switch_user, send_password_reset_mail, redirect, send_email_change_email from authentic2.a2_rbac.utils import get_default_ou from authentic2 import hooks from django_rbac.utils import get_role_model, get_role_parenting_model, get_ou_model @@ -26,8 +26,8 @@ from .views import BaseTableView, BaseAddView, \ BaseEditView, ActionMixin, OtherActionsMixin, Action, ExportMixin, \ BaseSubTableView, HideOUColumnMixin, BaseDeleteView, BaseDetailView from .tables import UserTable, UserRolesTable, OuUserRolesTable -from .forms import UserSearchForm, UserAddForm, UserEditForm, \ - UserChangePasswordForm, ChooseUserRoleForm, UserRoleSearchForm +from .forms import (UserSearchForm, UserAddForm, UserEditForm, + UserChangePasswordForm, ChooseUserRoleForm, UserRoleSearchForm, UserChangeEmailForm) from .resources import UserResource from . import app_settings @@ -164,6 +164,10 @@ class UserDetailView(OtherActionsMixin, BaseDetailView): permission='custom_user.change_password_user') if self.request.user.is_superuser: yield Action('switch_user', _('Impersonate this user')) + if self.object.ou and self.object.ou.validate_emails: + yield Action('change_email', _('Change user email'), + url_name='a2-manager-user-change-email', + permission='custom_user.change_email_user') def action_force_password_change(self, request, *args, **kwargs): PasswordReset.objects.get_or_create(user=self.object) @@ -256,7 +260,7 @@ class UserEditView(OtherActionsMixin, ActionMixin, BaseEditView): template_name = 'authentic2/manager/user_edit.html' form_class = UserEditForm permissions = ['custom_user.change_user'] - fields = ['username', 'ou', 'first_name', 'last_name', 'email'] + fields = ['username', 'ou', 'first_name', 'last_name'] success_url = '..' slug_field = 'uuid' action = _('Change') @@ -264,6 +268,8 @@ class UserEditView(OtherActionsMixin, ActionMixin, BaseEditView): def get_fields(self): fields = list(self.fields) + if not self.object.ou or not self.object.ou.validate_emails: + fields.append('email') for attribute in Attribute.objects.all(): fields.append(attribute.name) if self.request.user.is_superuser and \ @@ -325,6 +331,35 @@ class UserChangePasswordView(BaseEditView): user_change_password = UserChangePasswordView.as_view() +class UserChangeEmailView(BaseEditView): + template_name = 'authentic2/manager/user_change_email.html' + model = get_user_model() + form_class = UserChangeEmailForm + permissions = ['custom_user.change_email_user'] + success_url = '..' + slug_field = 'uuid' + 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 + + 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']) + return response + +user_change_email = UserChangeEmailView.as_view() + + class UserRolesView(HideOUColumnMixin, BaseSubTableView): model = get_user_model() form_class = ChooseUserRoleForm diff --git a/src/authentic2/settings.py b/src/authentic2/settings.py index a49fbbd..2b935d1 100644 --- a/src/authentic2/settings.py +++ b/src/authentic2/settings.py @@ -299,10 +299,11 @@ MELLON_ADAPTER = ('authentic2_auth_saml.adapters.AuthenticAdapter',) DJANGO_RBAC_PERMISSIONS_HIERARCHY = { 'view': ['search'], 'change_password': ['view', 'search'], + 'change_email': ['view', 'search'], 'reset_password': ['view', 'search'], 'activate': ['view', 'search'], 'admin': ['change', 'delete', 'add', 'view', 'change_password', 'reset_password', 'activate', - 'search'], + 'search', 'change_email'], 'change': ['view', 'search'], 'delete': ['view', 'search'], 'add': ['view', 'search'], diff --git a/src/authentic2/utils.py b/src/authentic2/utils.py index bdcb7b9..3a8bf23 100644 --- a/src/authentic2/utils.py +++ b/src/authentic2/utils.py @@ -981,7 +981,7 @@ def get_manager_login_url(): return app_settings.LOGIN_URL or settings.LOGIN_URL -def send_email_change_mail(user, email, request=None, context=None, template_names=None): +def send_email_change_email(user, email, request=None, context=None, template_names=None): '''Send an email to verify that user can take email as its new email''' assert user assert email diff --git a/src/authentic2/views.py b/src/authentic2/views.py index 85874b8..c89e796 100644 --- a/src/authentic2/views.py +++ b/src/authentic2/views.py @@ -162,7 +162,7 @@ class EmailChangeView(cbv.TemplateNamesMixin, FormView): def form_valid(self, form): email = form.cleaned_data['email'] - utils.send_email_change_mail(self.request.user, email, request=self.request) + utils.send_email_change_email(self.request.user, email, request=self.request) hooks.call_hooks('event', name='change-email', user=self.request.user, email=email) messages.info( self.request, diff --git a/tests/test_user_manager.py b/tests/test_user_manager.py new file mode 100644 index 0000000..786dde0 --- /dev/null +++ b/tests/test_user_manager.py @@ -0,0 +1,35 @@ +from django.core.urlresolvers import reverse + +from authentic2.a2_rbac.utils import get_default_ou +from utils import login, get_link_from_mail + + +def test_manager_user_change_email(app, superuser_or_admin, simple_user, mailoutbox): + ou = get_default_ou() + ou.validate_emails = True + ou.save() + + response = login(app, superuser_or_admin, + reverse('a2-manager-user-by-uuid-detail', + kwargs={'slug': unicode(simple_user.uuid)})) + assert 'Change user email' in response.content + # 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 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 + # logout + app.session.flush() + + link = get_link_from_mail(mailoutbox[0]) + response = app.get(link).maybe_follow() + assert ( + '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' -- 2.1.4