From bf68a5ae40bc9bebf52739d7037fbda75c34dfb7 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Wed, 12 Nov 2025 11:58:50 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=EF=B8=8F(backend)=20remove=20owner?= =?UTF-8?q?=20as=20valid=20role=20for=20ask=5Ffor=5Faccess=20serializer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a ask_for_access creation is made, we explicitly remove the owner role to prevent role escalation. --- CHANGELOG.md | 4 +++ src/backend/core/api/serializers.py | 8 +++--- .../test_api_documents_ask_for_access.py | 27 ++++++++++++++++--- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e42e2206..c05cae1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ and this project adheres to - 🐛(frontend) fix pdf embed to use full width #1526 - 🐛(pdf) fix table cell alignment issue in exported documents #1582 +### Security + +- mitigate role escalation in the ask_for_access viewset #1580 + ## [3.9.0] - 2025-11-10 ### Added diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 81b26d5e..c175745f 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -786,7 +786,9 @@ class DocumentAskForAccessCreateSerializer(serializers.Serializer): """Serializer for creating a document ask for access.""" role = serializers.ChoiceField( - choices=models.RoleChoices.choices, + choices=[ + role for role in choices.RoleChoices if role != models.RoleChoices.OWNER + ], required=False, default=models.RoleChoices.READER, ) @@ -810,11 +812,11 @@ class DocumentAskForAccessSerializer(serializers.ModelSerializer): ] read_only_fields = ["id", "document", "user", "role", "created_at", "abilities"] - def get_abilities(self, invitation) -> dict: + def get_abilities(self, instance) -> dict: """Return abilities of the logged-in user on the instance.""" request = self.context.get("request") if request: - return invitation.get_abilities(request.user) + return instance.get_abilities(request.user) return {} 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 321315e8..e566ff14 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 @@ -115,7 +115,10 @@ def test_api_documents_ask_for_access_create_authenticated_non_root_document(): assert response.status_code == 404 -def test_api_documents_ask_for_access_create_authenticated_specific_role(): +@pytest.mark.parametrize( + "role", [role for role in RoleChoices if role != RoleChoices.OWNER] +) +def test_api_documents_ask_for_access_create_authenticated_specific_role(role): """ Authenticated users should be able to create a document ask for access with a specific role. """ @@ -127,17 +130,35 @@ def test_api_documents_ask_for_access_create_authenticated_specific_role(): response = client.post( f"/api/v1.0/documents/{document.id}/ask-for-access/", - data={"role": RoleChoices.EDITOR}, + data={"role": role}, ) assert response.status_code == 201 assert DocumentAskForAccess.objects.filter( document=document, user=user, - role=RoleChoices.EDITOR, + role=role, ).exists() +def test_api_documents_ask_for_access_create_authenticated_owner_role(): + """ + Authenticated users should not be able to create a document ask for access with the owner role. + """ + document = DocumentFactory() + user = UserFactory() + + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/documents/{document.id}/ask-for-access/", + data={"role": RoleChoices.OWNER}, + ) + assert response.status_code == 400 + assert response.json() == {"role": ['"owner" is not a valid choice.']} + + def test_api_documents_ask_for_access_create_authenticated_already_has_access(): """Authenticated users with existing access can ask for access with a different role.""" user = UserFactory()