From 5b4ca84306d478a96853c934287c331c4e4bb7da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Norman=20J=C3=A4ckel?= <openslides@normanjaeckel.de>
Date: Fri, 24 Feb 2017 15:04:12 +0100
Subject: [PATCH] Removed restricted fields from PUT request where a users
 wants to update himself. Fixed #2986 and #2984.

---
 openslides/users/serializers.py         |  1 +
 openslides/users/views.py               | 15 +++++++++++----
 tests/integration/users/test_viewset.py | 25 +++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)

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):
     """