From aa0728ff60418b8e5aabb11711acdb3a65dd6b5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Mon, 3 Jun 2013 20:13:06 +0200 Subject: [PATCH] User lockout protection, fixed #666 Protection of updating and deleting users and groups if this caused a lockout of the requesting user. --- openslides/participant/forms.py | 52 ++++++++++++++++------ openslides/participant/models.py | 13 +++++- openslides/participant/views.py | 48 +++++++++++++++++---- tests/participant/test_views.py | 74 +++++++++++++++++++++++++++++++- 4 files changed, 163 insertions(+), 24 deletions(-) diff --git a/openslides/participant/forms.py b/openslides/participant/forms.py index 72710bf3d..fbd1a4167 100644 --- a/openslides/participant/forms.py +++ b/openslides/participant/forms.py @@ -13,12 +13,11 @@ from django import forms from django.contrib import messages from django.contrib.auth.models import Permission -from django.contrib.contenttypes.models import ContentType from django.utils.translation import ugettext as _, ugettext_lazy from django.conf import settings from openslides.utils.forms import CssClassMixin, LocalizedModelMultipleChoiceField -from openslides.participant.models import User, Group +from openslides.participant.models import User, Group, get_protected_perm from openslides.participant.api import get_registered_group @@ -64,19 +63,15 @@ class UserUpdateForm(UserCreateForm): def clean(self, *args, **kwargs): """ - Raises a validation error, if a non-superuser user edits himself + Raises a validation error if a non-superuser user edits himself and removes the last group containing the permission to manage participants. """ # TODO: Check this in clean_groups - if self.request.user == self.instance and not self.instance.is_superuser: - protected_perm = Permission.objects.get( - content_type=ContentType.objects.get(app_label='participant', - model='user'), - codename='can_manage_participant') - if not self.cleaned_data['groups'].filter(permissions__in=[protected_perm]).exists(): - error_msg = _('You can not remove the last group containing the permission to manage participants.') - messages.error(self.request, error_msg) - raise forms.ValidationError(error_msg) + if (self.request.user == self.instance and + not self.instance.is_superuser and + not self.cleaned_data['groups'].filter(permissions__in=[get_protected_perm()]).exists()): + error_msg = _('You can not remove the last group containing the permission to manage participants.') + raise forms.ValidationError(error_msg) return super(UserUpdateForm, self).clean(*args, **kwargs) @@ -87,7 +82,12 @@ class GroupForm(forms.ModelForm, CssClassMixin): users = forms.ModelMultipleChoiceField( queryset=User.objects.all(), label=ugettext_lazy('Participants'), required=False) + class Meta: + model = Group + def __init__(self, *args, **kwargs): + # Take request argument + self.request = kwargs.pop('request', None) # Initial users if kwargs.get('instance', None) is not None: initial = kwargs.setdefault('initial', {}) @@ -114,8 +114,32 @@ class GroupForm(forms.ModelForm, CssClassMixin): return instance - class Meta: - model = Group + def clean(self, *args, **kwargs): + """ + Raises a validation error if a non-superuser user removes himself + from the last group containing the permission to manage participants. + + Raises also a validation error if a non-superuser removes his last + permission to manage participants from the (last) group. + """ + # TODO: Check this in clean_users or clean_permissions + if (self.request and + not self.request.user.is_superuser and + not self.request.user in self.cleaned_data['users'] and + not Group.objects.exclude(pk=self.instance.pk).filter( + permissions__in=[get_protected_perm()], + user__pk=self.request.user.pk).exists()): + error_msg = _('You can not remove yourself from the last group containing the permission to manage participants.') + raise forms.ValidationError(error_msg) + if (self.request and + not self.request.user.is_superuser and + not get_protected_perm() in self.cleaned_data['permissions'] and + not Group.objects.exclude(pk=self.instance.pk).filter( + permissions__in=[get_protected_perm()], + user__pk=self.request.user.pk).exists()): + error_msg = _('You can not remove the permission to manage participants from the last group your are in.') + raise forms.ValidationError(error_msg) + return super(GroupForm, self).clean(*args, **kwargs) class UsersettingsForm(forms.ModelForm, CssClassMixin): diff --git a/openslides/participant/models.py b/openslides/participant/models.py index 448ea828e..ad614b2f0 100644 --- a/openslides/participant/models.py +++ b/openslides/participant/models.py @@ -10,7 +10,8 @@ :license: GNU GPL, see LICENSE for more details. """ -from django.contrib.auth.models import User as DjangoUser, Group as DjangoGroup +from django.contrib.auth.models import User as DjangoUser, Group as DjangoGroup, Permission +from django.contrib.contenttypes.models import ContentType from django.db import models from django.db.models import signals from django.dispatch import receiver @@ -245,3 +246,13 @@ def user_post_save(sender, instance, *args, **kwargs): registered = get_registered_group() instance.groups.add(registered) instance.save() + + +def get_protected_perm(): + """ + Returns the permission to manage participants. This function is a helper + function used to protect manager users from locking out themselves. + """ + return Permission.objects.get( + content_type=ContentType.objects.get(app_label='participant', model='user'), + codename='can_manage_participant') diff --git a/openslides/participant/views.py b/openslides/participant/views.py index 708b9fdb6..c59c66ac7 100644 --- a/openslides/participant/views.py +++ b/openslides/participant/views.py @@ -47,7 +47,7 @@ from openslides.participant.api import gen_username, gen_password, import_users from openslides.participant.forms import ( UserCreateForm, UserUpdateForm, UsersettingsForm, UserImportForm, GroupForm) -from openslides.participant.models import User, Group +from openslides.participant.models import User, Group, get_protected_perm class UserOverview(ListView): @@ -153,11 +153,17 @@ class UserDeleteView(DeleteView): success_url_name = 'user_overview' def pre_redirect(self, request, *args, **kwargs): - if self.get_object() == self.request.user: + if self.object == self.request.user: messages.error(request, _("You can not delete yourself.")) else: super(UserDeleteView, self).pre_redirect(request, *args, **kwargs) + def pre_post_redirect(self, request, *args, **kwargs): + if self.object == self.request.user: + messages.error(self.request, _("You can not delete yourself.")) + else: + super(UserDeleteView, self).pre_post_redirect(request, *args, **kwargs) + class SetUserStatusView(RedirectView, SingleObjectMixin): """ @@ -174,10 +180,10 @@ class SetUserStatusView(RedirectView, SingleObjectMixin): if action == 'activate': self.object.is_active = True elif action == 'deactivate': - if self.get_object().user == self.request.user: + if self.object.user == self.request.user: messages.error(request, _("You can not deactivate yourself.")) return - elif self.get_object().is_superuser: + elif self.object.is_superuser: messages.error(request, _("You can not deactivate the administrator.")) return self.object.is_active = False @@ -412,21 +418,47 @@ class GroupUpdateView(UpdateView): delete_default_permissions() return super(GroupUpdateView, self).get(request, *args, **kwargs) + def get_form_kwargs(self, *args, **kwargs): + form_kwargs = super(GroupUpdateView, self).get_form_kwargs(*args, **kwargs) + form_kwargs.update({'request': self.request}) + return form_kwargs + class GroupDeleteView(DeleteView): """ - Delete a Group. + Delete a group. """ permission_required = 'participant.can_manage_participant' model = Group success_url_name = 'user_group_overview' def pre_redirect(self, request, *args, **kwargs): - if self.get_object().pk in [1, 2]: - messages.error(request, _("You can not delete this Group.")) - else: + if not self.is_protected_from_deleting(): super(GroupDeleteView, self).pre_redirect(request, *args, **kwargs) + def pre_post_redirect(self, request, *args, **kwargs): + if not self.is_protected_from_deleting(): + super(GroupDeleteView, self).pre_post_redirect(request, *args, **kwargs) + + def is_protected_from_deleting(self): + """ + Checks whether the group is protected. + """ + if self.object.pk in [1, 2]: + messages.error(request, _('You can not delete this group.')) + return True + if (not self.request.user.is_superuser and + get_protected_perm() in self.object.permissions.all() and + not Group.objects.exclude(pk=self.object.pk).filter( + permissions__in=[get_protected_perm()], + user__pk=self.request.user.pk).exists()): + messages.error( + self.request, + _('You can not delete the last group containing the permission ' + 'to manage participants you are in.')) + return True + return False + def login(request): extra_content = {} diff --git a/tests/participant/test_views.py b/tests/participant/test_views.py index 3ea840e33..b4f6370b3 100644 --- a/tests/participant/test_views.py +++ b/tests/participant/test_views.py @@ -13,7 +13,7 @@ from django.test.client import Client from openslides.config.api import config from openslides.utils.test import TestCase -from openslides.participant.models import User, Group +from openslides.participant.models import User, Group, get_protected_perm class GroupViews(TestCase): @@ -52,3 +52,75 @@ class GroupViews(TestCase): match = re.findall(pattern, response.content) self.assertEqual(match[1], 'mi6iu2Te6ei9iohue3ex chahshah7eiqueip5eiW') self.assertEqual(match[0], 'aWei4ien6Se0vie0xeiv uquahx3Wohtieph9baer') + + +class LockoutProtection(TestCase): + """ + Tests that a manager user can not lockout himself by doing + something that removes his last permission to manage participants. + """ + def setUp(self): + self.user = User.objects.create(last_name='AQu9ie7ach2ek2Xoozoo', + first_name='guR3La9alah7lahsief6', + username='Iedei0eecoh1aiwahnoo') + self.user.reset_password('default') + self.user.groups.add(Group.objects.get(pk=4)) + self.client = Client() + self.client.login(username='Iedei0eecoh1aiwahnoo', password='default') + self.assertEqual(User.objects.count(), 1) + self.assertEqual(Group.objects.count(), 4) + + def test_delete_yourself(self): + response = self.client.get('/participant/1/del/') + self.assertRedirects(response, '/participant/1/') + self.assertTrue('You can not delete yourself.' in response.cookies['messages'].value) + response = self.client.post('/participant/1/del/', + {'yes': 'yes'}) + self.assertTrue('You can not delete yourself.' in response.cookies['messages'].value) + self.assertRedirects(response, '/participant/') + self.assertEqual(User.objects.count(), 1) + + def test_delete_last_manager_group(self): + response = self.client.get('/participant/group/4/del/') + self.assertRedirects(response, '/participant/group/4/') + self.assertTrue('You can not delete the last group containing the permission ' + 'to manage participants you are in.' in response.cookies['messages'].value) + response = self.client.post('/participant/group/4/del/', + {'yes': 'yes'}) + self.assertTrue('You can not delete the last group containing the permission ' + 'to manage participants you are in.' in response.cookies['messages'].value) + self.assertRedirects(response, '/participant/group/') + self.assertEqual(Group.objects.count(), 4) + + def test_remove_user_from_last_manager_group_via_UserUpdateView(self): + response = self.client.post('/participant/1/edit/', + {'username': 'arae0eQu8eeghoogeik0', + 'groups': '3'}) + self.assertFormError( + response=response, + form='form', + field=None, + errors='You can not remove the last group containing the permission to manage participants.') + + def test_remove_user_from_last_manager_group_via_GroupUpdateView(self): + User.objects.get_or_create(username='foo', pk=2) + response = self.client.post('/participant/group/4/edit/', + {'name': 'ChaeFaev4leephaiChae', + 'users': '2'}) + self.assertFormError( + response=response, + form='form', + field=None, + errors='You can not remove yourself from the last group containing the permission to manage participants.') + + def test_remove_perm_from_last_manager_group(self): + self.assertNotEqual(get_protected_perm().pk, 90) + response = self.client.post('/participant/group/4/edit/', + {'name': 'ChaeFaev4leephaiChae', + 'users': '1', + 'permissions': '90'}) + self.assertFormError( + response=response, + form='form', + field=None, + errors='You can not remove the permission to manage participants from the last group your are in.')