Merge branch 'bug-user-upgrade-privilege' into 'master'

Prevent a member to update its own membership freely

Closes #628

See merge request !1189

arkindex/users/api.py

@@ -620,17 +620,19 @@ class MembershipDetails(RetrieveUpdateDestroyAPIView):
 
         # Check the access level of the request user on the content object
         access_level = get_max_level(self.request.user, membership.content_object)
-
         if not access_level or access_level < Role.Guest.value:
-            # The request user does not have a read acces
             raise NotFound
 
-        if (
-            self.request.method not in SAFE_METHODS
-            # Allow an user to remove its own membership
-            and membership.user_id != self.request.user.id
-            and access_level < Role.Admin.value
-        ):
+        #  Allow any guest to retrieve informations
+        if self.request.method in SAFE_METHODS:
+            return
+
+        # Allow a user to remove its own membership
+        if self.request.method == 'DELETE' and membership.user_id == self.request.user.id:
+            return
+
+        # Only allow admins to edit the access level of a membership
+        if access_level < Role.Admin.value:
             raise PermissionDenied(
                 detail='Only admins of the target membership group can perform this action.'
             )

arkindex/users/tests/test_generic_memberships.py

@@ -21,7 +21,7 @@ class TestMembership(FixtureAPITestCase):
         cls.admin = User.objects.create_user('admin@address.com', 'P4$5w0Rd')
         cls.non_admin = User.objects.filter(
             rights__group_target=cls.group,
-            rights__level__lt=Role.Admin.value
+            rights__level=Role.Contributor.value
         ).first()
         cls.non_admin.verified_email = True
         cls.non_admin.save()
@@ -818,6 +818,26 @@ class TestMembership(FixtureAPITestCase):
             'level': 11
         })
 
+    def test_edit_privilege_escalation(self):
+        """
+        A user cannot update its own access level if he has no admin access
+        """
+        second_admin = User.objects.create(email='another_user@test.fr', verified_email=True)
+        self.group.memberships.create(user=second_admin, level=Role.Admin.value)
+        cases = [
+            (self.non_admin, Role.Guest.value, status.HTTP_403_FORBIDDEN),
+            (self.non_admin, Role.Admin.value, status.HTTP_403_FORBIDDEN),
+            (second_admin, Role.Guest.value, status.HTTP_200_OK),
+        ]
+        for user, level, response_status in cases:
+            self.client.force_login(user)
+            membership = self.group.memberships.get(user=user)
+            response = self.client.put(
+                reverse('api:membership-details', kwargs={'pk': str(membership.id)}),
+                {'level': level}
+            )
+            self.assertEqual(response.status_code, response_status)
+
     def test_edit_membership_superadmin(self):
         """
         Superadmins are allowed to update any membership level