diff --git a/CHANGELOG.md b/CHANGELOG.md index f1ce0771..085a1d5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to ### Added +- ✨(backend) include ancestors accesses on document accesses list view # 846 - ✨(backend) add ancestors links definitions to document abilities #846 - ✨(frontend) add customization for translations #857 - ✨(frontend) Duplicate a doc #1078 diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index c75a1f2f..dc1ba541 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -97,7 +97,7 @@ class BaseAccessSerializer(serializers.ModelSerializer): 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 @@ class BaseAccessSerializer(serializers.ModelSerializer): 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 dd6bdfaf..8f8b5816 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -8,7 +8,6 @@ from urllib.parse import unquote, urlencode, urlparse 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.cache import cache from django.core.exceptions import ValidationError @@ -1511,12 +1510,10 @@ class DocumentAccessViewSet( queryset = models.DocumentAccess.objects.select_related("user").all() resource_field_name = "document" serializer_class = serializers.DocumentAccessSerializer - is_current_user_owner_or_admin = False def list(self, request, *args, **kwargs): """Return accesses for the current document with filters and annotations.""" user = self.request.user - queryset = self.filter_queryset(self.get_queryset()) try: document = models.Document.objects.get(pk=self.kwargs["resource_id"]) @@ -1527,22 +1524,35 @@ class DocumentAccessViewSet( if not roles: return drf.response.Response([]) - 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: + 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() + + if highest_readable is None: + return drf.response.Response([]) + + 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 = self.get_serializer(queryset, many=True) + serializer = serializer_class( + queryset, many=True, context=self.get_serializer_context() + ) return drf.response.Response(serializer.data) - def get_serializer_class(self): - if self.action == "list" and not self.is_current_user_owner_or_admin: - return serializers.DocumentAccessLightSerializer - - return super().get_serializer_class() - def perform_create(self, serializer): """Add a new access to the document and send an email to the new added user.""" access = serializer.save() 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 946ffcf0..e30a6c36 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses.py +++ b/src/backend/core/tests/documents/test_api_document_accesses.py @@ -76,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, @@ -108,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) @@ -119,13 +125,16 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( f"/api/v1.0/documents/{document.id!s}/accesses/", ) - # Return only privileged roles - privileged_accesses = [ - access for access in accesses if access.role in models.PRIVILEGED_ROLES - ] assert response.status_code == 200 content = response.json() + + # 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( [ { @@ -147,33 +156,39 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( key=lambda x: x["id"], ) - for access in content: - 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, @@ -187,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) @@ -201,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) == 4 + + # 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"], ) @@ -331,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 e356973a..cd2d57eb 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": "",