diff --git a/arkindex/users/api.py b/arkindex/users/api.py index acbb2a1f28ff2b13582dfd9e0d8ba10208828ab7..a71bd7bb928090fe681820f707544f817901d56a 100644 --- a/arkindex/users/api.py +++ b/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.' ) diff --git a/arkindex/users/tests/test_generic_memberships.py b/arkindex/users/tests/test_generic_memberships.py index 85c25a438eaece6dc430f43db66803cf4545e7f4..a1cc51dbd98f306a5139f43f4f4a8eb984249f15 100644 --- a/arkindex/users/tests/test_generic_memberships.py +++ b/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