From 1292c33a58e4a826921b3fbf792aa385b55215a2 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Thu, 13 Nov 2025 14:44:28 +0100 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20rely=20on=20set?= =?UTF-8?q?=5Frole=5Fto=20from=20DocumentAskForAccess=20abilities?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Like in other abilities, we compute a set_role_to property on the abilities. This set_role_to contains all the roles lower or equal than the current user role. We rely on this propoerty to validate the accept endpoint and it will be used by the front allpication to built the role select list. --- src/backend/core/api/viewsets.py | 11 ++++----- src/backend/core/models.py | 24 +++++++------------ .../test_api_documents_ask_for_access.py | 19 +++++++++++++++ 3 files changed, 32 insertions(+), 22 deletions(-) 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, }, }