diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 6dbf1a32..90d5d235 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -457,7 +457,7 @@ class ItemViewSet( def get_queryset(self): """Get queryset performing all annotation and filtering on the item tree structure.""" user = self.request.user - queryset = super().get_queryset().select_related("creator") + queryset = super().get_queryset().select_related("creator").annotate_has_restriction() # Remove items with upload_state SUSPICIOUS for non-creators queryset = self._filter_suspicious_items(queryset, user) @@ -522,7 +522,7 @@ class ItemViewSet( for path in root_paths: path_list |= db.Q(path__descendants=path) - queryset = self.queryset.select_related("creator") + queryset = self.queryset.select_related("creator").annotate_has_restriction() # Remove items with upload_state SUSPICIOUS for non-creators queryset = self._filter_suspicious_items(queryset, user) queryset = self._exclude_pending_items(queryset) diff --git a/src/backend/core/permissions/backends/role.py b/src/backend/core/permissions/backends/role.py index 4554f093..1674afc6 100644 --- a/src/backend/core/permissions/backends/role.py +++ b/src/backend/core/permissions/backends/role.py @@ -2,6 +2,8 @@ from __future__ import annotations +from functools import cached_property + from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.db.models import Q, QuerySet @@ -37,6 +39,16 @@ class ItemAbilities: # pylint: disable=too-many-public-methods self.is_owner = self.access_role == RoleChoices.OWNER self.is_owner_or_admin = self.is_owner or self.access_role == RoleChoices.ADMIN + @cached_property + def is_restricted(self) -> bool: + """Return whether the item is a restricted folder, querying once and only for roots.""" + # Only a folder at the tree root can be restricted + return ( + self.item.type == models.ItemTypeChoices.FOLDER + and self.item.is_root + and self.item.is_restricted + ) + def has_access_role(self) -> bool: """Return whether the user holds a role through accesses on a live item.""" # Based on accesses only so that anonymous users granted by a link @@ -61,6 +73,10 @@ class ItemAbilities: # pylint: disable=too-many-public-methods """Return whether the user can manage the item and its accesses.""" return self.is_owner_or_admin and not self.is_deleted + def can_move(self) -> bool: + """Return whether the user can move the item: a restricted folder must stay at the root.""" + return self.can_manage() and not self.is_restricted + def can_update(self) -> bool: """Return whether the user can modify the item.""" return (self.is_owner_or_admin or self.role == RoleChoices.EDITOR) and not self.is_deleted @@ -111,6 +127,17 @@ class ItemAbilities: # pylint: disable=too-many-public-methods and bool(settings.WOPI_ONLYOFFICE_CONVERT_JWT_SECRET) ) + def can_restrict(self) -> bool: + """Return whether the user can toggle restriction on the folder.""" + # A restricted folder lives at the tree root: deactivation must stay + # possible there, while activation requires a parent for the restriction + return ( + self.is_owner + and not self.is_deleted + and self.item.type == models.ItemTypeChoices.FOLDER + and (self.item.depth > 1 or self.is_restricted) + ) + def can_favorite(self) -> bool: """Return whether the user can mark the item as favorite.""" return self.can_get() and self.user.is_authenticated @@ -144,7 +171,8 @@ class ItemAbilities: # pylint: disable=too-many-public-methods "link_configuration": self.can_manage(), "invite_owner": self.can_invite_owner(), "link_select_options": self.link_select_options(), - "move": self.can_manage(), + "move": self.can_move(), + "restrict": self.can_restrict(), "restore": self.can_restore(), "retrieve": self.can_retrieve(), "tree": self.can_get(), diff --git a/src/backend/core/tests/test_models_items.py b/src/backend/core/tests/test_models_items.py index eb3f6181..f8b6c1d5 100644 --- a/src/backend/core/tests/test_models_items.py +++ b/src/backend/core/tests/test_models_items.py @@ -319,6 +319,7 @@ def test_models_items_get_abilities_forbidden( "link_configuration": False, "link_select_options": {}, "partial_update": False, + "restrict": False, "restore": False, "retrieve": False, "tree": False, @@ -369,6 +370,7 @@ def test_models_items_get_abilities_reader(is_authenticated, reach, django_asser "download": True, "move": False, "partial_update": False, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -474,6 +476,7 @@ def test_models_items_get_abilities_editor( # noqa: PLR0913 "download": True, "move": False, "partial_update": True, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -544,6 +547,7 @@ def test_models_items_not_root_get_abilities_owner( "download": True, "move": True, "partial_update": True, + "restrict": False, "restore": True, "retrieve": True, "tree": True, @@ -552,7 +556,9 @@ def test_models_items_not_root_get_abilities_owner( "wopi": True, "convert": False, } - with django_assert_num_queries(1): + # A folder at the tree root checks for a targeting restriction + nb_queries = 2 if item_type == models.ItemTypeChoices.FOLDER else 1 + with django_assert_num_queries(nb_queries): assert item.get_abilities(user) == expected_abilities item.soft_delete() item.refresh_from_db() @@ -574,6 +580,7 @@ def test_models_items_not_root_get_abilities_owner( "download": False, "move": False, "partial_update": False, + "restrict": False, "restore": True, "retrieve": True, "tree": False, @@ -634,6 +641,7 @@ def test_models_items_not_root_get_abilities_administrator( "download": True, "move": True, "partial_update": True, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -642,7 +650,9 @@ def test_models_items_not_root_get_abilities_administrator( "wopi": True, "convert": False, } - with django_assert_num_queries(1): + # A folder at the tree root checks for a targeting restriction + nb_queries = 2 if item_type == models.ItemTypeChoices.FOLDER else 1 + with django_assert_num_queries(nb_queries): assert item.get_abilities(user) == expected_abilities item.soft_delete() item.refresh_from_db() @@ -704,6 +714,7 @@ def test_models_items_not_root_get_abilities_editor_user( "download": True, "move": False, "partial_update": True, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -757,6 +768,7 @@ def test_models_items_not_root_get_abilities_reader_user(django_assert_num_queri "download": True, "move": False, "partial_update": access_from_link, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -815,6 +827,7 @@ def test_models_items_get_abilities_hard_delete_non_root_by_non_creator( "media_auth": True, "move": True, "partial_update": True, + "restrict": False, "restore": True, "retrieve": True, "tree": True, @@ -846,6 +859,7 @@ def test_models_items_get_abilities_hard_delete_non_root_by_non_creator( "media_auth": False, "move": False, "partial_update": False, + "restrict": False, "restore": True, "retrieve": True, "tree": False, diff --git a/src/backend/core/tests/test_models_items_restricted.py b/src/backend/core/tests/test_models_items_restricted.py index 1ee2ef8d..6de925b7 100644 --- a/src/backend/core/tests/test_models_items_restricted.py +++ b/src/backend/core/tests/test_models_items_restricted.py @@ -9,6 +9,94 @@ from core import factories, models pytestmark = pytest.mark.django_db +@pytest.mark.parametrize( + "role,expected", + [ + ("owner", True), + ("administrator", False), + ("editor", False), + ("reader", False), + ], +) +def test_models_items_restricted_get_abilities_restrict_requires_owner(role, expected): + """Only an owner can restrict a folder.""" + user = factories.UserFactory() + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + folder = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + factories.UserItemAccessFactory(item=folder, user=user, role=role) + + abilities = folder.get_abilities(user) + + assert abilities["restrict"] is expected + + +def test_models_items_restricted_get_abilities_restrict_forbidden_on_roots(): + """A root folder cannot be restricted: no parent can hold its restriction.""" + user = factories.UserFactory() + folder = factories.ItemFactory( + type=models.ItemTypeChoices.FOLDER, + users=[(user, models.RoleChoices.OWNER)], + ) + + abilities = folder.get_abilities(user) + + assert abilities["restrict"] is False + + +def test_models_items_restricted_get_abilities_restrict_allowed_on_restricted_root(): + """An explicit owner can deactivate a restricted folder moved to the root.""" + user = factories.UserFactory() + folder = factories.ItemFactory( + type=models.ItemTypeChoices.FOLDER, + users=[(user, models.RoleChoices.OWNER)], + ) + factories.RestrictionFactory(target=folder) + + abilities = folder.get_abilities(user) + + assert abilities["restrict"] is True + + +def test_models_items_restricted_get_abilities_move_forbidden_on_restricted_root(): + """A restricted folder cannot be moved, even by an explicit owner.""" + user = factories.UserFactory() + folder = factories.ItemFactory( + type=models.ItemTypeChoices.FOLDER, + users=[(user, models.RoleChoices.OWNER)], + ) + + assert folder.get_abilities(user)["move"] is True + + factories.RestrictionFactory(target=folder) + + assert folder.get_abilities(user)["move"] is False + + +def test_models_items_restricted_get_abilities_restrict_forbidden_on_files(): + """A file cannot be restricted.""" + user = factories.UserFactory() + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + item = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FILE) + factories.UserItemAccessFactory(item=item, user=user, role="owner") + + abilities = item.get_abilities(user) + + assert abilities["restrict"] is False + + +def test_models_items_restricted_get_abilities_restrict_forbidden_when_deleted(): + """A soft deleted folder cannot be restricted.""" + user = factories.UserFactory() + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + folder = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + factories.UserItemAccessFactory(item=folder, user=user, role="owner") + folder.soft_delete() + + abilities = folder.get_abilities(user) + + assert abilities["restrict"] is False + + def test_models_items_restricted_folder_can_be_restricted(): """A folder is restricted when a restriction targets it.""" folder = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) diff --git a/src/backend/core/tests/test_models_items_root.py b/src/backend/core/tests/test_models_items_root.py index ccb62272..0c8378e2 100644 --- a/src/backend/core/tests/test_models_items_root.py +++ b/src/backend/core/tests/test_models_items_root.py @@ -65,6 +65,7 @@ def test_models_sub_item_abilities_downgraded(): "download": True, "move": False, "partial_update": True, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -101,6 +102,7 @@ def test_models_sub_item_abilities_downgraded(): "download": True, "move": False, "partial_update": False, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -155,6 +157,7 @@ def test_models_items_root_get_abilities_owner( "download": True, "move": True, "partial_update": True, + "restrict": False, "restore": True, "retrieve": True, "tree": True, @@ -163,7 +166,9 @@ def test_models_items_root_get_abilities_owner( "wopi": True, "convert": False, } - with django_assert_num_queries(1): + # A folder at the tree root checks for a targeting restriction + nb_queries = 2 if item_type == models.ItemTypeChoices.FOLDER else 1 + with django_assert_num_queries(nb_queries): assert item.get_abilities(user) == expected_abilities item.soft_delete() item.refresh_from_db() @@ -185,6 +190,7 @@ def test_models_items_root_get_abilities_owner( "download": False, "move": False, "partial_update": False, + "restrict": False, "restore": True, "retrieve": True, "tree": False, @@ -242,6 +248,7 @@ def test_models_items_root_get_abilities_administrator( "download": True, "move": True, "partial_update": True, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -250,7 +257,9 @@ def test_models_items_root_get_abilities_administrator( "wopi": True, "convert": False, } - with django_assert_num_queries(1): + # A folder at the tree root checks for a targeting restriction + nb_queries = 2 if item_type == models.ItemTypeChoices.FOLDER else 1 + with django_assert_num_queries(nb_queries): assert item.get_abilities(user) == expected_abilities item.soft_delete() item.refresh_from_db() @@ -308,6 +317,7 @@ def test_models_items_root_get_abilities_editor_user( "download": True, "move": False, "partial_update": True, + "restrict": False, "restore": False, "retrieve": True, "tree": True, @@ -361,6 +371,7 @@ def test_models_items_root_get_abilities_reader_user( "download": True, "move": False, "partial_update": access_from_link, + "restrict": False, "restore": False, "retrieve": True, "tree": True,