From c791682b62acf8081510269d64a038b8e1b46e08 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 7 May 2024 15:17:42 -0500 Subject: [PATCH 1/3] feat: Update permissions to remove object tags * chore: Bump version * fix: Issue with remove object tag permission --- openedx_learning/__init__.py | 2 +- .../core/tagging/rest_api/v1/serializers.py | 9 ++++++++- openedx_tagging/core/tagging/rules.py | 11 +++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 16f5d5908..bfa3cbd91 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.9.2" +__version__ = "0.9.3" diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 75fe2fbe9..7e4d71671 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -140,7 +140,14 @@ class Meta: fields = ["value", "lineage", "can_delete_objecttag"] lineage = serializers.ListField(child=serializers.CharField(), source="get_lineage", read_only=True) - can_delete_objecttag = serializers.SerializerMethodField(method_name='get_can_delete') + can_delete_objecttag = serializers.SerializerMethodField() + + def get_can_delete_objecttag(self, instance) -> bool | None: + """ + Returns True if the current request user may delete object tags on this taxonomy + """ + perm_name = f'{self.app_label}.remove_objecttag_objectid' + return self._can(perm_name, instance.object_id) class ObjectTagSerializer(ObjectTagMinimalSerializer): diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index 36f67ffef..81a56b698 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -99,6 +99,16 @@ def can_view_object_tag_objectid(_user: UserType, _object_id: str) -> bool: return True +@rules.predicate +def can_remove_object_tag_objectid(_user: UserType, _object_id: str) -> bool: + """ + Everybody can remove object tags from any objects. + + This rule could be defined in other apps for proper permission checking. + """ + return True + + @rules.predicate def can_view_object_tag( user: UserType, perm_obj: ObjectTagPermissionItem | None = None @@ -194,3 +204,4 @@ def can_change_object_tag( rules.add_perm("oel_tagging.view_objecttag_taxonomy", can_view_object_tag_taxonomy) rules.add_perm("oel_tagging.change_objecttag_taxonomy", can_view_object_tag_taxonomy) rules.add_perm("oel_tagging.change_objecttag_objectid", can_change_object_tag_objectid) +rules.add_perm("oel_tagging.remove_objecttag_objectid", can_remove_object_tag_objectid) From bf49f694ece7c1bcc0fc50b8919111ecb1b158eb Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 8 May 2024 13:04:53 -0500 Subject: [PATCH 2/3] refactor: Update default value of can_remove_object_tag_objectid --- openedx_tagging/core/tagging/rules.py | 20 +++++++++---------- .../core/tagging/test_views.py | 13 ++++++------ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index 81a56b698..02aac1a77 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -99,16 +99,6 @@ def can_view_object_tag_objectid(_user: UserType, _object_id: str) -> bool: return True -@rules.predicate -def can_remove_object_tag_objectid(_user: UserType, _object_id: str) -> bool: - """ - Everybody can remove object tags from any objects. - - This rule could be defined in other apps for proper permission checking. - """ - return True - - @rules.predicate def can_view_object_tag( user: UserType, perm_obj: ObjectTagPermissionItem | None = None @@ -147,6 +137,16 @@ def can_change_object_tag_objectid(_user: UserType, _object_id: str) -> bool: return False +@rules.predicate +def can_remove_object_tag_objectid(_user: UserType, _object_id: str) -> bool: + """ + Nobody can remove object tags without checking the permission for the tagged object. + + This rule could be defined in other apps for proper permission checking. + """ + return can_change_object_tag_objectid(_user, _object_id) + + @rules.predicate def can_change_object_tag( user: UserType, perm_obj: ObjectTagPermissionItem | None = None diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index a9e944eda..531d4fa2a 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -767,12 +767,12 @@ def test_retrieve_object_tags(self, user_attr, expected_status): { "value": "Mammalia", "lineage": ["Eukaryota", "Animalia", "Chordata", "Mammalia"], - "can_delete_objecttag": True, + "can_delete_objecttag": False, }, { "value": "Fungi", "lineage": ["Eukaryota", "Fungi"], - "can_delete_objecttag": True, + "can_delete_objecttag": False, }, ], "export_id": "life_on_earth" @@ -785,7 +785,7 @@ def test_retrieve_object_tags(self, user_attr, expected_status): { "value": "test_user_1", "lineage": ["test_user_1"], - "can_delete_objecttag": True, + "can_delete_objecttag": False, }, ], "export_id": "user_authors" @@ -860,7 +860,7 @@ def test_retrieve_object_tags_sorted(self): Test the sort order of the object tags retrieved from the get object tags API. """ - object_id, sort_test_applied_result = self.prepare_for_sort_test(expected_perm=True) + object_id, sort_test_applied_result = self.prepare_for_sort_test(expected_perm=False) url = OBJECT_TAGS_RETRIEVE_URL.format(object_id=object_id) self.client.force_authenticate(user=self.user_1) @@ -874,7 +874,8 @@ def test_retrieve_object_tags_query_count(self): Test how many queries are used when retrieving object tags and permissions """ expected_perm = True - object_id, sort_test_applied_result = self.prepare_for_sort_test(expected_perm) + expoected_remove_perm = False + object_id, sort_test_applied_result = self.prepare_for_sort_test(expoected_remove_perm) url = OBJECT_TAGS_RETRIEVE_URL.format(object_id=object_id) self.client.force_authenticate(user=self.user_1) @@ -936,7 +937,7 @@ def test_retrieve_object_tags_taxonomy_queryparam( { "value": "test_user_1", "lineage": ["test_user_1"], - "can_delete_objecttag": True, + "can_delete_objecttag": False, }, ], "export_id": "user_authors", From 0df63a1e8d7d6fe15bc8c824b3519d5d09379f4b Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 8 May 2024 14:08:05 -0500 Subject: [PATCH 3/3] style: Typo on tests --- tests/openedx_tagging/core/tagging/test_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 531d4fa2a..587d2de8e 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -874,8 +874,8 @@ def test_retrieve_object_tags_query_count(self): Test how many queries are used when retrieving object tags and permissions """ expected_perm = True - expoected_remove_perm = False - object_id, sort_test_applied_result = self.prepare_for_sort_test(expoected_remove_perm) + expected_remove_perm = False + object_id, sort_test_applied_result = self.prepare_for_sort_test(expected_remove_perm) url = OBJECT_TAGS_RETRIEVE_URL.format(object_id=object_id) self.client.force_authenticate(user=self.user_1)