From 20315e9b607386028cfea9a1ce46c39c3f22ed9d Mon Sep 17 00:00:00 2001 From: Samuel Paccoud - DINUM Date: Tue, 18 Feb 2025 08:32:49 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20limit=20link=20reach/role?= =?UTF-8?q?=20select=20options=20depending=20on=20ancestors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a document already gets a link reach/role inheriting from one of its ancestors, we should not propose setting link reach/role on the document that would be more restrictive than what we inherited from ancestors. --- CHANGELOG.md | 1 + src/backend/core/models.py | 69 +++++- .../documents/test_api_documents_retrieve.py | 16 ++ .../documents/test_api_documents_trashbin.py | 5 + .../core/tests/test_models_documents.py | 210 +++++++++++++++++- 5 files changed, 284 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc2b8e02..ec2eca97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ and this project adheres to ## Added +- ✨(backend) limit link reach/role select options depending on ancestors #645 - ✨(backend) add new "descendants" action to document API endpoint #645 - ✨(backend) new "tree" action on document detail endpoint #645 - ✨(backend) allow forcing page size within limits #645 diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 80e4a658..1874f565 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -81,6 +81,55 @@ class LinkReachChoices(models.TextChoices): ) # Any authenticated user can access the document PUBLIC = "public", _("Public") # Even anonymous users can access the document + @classmethod + 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 {reach: LinkRoleChoices.values 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} + + # 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) + + if LinkRoleChoices.EDITOR in reach_roles[cls.AUTHENTICATED]: + 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) + 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] + + return result + class DuplicateEmailError(Exception): """Raised when an email is already associated with a pre-existing user.""" @@ -650,20 +699,12 @@ class Document(MP_Node, BaseModel): roles = [] return roles - def get_links_definitions(self, ancestors_links=None): + def get_links_definitions(self, ancestors_links): """Get links reach/role definitions for the current document and its ancestors.""" links_definitions = defaultdict(set) links_definitions[self.link_reach].add(self.link_role) - # Skip ancestor processing if the document is the highest accessible ancestor - if self.depth <= 1 or getattr(self, "is_highest_ancestor_for_user", False): - return links_definitions - - # Fallback to querying the DB if ancestors links are not provided - if ancestors_links is None: - ancestors_links = self.get_ancestors().values("link_reach", "link_role") - # Merge ancestor link definitions for ancestor in ancestors_links: links_definitions[ancestor["link_reach"]].add(ancestor["link_role"]) @@ -674,6 +715,11 @@ class Document(MP_Node, BaseModel): """ Compute and return abilities for a given user on the document. """ + if self.depth <= 1 or getattr(self, "is_highest_ancestor_for_user", False): + ancestors_links = [] + elif ancestors_links is None: + ancestors_links = self.get_ancestors().values("link_reach", "link_role") + roles = set( self.get_roles(user) ) # at this point only roles based on specific access @@ -693,9 +739,7 @@ class Document(MP_Node, BaseModel): ) and not is_deleted # Add roles provided by the document link, taking into account its ancestors - - # Add roles provided by the document link - links_definitions = self.get_links_definitions(ancestors_links=ancestors_links) + links_definitions = self.get_links_definitions(ancestors_links) public_roles = links_definitions.get(LinkReachChoices.PUBLIC, set()) authenticated_roles = ( links_definitions.get(LinkReachChoices.AUTHENTICATED, set()) @@ -740,6 +784,7 @@ class Document(MP_Node, BaseModel): "restore": is_owner, "retrieve": can_get, "media_auth": can_get, + "link_select_options": LinkReachChoices.get_select_options(ancestors_links), "tree": can_get, "update": can_update, "versions_destroy": is_owner_or_admin, 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 b4024e62..e88ad035 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -40,6 +40,11 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "favorite": False, "invite_owner": False, "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": False, "partial_update": document.link_role == "editor", @@ -81,6 +86,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/") assert response.status_code == 200 + links = document.get_ancestors().values("link_reach", "link_role") assert response.json() == { "id": str(document.id), "abilities": { @@ -98,6 +104,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "favorite": False, "invite_owner": False, "link_configuration": False, + "link_select_options": models.LinkReachChoices.get_select_options(links), "media_auth": True, "move": False, "partial_update": grand_parent.link_role == "editor", @@ -189,6 +196,11 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "favorite": True, "invite_owner": False, "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": False, "partial_update": document.link_role == "editor", @@ -238,6 +250,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea response = client.get(f"/api/v1.0/documents/{document.id!s}/") assert response.status_code == 200 + links = document.get_ancestors().values("link_reach", "link_role") assert response.json() == { "id": str(document.id), "abilities": { @@ -254,6 +267,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "favorite": True, "invite_owner": False, "link_configuration": False, + "link_select_options": models.LinkReachChoices.get_select_options(links), "move": False, "media_auth": True, "partial_update": grand_parent.link_role == "editor", @@ -412,6 +426,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): f"/api/v1.0/documents/{document.id!s}/", ) assert response.status_code == 200 + links = document.get_ancestors().values("link_reach", "link_role") assert response.json() == { "id": str(document.id), "abilities": { @@ -428,6 +443,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): "favorite": True, "invite_owner": access.role == "owner", "link_configuration": access.role in ["administrator", "owner"], + "link_select_options": models.LinkReachChoices.get_select_options(links), "media_auth": True, "move": access.role in ["administrator", "owner"], "partial_update": access.role != "reader", 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 9f1d8d7f..b2fcd4b6 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -83,6 +83,11 @@ def test_api_documents_trashbin_format(): "favorite": True, "invite_owner": True, "link_configuration": True, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": False, # Can't move a deleted document "partial_update": True, diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 53a7cd79..ae529a27 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -164,6 +164,11 @@ def test_models_documents_get_abilities_forbidden( "media_auth": False, "move": False, "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "partial_update": False, "restore": False, "retrieve": False, @@ -215,6 +220,11 @@ def test_models_documents_get_abilities_reader( "favorite": is_authenticated, "invite_owner": False, "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": False, "partial_update": False, @@ -229,9 +239,14 @@ def test_models_documents_get_abilities_reader( nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): assert document.get_abilities(user) == expected_abilities + document.soft_delete() document.refresh_from_db() - assert all(value is False for value in document.get_abilities(user).values()) + assert all( + value is False + for key, value in document.get_abilities(user).items() + if key != "link_select_options" + ) @pytest.mark.parametrize( @@ -265,6 +280,11 @@ def test_models_documents_get_abilities_editor( "favorite": is_authenticated, "invite_owner": False, "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": False, "partial_update": True, @@ -281,7 +301,11 @@ def test_models_documents_get_abilities_editor( assert document.get_abilities(user) == expected_abilities document.soft_delete() document.refresh_from_db() - assert all(value is False for value in document.get_abilities(user).values()) + assert all( + value is False + for key, value in document.get_abilities(user).items() + if key != "link_select_options" + ) @override_settings( @@ -305,6 +329,11 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "favorite": True, "invite_owner": True, "link_configuration": True, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": True, "partial_update": True, @@ -346,6 +375,11 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "favorite": True, "invite_owner": False, "link_configuration": True, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": True, "partial_update": True, @@ -362,7 +396,11 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) document.soft_delete() document.refresh_from_db() - assert all(value is False for value in document.get_abilities(user).values()) + assert all( + value is False + for key, value in document.get_abilities(user).items() + if key != "link_select_options" + ) @override_settings( @@ -386,6 +424,11 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "favorite": True, "invite_owner": False, "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": False, "partial_update": True, @@ -402,7 +445,11 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): document.soft_delete() document.refresh_from_db() - assert all(value is False for value in document.get_abilities(user).values()) + assert all( + value is False + for key, value in document.get_abilities(user).items() + if key != "link_select_options" + ) @pytest.mark.parametrize("ai_access_setting", ["public", "authenticated", "restricted"]) @@ -433,6 +480,11 @@ def test_models_documents_get_abilities_reader_user( "favorite": True, "invite_owner": False, "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": False, "partial_update": access_from_link, @@ -451,7 +503,11 @@ def test_models_documents_get_abilities_reader_user( document.soft_delete() document.refresh_from_db() - assert all(value is False for value in document.get_abilities(user).values()) + assert all( + value is False + for key, value in document.get_abilities(user).items() + if key != "link_select_options" + ) def test_models_documents_get_abilities_preset_role(django_assert_num_queries): @@ -478,6 +534,11 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "favorite": True, "invite_owner": False, "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, "media_auth": True, "move": False, "partial_update": False, @@ -931,3 +992,142 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): assert document.ancestors_deleted_at == document.deleted_at assert child1.ancestors_deleted_at == document.deleted_at assert child2.ancestors_deleted_at == document.deleted_at + +@pytest.mark.parametrize( + "ancestors_links, select_options", + [ + # One ancestor + ( + [{"link_reach": "public", "link_role": "reader"}], + { + "restricted": ["editor"], + "authenticated": ["editor"], + "public": ["reader", "editor"], + }, + ), + ([{"link_reach": "public", "link_role": "editor"}], {"public": ["editor"]}), + ( + [{"link_reach": "authenticated", "link_role": "reader"}], + { + "restricted": ["editor"], + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + }, + ), + ( + [{"link_reach": "authenticated", "link_role": "editor"}], + {"authenticated": ["editor"], "public": ["reader", "editor"]}, + ), + ( + [{"link_reach": "restricted", "link_role": "reader"}], + { + "restricted": ["reader", "editor"], + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + }, + ), + ( + [{"link_reach": "restricted", "link_role": "editor"}], + { + "restricted": ["editor"], + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + }, + ), + # Multiple ancestors with different roles + ( + [ + {"link_reach": "public", "link_role": "reader"}, + {"link_reach": "public", "link_role": "editor"}, + ], + {"public": ["editor"]}, + ), + ( + [ + {"link_reach": "authenticated", "link_role": "reader"}, + {"link_reach": "authenticated", "link_role": "editor"}, + ], + {"authenticated": ["editor"], "public": ["reader", "editor"]}, + ), + ( + [ + {"link_reach": "restricted", "link_role": "reader"}, + {"link_reach": "restricted", "link_role": "editor"}, + ], + { + "restricted": ["editor"], + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + }, + ), + # Multiple ancestors with different reaches + ( + [ + {"link_reach": "authenticated", "link_role": "reader"}, + {"link_reach": "public", "link_role": "reader"}, + ], + { + "restricted": ["editor"], + "authenticated": ["editor"], + "public": ["reader", "editor"], + }, + ), + ( + [ + {"link_reach": "restricted", "link_role": "reader"}, + {"link_reach": "authenticated", "link_role": "reader"}, + {"link_reach": "public", "link_role": "reader"}, + ], + { + "restricted": ["editor"], + "authenticated": ["editor"], + "public": ["reader", "editor"], + }, + ), + # Multiple ancestors with mixed reaches and roles + ( + [ + {"link_reach": "authenticated", "link_role": "editor"}, + {"link_reach": "public", "link_role": "reader"}, + ], + {"authenticated": ["editor"], "public": ["reader", "editor"]}, + ), + ( + [ + {"link_reach": "authenticated", "link_role": "reader"}, + {"link_reach": "public", "link_role": "editor"}, + ], + {"public": ["editor"]}, + ), + ( + [ + {"link_reach": "restricted", "link_role": "editor"}, + {"link_reach": "authenticated", "link_role": "reader"}, + ], + { + "restricted": ["editor"], + "authenticated": ["reader", "editor"], + "public": ["reader", "editor"], + }, + ), + ( + [ + {"link_reach": "restricted", "link_role": "reader"}, + {"link_reach": "authenticated", "link_role": "editor"}, + ], + {"authenticated": ["editor"], "public": ["reader", "editor"]}, + ), + # No ancestors (edge case) + ( + [], + { + "public": ["reader", "editor"], + "authenticated": ["reader", "editor"], + "restricted": ["reader", "editor"], + }, + ), + ], +) +def test_models_documents_get_select_options(ancestors_links, select_options): + """Validate that the "get_select_options" method operates as expected.""" + assert models.LinkReachChoices.get_select_options(ancestors_links) == select_options