From 1ab237af3b6c0caab04e2efb78a835621995b2e9 Mon Sep 17 00:00:00 2001 From: Samuel Paccoud - DINUM Date: Fri, 2 May 2025 19:18:30 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20add=20max=20ancestors=20ro?= =?UTF-8?q?le=20field=20to=20document=20access=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This field is set only on the list view when all accesses for a given document and all its ancestors are listed. It gives the highest role among all accesses related to each document. --- src/backend/core/api/serializers.py | 10 +- src/backend/core/api/viewsets.py | 49 ++-- src/backend/core/models.py | 4 +- .../documents/test_api_document_accesses.py | 242 ++++++++++++++++++ .../test_api_document_accesses_create.py | 3 + 5 files changed, 291 insertions(+), 17 deletions(-) diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index b2b6a79e..fcf2a1ea 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -124,6 +124,7 @@ class DocumentAccessSerializer(BaseAccessSerializer): allow_null=True, ) user = UserSerializer(read_only=True) + max_ancestors_role = serializers.SerializerMethodField(read_only=True) class Meta: model = models.DocumentAccess @@ -136,8 +137,13 @@ class DocumentAccessSerializer(BaseAccessSerializer): "team", "role", "abilities", + "max_ancestors_role", ] - read_only_fields = ["id", "document_id", "abilities"] + read_only_fields = ["id", "document_id", "abilities", "max_ancestors_role"] + + def get_max_ancestors_role(self, instance): + """Return max_ancestors_role if annotated; else None.""" + return getattr(instance, "max_ancestors_role", None) class DocumentAccessLightSerializer(DocumentAccessSerializer): @@ -155,6 +161,7 @@ class DocumentAccessLightSerializer(DocumentAccessSerializer): "team", "role", "abilities", + "max_ancestors_role", ] read_only_fields = [ "id", @@ -162,6 +169,7 @@ class DocumentAccessLightSerializer(DocumentAccessSerializer): "team", "role", "abilities", + "max_ancestors_role", ] diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 188f068c..4675a0fd 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1551,23 +1551,35 @@ class DocumentAccessViewSet( ) # Annotate more information on roles + path_to_key_to_max_ancestors_role = defaultdict( + lambda: defaultdict(lambda: None) + ) 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] = [] + key = access.target_key + path = access.document.path + parent_path = path[: -models.Document.steplen] - path_to_role[access.document_path] = choices.RoleChoices.max( - path_to_role[access.document_path], access.role + path_to_key_to_max_ancestors_role[path][key] = choices.RoleChoices.max( + path_to_key_to_max_ancestors_role[path][key], access.role + ) + + if parent_path: + path_to_key_to_max_ancestors_role[path][key] = choices.RoleChoices.max( + path_to_key_to_max_ancestors_role[parent_path][key], + path_to_key_to_max_ancestors_role[path][key], + ) + path_to_ancestors_roles[path].extend( + path_to_ancestors_roles[parent_path] + ) + path_to_ancestors_roles[path].append(path_to_role[parent_path]) + else: + path_to_ancestors_roles[path] = [] + + if access.user_id == user.id or access.team in user.teams: + path_to_role[path] = choices.RoleChoices.max( + path_to_role[path], access.role ) # serialize and return the response @@ -1575,9 +1587,16 @@ class DocumentAccessViewSet( serializer_class = self.get_serializer_class() serialized_data = [] for access in accesses: + path = access.document.path + parent_path = path[: -models.Document.steplen] + access.max_ancestors_role = ( + path_to_key_to_max_ancestors_role[parent_path][access.target_key] + if parent_path + else None + ) access.set_user_roles_tuple( - choices.RoleChoices.max(*path_to_ancestors_roles[access.document_path]), - path_to_role.get(access.document_path), + choices.RoleChoices.max(*path_to_ancestors_roles[path]), + path_to_role.get(path), ) serializer = serializer_class(access, context=context) serialized_data.append(serializer.data) diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 8aff1fca..74ef4520 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1140,7 +1140,9 @@ class DocumentAccess(BaseAccess): set_role_to.append(RoleChoices.OWNER) # Filter out roles that would be lower than the one the user already has - ancestors_role_priority = RoleChoices.get_priority(ancestors_role) + ancestors_role_priority = RoleChoices.get_priority( + getattr(self, "max_ancestors_role", None) + ) set_role_to = [ candidate_role for candidate_role in set_role_to 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 1b4151f9..593d2a7b 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses.py +++ b/src/backend/core/tests/documents/test_api_document_accesses.py @@ -148,6 +148,7 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( else None, "team": access.team, "role": access.role, + "max_ancestors_role": None, "abilities": { "destroy": False, "partial_update": False, @@ -248,6 +249,7 @@ def test_api_document_accesses_list_authenticated_related_privileged( } if access.user else None, + "max_ancestors_role": None, "team": access.team, "role": access.role, "abilities": access.get_abilities(user), @@ -258,6 +260,245 @@ def test_api_document_accesses_list_authenticated_related_privileged( ) +def test_api_document_accesses_retrieve_set_role_to_child(): + """Check set_role_to for an access with no access on the ancestor.""" + user, other_user = factories.UserFactory.create_batch(2) + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory() + parent_access = factories.UserDocumentAccessFactory( + document=parent, user=user, role="owner" + ) + + document = factories.DocumentFactory(parent=parent) + document_access_other_user = factories.UserDocumentAccessFactory( + document=document, user=other_user, role="editor" + ) + + response = client.get(f"/api/v1.0/documents/{document.id!s}/accesses/") + + assert response.status_code == 200 + content = response.json() + assert len(content) == 2 + + result_dict = { + result["id"]: result["abilities"]["set_role_to"] for result in content + } + assert result_dict[str(document_access_other_user.id)] == [ + "reader", + "editor", + "administrator", + "owner", + ] + assert result_dict[str(parent_access.id)] == [] + + # Add an access for the other user on the parent + parent_access_other_user = factories.UserDocumentAccessFactory( + document=parent, user=other_user, role="editor" + ) + + response = client.get(f"/api/v1.0/documents/{document.id!s}/accesses/") + + assert response.status_code == 200 + content = response.json() + assert len(content) == 3 + + result_dict = { + result["id"]: result["abilities"]["set_role_to"] for result in content + } + assert result_dict[str(document_access_other_user.id)] == [ + "editor", + "administrator", + "owner", + ] + assert result_dict[str(parent_access.id)] == [] + assert result_dict[str(parent_access_other_user.id)] == [ + "reader", + "editor", + "administrator", + "owner", + ] + + +@pytest.mark.parametrize( + "roles,results", + [ + [ + ["administrator", "reader", "reader", "reader"], + [ + ["reader", "editor", "administrator"], + [], + [], + ["reader", "editor", "administrator"], + ], + ], + [ + ["owner", "reader", "reader", "reader"], + [[], [], [], ["reader", "editor", "administrator", "owner"]], + ], + [ + ["owner", "reader", "reader", "owner"], + [ + ["reader", "editor", "administrator", "owner"], + [], + [], + ["reader", "editor", "administrator", "owner"], + ], + ], + ], +) +def test_api_document_accesses_list_authenticated_related_same_user(roles, results): + """ + The maximum role across ancestor documents and set_role_to optionsfor + a given user should be filled as expected. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + # Create documents structured as a tree + grand_parent = factories.DocumentFactory(link_reach="authenticated") + parent = factories.DocumentFactory(parent=grand_parent) + document = factories.DocumentFactory(parent=parent) + + # Create accesses for another user + other_user = factories.UserFactory() + accesses = [ + factories.UserDocumentAccessFactory( + document=document, user=user, role=roles[0] + ), + factories.UserDocumentAccessFactory( + document=grand_parent, user=other_user, role=roles[1] + ), + factories.UserDocumentAccessFactory( + document=parent, user=other_user, role=roles[2] + ), + factories.UserDocumentAccessFactory( + document=document, user=other_user, role=roles[3] + ), + ] + + response = client.get(f"/api/v1.0/documents/{document.id!s}/accesses/") + + assert response.status_code == 200 + content = response.json() + assert len(content) == 4 + + for result in content: + assert ( + result["max_ancestors_role"] is None + if result["user"]["id"] == str(user.id) + else choices.RoleChoices.max(roles[1], roles[2]) + ) + + result_dict = { + result["id"]: result["abilities"]["set_role_to"] for result in content + } + assert [result_dict[str(access.id)] for access in accesses] == results + + +@pytest.mark.parametrize( + "roles,results", + [ + [ + ["administrator", "reader", "reader", "reader"], + [ + ["reader", "editor", "administrator"], + [], + [], + ["reader", "editor", "administrator"], + ], + ], + [ + ["owner", "reader", "reader", "reader"], + [[], [], [], ["reader", "editor", "administrator", "owner"]], + ], + [ + ["owner", "reader", "reader", "owner"], + [ + ["reader", "editor", "administrator", "owner"], + [], + [], + ["reader", "editor", "administrator", "owner"], + ], + ], + [ + ["reader", "reader", "reader", "owner"], + [["reader", "editor", "administrator", "owner"], [], [], []], + ], + [ + ["reader", "administrator", "reader", "editor"], + [ + ["reader", "editor", "administrator"], + ["reader", "editor", "administrator"], + [], + [], + ], + ], + [ + ["editor", "editor", "administrator", "editor"], + [ + ["reader", "editor", "administrator"], + [], + ["editor", "administrator"], + [], + ], + ], + ], +) +def test_api_document_accesses_list_authenticated_related_same_team( + roles, results, mock_user_teams +): + """ + The maximum role across ancestor documents and set_role_to optionsfor + a given team should be filled as expected. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + # Create documents structured as a tree + grand_parent = factories.DocumentFactory(link_reach="authenticated") + parent = factories.DocumentFactory(parent=grand_parent) + document = factories.DocumentFactory(parent=parent) + + mock_user_teams.return_value = ["lasuite", "unknown"] + accesses = [ + factories.UserDocumentAccessFactory( + document=document, user=user, role=roles[0] + ), + # Create accesses for a team + factories.TeamDocumentAccessFactory( + document=grand_parent, team="lasuite", role=roles[1] + ), + factories.TeamDocumentAccessFactory( + document=parent, team="lasuite", role=roles[2] + ), + factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role=roles[3] + ), + ] + + response = client.get(f"/api/v1.0/documents/{document.id!s}/accesses/") + + assert response.status_code == 200 + content = response.json() + assert len(content) == 4 + + for result in content: + assert ( + result["max_ancestors_role"] is None + if result["user"] and result["user"]["id"] == str(user.id) + else choices.RoleChoices.max(roles[1], roles[2]) + ) + + result_dict = { + result["id"]: result["abilities"]["set_role_to"] for result in content + } + assert [result_dict[str(access.id)] for access in accesses] == results + + def test_api_document_accesses_retrieve_anonymous(): """ Anonymous users should not be allowed to retrieve a document access. @@ -353,6 +594,7 @@ def test_api_document_accesses_retrieve_authenticated_related( "user": access_user, "team": "", "role": access.role, + "max_ancestors_role": None, "abilities": access.get_abilities(user), } 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 cd2d57eb..8b831980 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 @@ -169,6 +169,7 @@ def test_api_document_accesses_create_authenticated_administrator(via, mock_user "id": str(new_document_access.id), "team": "", "role": role, + "max_ancestors_role": None, "user": other_user, } assert len(mail.outbox) == 1 @@ -228,6 +229,7 @@ def test_api_document_accesses_create_authenticated_owner(via, mock_user_teams): "user": other_user, "team": "", "role": role, + "max_ancestors_role": None, "abilities": new_document_access.get_abilities(user), } assert len(mail.outbox) == 1 @@ -293,6 +295,7 @@ def test_api_document_accesses_create_email_in_receivers_language(via, mock_user "user": other_user_data, "team": "", "role": role, + "max_ancestors_role": None, "abilities": new_document_access.get_abilities(user), } assert len(mail.outbox) == index + 1