diff --git a/CHANGELOG.md b/CHANGELOG.md index 3738603cf..02ef43a37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to ## Added +- ✨(backend) add ancestors links definitions to document abilities #846 - 🚩(backend) add feature flag for the footer #841 - 🔧(backend) add view to manage footer json #841 - ✨(frontend) add custom css style #771 @@ -23,6 +24,7 @@ and this project adheres to ## Fixed +- 🐛(backend) fix link definition select options linked to ancestors #846 - 🐛(back) validate document content in serializer #822 - 🐛(frontend) fix selection click past end of content #840 diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index e86288bb3..b0772412b 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -97,7 +97,7 @@ def validate(self, attrs): if not self.Meta.model.objects.filter( # pylint: disable=no-member Q(user=user) | Q(team__in=user.teams), - role__in=[models.RoleChoices.OWNER, models.RoleChoices.ADMIN], + role__in=models.PRIVILEGED_ROLES, **{self.Meta.resource_field_name: resource_id}, # pylint: disable=no-member ).exists(): raise exceptions.PermissionDenied( @@ -124,6 +124,10 @@ def validate(self, attrs): class DocumentAccessSerializer(BaseAccessSerializer): """Serialize document accesses.""" + document_id = serializers.PrimaryKeyRelatedField( + read_only=True, + source="document", + ) user_id = serializers.PrimaryKeyRelatedField( queryset=models.User.objects.all(), write_only=True, @@ -136,11 +140,11 @@ class DocumentAccessSerializer(BaseAccessSerializer): class Meta: model = models.DocumentAccess resource_field_name = "document" - fields = ["id", "user", "user_id", "team", "role", "abilities"] - read_only_fields = ["id", "abilities"] + fields = ["id", "document_id", "user", "user_id", "team", "role", "abilities"] + read_only_fields = ["id", "document_id", "abilities"] -class DocumentAccessLightSerializer(DocumentAccessSerializer): +class DocumentAccessLightSerializer(BaseAccessSerializer): """Serialize document accesses with limited fields.""" user = UserLightSerializer(read_only=True) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 9f8a12557..8225cce6f 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -7,7 +7,6 @@ from django.conf import settings from django.contrib.postgres.aggregates import ArrayAgg -from django.contrib.postgres.fields import ArrayField from django.contrib.postgres.search import TrigramSimilarity from django.core.exceptions import ValidationError from django.core.files.storage import default_storage @@ -219,14 +218,10 @@ def get_me(self, request): class ResourceAccessViewsetMixin: """Mixin with methods common to all access viewsets.""" - def get_permissions(self): - """User only needs to be authenticated to list resource accesses""" - if self.action == "list": - permission_classes = [permissions.IsAuthenticated] - else: - return super().get_permissions() - - return [permission() for permission in permission_classes] + def filter_queryset(self, queryset): + """Override to filter on related resource.""" + queryset = super().filter_queryset(queryset) + return queryset.filter(**{self.resource_field_name: self.kwargs["resource_id"]}) def get_serializer_context(self): """Extra context provided to the serializer class.""" @@ -234,43 +229,6 @@ def get_serializer_context(self): context["resource_id"] = self.kwargs["resource_id"] return context - def get_queryset(self): - """Return the queryset according to the action.""" - queryset = super().get_queryset() - queryset = queryset.filter( - **{self.resource_field_name: self.kwargs["resource_id"]} - ) - - if self.action == "list": - user = self.request.user - teams = user.teams - user_roles_query = ( - queryset.filter( - db.Q(user=user) | db.Q(team__in=teams), - **{self.resource_field_name: self.kwargs["resource_id"]}, - ) - .values(self.resource_field_name) - .annotate(roles_array=ArrayAgg("role")) - .values("roles_array") - ) - - # Limit to resource access instances related to a resource THAT also has - # a resource access - # instance for the logged-in user (we don't want to list only the resource - # access instances pointing to the logged-in user) - queryset = ( - queryset.filter( - db.Q(**{f"{self.resource_field_name}__accesses__user": user}) - | db.Q( - **{f"{self.resource_field_name}__accesses__team__in": teams} - ), - **{self.resource_field_name: self.kwargs["resource_id"]}, - ) - .annotate(user_roles=db.Subquery(user_roles_query)) - .distinct() - ) - return queryset - def destroy(self, request, *args, **kwargs): """Forbid deleting the last owner access""" instance = self.get_object() @@ -441,44 +399,6 @@ class DocumentViewSet( trashbin_serializer_class = serializers.ListDocumentSerializer tree_serializer_class = serializers.ListDocumentSerializer - def annotate_is_favorite(self, queryset): - """ - Annotate document queryset with the favorite status for the current user. - """ - user = self.request.user - - if user.is_authenticated: - favorite_exists_subquery = models.DocumentFavorite.objects.filter( - document_id=db.OuterRef("pk"), user=user - ) - return queryset.annotate(is_favorite=db.Exists(favorite_exists_subquery)) - - return queryset.annotate(is_favorite=db.Value(False)) - - def annotate_user_roles(self, queryset): - """ - Annotate document queryset with the roles of the current user - on the document or its ancestors. - """ - user = self.request.user - output_field = ArrayField(base_field=db.CharField()) - - if user.is_authenticated: - user_roles_subquery = models.DocumentAccess.objects.filter( - db.Q(user=user) | db.Q(team__in=user.teams), - document__path=Left(db.OuterRef("path"), Length("document__path")), - ).values_list("role", flat=True) - - return queryset.annotate( - user_roles=db.Func( - user_roles_subquery, function="ARRAY", output_field=output_field - ) - ) - - return queryset.annotate( - user_roles=db.Value([], output_field=output_field), - ) - def get_queryset(self): """Get queryset performing all annotation and filtering on the document tree structure.""" user = self.request.user @@ -514,8 +434,9 @@ def get_queryset(self): def filter_queryset(self, queryset): """Override to apply annotations to generic views.""" queryset = super().filter_queryset(queryset) - queryset = self.annotate_is_favorite(queryset) - queryset = self.annotate_user_roles(queryset) + user = self.request.user + queryset = queryset.annotate_is_favorite(user) + queryset = queryset.annotate_user_roles(user) return queryset def get_response_for_queryset(self, queryset): @@ -539,9 +460,10 @@ def list(self, request, *args, **kwargs): Additional annotations (e.g., `is_highest_ancestor_for_user`, favorite status) are applied before ordering and returning the response. """ - queryset = ( - self.get_queryset() - ) # Not calling filter_queryset. We do our own cooking. + user = self.request.user + + # Not calling filter_queryset. We do our own cooking. + queryset = self.get_queryset() filterset = ListDocumentFilter( self.request.GET, queryset=queryset, request=self.request @@ -554,7 +476,7 @@ def list(self, request, *args, **kwargs): for field in ["is_creator_me", "title"]: queryset = filterset.filters[field].filter(queryset, filter_data[field]) - queryset = self.annotate_user_roles(queryset) + queryset = queryset.annotate_user_roles(user) # Among the results, we may have documents that are ancestors/descendants # of each other. In this case we want to keep only the highest ancestors. @@ -571,7 +493,7 @@ def list(self, request, *args, **kwargs): ) # Annotate favorite status and filter if applicable as late as possible - queryset = self.annotate_is_favorite(queryset) + queryset = queryset.annotate_is_favorite(user) queryset = filterset.filters["is_favorite"].filter( queryset, filter_data["is_favorite"] ) @@ -654,7 +576,7 @@ def trashbin(self, request, *args, **kwargs): deleted_at__isnull=False, deleted_at__gte=models.get_trashbin_cutoff(), ) - queryset = self.annotate_user_roles(queryset) + queryset = queryset.annotate_user_roles(self.request.user) queryset = queryset.filter(user_roles__contains=[models.RoleChoices.OWNER]) return self.get_response_for_queryset(queryset) @@ -834,6 +756,8 @@ def tree(self, request, pk, *args, **kwargs): List ancestors tree above the document. What we need to display is the tree structure opened for the current document. """ + user = self.request.user + try: current_document = self.queryset.only("depth", "path").get(pk=pk) except models.Document.DoesNotExist as excpt: @@ -888,8 +812,8 @@ def tree(self, request, pk, *args, **kwargs): output_field=db.BooleanField(), ) ) - queryset = self.annotate_user_roles(queryset) - queryset = self.annotate_is_favorite(queryset) + queryset = queryset.annotate_user_roles(user) + queryset = queryset.annotate_is_favorite(user) # Pass ancestors' links definitions to the serializer as a context variable # in order to allow saving time while computing abilities on the instance @@ -1373,7 +1297,11 @@ def cors_proxy(self, request, *args, **kwargs): class DocumentAccessViewSet( ResourceAccessViewsetMixin, - viewsets.ModelViewSet, + drf.mixins.CreateModelMixin, + drf.mixins.RetrieveModelMixin, + drf.mixins.UpdateModelMixin, + drf.mixins.DestroyModelMixin, + viewsets.GenericViewSet, ): """ API ViewSet for all interactions with document accesses. @@ -1400,37 +1328,52 @@ class DocumentAccessViewSet( """ lookup_field = "pk" - pagination_class = Pagination permission_classes = [permissions.IsAuthenticated, permissions.AccessPermission] queryset = models.DocumentAccess.objects.select_related("user").all() resource_field_name = "document" serializer_class = serializers.DocumentAccessSerializer - is_current_user_owner_or_admin = False - def get_queryset(self): - """Return the queryset according to the action.""" - queryset = super().get_queryset() + def list(self, request, *args, **kwargs): + """Return accesses for the current document with filters and annotations.""" + user = self.request.user - if self.action == "list": - try: - document = models.Document.objects.get(pk=self.kwargs["resource_id"]) - except models.Document.DoesNotExist: - return queryset.none() + try: + document = models.Document.objects.get(pk=self.kwargs["resource_id"]) + except models.Document.DoesNotExist: + return drf.response.Response([]) - roles = set(document.get_roles(self.request.user)) - is_owner_or_admin = bool(roles.intersection(set(models.PRIVILEGED_ROLES))) - self.is_current_user_owner_or_admin = is_owner_or_admin - if not is_owner_or_admin: - # Return only the document owner access - queryset = queryset.filter(role__in=models.PRIVILEGED_ROLES) + roles = set(document.get_roles(user)) + if not roles: + return drf.response.Response([]) - return queryset + ancestors = ( + (document.get_ancestors() | models.Document.objects.filter(pk=document.pk)) + .filter(ancestors_deleted_at__isnull=True) + .order_by("path") + ) + highest_readable = ancestors.readable_per_se(user).only("depth").first() - def get_serializer_class(self): - if self.action == "list" and not self.is_current_user_owner_or_admin: - return serializers.DocumentAccessLightSerializer + if highest_readable is None: + return drf.response.Response([]) - return super().get_serializer_class() + queryset = self.get_queryset() + queryset = queryset.filter( + document__in=ancestors.filter(depth__gte=highest_readable.depth) + ) + + is_privileged = bool(roles.intersection(set(models.PRIVILEGED_ROLES))) + if is_privileged: + serializer_class = serializers.DocumentAccessSerializer + else: + # Return only the document's privileged accesses + queryset = queryset.filter(role__in=models.PRIVILEGED_ROLES) + serializer_class = serializers.DocumentAccessLightSerializer + + queryset = queryset.distinct() + serializer = serializer_class( + queryset, many=True, context=self.get_serializer_context() + ) + return drf.response.Response(serializer.data) def perform_create(self, serializer): """Add a new access to the document and send an email to the new added user.""" @@ -1542,7 +1485,6 @@ class TemplateAccessViewSet( ResourceAccessViewsetMixin, drf.mixins.CreateModelMixin, drf.mixins.DestroyModelMixin, - drf.mixins.ListModelMixin, drf.mixins.RetrieveModelMixin, drf.mixins.UpdateModelMixin, viewsets.GenericViewSet, @@ -1572,12 +1514,28 @@ class TemplateAccessViewSet( """ lookup_field = "pk" - pagination_class = Pagination permission_classes = [permissions.IsAuthenticated, permissions.AccessPermission] queryset = models.TemplateAccess.objects.select_related("user").all() resource_field_name = "template" serializer_class = serializers.TemplateAccessSerializer + def list(self, request, *args, **kwargs): + """Restrict templates returned by the list endpoint""" + user = self.request.user + teams = user.teams + queryset = self.filter_queryset(self.get_queryset()) + + # Limit to resource access instances related to a resource THAT also has + # a resource access instance for the logged-in user (we don't want to list + # only the resource access instances pointing to the logged-in user) + queryset = queryset.filter( + db.Q(template__accesses__user=user) + | db.Q(template__accesses__team__in=teams), + ).distinct() + + serializer = self.get_serializer(queryset, many=True) + return drf.response.Response(serializer.data) + class InvitationViewset( drf.mixins.CreateModelMixin, diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 2c5239ead..9f3a78b95 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -87,49 +87,61 @@ def get_select_options(cls, ancestors_links): """ Determines the valid select options for link reach and link role depending on the list of ancestors' link reach/role. - Args: ancestors_links: List of dictionaries, each with 'link_reach' and 'link_role' keys representing the reach and role of ancestors links. - Returns: Dictionary mapping possible reach levels to their corresponding possible roles. """ # If no ancestors, return all options if not ancestors_links: - return dict.fromkeys(cls.values, LinkRoleChoices.values) + return { + reach: LinkRoleChoices.values if reach != cls.RESTRICTED else None + for reach in cls.values + } # Initialize result with all possible reaches and role options as sets - result = {reach: set(LinkRoleChoices.values) for reach in cls.values} + result = { + reach: set(LinkRoleChoices.values) if reach != cls.RESTRICTED else None + for reach in cls.values + } # Group roles by reach level reach_roles = defaultdict(set) for link in ancestors_links: reach_roles[link["link_reach"]].add(link["link_role"]) - # Apply constraints based on ancestor links - if LinkRoleChoices.EDITOR in reach_roles[cls.RESTRICTED]: - result[cls.RESTRICTED].discard(LinkRoleChoices.READER) + # Rule 1: public/editor → override everything + if LinkRoleChoices.EDITOR in reach_roles.get(cls.PUBLIC, set()): + return {cls.PUBLIC: [LinkRoleChoices.EDITOR]} - if LinkRoleChoices.EDITOR in reach_roles[cls.AUTHENTICATED]: + # Rule 2: authenticated/editor + if LinkRoleChoices.EDITOR in reach_roles.get(cls.AUTHENTICATED, set()): result[cls.AUTHENTICATED].discard(LinkRoleChoices.READER) result.pop(cls.RESTRICTED, None) - elif LinkRoleChoices.READER in reach_roles[cls.AUTHENTICATED]: - result[cls.RESTRICTED].discard(LinkRoleChoices.READER) - if LinkRoleChoices.EDITOR in reach_roles[cls.PUBLIC]: - result[cls.PUBLIC].discard(LinkRoleChoices.READER) + # Rule 3: public/reader + if LinkRoleChoices.READER in reach_roles.get(cls.PUBLIC, set()): result.pop(cls.AUTHENTICATED, None) result.pop(cls.RESTRICTED, None) - elif LinkRoleChoices.READER in reach_roles[cls.PUBLIC]: - result[cls.AUTHENTICATED].discard(LinkRoleChoices.READER) - result.get(cls.RESTRICTED, set()).discard(LinkRoleChoices.READER) - # Convert roles sets to lists while maintaining the order from LinkRoleChoices - for reach, roles in result.items(): - result[reach] = [role for role in LinkRoleChoices.values if role in roles] + # Rule 4: authenticated/reader + if LinkRoleChoices.READER in reach_roles.get(cls.AUTHENTICATED, set()): + result.pop(cls.RESTRICTED, None) + + # Clean up: remove empty entries and convert sets to ordered lists + cleaned = {} + for reach in cls.values: + if reach in result: + if result[reach]: + cleaned[reach] = [ + r for r in LinkRoleChoices.values if r in result[reach] + ] + else: + # Could be [] or None (for RESTRICTED reach) + cleaned[reach] = result[reach] - return result + return cleaned class DuplicateEmailError(Exception): @@ -452,6 +464,41 @@ def readable_per_se(self, user): return self.filter(link_reach=LinkReachChoices.PUBLIC) + def annotate_is_favorite(self, user): + """ + Annotate document queryset with the favorite status for the current user. + """ + if user.is_authenticated: + favorite_exists_subquery = DocumentFavorite.objects.filter( + document_id=models.OuterRef("pk"), user=user + ) + return self.annotate(is_favorite=models.Exists(favorite_exists_subquery)) + + return self.annotate(is_favorite=models.Value(False)) + + def annotate_user_roles(self, user): + """ + Annotate document queryset with the roles of the current user + on the document or its ancestors. + """ + output_field = ArrayField(base_field=models.CharField()) + + if user.is_authenticated: + user_roles_subquery = DocumentAccess.objects.filter( + models.Q(user=user) | models.Q(team__in=user.teams), + document__path=Left(models.OuterRef("path"), Length("document__path")), + ).values_list("role", flat=True) + + return self.annotate( + user_roles=models.Func( + user_roles_subquery, function="ARRAY", output_field=output_field + ) + ) + + return self.annotate( + user_roles=models.Value([], output_field=output_field), + ) + class DocumentManager(MP_NodeManager.from_queryset(DocumentQuerySet)): """ @@ -737,17 +784,16 @@ def get_roles(self, user): roles = [] return roles - def get_links_definitions(self, ancestors_links): - """Get links reach/role definitions for the current document and its ancestors.""" + def get_ancestors_links_definitions(self, ancestors_links): + """Get links reach/role definitions for ancestors of the current document.""" - links_definitions = defaultdict(set) - links_definitions[self.link_reach].add(self.link_role) - - # Merge ancestor link definitions + ancestors_links_definitions = defaultdict(set) for ancestor in ancestors_links: - links_definitions[ancestor["link_reach"]].add(ancestor["link_role"]) + ancestors_links_definitions[ancestor["link_reach"]].add( + ancestor["link_role"] + ) - return dict(links_definitions) # Convert defaultdict back to a normal dict + return ancestors_links_definitions def compute_ancestors_links(self, user): """ @@ -803,10 +849,20 @@ def get_abilities(self, user, ancestors_links=None): ) and not is_deleted # Add roles provided by the document link, taking into account its ancestors - links_definitions = self.get_links_definitions(ancestors_links) - public_roles = links_definitions.get(LinkReachChoices.PUBLIC, set()) + ancestors_links_definitions = self.get_ancestors_links_definitions( + ancestors_links + ) + + public_roles = ancestors_links_definitions.get( + LinkReachChoices.PUBLIC, set() + ) | ({self.link_role} if self.link_reach == LinkReachChoices.PUBLIC else set()) authenticated_roles = ( - links_definitions.get(LinkReachChoices.AUTHENTICATED, set()) + ancestors_links_definitions.get(LinkReachChoices.AUTHENTICATED, set()) + | ( + {self.link_role} + if self.link_reach == LinkReachChoices.AUTHENTICATED + else set() + ) if user.is_authenticated else set() ) @@ -850,6 +906,9 @@ def get_abilities(self, user, ancestors_links=None): "restore": is_owner, "retrieve": can_get, "media_auth": can_get, + "ancestors_links_definitions": { + k: list(v) for k, v in ancestors_links_definitions.items() + }, "link_select_options": LinkReachChoices.get_select_options(ancestors_links), "tree": can_get, "update": can_update, diff --git a/src/backend/core/tests/documents/test_api_document_accesses.py b/src/backend/core/tests/documents/test_api_document_accesses.py index bf5ef1827..e30a6c364 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses.py +++ b/src/backend/core/tests/documents/test_api_document_accesses.py @@ -51,12 +51,7 @@ def test_api_document_accesses_list_authenticated_unrelated(): f"/api/v1.0/documents/{document.id!s}/accesses/", ) assert response.status_code == 200 - assert response.json() == { - "count": 0, - "next": None, - "previous": None, - "results": [], - } + assert response.json() == [] def test_api_document_accesses_list_unexisting_document(): @@ -70,12 +65,7 @@ def test_api_document_accesses_list_unexisting_document(): response = client.get(f"/api/v1.0/documents/{uuid4()!s}/accesses/") assert response.status_code == 200 - assert response.json() == { - "count": 0, - "next": None, - "previous": None, - "results": [], - } + assert response.json() == [] @pytest.mark.parametrize("via", VIA) @@ -86,22 +76,30 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( via, role, mock_user_teams ): """ - Authenticated users should be able to list document accesses for a document - to which they are directly related, whatever their role in the document. + Authenticated users with no privileged role should only be able to list document + accesses associated with privileged roles for a document, including from ancestors. """ user = factories.UserFactory() - client = APIClient() client.force_login(user) - owner = factories.UserFactory() - accesses = [] - - document_access = factories.UserDocumentAccessFactory( - user=owner, role=models.RoleChoices.OWNER + # Create documents structured as a tree + unreadable_ancestor = factories.DocumentFactory(link_reach="restricted") + # make all documents below the grand parent readable without a specific access for the user + grand_parent = factories.DocumentFactory( + parent=unreadable_ancestor, link_reach="authenticated" ) - accesses.append(document_access) - document = document_access.document + parent = factories.DocumentFactory(parent=grand_parent) + document = factories.DocumentFactory(parent=parent) + child = factories.DocumentFactory(parent=document) + + # Create accesses related to each document + factories.UserDocumentAccessFactory(document=unreadable_ancestor) + grand_parent_access = factories.UserDocumentAccessFactory(document=grand_parent) + parent_access = factories.UserDocumentAccessFactory(document=parent) + document_access = factories.UserDocumentAccessFactory(document=document) + factories.UserDocumentAccessFactory(document=child) + if via == USER: models.DocumentAccess.objects.create( document=document, @@ -118,8 +116,6 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( access1 = factories.TeamDocumentAccessFactory(document=document) access2 = factories.UserDocumentAccessFactory(document=document) - accesses.append(access1) - accesses.append(access2) # Accesses for other documents to which the user is related should not be listed either other_access = factories.UserDocumentAccessFactory(user=user) @@ -129,14 +125,17 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( f"/api/v1.0/documents/{document.id!s}/accesses/", ) - # Return only owners - owners_accesses = [ - access for access in accesses if access.role in models.PRIVILEGED_ROLES - ] assert response.status_code == 200 content = response.json() - assert content["count"] == len(owners_accesses) - assert sorted(content["results"], key=lambda x: x["id"]) == sorted( + + # Make sure only privileged roles are returned + accesses = [grand_parent_access, parent_access, document_access, access1, access2] + privileged_accesses = [ + acc for acc in accesses if acc.role in models.PRIVILEGED_ROLES + ] + assert len(content) == len(privileged_accesses) + + assert sorted(content, key=lambda x: x["id"]) == sorted( [ { "id": str(access.id), @@ -152,38 +151,44 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( "role": access.role, "abilities": access.get_abilities(user), } - for access in owners_accesses + for access in privileged_accesses ], key=lambda x: x["id"], ) - for access in content["results"]: - assert access["role"] in models.PRIVILEGED_ROLES - @pytest.mark.parametrize("via", VIA) -@pytest.mark.parametrize("role", models.PRIVILEGED_ROLES) -def test_api_document_accesses_list_authenticated_related_privileged_roles( +@pytest.mark.parametrize( + "role", [role for role in models.RoleChoices if role in models.PRIVILEGED_ROLES] +) +def test_api_document_accesses_list_authenticated_related_privileged( via, role, mock_user_teams ): """ - Authenticated users should be able to list document accesses for a document - to which they are directly related, whatever their role in the document. + Authenticated users with a privileged role should be able to list all + document accesses whatever the role, including from ancestors. """ user = factories.UserFactory() - client = APIClient() client.force_login(user) - owner = factories.UserFactory() - accesses = [] - - document_access = factories.UserDocumentAccessFactory( - user=owner, role=models.RoleChoices.OWNER + # Create documents structured as a tree + unreadable_ancestor = factories.DocumentFactory(link_reach="restricted") + # make all documents below the grand parent readable without a specific access for the user + grand_parent = factories.DocumentFactory( + parent=unreadable_ancestor, link_reach="authenticated" ) - accesses.append(document_access) - document = document_access.document - user_access = None + parent = factories.DocumentFactory(parent=grand_parent) + document = factories.DocumentFactory(parent=parent) + child = factories.DocumentFactory(parent=document) + + # Create accesses related to each document + factories.UserDocumentAccessFactory(document=unreadable_ancestor) + grand_parent_access = factories.UserDocumentAccessFactory(document=grand_parent) + parent_access = factories.UserDocumentAccessFactory(document=parent) + document_access = factories.UserDocumentAccessFactory(document=document) + factories.UserDocumentAccessFactory(document=child) + if via == USER: user_access = models.DocumentAccess.objects.create( document=document, @@ -197,11 +202,11 @@ def test_api_document_accesses_list_authenticated_related_privileged_roles( team="lasuite", role=role, ) + else: + raise RuntimeError() access1 = factories.TeamDocumentAccessFactory(document=document) access2 = factories.UserDocumentAccessFactory(document=document) - accesses.append(access1) - accesses.append(access2) # Accesses for other documents to which the user is related should not be listed either other_access = factories.UserDocumentAccessFactory(user=user) @@ -211,42 +216,39 @@ def test_api_document_accesses_list_authenticated_related_privileged_roles( f"/api/v1.0/documents/{document.id!s}/accesses/", ) - access2_user = serializers.UserSerializer(instance=access2.user).data - base_user = serializers.UserSerializer(instance=user).data - assert response.status_code == 200 content = response.json() - assert len(content["results"]) == 4 - assert sorted(content["results"], key=lambda x: x["id"]) == sorted( + + # Make sure all expected accesses are returned + accesses = [ + user_access, + grand_parent_access, + parent_access, + document_access, + access1, + access2, + ] + assert len(content) == 6 + + assert sorted(content, key=lambda x: x["id"]) == sorted( [ { - "id": str(user_access.id), - "user": base_user if via == "user" else None, - "team": "lasuite" if via == "team" else "", - "role": user_access.role, - "abilities": user_access.get_abilities(user), - }, - { - "id": str(access1.id), - "user": None, - "team": access1.team, - "role": access1.role, - "abilities": access1.get_abilities(user), - }, - { - "id": str(access2.id), - "user": access2_user, - "team": "", - "role": access2.role, - "abilities": access2.get_abilities(user), - }, - { - "id": str(document_access.id), - "user": serializers.UserSerializer(instance=owner).data, - "team": "", - "role": models.RoleChoices.OWNER, - "abilities": document_access.get_abilities(user), - }, + "id": str(access.id), + "document_id": str(access.document_id), + "user": { + "id": str(access.user.id), + "email": access.user.email, + "language": access.user.language, + "full_name": access.user.full_name, + "short_name": access.user.short_name, + } + if access.user + else None, + "team": access.team, + "role": access.role, + "abilities": access.get_abilities(user), + } + for access in accesses ], key=lambda x: x["id"], ) @@ -341,6 +343,7 @@ def test_api_document_accesses_retrieve_authenticated_related( assert response.status_code == 200 assert response.json() == { "id": str(access.id), + "document_id": str(access.document_id), "user": access_user, "team": "", "role": access.role, diff --git a/src/backend/core/tests/documents/test_api_document_accesses_create.py b/src/backend/core/tests/documents/test_api_document_accesses_create.py index db67ece18..dc3f752de 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses_create.py +++ b/src/backend/core/tests/documents/test_api_document_accesses_create.py @@ -165,6 +165,7 @@ def test_api_document_accesses_create_authenticated_administrator(via, mock_user other_user = serializers.UserSerializer(instance=other_user).data assert response.json() == { "abilities": new_document_access.get_abilities(user), + "document_id": str(new_document_access.document_id), "id": str(new_document_access.id), "team": "", "role": role, @@ -222,6 +223,7 @@ def test_api_document_accesses_create_authenticated_owner(via, mock_user_teams): new_document_access = models.DocumentAccess.objects.filter(user=other_user).get() other_user = serializers.UserSerializer(instance=other_user).data assert response.json() == { + "document_id": str(new_document_access.document_id), "id": str(new_document_access.id), "user": other_user, "team": "", @@ -286,6 +288,7 @@ def test_api_document_accesses_create_email_in_receivers_language(via, mock_user ).get() other_user_data = serializers.UserSerializer(instance=other_user).data assert response.json() == { + "document_id": str(new_document_access.document_id), "id": str(new_document_access.id), "user": other_user_data, "team": "", diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index 38d66cd46..88507147a 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -42,10 +42,11 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "favorite": False, "invite_owner": False, "link_configuration": False, + "ancestors_links_definitions": {}, "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": False, @@ -97,6 +98,10 @@ def test_api_documents_retrieve_anonymous_public_parent(): "accesses_view": False, "ai_transform": False, "ai_translate": False, + "ancestors_links_definitions": { + "public": [grand_parent.link_role], + parent.link_reach: [parent.link_role], + }, "attachment_upload": grand_parent.link_role == "editor", "children_create": False, "children_list": True, @@ -193,6 +198,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "accesses_view": False, "ai_transform": document.link_role == "editor", "ai_translate": document.link_role == "editor", + "ancestors_links_definitions": {}, "attachment_upload": document.link_role == "editor", "children_create": document.link_role == "editor", "children_list": True, @@ -207,7 +213,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": False, @@ -267,6 +273,10 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "accesses_view": False, "ai_transform": grand_parent.link_role == "editor", "ai_translate": grand_parent.link_role == "editor", + "ancestors_links_definitions": { + grand_parent.link_reach: [grand_parent.link_role], + "restricted": [parent.link_role], + }, "attachment_upload": grand_parent.link_role == "editor", "children_create": grand_parent.link_role == "editor", "children_list": True, @@ -440,6 +450,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): ) assert response.status_code == 200 links = document.get_ancestors().values("link_reach", "link_role") + ancestors_roles = list({grand_parent.link_role, parent.link_role}) assert response.json() == { "id": str(document.id), "abilities": { @@ -447,6 +458,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): "accesses_view": True, "ai_transform": access.role != "reader", "ai_translate": access.role != "reader", + "ancestors_links_definitions": {"restricted": ancestors_roles}, "attachment_upload": access.role != "reader", "children_create": access.role != "reader", "children_list": True, diff --git a/src/backend/core/tests/documents/test_api_documents_trashbin.py b/src/backend/core/tests/documents/test_api_documents_trashbin.py index 6db898eaf..b48ba252b 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -74,6 +74,7 @@ def test_api_documents_trashbin_format(): "accesses_view": True, "ai_transform": True, "ai_translate": True, + "ancestors_links_definitions": {}, "attachment_upload": True, "children_create": True, "children_list": True, @@ -88,7 +89,7 @@ def test_api_documents_trashbin_format(): "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": False, # Can't move a deleted document diff --git a/src/backend/core/tests/templates/test_api_template_accesses.py b/src/backend/core/tests/templates/test_api_template_accesses.py index 86e5f2bd5..6d1107768 100644 --- a/src/backend/core/tests/templates/test_api_template_accesses.py +++ b/src/backend/core/tests/templates/test_api_template_accesses.py @@ -48,12 +48,7 @@ def test_api_template_accesses_list_authenticated_unrelated(): f"/api/v1.0/templates/{template.id!s}/accesses/", ) assert response.status_code == 200 - assert response.json() == { - "count": 0, - "next": None, - "previous": None, - "results": [], - } + assert response.json() == [] @pytest.mark.parametrize("via", VIA) @@ -96,8 +91,8 @@ def test_api_template_accesses_list_authenticated_related(via, mock_user_teams): assert response.status_code == 200 content = response.json() - assert len(content["results"]) == 3 - assert sorted(content["results"], key=lambda x: x["id"]) == sorted( + assert len(content) == 3 + assert sorted(content, key=lambda x: x["id"]) == sorted( [ { "id": str(user_access.id), diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 3f2b8d6e0..908215369 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -154,6 +154,7 @@ def test_models_documents_get_abilities_forbidden( "accesses_view": False, "ai_transform": False, "ai_translate": False, + "ancestors_links_definitions": {}, "attachment_upload": False, "children_create": False, "children_list": False, @@ -170,7 +171,7 @@ def test_models_documents_get_abilities_forbidden( "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "partial_update": False, "restore": False, @@ -214,6 +215,7 @@ def test_models_documents_get_abilities_reader( "accesses_view": False, "ai_transform": False, "ai_translate": False, + "ancestors_links_definitions": {}, "attachment_upload": False, "children_create": False, "children_list": True, @@ -228,7 +230,7 @@ def test_models_documents_get_abilities_reader( "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": False, @@ -250,7 +252,7 @@ def test_models_documents_get_abilities_reader( assert all( value is False for key, value in document.get_abilities(user).items() - if key != "link_select_options" + if key not in ["link_select_options", "ancestors_links_definitions"] ) @@ -276,6 +278,7 @@ def test_models_documents_get_abilities_editor( "accesses_view": False, "ai_transform": is_authenticated, "ai_translate": is_authenticated, + "ancestors_links_definitions": {}, "attachment_upload": True, "children_create": is_authenticated, "children_list": True, @@ -290,7 +293,7 @@ def test_models_documents_get_abilities_editor( "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": False, @@ -311,7 +314,7 @@ def test_models_documents_get_abilities_editor( assert all( value is False for key, value in document.get_abilities(user).items() - if key != "link_select_options" + if key not in ["link_select_options", "ancestors_links_definitions"] ) @@ -327,6 +330,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "accesses_view": True, "ai_transform": True, "ai_translate": True, + "ancestors_links_definitions": {}, "attachment_upload": True, "children_create": True, "children_list": True, @@ -341,7 +345,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": True, @@ -375,6 +379,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "accesses_view": True, "ai_transform": True, "ai_translate": True, + "ancestors_links_definitions": {}, "attachment_upload": True, "children_create": True, "children_list": True, @@ -389,7 +394,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": True, @@ -410,7 +415,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) assert all( value is False for key, value in document.get_abilities(user).items() - if key != "link_select_options" + if key not in ["link_select_options", "ancestors_links_definitions"] ) @@ -426,6 +431,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "accesses_view": True, "ai_transform": True, "ai_translate": True, + "ancestors_links_definitions": {}, "attachment_upload": True, "children_create": True, "children_list": True, @@ -440,7 +446,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": False, @@ -461,7 +467,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): assert all( value is False for key, value in document.get_abilities(user).items() - if key != "link_select_options" + if key not in ["link_select_options", "ancestors_links_definitions"] ) @@ -484,6 +490,7 @@ def test_models_documents_get_abilities_reader_user( # You should not access AI if it's restricted to users with specific access "ai_transform": access_from_link and ai_access_setting != "restricted", "ai_translate": access_from_link and ai_access_setting != "restricted", + "ancestors_links_definitions": {}, "attachment_upload": access_from_link, "children_create": access_from_link, "children_list": True, @@ -498,7 +505,7 @@ def test_models_documents_get_abilities_reader_user( "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": False, @@ -521,7 +528,7 @@ def test_models_documents_get_abilities_reader_user( assert all( value is False for key, value in document.get_abilities(user).items() - if key != "link_select_options" + if key not in ["link_select_options", "ancestors_links_definitions"] ) @@ -540,6 +547,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "accesses_view": True, "ai_transform": False, "ai_translate": False, + "ancestors_links_definitions": {}, "attachment_upload": False, "children_create": False, "children_list": True, @@ -554,7 +562,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "link_select_options": { "authenticated": ["reader", "editor"], "public": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, "media_auth": True, "move": False, @@ -1174,8 +1182,6 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): ( [{"link_reach": "public", "link_role": "reader"}], { - "restricted": ["editor"], - "authenticated": ["editor"], "public": ["reader", "editor"], }, ), @@ -1183,7 +1189,6 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): ( [{"link_reach": "authenticated", "link_role": "reader"}], { - "restricted": ["editor"], "authenticated": ["reader", "editor"], "public": ["reader", "editor"], }, @@ -1195,7 +1200,7 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): ( [{"link_reach": "restricted", "link_role": "reader"}], { - "restricted": ["reader", "editor"], + "restricted": None, "authenticated": ["reader", "editor"], "public": ["reader", "editor"], }, @@ -1203,7 +1208,7 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): ( [{"link_reach": "restricted", "link_role": "editor"}], { - "restricted": ["editor"], + "restricted": None, "authenticated": ["reader", "editor"], "public": ["reader", "editor"], }, @@ -1229,7 +1234,7 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): {"link_reach": "restricted", "link_role": "editor"}, ], { - "restricted": ["editor"], + "restricted": None, "authenticated": ["reader", "editor"], "public": ["reader", "editor"], }, @@ -1241,8 +1246,6 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): {"link_reach": "public", "link_role": "reader"}, ], { - "restricted": ["editor"], - "authenticated": ["editor"], "public": ["reader", "editor"], }, ), @@ -1253,8 +1256,6 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): {"link_reach": "public", "link_role": "reader"}, ], { - "restricted": ["editor"], - "authenticated": ["editor"], "public": ["reader", "editor"], }, ), @@ -1264,7 +1265,7 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): {"link_reach": "authenticated", "link_role": "editor"}, {"link_reach": "public", "link_role": "reader"}, ], - {"authenticated": ["editor"], "public": ["reader", "editor"]}, + {"public": ["reader", "editor"]}, ), ( [ @@ -1279,7 +1280,6 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): {"link_reach": "authenticated", "link_role": "reader"}, ], { - "restricted": ["editor"], "authenticated": ["reader", "editor"], "public": ["reader", "editor"], }, @@ -1297,7 +1297,7 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): { "public": ["reader", "editor"], "authenticated": ["reader", "editor"], - "restricted": ["reader", "editor"], + "restricted": None, }, ), ],