🐛(backend) allow creating accesses when privileged by heritage

We took the opportunity of this bug to refactor serializers and
permissions as advised one day by @qbey: no permission checks in
serializers.

Author: sampaccoud

src/backend/core/api/permissions.py

@@ -6,6 +6,7 @@
 
 from rest_framework import permissions
 
+from core import choices
 from core.models import DocumentAccess, RoleChoices, get_trashbin_cutoff
 
 ACTION_FOR_METHOD_TO_PERMISSION = {
@@ -96,26 +97,27 @@ def has_permission(self, request, view):
         ).exists()
 
 
-class AccessPermission(permissions.BasePermission):
-    """Permission class for access objects."""
+class ResourceWithAccessPermission(permissions.BasePermission):
+    """A permission class for templates."""
 
     def has_permission(self, request, view):
+        """check create permission for templates."""
         return request.user.is_authenticated or view.action != "create"
 
     def has_object_permission(self, request, view, obj):
         """Check permission for a given object."""
         abilities = obj.get_abilities(request.user)
         action = view.action
-        try:
-            action = ACTION_FOR_METHOD_TO_PERMISSION[view.action][request.method]
-        except KeyError:
-            pass
         return abilities.get(action, False)
 
 
-class DocumentAccessPermission(AccessPermission):
+class DocumentPermission(permissions.BasePermission):
     """Subclass to handle soft deletion specificities."""
 
+    def has_permission(self, request, view):
+        """check create permission for documents."""
+        return request.user.is_authenticated or view.action != "create"
+
     def has_object_permission(self, request, view, obj):
         """
         Return a 404 on deleted documents
@@ -127,10 +129,74 @@ def has_object_permission(self, request, view, obj):
         ) and deleted_at < get_trashbin_cutoff():
             raise Http404
 
-        # Compute permission first to ensure the "user_roles" attribute is set
-        has_permission = super().has_object_permission(request, view, obj)
+        abilities = obj.get_abilities(request.user)
+        action = view.action
+        try:
+            action = ACTION_FOR_METHOD_TO_PERMISSION[view.action][request.method]
+        except KeyError:
+            pass
+
+        has_permission = abilities.get(action, False)
 
         if obj.ancestors_deleted_at and not RoleChoices.OWNER in obj.user_roles:
             raise Http404
 
         return has_permission
+
+
+class DocumentAccessPermission(IsAuthenticated):
+    """Permission class for document access objects."""
+
+    def has_permission(self, request, view):
+        """check create permission for accesses in documents tree."""
+        if super().has_permission(request, view) is False:
+            return False
+
+        if view.action == "create":
+            role = view.document.get_role(request.user)
+            if role not in choices.PRIVILEGED_ROLES:
+                raise exceptions.PermissionDenied(
+                    "You are not allowed to manage accesses for this resource."
+                )
+
+        return True
+
+    def has_object_permission(self, request, view, obj):
+        """Check permission for a given object."""
+        abilities = obj.get_abilities(request.user)
+
+        requested_role = request.data.get("role")
+        if requested_role and requested_role not in abilities.get("set_role_to", []):
+            return False
+
+        action = view.action
+        return abilities.get(action, False)
+
+
+class TemplateAccessPermission(IsAuthenticated):
+    """Permission class for template access objects."""
+
+    def has_permission(self, request, view):
+        """check create permission for template accesses."""
+        if super().has_permission(request, view) is False:
+            return False
+
+        if view.action == "create":
+            roles = view.template.get_roles(request.user)
+            if not set(roles).intersection(set(choices.PRIVILEGED_ROLES)):
+                raise exceptions.PermissionDenied(
+                    "You are not allowed to manage accesses for this template."
+                )
+
+        return True
+
+    def has_object_permission(self, request, view, obj):
+        """Check permission for a given object."""
+        abilities = obj.get_abilities(request.user)
+
+        requested_role = request.data.get("role")
+        if requested_role and requested_role not in abilities.get("set_role_to", []):
+            return False
+
+        action = view.action
+        return abilities.get(action, False)

src/backend/core/api/serializers.py

@@ -10,7 +10,7 @@
 from django.utils.translation import gettext_lazy as _
 
 import magic
-from rest_framework import exceptions, serializers
+from rest_framework import serializers
 
 from core import choices, enums, models, utils
 from core.services.ai_services import AI_ACTIONS
@@ -38,78 +38,7 @@ class Meta:
         read_only_fields = ["full_name", "short_name"]
 
 
-class BaseAccessSerializer(serializers.ModelSerializer):
-    """Serialize template accesses."""
-
-    abilities = serializers.SerializerMethodField(read_only=True)
-
-    def update(self, instance, validated_data):
-        """Make "user" field is readonly but only on update."""
-        validated_data.pop("user", None)
-        return super().update(instance, validated_data)
-
-    def get_abilities(self, instance) -> dict:
-        """Return abilities of the logged-in user on the instance."""
-        request = self.context.get("request")
-        if request:
-            return instance.get_abilities(request.user)
-        return {}
-
-    def validate(self, attrs):
-        """
-        Check access rights specific to writing (create/update)
-        """
-        request = self.context.get("request")
-        user = getattr(request, "user", None)
-        role = attrs.get("role")
-
-        # Update
-        if self.instance:
-            can_set_role_to = self.instance.get_abilities(user)["set_role_to"]
-            if role and role not in can_set_role_to:
-                message = (
-                    f"You are only allowed to set role to {', '.join(can_set_role_to)}"
-                    if can_set_role_to
-                    else "You are not allowed to set this role for this template."
-                )
-                raise exceptions.PermissionDenied(message)
-
-        # Create
-        else:
-            try:
-                resource_id = self.context["resource_id"]
-            except KeyError as exc:
-                raise exceptions.ValidationError(
-                    "You must set a resource ID in kwargs to create a new access."
-                ) from exc
-
-            if not self.Meta.model.objects.filter(  # pylint: disable=no-member
-                Q(user=user) | Q(team__in=user.teams),
-                role__in=choices.PRIVILEGED_ROLES,
-                **{self.Meta.resource_field_name: resource_id},  # pylint: disable=no-member
-            ).exists():
-                raise exceptions.PermissionDenied(
-                    "You are not allowed to manage accesses for this resource."
-                )
-
-            if (
-                role == models.RoleChoices.OWNER
-                and not self.Meta.model.objects.filter(  # pylint: disable=no-member
-                    Q(user=user) | Q(team__in=user.teams),
-                    role=models.RoleChoices.OWNER,
-                    **{self.Meta.resource_field_name: resource_id},  # pylint: disable=no-member
-                ).exists()
-            ):
-                raise exceptions.PermissionDenied(
-                    "Only owners of a resource can assign other users as owners."
-                )
-
-        # pylint: disable=no-member
-        attrs[f"{self.Meta.resource_field_name}_id"] = self.context["resource_id"]
-        return attrs
-
-
-class DocumentAccessSerializer(BaseAccessSerializer):
+class DocumentAccessSerializer(serializers.ModelSerializer):
     """Serialize document accesses."""
 
     document_id = serializers.PrimaryKeyRelatedField(
@@ -124,6 +53,7 @@ class DocumentAccessSerializer(BaseAccessSerializer):
         allow_null=True,
     )
     user = UserSerializer(read_only=True)
+    abilities = serializers.SerializerMethodField(read_only=True)
     max_ancestors_role = serializers.SerializerMethodField(read_only=True)
 
     class Meta:
@@ -141,10 +71,22 @@ class Meta:
         ]
         read_only_fields = ["id", "document_id", "abilities", "max_ancestors_role"]
 
+    def get_abilities(self, instance) -> dict:
+        """Return abilities of the logged-in user on the instance."""
+        request = self.context.get("request")
+        if request:
+            return instance.get_abilities(request.user)
+        return {}
+
     def get_max_ancestors_role(self, instance):
         """Return max_ancestors_role if annotated; else None."""
         return getattr(instance, "max_ancestors_role", None)
 
+    def update(self, instance, validated_data):
+        """Make "user" field is readonly but only on update."""
+        validated_data.pop("user", None)
+        return super().update(instance, validated_data)
+
 
 class DocumentAccessLightSerializer(DocumentAccessSerializer):
     """Serialize document accesses with limited fields."""
@@ -173,15 +115,29 @@ class Meta:
         ]
 
 
-class TemplateAccessSerializer(BaseAccessSerializer):
+class TemplateAccessSerializer(serializers.ModelSerializer):
     """Serialize template accesses."""
 
+    abilities = serializers.SerializerMethodField(read_only=True)
+
     class Meta:
         model = models.TemplateAccess
         resource_field_name = "template"
         fields = ["id", "user", "team", "role", "abilities"]
         read_only_fields = ["id", "abilities"]
 
+    def get_abilities(self, instance) -> dict:
+        """Return abilities of the logged-in user on the instance."""
+        request = self.context.get("request")
+        if request:
+            return instance.get_abilities(request.user)
+        return {}
+
+    def update(self, instance, validated_data):
+        """Make "user" field is readonly but only on update."""
+        validated_data.pop("user", None)
+        return super().update(instance, validated_data)
+
 
 class ListDocumentSerializer(serializers.ModelSerializer):
     """Serialize documents with limited fields for display in lists."""