Mitigate role escalation in the ask_for_access viewset

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

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 Meta:
         ]
         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 {}
 
 

src/backend/core/api/viewsets.py

@@ -2162,7 +2162,18 @@ def accept(self, request, *args, **kwargs):
         serializer = serializers.RoleSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
 
-        document_ask_for_access.accept(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 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,
+            )
+
+        document_ask_for_access.accept(role=target_role)
         return drf.response.Response(status=drf.status.HTTP_204_NO_CONTENT)
 
 

src/backend/core/models.py

@@ -1205,30 +1205,22 @@ def __str__(self):
 
     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,
             "update": is_admin_or_owner,
             "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):

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()
@@ -266,6 +287,7 @@ def test_api_documents_ask_for_access_list_authenticated_own_request():
                     "update": False,
                     "partial_update": False,
                     "retrieve": False,
+                    "set_role_to": [],
                 },
             }
         ],
@@ -335,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,
@@ -354,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
@@ -446,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),
@@ -460,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,
         },
     }
 
@@ -749,6 +789,53 @@ def test_api_documents_ask_for_access_accept_authenticated_owner_or_admin_update
     assert document_access.role == RoleChoices.ADMIN
 
 
+def test_api_documents_ask_for_access_accept_admin_cannot_accept_owner_role():
+    """
+    Admin users should not be able to accept document ask for access with the owner role.
+    """
+    user = UserFactory()
+    document = DocumentFactory(users=[(user, RoleChoices.ADMIN)])
+    document_ask_for_access = DocumentAskForAccessFactory(
+        document=document, role=RoleChoices.READER
+    )
+
+    client = APIClient()
+    client.force_login(user)
+
+    response = client.post(
+        f"/api/v1.0/documents/{document.id}/ask-for-access/{document_ask_for_access.id}/accept/",
+        data={"role": RoleChoices.OWNER},
+    )
+    assert response.status_code == 400
+    assert response.json() == {
+        "detail": "You cannot accept a role higher than your own."
+    }
+
+
+def test_api_documents_ask_for_access_accept_owner_can_accept_owner_role():
+    """
+    Owner users should be able to accept document ask for access with the owner role.
+    """
+    user = UserFactory()
+    document = DocumentFactory(users=[(user, RoleChoices.OWNER)])
+    document_ask_for_access = DocumentAskForAccessFactory(
+        document=document, role=RoleChoices.READER
+    )
+
+    client = APIClient()
+    client.force_login(user)
+
+    response = client.post(
+        f"/api/v1.0/documents/{document.id}/ask-for-access/{document_ask_for_access.id}/accept/",
+        data={"role": RoleChoices.OWNER},
+    )
+    assert response.status_code == 204
+
+    assert not DocumentAskForAccess.objects.filter(
+        id=document_ask_for_access.id
+    ).exists()
+
+
 @pytest.mark.parametrize("role", [RoleChoices.OWNER, RoleChoices.ADMIN])
 def test_api_documents_ask_for_access_accept_authenticated_non_root_document(role):
     """

src/frontend/apps/e2e/__tests__/app-impress/doc-member-list.spec.ts

@@ -160,6 +160,9 @@ test.describe('Document list members', () => {
       `You are the sole owner of this group, make another member the group owner before you can change your own role or be removed from your document.`,
     );
     await expect(soloOwner).toBeVisible();
+    await expect(
+      page.getByRole('menuitem', { name: 'Administrator' }),
+    ).toBeDisabled();
 
     await list.click({
       force: true, // Force click to close the dropdown
@@ -186,9 +189,17 @@ test.describe('Document list members', () => {
     await list.click();
     await expect(currentUserRole).toBeVisible();
 
+    await newUserRoles.click();
+    await expect(page.getByRole('menuitem', { name: 'Owner' })).toBeDisabled();
+    await list.click({
+      force: true, // Force click to close the dropdown
+    });
+
     await currentUserRole.click();
     await page.getByRole('menuitem', { name: 'Reader' }).click();
-    await list.click();
+    await list.click({
+      force: true, // Force click to close the dropdown
+    });
     await expect(currentUserRole).toBeHidden();
   });
 

src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts

@@ -7,14 +7,14 @@ import {
   verifyDocName,
 } from './utils-common';
 
-export type Role = 'Administrator' | 'Owner' | 'Member' | 'Editor' | 'Reader';
+export type Role = 'Administrator' | 'Owner' | 'Editor' | 'Reader';
 export type LinkReach = 'Private' | 'Connected' | 'Public';
 export type LinkRole = 'Reading' | 'Editing';
 
 export const addNewMember = async (
   page: Page,
   index: number,
-  role: 'Administrator' | 'Owner' | 'Editor' | 'Reader',
+  role: Role,
   fillText = 'user.test',
 ) => {
   const responsePromiseSearchUser = page.waitForResponse(

src/frontend/apps/impress/src/features/docs/doc-management/types.tsx

@@ -132,5 +132,6 @@ export interface AccessRequest {
     partial_update: boolean;
     retrieve: boolean;
     accept: boolean;
+    set_role_to: Role[];
   };
 }

src/frontend/apps/impress/src/features/docs/doc-share/components/DocRoleDropdown.tsx

@@ -90,12 +90,14 @@ export const DocRoleDropdown = ({
   const roles: DropdownMenuOption[] = Object.keys(translatedRoles).map(
     (key, index) => {
       const isLast = index === Object.keys(translatedRoles).length - 1;
+      const isRoleAllowed = rolesAllowed?.includes(key as Role) ?? true;
+
       return {
         label: transRole(key as Role),
         callback: () => onSelectRole?.(key as Role),
         isSelected: currentRole === (key as Role),
         showSeparator: isLast,
-        disabled: isLastOwner && key !== 'owner',
+        disabled: (isLastOwner && key !== 'owner') || !isRoleAllowed,
       };
     },
   );

src/frontend/apps/impress/src/features/docs/doc-share/components/DocShareAccessRequest.tsx

@@ -77,6 +77,7 @@ const DocShareAccessRequestItem = ({ doc, accessRequest }: Props) => {
               currentRole={role}
               onSelectRole={setRole}
               canUpdate={doc.abilities.accesses_manage}
+              rolesAllowed={accessRequest.abilities.set_role_to}
             />
             <Button
               color="tertiary"