diff --git a/CHANGELOG.txt b/CHANGELOG.txt index a34fc9aa28..fa594087df 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -21,6 +21,7 @@ Changelog * Added formal APIs for customising the display of StructBlock forms within the page editor (Matt Westcott) * `wagtailforms.models.AbstractEmailForm` now supports multiple email recipients (Serafeim Papastefanos) * Added the ``include_block`` template tag for improved StreamField template inclusion (Matt Westcott) + * Added ability to delete users through Settings -> Users (Vincent Audebert; thanks also to Ludolf Takens and Tobias Schmidt for alternative implementations) * Fix: Email templates and document uploader now support custom `STATICFILES_STORAGE` (Jonny Scholes) * Fix: Removed alignment options (deprecated in HTML and not rendered by Wagtail) from `TableBlock` context menu (Moritz Pfeiffer) * Fix: Fixed incorrect CSS path on ModelAdmin's "choose a parent page" view diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.rst index 09a6a11c4d..2a5e496dd1 100644 --- a/CONTRIBUTORS.rst +++ b/CONTRIBUTORS.rst @@ -152,6 +152,7 @@ Contributors * Raphael Stolt * Tim Graham * Thibaud Colas +* Tobias Schmidt Translators =========== diff --git a/docs/releases/1.6.rst b/docs/releases/1.6.rst index b0bbea36dc..fda4abe9e3 100644 --- a/docs/releases/1.6.rst +++ b/docs/releases/1.6.rst @@ -36,6 +36,7 @@ Minor features * Added formal APIs for customising the display of StructBlock forms within the page editor - see :ref:`custom_editing_interfaces_for_structblock` (Matt Westcott) * ``wagtailforms.models.AbstractEmailForm`` now supports multiple email recipients (Serafeim Papastefanos) * Added the ``include_block`` template tag for improved StreamField template inclusion. See :doc:`/topics/streamfield` for documentation (Matt Westcott) + * Added ability to delete users through Settings -> Users (Vincent Audebert; thanks also to Ludolf Takens and Tobias Schmidt for alternative implementations) Bug fixes diff --git a/wagtail/wagtailusers/templates/wagtailusers/users/confirm_delete.html b/wagtail/wagtailusers/templates/wagtailusers/users/confirm_delete.html new file mode 100644 index 0000000000..47236371b1 --- /dev/null +++ b/wagtail/wagtailusers/templates/wagtailusers/users/confirm_delete.html @@ -0,0 +1,16 @@ +{% extends "wagtailadmin/base.html" %} +{% load i18n %} +{% block titletag %}{% trans "Delete user" %}{% endblock %} + +{% block content %} + {% trans "Delete user" as del_str %} + {% include "wagtailadmin/shared/header.html" with title=del_str subtitle=user icon="user" %} + +
+

{% trans "Are you sure you want to delete this user?" %}

+
+ {% csrf_token %} + +
+
+{% endblock %} diff --git a/wagtail/wagtailusers/templates/wagtailusers/users/edit.html b/wagtail/wagtailusers/templates/wagtailusers/users/edit.html index 70c6cba08b..f89be03b16 100644 --- a/wagtail/wagtailusers/templates/wagtailusers/users/edit.html +++ b/wagtail/wagtailusers/templates/wagtailusers/users/edit.html @@ -30,14 +30,24 @@ {% include "wagtailadmin/shared/field_as_li.html" with field=form.password2 %} {% include "wagtailadmin/shared/field_as_li.html" with field=form.is_active %} {% endblock fields %} -
  • +
  • + + {% if can_delete %} + {% trans "Delete user" %} + {% endif %} +
  • diff --git a/wagtail/wagtailusers/templates/wagtailusers/users/list.html b/wagtail/wagtailusers/templates/wagtailusers/users/list.html index ee4f9c04f2..13846161e6 100644 --- a/wagtail/wagtailusers/templates/wagtailusers/users/list.html +++ b/wagtail/wagtailusers/templates/wagtailusers/users/list.html @@ -1,4 +1,4 @@ -{% load i18n %} +{% load i18n wagtailusers_tags %} {% load gravatar %} @@ -26,15 +26,18 @@ {% for user in users %} - - - - + + + {% endfor %} diff --git a/wagtail/wagtailusers/templatetags/wagtailusers_tags.py b/wagtail/wagtailusers/templatetags/wagtailusers_tags.py index 72ac9175d4..d51b6039d8 100644 --- a/wagtail/wagtailusers/templatetags/wagtailusers_tags.py +++ b/wagtail/wagtailusers/templatetags/wagtailusers_tags.py @@ -1,6 +1,9 @@ from __future__ import absolute_import, unicode_literals +import itertools + from django import template +from wagtail.wagtailcore import hooks register = template.Library() @@ -55,3 +58,13 @@ def format_permissions(permission_bound_field): 'object_perms': object_perms, 'other_perms': other_perms, } + + +@register.inclusion_tag("wagtailadmin/pages/listing/_buttons.html", + takes_context=True) +def user_listing_buttons(context, user): + button_hooks = hooks.get_hooks('register_user_listing_buttons') + buttons = sorted(itertools.chain.from_iterable( + hook(context, user) + for hook in button_hooks)) + return {'user': user, 'buttons': buttons} diff --git a/wagtail/wagtailusers/tests.py b/wagtail/wagtailusers/tests.py index ab570201ff..ae894d68d1 100644 --- a/wagtail/wagtailusers/tests.py +++ b/wagtail/wagtailusers/tests.py @@ -10,6 +10,7 @@ from django.utils import six from wagtail.tests.utils import WagtailTestUtils from wagtail.wagtailcore import hooks +from wagtail.wagtailcore.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME from wagtail.wagtailcore.models import ( Collection, GroupCollectionPermission, GroupPagePermission, Page) from wagtail.wagtailusers.forms import UserCreationForm, UserEditForm @@ -17,6 +18,9 @@ from wagtail.wagtailusers.models import UserProfile from wagtail.wagtailusers.views.users import get_user_creation_form, get_user_edit_form +delete_user_perm_codename = "delete_{0}".format(AUTH_USER_MODEL_NAME.lower()) + + class CustomUserCreationForm(UserCreationForm): country = forms.CharField(required=True, label="Country") @@ -179,6 +183,122 @@ class TestUserCreateView(TestCase, WagtailTestUtils): self.assertEqual(users.count(), 0) +class TestUserDeleteView(TestCase, WagtailTestUtils): + def setUp(self): + # create a user that should be visible in the listing + self.test_user = get_user_model().objects.create_user( + username='testuser', + email='testuser@email.com', + password='password' + ) + # also create a superuser to delete + self.superuser = get_user_model().objects.create_superuser( + username='testsuperuser', + email='testsuperuser@email.com', + password='password' + ) + self.current_user = self.login() + + def get(self, params={}): + return self.client.get(reverse('wagtailusers_users:delete', args=(self.test_user.pk,)), params) + + def post(self, post_data={}): + return self.client.post(reverse('wagtailusers_users:delete', args=(self.test_user.pk,)), post_data) + + def test_simple(self): + response = self.get() + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailusers/users/confirm_delete.html') + + def test_delete(self): + response = self.post() + + # Should redirect back to index + self.assertRedirects(response, reverse('wagtailusers_users:index')) + + # Check that the user was deleted + users = get_user_model().objects.filter(username='testuser') + self.assertEqual(users.count(), 0) + + def test_user_cannot_delete_self(self): + response = self.client.get(reverse('wagtailusers_users:delete', args=(self.current_user.pk,))) + + # Should redirect to admin index (permission denied) + self.assertRedirects(response, reverse('wagtailadmin_home')) + # Check user was not deleted + self.assertTrue(get_user_model().objects.filter(pk=self.current_user.pk).exists()) + + def test_user_can_delete_other_superuser(self): + response = self.client.get(reverse('wagtailusers_users:delete', args=(self.superuser.pk,))) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailusers/users/confirm_delete.html') + + response = self.client.post(reverse('wagtailusers_users:delete', args=(self.superuser.pk,))) + # Should redirect back to index + self.assertRedirects(response, reverse('wagtailusers_users:index')) + + # Check that the user was deleted + users = get_user_model().objects.filter(username='testsuperuser') + self.assertEqual(users.count(), 0) + + +class TestUserDeleteViewForNonSuperuser(TestCase, WagtailTestUtils): + def setUp(self): + # create a user that should be visible in the listing + self.test_user = get_user_model().objects.create_user( + username='testuser', + email='testuser@email.com', + password='password' + ) + # create a user with delete permission + self.deleter_user = get_user_model().objects.create_user( + username='deleter', + email='deleter@email.com', + password='password' + ) + deleters_group = Group.objects.create(name='User deleters') + deleters_group.permissions.add(Permission.objects.get(codename='access_admin')) + deleters_group.permissions.add(Permission.objects.get( + content_type__app_label=AUTH_USER_APP_LABEL, codename=delete_user_perm_codename + )) + self.deleter_user.groups.add(deleters_group) + + self.superuser = self.create_test_user() + + self.client.login(username='deleter', password='password') + + def test_simple(self): + response = self.client.get(reverse('wagtailusers_users:delete', args=(self.test_user.pk,))) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailusers/users/confirm_delete.html') + + def test_delete(self): + response = self.client.post(reverse('wagtailusers_users:delete', args=(self.test_user.pk,))) + + # Should redirect back to index + self.assertRedirects(response, reverse('wagtailusers_users:index')) + + # Check that the user was deleted + users = get_user_model().objects.filter(username='testuser') + self.assertEqual(users.count(), 0) + + def test_user_cannot_delete_self(self): + response = self.client.post(reverse('wagtailusers_users:delete', args=(self.deleter_user.pk,))) + + # Should redirect to admin index (permission denied) + self.assertRedirects(response, reverse('wagtailadmin_home')) + # Check user was not deleted + self.assertTrue(get_user_model().objects.filter(pk=self.deleter_user.pk).exists()) + + def test_user_cannot_delete_superuser(self): + response = self.client.post(reverse('wagtailusers_users:delete', args=(self.superuser.pk,))) + + # Should redirect to admin index (permission denied) + self.assertRedirects(response, reverse('wagtailadmin_home')) + # Check user was not deleted + self.assertTrue(get_user_model().objects.filter(pk=self.superuser.pk).exists()) + + class TestUserEditView(TestCase, WagtailTestUtils): def setUp(self): # Create a user to edit diff --git a/wagtail/wagtailusers/urls/users.py b/wagtail/wagtailusers/urls/users.py index 1b520da724..11b22dd43f 100644 --- a/wagtail/wagtailusers/urls/users.py +++ b/wagtail/wagtailusers/urls/users.py @@ -8,4 +8,5 @@ urlpatterns = [ url(r'^$', users.index, name='index'), url(r'^add/$', users.create, name='add'), url(r'^([^\/]+)/$', users.edit, name='edit'), + url(r'^([^\/]+)/delete/$', users.delete, name='delete'), ] diff --git a/wagtail/wagtailusers/utils.py b/wagtail/wagtailusers/utils.py new file mode 100644 index 0000000000..ae82e823cf --- /dev/null +++ b/wagtail/wagtailusers/utils.py @@ -0,0 +1,21 @@ +from __future__ import absolute_import, unicode_literals + +from wagtail.wagtailcore.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME + + +delete_user_perm = "{0}.delete_{1}".format(AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME.lower()) + + +def user_can_delete_user(current_user, user_to_delete): + if not current_user.has_perm(delete_user_perm): + return False + + if current_user == user_to_delete: + # users may not delete themselves + return False + + if user_to_delete.is_superuser and not current_user.is_superuser: + # ordinary users may not delete superusers + return False + + return True diff --git a/wagtail/wagtailusers/views/users.py b/wagtail/wagtailusers/views/users.py index 9de120ecb1..ed8029041b 100644 --- a/wagtail/wagtailusers/views/users.py +++ b/wagtail/wagtailusers/views/users.py @@ -13,9 +13,11 @@ from django.views.decorators.vary import vary_on_headers from wagtail.utils.pagination import paginate from wagtail.wagtailadmin import messages from wagtail.wagtailadmin.forms import SearchForm -from wagtail.wagtailadmin.utils import any_permission_required, permission_required +from wagtail.wagtailadmin.utils import ( + any_permission_required, permission_denied, permission_required) from wagtail.wagtailcore.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME from wagtail.wagtailusers.forms import UserCreationForm, UserEditForm +from wagtail.wagtailusers.utils import user_can_delete_user User = get_user_model() @@ -140,6 +142,8 @@ def create(request): @permission_required(change_user_perm) def edit(request, user_id): user = get_object_or_404(User, pk=user_id) + can_delete = user_can_delete_user(request.user, user) + if request.method == 'POST': form = get_user_edit_form()(request.POST, instance=user) if form.is_valid(): @@ -156,4 +160,22 @@ def edit(request, user_id): return render(request, 'wagtailusers/users/edit.html', { 'user': user, 'form': form, + 'can_delete': can_delete, + }) + + +@permission_required(delete_user_perm) +def delete(request, user_id): + user = get_object_or_404(User, pk=user_id) + + if not user_can_delete_user(request.user, user): + return permission_denied(request) + + if request.method == 'POST': + user.delete() + messages.success(request, _("User '{0}' deleted.").format(user)) + return redirect('wagtailusers_users:index') + + return render(request, "wagtailusers/users/confirm_delete.html", { + 'user': user, }) diff --git a/wagtail/wagtailusers/wagtail_hooks.py b/wagtail/wagtailusers/wagtail_hooks.py index 571491c8c5..c3f25400c6 100644 --- a/wagtail/wagtailusers/wagtail_hooks.py +++ b/wagtail/wagtailusers/wagtail_hooks.py @@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals from django.conf.urls import include, url from django.contrib.auth.models import Permission from django.core import urlresolvers +from django.core.urlresolvers import reverse from django.db.models import Q from django.utils.translation import ugettext_lazy as _ @@ -11,6 +12,8 @@ from wagtail.wagtailadmin.search import SearchArea from wagtail.wagtailcore import hooks from wagtail.wagtailcore.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME from wagtail.wagtailusers.urls import groups, users +from wagtail.wagtailusers.utils import user_can_delete_user +from wagtail.wagtailusers.widgets import UserListingButton @hooks.register('register_admin_urls') @@ -95,3 +98,10 @@ def register_users_search_area(): name='users', classnames='icon icon-user', order=600) + + +@hooks.register('register_user_listing_buttons') +def user_listing_buttons(context, user): + yield UserListingButton(_('Edit'), reverse('wagtailusers_users:edit', args=[user.pk]), attrs={'title': _('Edit this user')}, priority=10) + if user_can_delete_user(context.request.user, user): + yield UserListingButton(_('Delete'), reverse('wagtailusers_users:delete', args=[user.pk]), classes={'no'}, attrs={'title': _('Delete this user')}, priority=20) diff --git a/wagtail/wagtailusers/widgets.py b/wagtail/wagtailusers/widgets.py new file mode 100644 index 0000000000..8950be0ae3 --- /dev/null +++ b/wagtail/wagtailusers/widgets.py @@ -0,0 +1,9 @@ +from __future__ import absolute_import, unicode_literals + +from wagtail.wagtailadmin.widgets import Button + + +class UserListingButton(Button): + def __init__(self, label, url, classes=set(), **kwargs): + classes = {'button', 'button-small', 'button-secondary'} | set(classes) + super(UserListingButton, self).__init__(label, url, classes=classes, **kwargs)
    +

    {{ user.get_full_name|default:user.get_username }}

    +
      + {% user_listing_buttons user %} +
    {{ user.get_username }}{% if user.is_superuser %}{% trans "Admin" %}{% endif %}
    {% if user.is_active %}{% trans "Active" %}{% else %}{% trans "Inactive" %}{% endif %}
    {{ user.get_username }}{% if user.is_superuser %}{% trans "Admin" %}{% endif %}
    {% if user.is_active %}{% trans "Active" %}{% else %}{% trans "Inactive" %}{% endif %}