diff --git a/openslides/users/serializers.py b/openslides/users/serializers.py index f2b82a076..4c57f65a0 100644 --- a/openslides/users/serializers.py +++ b/openslides/users/serializers.py @@ -41,6 +41,7 @@ class UserFullSerializer(ModelSerializer): """ groups = IdPrimaryKeyRelatedField( many=True, + required=False, queryset=Group.objects.exclude(pk=1), help_text=ugettext_lazy('The groups this user belongs to. A user will ' 'get all permissions granted to each of ' diff --git a/openslides/users/views.py b/openslides/users/views.py index 2d7a3b49d..385d3eb98 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -61,16 +61,23 @@ class UserViewSet(ModelViewSet): self.check_view_permissions()). Also it is evaluated whether he wants to update himself or is manager. """ - # Check manager perms - if (has_perm(request.user, 'users.can_see_extra_data') and + # Check permissions. + if (has_perm(self.request.user, 'users.can_see_name') and + has_perm(request.user, 'users.can_see_extra_data') and has_perm(request.user, 'users.can_manage')): + # The user has all permissions so he may update every user. if request.data.get('is_active') is False and self.get_object() == request.user: - # A user can not deactivate himself. + # But a user can not deactivate himself. raise ValidationError({'detail': _('You can not deactivate yourself.')}) else: - # Check permissions only to update yourself. + # The user does not have all permissions so he may only update himself. if str(request.user.pk) != self.kwargs['pk']: self.permission_denied(request) + # Remove fields that the user is not allowed to change. + # The list() is required because we want to use del inside the loop. + for key in list(request.data.keys()): + if key not in ('username', 'about_me'): + del request.data[key] response = super().update(request, *args, **kwargs) return response diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index 0d5f79c62..06a2e94ea 100644 --- a/tests/integration/users/test_viewset.py +++ b/tests/integration/users/test_viewset.py @@ -207,6 +207,31 @@ class UserUpdate(TestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + def test_update_yourself_non_manager(self): + """ + Tests that an user can update himself even if he is not a manager. + """ + user = User.objects.create_user( + username='non-admin zeiyeGhaoXoh4awe3xai', + password='non-admin chah1hoshohN5Oh7zouj') + client = APIClient() + client.login( + username='non-admin zeiyeGhaoXoh4awe3xai', + password='non-admin chah1hoshohN5Oh7zouj') + + response = client.put( + reverse('user-detail', args=[user.pk]), + {'username': 'New username IeWeipee5mahpi4quupo', + 'last_name': 'New name fae1Bu1Eyeis9eRox4xu', + 'about_me': 'New profile text Faemahphi3Hilokangei'}) + + self.assertEqual(response.status_code, 200) + user = User.objects.get(pk=user.pk) + self.assertEqual(user.username, 'New username IeWeipee5mahpi4quupo') + self.assertEqual(user.about_me, 'New profile text Faemahphi3Hilokangei') + # The user is not allowed to change some other fields (like last_name). + self.assertNotEqual(user.last_name, 'New name fae1Bu1Eyeis9eRox4xu') + class UserDelete(TestCase): """