kopia lustrzana https://github.com/wagtail/wagtail
Prevent users from demoting or deactivating themself
Remove is_active and is_superuser fields completely when editing oneself, to avoid locking oneself out.pull/3719/head
Stein Strindhaug
Matt Westcott
rodzic
15965fd0cc
commit
f663c76490
|
|
@ -10,6 +10,7 @@ Changelog
|
|||
* Improved performance of sitemap generation (Levi Adler)
|
||||
* StreamField now respects the `blank` setting; StreamBlock accepts a `required` setting (Loic Teixeira)
|
||||
* StreamBlock now accepts `min_num`, `max_num` and `block_counts` settings to control the minimum and maximum numbers of blocks (Edwar Baron, Matt Westcott)
|
||||
* Users can no longer remove their own active / superuser flags through Settings -> Users (Stein Strindhaug, Huub Bouma)
|
||||
* Fix: FieldBlocks in StreamField now call the field's `prepare_value` method (Tim Heap)
|
||||
* Fix: Initial disabled state of InlinePanel add button is now set correctly on non-default tabs (Matthew Downey)
|
||||
* Fix: Redirects with unicode characters now work (Rich Brennan)
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ Other features
|
|||
* Improved performance of sitemap generation (Levi Adler)
|
||||
* StreamField now respects the ``blank`` setting; StreamBlock accepts a ``required`` setting (Loic Teixeira)
|
||||
* StreamBlock now accepts ``min_num``, ``max_num`` and ``block_counts`` settings to control the minimum and maximum numbers of blocks (Edwar Baron, Matt Westcott)
|
||||
* Users can no longer remove their own active / superuser flags through Settings -> Users (Stein Strindhaug, Huub Bouma)
|
||||
|
||||
Bug fixes
|
||||
~~~~~~~~~
|
||||
|
|
|
|||
|
|
@ -166,6 +166,14 @@ class UserCreationForm(UserForm):
|
|||
class UserEditForm(UserForm):
|
||||
password_required = False
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
editing_self = kwargs.pop('editing_self', False)
|
||||
super(UserEditForm, self).__init__(*args, **kwargs)
|
||||
|
||||
if editing_self:
|
||||
del self.fields["is_active"]
|
||||
del self.fields["is_superuser"]
|
||||
|
||||
class Meta:
|
||||
model = User
|
||||
fields = set([User.USERNAME_FIELD, "is_active"]) | standard_fields | custom_fields
|
||||
|
|
|
|||
|
|
@ -28,7 +28,10 @@
|
|||
{% block extra_fields %}{% endblock extra_fields %}
|
||||
{% include "wagtailadmin/shared/field_as_li.html" with field=form.password1 %}
|
||||
{% include "wagtailadmin/shared/field_as_li.html" with field=form.password2 %}
|
||||
{% include "wagtailadmin/shared/field_as_li.html" with field=form.is_active %}
|
||||
{% if form.is_active %}
|
||||
{% include "wagtailadmin/shared/field_as_li.html" with field=form.is_active %}
|
||||
{% endif %}
|
||||
|
||||
{% endblock fields %}
|
||||
<li>
|
||||
<input type="submit" value="{% trans 'Save' %}" class="button" />
|
||||
|
|
@ -40,7 +43,10 @@
|
|||
</section>
|
||||
<section id="roles" class="nice-padding">
|
||||
<ul class="fields">
|
||||
{% include "wagtailadmin/shared/field_as_li.html" with field=form.is_superuser %}
|
||||
{% if form.is_superuser %}
|
||||
{% include "wagtailadmin/shared/field_as_li.html" with field=form.is_superuser %}
|
||||
{% endif %}
|
||||
|
||||
{% include "wagtailadmin/shared/field_as_li.html" with field=form.groups %}
|
||||
<li>
|
||||
<input type="submit" value="{% trans 'Save' %}" class="button" />
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ from wagtail.wagtailusers.views.users import get_user_creation_form, get_user_ed
|
|||
|
||||
|
||||
delete_user_perm_codename = "delete_{0}".format(AUTH_USER_MODEL_NAME.lower())
|
||||
change_user_perm_codename = "change_{0}".format(AUTH_USER_MODEL_NAME.lower())
|
||||
|
||||
|
||||
class CustomUserCreationForm(UserCreationForm):
|
||||
|
|
@ -314,7 +315,7 @@ class TestUserEditView(TestCase, WagtailTestUtils):
|
|||
)
|
||||
|
||||
# Login
|
||||
self.login()
|
||||
self.current_user = self.login()
|
||||
|
||||
def get(self, params={}, user_id=None):
|
||||
return self.client.get(reverse('wagtailusers_users:edit', args=(user_id or self.test_user.pk, )), params)
|
||||
|
|
@ -330,7 +331,7 @@ class TestUserEditView(TestCase, WagtailTestUtils):
|
|||
def test_nonexistant_redirect(self):
|
||||
self.assertEqual(self.get(user_id=100000).status_code, 404)
|
||||
|
||||
def test_edit(self):
|
||||
def test_edit_and_deactivate(self):
|
||||
response = self.post({
|
||||
'username': "testuser",
|
||||
'email': "test@user.com",
|
||||
|
|
@ -338,6 +339,9 @@ class TestUserEditView(TestCase, WagtailTestUtils):
|
|||
'last_name': "User",
|
||||
'password1': "password",
|
||||
'password2': "password",
|
||||
# Leaving out these fields, thus setting them to False:
|
||||
# 'is_active': 'on'
|
||||
# 'is_superuser': 'on',
|
||||
})
|
||||
|
||||
# Should redirect back to index
|
||||
|
|
@ -346,6 +350,88 @@ class TestUserEditView(TestCase, WagtailTestUtils):
|
|||
# Check that the user was edited
|
||||
user = get_user_model().objects.get(pk=self.test_user.pk)
|
||||
self.assertEqual(user.first_name, 'Edited')
|
||||
# Check that the user is no longer superuser
|
||||
self.assertEqual(user.is_superuser, False)
|
||||
# Check that the user is no longer active
|
||||
self.assertEqual(user.is_active, False)
|
||||
|
||||
|
||||
def test_edit_and_make_superuser(self):
|
||||
response = self.post({
|
||||
'username': "testuser",
|
||||
'email': "test@user.com",
|
||||
'first_name': "Edited",
|
||||
'last_name': "User",
|
||||
'password1': "password",
|
||||
'password2': "password",
|
||||
'is_active': 'on',
|
||||
'is_superuser': 'on',
|
||||
})
|
||||
|
||||
# Should redirect back to index
|
||||
self.assertRedirects(response, reverse('wagtailusers_users:index'))
|
||||
|
||||
# Check that the user was edited
|
||||
user = get_user_model().objects.get(pk=self.test_user.pk)
|
||||
|
||||
# Check that the user is now superuser
|
||||
self.assertEqual(user.is_superuser, True)
|
||||
# Check that the user is now active
|
||||
self.assertEqual(user.is_active, True)
|
||||
|
||||
def test_edit_self(self):
|
||||
response = self.post({
|
||||
'username': 'test@email.com',
|
||||
'email': 'test@email.com',
|
||||
'first_name': "Edited Myself",
|
||||
'last_name': "User",
|
||||
# 'password1': "password",
|
||||
# 'password2': "password",
|
||||
'is_active': 'on',
|
||||
'is_superuser': 'on',
|
||||
}, self.current_user.pk)
|
||||
|
||||
# Should redirect back to index
|
||||
self.assertRedirects(response, reverse('wagtailusers_users:index'))
|
||||
|
||||
# Check that the user was edited
|
||||
user = get_user_model().objects.get(pk=self.current_user.pk)
|
||||
self.assertEqual(user.first_name, 'Edited Myself')
|
||||
|
||||
# Check that the user is still superuser
|
||||
self.assertEqual(user.is_superuser, True)
|
||||
# Check that the user is still active
|
||||
self.assertEqual(user.is_active, True)
|
||||
|
||||
def test_cannot_demote_self(self):
|
||||
"""
|
||||
check that unsetting a user's own is_active or is_superuser flag has no effect
|
||||
"""
|
||||
response = self.post({
|
||||
'username': 'test@email.com',
|
||||
'email': 'test@email.com',
|
||||
'first_name': "Edited Myself",
|
||||
'last_name': "User",
|
||||
# 'password1': "password",
|
||||
# 'password2': "password",
|
||||
|
||||
# failing to submit is_active or is_superuser would unset those flags,
|
||||
# if we didn't explicitly prevent that when editing self
|
||||
# 'is_active': 'on',
|
||||
# 'is_superuser': 'on',
|
||||
}, self.current_user.pk)
|
||||
|
||||
# Should redirect back to index
|
||||
self.assertRedirects(response, reverse('wagtailusers_users:index'))
|
||||
|
||||
# Check that the user was edited
|
||||
user = get_user_model().objects.get(pk=self.current_user.pk)
|
||||
self.assertEqual(user.first_name, 'Edited Myself')
|
||||
|
||||
# Check that the user is still superuser
|
||||
self.assertEqual(user.is_superuser, True)
|
||||
# Check that the user is still active
|
||||
self.assertEqual(user.is_active, True)
|
||||
|
||||
@override_settings(
|
||||
WAGTAIL_USER_EDIT_FORM='wagtail.wagtailusers.tests.CustomUserEditForm',
|
||||
|
|
@ -406,6 +492,60 @@ class TestUserProfileCreation(TestCase, WagtailTestUtils):
|
|||
self.assertEqual(UserProfile.objects.filter(user=self.test_user).count(), 1)
|
||||
|
||||
|
||||
class TestUserEditViewForNonSuperuser(TestCase, WagtailTestUtils):
|
||||
def setUp(self):
|
||||
# create a user with edit permission
|
||||
self.editor_user = get_user_model().objects.create_user(
|
||||
username='editor',
|
||||
email='editor@email.com',
|
||||
password='password'
|
||||
)
|
||||
editors_group = Group.objects.create(name='User editors')
|
||||
editors_group.permissions.add(Permission.objects.get(codename='access_admin'))
|
||||
editors_group.permissions.add(Permission.objects.get(
|
||||
content_type__app_label=AUTH_USER_APP_LABEL, codename=change_user_perm_codename
|
||||
))
|
||||
self.editor_user.groups.add(editors_group)
|
||||
|
||||
self.client.login(username='editor', password='password')
|
||||
|
||||
def test_user_cannot_escalate_privileges(self):
|
||||
"""
|
||||
Check that a non-superuser cannot edit their own is_active or is_superuser flag.
|
||||
(note: this doesn't necessarily guard against other routes to escalating privileges, such
|
||||
as creating a new user with is_superuser=True or adding oneself to a group with additional
|
||||
privileges - the latter will be dealt with by #537)
|
||||
"""
|
||||
editors_group = Group.objects.get(name='User editors')
|
||||
post_data = {
|
||||
'username': "editor",
|
||||
'email': "editor@email.com",
|
||||
'first_name': "Escalating",
|
||||
'last_name': "User",
|
||||
'password1': "",
|
||||
'password2': "",
|
||||
'groups': [editors_group.id, ],
|
||||
# These should not be possible without manipulating the form in the DOM:
|
||||
'is_superuser': 'on',
|
||||
'is_active': 'on',
|
||||
}
|
||||
response = self.client.post(
|
||||
reverse('wagtailusers_users:edit', args=(self.editor_user.pk, )),
|
||||
post_data)
|
||||
# Should redirect back to index
|
||||
self.assertRedirects(response, reverse('wagtailusers_users:index'))
|
||||
|
||||
user = get_user_model().objects.get(pk=self.editor_user.pk)
|
||||
# check if user is still in the editors group
|
||||
self.assertTrue(user.groups.filter(name='User editors').exists())
|
||||
|
||||
# check that non-permission-related edits went ahead
|
||||
self.assertEqual(user.first_name, "Escalating")
|
||||
|
||||
# Check that the user did not escalate its is_superuser status
|
||||
self.assertEqual(user.is_superuser, False)
|
||||
|
||||
|
||||
class TestGroupIndexView(TestCase, WagtailTestUtils):
|
||||
def setUp(self):
|
||||
self.login()
|
||||
|
|
|
|||
|
|
@ -143,9 +143,10 @@ def create(request):
|
|||
def edit(request, user_id):
|
||||
user = get_object_or_404(User, pk=user_id)
|
||||
can_delete = user_can_delete_user(request.user, user)
|
||||
editing_self = request.user == user
|
||||
|
||||
if request.method == 'POST':
|
||||
form = get_user_edit_form()(request.POST, request.FILES, instance=user)
|
||||
form = get_user_edit_form()(request.POST, request.FILES, instance=user, editing_self=editing_self)
|
||||
if form.is_valid():
|
||||
user = form.save()
|
||||
messages.success(request, _("User '{0}' updated.").format(user), buttons=[
|
||||
|
|
@ -155,7 +156,7 @@ def edit(request, user_id):
|
|||
else:
|
||||
messages.error(request, _("The user could not be saved due to errors."))
|
||||
else:
|
||||
form = get_user_edit_form()(instance=user)
|
||||
form = get_user_edit_form()(instance=user, editing_self=editing_self)
|
||||
|
||||
return render(request, 'wagtailusers/users/edit.html', {
|
||||
'user': user,
|
||||
|
|
|
|||
Ładowanie…
Reference in New Issue