diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index a9f99d04..21cd6473 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -2162,13 +2162,12 @@ class DocumentAskForAccessViewSet( serializer = serializers.RoleSerializer(data=request.data) serializer.is_valid(raise_exception=True) - document = self.get_document_or_404() - user_role = document.get_role(request.user) - target_role = serializer.validated_data.get("role") + target_role = serializer.validated_data.get( + "role", document_ask_for_access.role + ) + abilities = document_ask_for_access.get_abilities(request.user) - if models.RoleChoices.get_priority(user_role) < models.RoleChoices.get_priority( - target_role - ): + if target_role not in abilities["set_role_to"]: return drf.response.Response( {"detail": "You cannot accept a role higher than your own."}, status=drf.status.HTTP_400_BAD_REQUEST, diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 6e0ad69e..7a93b82c 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1205,23 +1205,14 @@ class DocumentAskForAccess(BaseModel): def get_abilities(self, user): """Compute and return abilities for a given user.""" - roles = [] + user_role = self.document.get_role(user) + is_admin_or_owner = user_role in PRIVILEGED_ROLES - if user.is_authenticated: - teams = user.teams - try: - roles = self.user_roles or [] - except AttributeError: - try: - roles = self.document.accesses.filter( - models.Q(user=user) | models.Q(team__in=teams), - ).values_list("role", flat=True) - except (self._meta.model.DoesNotExist, IndexError): - roles = [] - - is_admin_or_owner = bool( - set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN}) - ) + set_role_to = [ + role + for role in RoleChoices.values + if RoleChoices.get_priority(role) <= RoleChoices.get_priority(user_role) + ] return { "destroy": is_admin_or_owner, @@ -1229,6 +1220,7 @@ class DocumentAskForAccess(BaseModel): "partial_update": is_admin_or_owner, "retrieve": is_admin_or_owner, "accept": is_admin_or_owner, + "set_role_to": set_role_to, } def accept(self, role=None): diff --git a/src/backend/core/tests/documents/test_api_documents_ask_for_access.py b/src/backend/core/tests/documents/test_api_documents_ask_for_access.py index e566ff14..2d93b753 100644 --- a/src/backend/core/tests/documents/test_api_documents_ask_for_access.py +++ b/src/backend/core/tests/documents/test_api_documents_ask_for_access.py @@ -287,6 +287,7 @@ def test_api_documents_ask_for_access_list_authenticated_own_request(): "update": False, "partial_update": False, "retrieve": False, + "set_role_to": [], }, } ], @@ -356,6 +357,15 @@ def test_api_documents_ask_for_access_list_owner_or_admin(role): response = client.get(f"/api/v1.0/documents/{document.id}/ask-for-access/") assert response.status_code == 200 + + expected_set_role_to = [ + RoleChoices.READER, + RoleChoices.EDITOR, + RoleChoices.ADMIN, + ] + if role == RoleChoices.OWNER: + expected_set_role_to.append(RoleChoices.OWNER) + assert response.json() == { "count": 3, "next": None, @@ -375,6 +385,7 @@ def test_api_documents_ask_for_access_list_owner_or_admin(role): "update": True, "partial_update": True, "retrieve": True, + "set_role_to": expected_set_role_to, }, } for document_ask_for_access in document_ask_for_accesses @@ -467,6 +478,13 @@ def test_api_documents_ask_for_access_retrieve_owner_or_admin(role): f"/api/v1.0/documents/{document.id}/ask-for-access/{document_ask_for_access.id}/" ) assert response.status_code == 200 + expected_set_role_to = [ + RoleChoices.READER, + RoleChoices.EDITOR, + RoleChoices.ADMIN, + ] + if role == RoleChoices.OWNER: + expected_set_role_to.append(RoleChoices.OWNER) assert response.json() == { "id": str(document_ask_for_access.id), "document": str(document.id), @@ -481,6 +499,7 @@ def test_api_documents_ask_for_access_retrieve_owner_or_admin(role): "update": True, "partial_update": True, "retrieve": True, + "set_role_to": expected_set_role_to, }, }