Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add object tag view permissions [FC-0036] #94

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.3"
__version__ = "0.2.4"
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:
"""
Everybody can view object tags from any objects.

This rule could be defined in other apps for proper permission checking.
"""
return True


@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)
pomegranited marked this conversation as resolved.
Show resolved Hide resolved
55 changes: 42 additions & 13 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"
rpenido marked this conversation as resolved.
Show resolved Hide resolved
"""
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 all objects but "unauthorized_id"
rpenido marked this conversation as resolved.
Show resolved Hide resolved
"""
if object_id == "unauthorized_id":
return False

return can_view_object_tag_objectid(user, object_id)

super().setUp()

Expand Down Expand Up @@ -478,22 +491,20 @@ 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),
(None, status.HTTP_403_FORBIDDEN, None),
("user", status.HTTP_200_OK, 81),
("staff", status.HTTP_200_OK, 81),
)
@ddt.unpack
def test_retrieve_object_tags(self, user_attr, object_id, expected_status, expected_count):
def test_retrieve_object_tags(self, user_attr, expected_status, expected_count):
"""
Test retrieving object tags
"""
url = OBJECT_TAGS_RETRIEVE_URL.format(object_id=object_id)
url = OBJECT_TAGS_RETRIEVE_URL.format(object_id="abc")

if user_attr:
user = getattr(self, user_attr)
Expand All @@ -505,6 +516,24 @@ def test_retrieve_object_tags(self, user_attr, object_id, expected_status, expec
if status.is_success(expected_status):
assert len(response.data) == expected_count

@ddt.data(
None,
"user",
"staff"
)
def test_retrieve_object_tags_unauthorized(self, user_attr):
"""
Test retrieving object tags from an unauthorized object_id
"""
url = OBJECT_TAGS_RETRIEVE_URL.format(object_id="unauthorized_id")

if user_attr:
user = getattr(self, user_attr)
self.client.force_authenticate(user=user)

response = self.client.get(url)
assert response.status_code == status.HTTP_403_FORBIDDEN

rpenido marked this conversation as resolved.
Show resolved Hide resolved
@ddt.data(
(None, "abc", status.HTTP_403_FORBIDDEN, None),
("user", "abc", status.HTTP_200_OK, 20),
Expand Down Expand Up @@ -744,7 +773,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)
rpenido marked this conversation as resolved.
Show resolved Hide resolved

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