♻️(backend) optimize refactoring access abilities and fix inheritance

The latest refactoring in a445278 kept some factorizations that are
not legit anymore after the refactoring.

It is also cleaner to not make serializer choice in the list view if
the reason for this choice is related to something else b/c other
views would then use the wrong serializer and that would be a
security leak.

This commit also fixes a bug in the access rights inheritance: if a
user is allowed to see accesses on a document, he should see all
acesses related to ancestors, even the ancestors that he can not
read. This is because the access that was granted on all ancestors
also apply on the current document... so it must be displayed.

Lastly, we optimize database queries because the number of accesses
we fetch is going up with multi-pages and we were generating a lot
of useless queries.

Author: sampaccoud

src/backend/core/api/serializers.py

@@ -32,21 +32,10 @@ class Meta:
 class UserLightSerializer(UserSerializer):
     """Serialize users with limited fields."""
 
-    id = serializers.SerializerMethodField(read_only=True)
-    email = serializers.SerializerMethodField(read_only=True)
-
-    def get_id(self, _user):
-        """Return always None. Here to have the same fields than in UserSerializer."""
-        return None
-
-    def get_email(self, _user):
-        """Return always None. Here to have the same fields than in UserSerializer."""
-        return None
-
     class Meta:
         model = models.User
-        fields = ["id", "email", "full_name", "short_name"]
-        read_only_fields = ["id", "email", "full_name", "short_name"]
+        fields = ["full_name", "short_name"]
+        read_only_fields = ["full_name", "short_name"]
 
 
 class BaseAccessSerializer(serializers.ModelSerializer):
@@ -59,11 +48,11 @@ def update(self, instance, validated_data):
         validated_data.pop("user", None)
         return super().update(instance, validated_data)
 
-    def get_abilities(self, access) -> 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 access.get_abilities(request.user)
+            return instance.get_abilities(request.user)
         return {}
 
     def validate(self, attrs):
@@ -77,7 +66,6 @@ def validate(self, attrs):
         # 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)}"
@@ -140,19 +128,41 @@ class DocumentAccessSerializer(BaseAccessSerializer):
     class Meta:
         model = models.DocumentAccess
         resource_field_name = "document"
-        fields = ["id", "document_id", "user", "user_id", "team", "role", "abilities"]
+        fields = [
+            "id",
+            "document_id",
+            "user",
+            "user_id",
+            "team",
+            "role",
+            "abilities",
+        ]
         read_only_fields = ["id", "document_id", "abilities"]
 
 
-class DocumentAccessLightSerializer(BaseAccessSerializer):
+class DocumentAccessLightSerializer(DocumentAccessSerializer):
     """Serialize document accesses with limited fields."""
 
     user = UserLightSerializer(read_only=True)
 
     class Meta:
         model = models.DocumentAccess
-        fields = ["id", "user", "team", "role", "abilities"]
-        read_only_fields = ["id", "team", "role", "abilities"]
+        resource_field_name = "document"
+        fields = [
+            "id",
+            "document_id",
+            "user",
+            "team",
+            "role",
+            "abilities",
+        ]
+        read_only_fields = [
+            "id",
+            "document_id",
+            "team",
+            "role",
+            "abilities",
+        ]
 
 
 class TemplateAccessSerializer(BaseAccessSerializer):

src/backend/core/api/viewsets.py

@@ -4,6 +4,7 @@
 import json
 import logging
 import uuid
+from collections import defaultdict
 from urllib.parse import unquote, urlencode, urlparse
 
 from django.conf import settings
@@ -1421,49 +1422,88 @@ class DocumentAccessViewSet(
     permission_classes = [permissions.IsAuthenticated, permissions.AccessPermission]
     queryset = models.DocumentAccess.objects.select_related("user").all()
     resource_field_name = "document"
-    serializer_class = serializers.DocumentAccessSerializer
 
-    def list(self, request, *args, **kwargs):
-        """Return accesses for the current document with filters and annotations."""
-        user = self.request.user
+    def __init__(self, *args, **kwargs):
+        """Initialize the viewset and define default value for contextual document."""
+        super().__init__(*args, **kwargs)
+        self.document = None
 
-        try:
-            document = models.Document.objects.get(pk=self.kwargs["resource_id"])
-        except models.Document.DoesNotExist:
-            return drf.response.Response([])
+    def initial(self, request, *args, **kwargs):
+        """Retrieve self.document with annotated user roles."""
+        super().initial(request, *args, **kwargs)
 
-        role = document.get_role(user)
-        if role is None:
-            return drf.response.Response([])
+        try:
+            self.document = models.Document.objects.annotate_user_roles(
+                self.request.user
+            ).get(pk=self.kwargs["resource_id"])
+        except models.Document.DoesNotExist as excpt:
+            raise Http404() from excpt
 
-        ancestors = (
-            (document.get_ancestors() | models.Document.objects.filter(pk=document.pk))
-            .filter(ancestors_deleted_at__isnull=True)
-            .order_by("path")
+    def get_serializer_class(self):
+        """Use light serializer for unprivileged users."""
+        return (
+            serializers.DocumentAccessSerializer
+            if self.document.get_role(self.request.user) in choices.PRIVILEGED_ROLES
+            else serializers.DocumentAccessLightSerializer
         )
-        highest_readable = ancestors.readable_per_se(user).only("depth").first()
 
-        if highest_readable is None:
+    def list(self, request, *args, **kwargs):
+        """Return accesses for the current document with filters and annotations."""
+        user = request.user
+
+        role = self.document.get_role(user)
+        if not role:
             return drf.response.Response([])
 
-        queryset = self.get_queryset()
-        queryset = queryset.filter(
-            document__in=ancestors.filter(depth__gte=highest_readable.depth)
-        )
+        ancestors = (
+            self.document.get_ancestors()
+            | models.Document.objects.filter(pk=self.document.pk)
+        ).filter(ancestors_deleted_at__isnull=True)
 
-        is_privileged = role in choices.PRIVILEGED_ROLES
-        if is_privileged:
-            serializer_class = serializers.DocumentAccessSerializer
-        else:
-            # Return only the document's privileged accesses
+        queryset = self.get_queryset().filter(document__in=ancestors)
+
+        if role not in choices.PRIVILEGED_ROLES:
             queryset = queryset.filter(role__in=choices.PRIVILEGED_ROLES)
-            serializer_class = serializers.DocumentAccessLightSerializer
 
-        queryset = queryset.distinct()
-        serializer = serializer_class(
-            queryset, many=True, context=self.get_serializer_context()
+        accesses = list(
+            queryset.annotate(document_path=db.F("document__path")).order_by(
+                "document_path"
+            )
         )
-        return drf.response.Response(serializer.data)
+
+        # Annotate more information on roles
+        path_to_ancestors_roles = defaultdict(list)
+        path_to_role = defaultdict(lambda: None)
+        for access in accesses:
+            if access.user_id == user.id or access.team in user.teams:
+                parent_path = access.document_path[: -models.Document.steplen]
+                if parent_path:
+                    path_to_ancestors_roles[access.document_path].extend(
+                        path_to_ancestors_roles[parent_path]
+                    )
+                    path_to_ancestors_roles[access.document_path].append(
+                        path_to_role[parent_path]
+                    )
+                else:
+                    path_to_ancestors_roles[access.document_path] = []
+
+                path_to_role[access.document_path] = choices.RoleChoices.max(
+                    path_to_role[access.document_path], access.role
+                )
+
+        # serialize and return the response
+        context = self.get_serializer_context()
+        serializer_class = self.get_serializer_class()
+        serialized_data = []
+        for access in accesses:
+            access.set_user_roles_tuple(
+                choices.RoleChoices.max(*path_to_ancestors_roles[access.document_path]),
+                path_to_role.get(access.document_path),
+            )
+            serializer = serializer_class(access, context=context)
+            serialized_data.append(serializer.data)
+
+        return drf.response.Response(serialized_data)
 
     def perform_create(self, serializer):
         """Add a new access to the document and send an email to the new added user."""