Skip to content

Commit

Permalink
feat: add ObjectTag view permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
rpenido committed Oct 10, 2023
1 parent ce85d72 commit c1502d0
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 9 deletions.
2 changes: 1 addition & 1 deletion openedx_learning/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
Open edX Learning ("Learning Core").
"""
__version__ = "0.2.2"
__version__ = "0.2.3"
14 changes: 13 additions & 1 deletion openedx_tagging/core/tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,18 @@ def get_queryset(self) -> models.QuerySet:
)
query_params.is_valid(raise_exception=True)
taxonomy_id = query_params.data.get("taxonomy", None)

# Checks the permission for the object_id
objectid_perm = self.request.user.has_perm(
"oel_tagging.view_objecttag_objectid",
# The obj arg expects an object, but we are passing a string
object_id, # type: ignore[arg-type]
)

if not objectid_perm:
raise PermissionDenied(
"You do not have permission to view object tags for this object_id."
)
return get_object_tags(object_id, taxonomy_id)

def retrieve(self, request, *args, **kwargs):
Expand All @@ -255,7 +267,7 @@ def retrieve(self, request, *args, **kwargs):
path and returns a it as a single result however that is not
behavior we want.
"""
object_tags = self.get_queryset()
object_tags = self.filter_queryset(self.get_queryset())
serializer = ObjectTagSerializer(object_tags, many=True)
return Response(serializer.data)

Expand Down
11 changes: 11 additions & 0 deletions openedx_tagging/core/tagging/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ def can_change_tag(user: UserType, tag: Tag | None = None) -> bool:
)


@rules.predicate
def can_view_object_tag_objectid(_user: UserType, _object_id: str) -> bool:
"""
Nobody can view object tags without checking the permission for the tagged object.
This rule should be defined in other apps for proper permission checking.
"""
return False


@rules.predicate
def can_change_object_tag_objectid(_user: UserType, _object_id: str) -> bool:
"""
Expand Down Expand Up @@ -124,5 +134,6 @@ def can_change_object_tag(
rules.add_perm("oel_tagging.view_objecttag", rules.always_allow)

# Users can tag objects using tags from any taxonomy that they have permission to view
rules.add_perm("oel_tagging.view_objecttag_objectid", can_view_object_tag_objectid)
rules.add_perm("oel_tagging.change_objecttag_taxonomy", can_view_taxonomy)
rules.add_perm("oel_tagging.change_objecttag_objectid", can_change_object_tag_objectid)
28 changes: 21 additions & 7 deletions tests/openedx_tagging/core/tagging/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy
from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy
from openedx_tagging.core.tagging.rest_api.paginators import TagsPagination
from openedx_tagging.core.tagging.rules import can_change_object_tag_objectid, can_view_object_tag_objectid

User = get_user_model()

Expand Down Expand Up @@ -404,11 +405,23 @@ class TestObjectTagViewSet(APITestCase):

def setUp(self):

def _object_permission(_user, object_id: str) -> bool:
def _change_object_permission(user, object_id: str) -> bool:
"""
Everyone have object permission on object_id "abc"
Everyone have object permission on object_id "abc" and "limit_tag_count"
"""
return object_id in ("abc", "limit_tag_count")
if object_id in ("abc", "limit_tag_count"):
return True

return can_change_object_tag_objectid(user, object_id)

def _view_object_permission(user, object_id: str) -> bool:
"""
Everyone have object permission on object_id "abc", "limit_tag_count" and "view_only"
"""
if object_id in ("abc", "limit_tag_count", "view_only"):
return True

return can_view_object_tag_objectid(user, object_id)

super().setUp()

Expand Down Expand Up @@ -478,15 +491,16 @@ def _object_permission(_user, object_id: str) -> bool:
self.dummy_taxonomies.append(taxonomy)

# Override the object permission for the test
rules.set_perm("oel_tagging.change_objecttag_objectid", _object_permission)
rules.set_perm("oel_tagging.change_objecttag_objectid", _change_object_permission)
rules.set_perm("oel_tagging.view_objecttag_objectid", _view_object_permission)

@ddt.data(
(None, "abc", status.HTTP_403_FORBIDDEN, None),
("user", "abc", status.HTTP_200_OK, 81),
("staff", "abc", status.HTTP_200_OK, 81),
(None, "non-existing-id", status.HTTP_403_FORBIDDEN, None),
("user", "non-existing-id", status.HTTP_200_OK, 0),
("staff", "non-existing-id", status.HTTP_200_OK, 0),
("user", "non-existing-id", status.HTTP_403_FORBIDDEN, None),
("staff", "non-existing-id", status.HTTP_403_FORBIDDEN, None),
)
@ddt.unpack
def test_retrieve_object_tags(self, user_attr, object_id, expected_status, expected_count):
Expand Down Expand Up @@ -744,7 +758,7 @@ def test_tag_object_without_permission(self, user_attr, expected_status):
user = getattr(self, user_attr)
self.client.force_authenticate(user=user)

url = OBJECT_TAGS_UPDATE_URL.format(object_id="not abc", taxonomy_id=self.enabled_taxonomy.pk)
url = OBJECT_TAGS_UPDATE_URL.format(object_id="view_only", taxonomy_id=self.enabled_taxonomy.pk)

response = self.client.put(url, {"tags": ["Tag 1"]}, format="json")
assert response.status_code == expected_status
Expand Down

0 comments on commit c1502d0

Please sign in to comment.