From d66d6294c1b27fbb285cda365802925f528bfe39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Fri, 13 Oct 2023 11:43:51 -0500 Subject: [PATCH] feat: Authentication classes added to tagging API views (#98) * Authentication classes added to tagging API views * Cast taxonomies before serialize in TaxonomySerializer * Test updated --- .../core/tagging/rest_api/v1/serializers.py | 10 +++ .../core/tagging/rest_api/v1/utils.py | 24 +++++++ .../core/tagging/rest_api/v1/views.py | 4 ++ .../core/tagging/test_views.py | 70 +++++++++---------- 4 files changed, 73 insertions(+), 35 deletions(-) create mode 100644 openedx_tagging/core/tagging/rest_api/v1/utils.py diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index c5472afb..388ee6cc 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -17,6 +17,9 @@ class TaxonomyListQueryParamsSerializer(serializers.Serializer): # pylint: disa class TaxonomySerializer(serializers.ModelSerializer): + """ + Serializer for the Taxonomy model. + """ class Meta: model = Taxonomy fields = [ @@ -30,6 +33,13 @@ class Meta: "visible_to_authors", ] + def to_representation(self, instance): + """ + Cast the taxonomy before serialize + """ + instance = instance.cast() + return super().to_representation(instance) + class ObjectTagListQueryParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method """ diff --git a/openedx_tagging/core/tagging/rest_api/v1/utils.py b/openedx_tagging/core/tagging/rest_api/v1/utils.py new file mode 100644 index 00000000..0667fb3b --- /dev/null +++ b/openedx_tagging/core/tagging/rest_api/v1/utils.py @@ -0,0 +1,24 @@ +""" +Utilities for the API +""" +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication # type: ignore[import] +from edx_rest_framework_extensions.auth.session.authentication import ( # type: ignore[import] + SessionAuthenticationAllowInactiveUser, +) + + +def view_auth_classes(func_or_class): + """ + Function and class decorator that abstracts the authentication classes for api views. + """ + def _decorator(func_or_class): + """ + Requires either OAuth2 or Session-based authentication; + are the same authentication classes used on edx-platform + """ + func_or_class.authentication_classes = ( + JwtAuthentication, + SessionAuthenticationAllowInactiveUser, + ) + return func_or_class + return _decorator(func_or_class) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index f940add1..c00f78dc 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -38,8 +38,10 @@ TaxonomyListQueryParamsSerializer, TaxonomySerializer, ) +from .utils import view_auth_classes +@view_auth_classes class TaxonomyView(ModelViewSet): """ View to list, create, retrieve, update, or delete Taxonomies. @@ -182,6 +184,7 @@ def perform_create(self, serializer) -> None: serializer.instance = create_taxonomy(**serializer.validated_data) +@view_auth_classes class ObjectTagView( mixins.RetrieveModelMixin, mixins.UpdateModelMixin, @@ -322,6 +325,7 @@ def update(self, request, *args, **kwargs): return self.retrieve(request, object_id) +@view_auth_classes class TaxonomyTagsView(ListAPIView): """ View to list tags of a taxonomy. diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 9183ee5f..5092c414 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -113,7 +113,7 @@ def test_list_taxonomy_queryparams(self, enabled, expected_status: int, expected assert len(response.data["results"]) == expected_count @ddt.data( - (None, status.HTTP_403_FORBIDDEN), + (None, status.HTTP_401_UNAUTHORIZED), ("user", status.HTTP_200_OK), ("staff", status.HTTP_200_OK), ) @@ -161,8 +161,8 @@ def test_list_invalid_page(self) -> None: assert response.status_code == status.HTTP_404_NOT_FOUND @ddt.data( - (None, {"enabled": True}, status.HTTP_403_FORBIDDEN), - (None, {"enabled": False}, status.HTTP_403_FORBIDDEN), + (None, {"enabled": True}, status.HTTP_401_UNAUTHORIZED), + (None, {"enabled": False}, status.HTTP_401_UNAUTHORIZED), ("user", {"enabled": True}, status.HTTP_200_OK), ("user", {"enabled": False}, status.HTTP_404_NOT_FOUND), ("staff", {"enabled": True}, status.HTTP_200_OK), @@ -192,7 +192,7 @@ def test_detail_taxonomy_404(self) -> None: assert response.status_code == status.HTTP_404_NOT_FOUND @ddt.data( - (None, status.HTTP_403_FORBIDDEN), + (None, status.HTTP_401_UNAUTHORIZED), ("user", status.HTTP_403_FORBIDDEN), ("staff", status.HTTP_201_CREATED), ) @@ -246,7 +246,7 @@ def test_create_taxonomy_system_defined(self, create_data): assert not response.data["system_defined"] @ddt.data( - (None, status.HTTP_403_FORBIDDEN), + (None, status.HTTP_401_UNAUTHORIZED), ("user", status.HTTP_403_FORBIDDEN), ("staff", status.HTTP_200_OK), ) @@ -308,7 +308,7 @@ def test_update_taxonomy_404(self): assert response.status_code == status.HTTP_404_NOT_FOUND @ddt.data( - (None, status.HTTP_403_FORBIDDEN), + (None, status.HTTP_401_UNAUTHORIZED), ("user", status.HTTP_403_FORBIDDEN), ("staff", status.HTTP_200_OK), ) @@ -365,7 +365,7 @@ def test_patch_taxonomy_404(self): assert response.status_code == status.HTTP_404_NOT_FOUND @ddt.data( - (None, status.HTTP_403_FORBIDDEN), + (None, status.HTTP_401_UNAUTHORIZED), ("user", status.HTTP_403_FORBIDDEN), ("staff", status.HTTP_204_NO_CONTENT), ) @@ -481,10 +481,10 @@ def _object_permission(_user, object_id: str) -> bool: rules.set_perm("oel_tagging.change_objecttag_objectid", _object_permission) @ddt.data( - (None, "abc", status.HTTP_403_FORBIDDEN, None), + (None, "abc", status.HTTP_401_UNAUTHORIZED, None), ("user", "abc", status.HTTP_200_OK, 81), ("staff", "abc", status.HTTP_200_OK, 81), - (None, "non-existing-id", status.HTTP_403_FORBIDDEN, None), + (None, "non-existing-id", status.HTTP_401_UNAUTHORIZED, None), ("user", "non-existing-id", status.HTTP_200_OK, 0), ("staff", "non-existing-id", status.HTTP_200_OK, 0), ) @@ -506,7 +506,7 @@ def test_retrieve_object_tags(self, user_attr, object_id, expected_status, expec assert len(response.data) == expected_count @ddt.data( - (None, "abc", status.HTTP_403_FORBIDDEN, None), + (None, "abc", status.HTTP_401_UNAUTHORIZED, None), ("user", "abc", status.HTTP_200_OK, 20), ("staff", "abc", status.HTTP_200_OK, 20), ) @@ -532,7 +532,7 @@ def test_retrieve_object_tags_taxonomy_queryparam( assert object_tag.get("taxonomy_id") == self.enabled_taxonomy.pk @ddt.data( - (None, "abc", status.HTTP_403_FORBIDDEN), + (None, "abc", status.HTTP_401_UNAUTHORIZED), ("user", "abc", status.HTTP_400_BAD_REQUEST), ("staff", "abc", status.HTTP_400_BAD_REQUEST), ) @@ -552,9 +552,9 @@ def test_retrieve_object_tags_invalid_taxonomy_queryparam(self, user_attr, objec assert response.status_code == expected_status @ddt.data( - (None, "POST", status.HTTP_403_FORBIDDEN), - (None, "PATCH", status.HTTP_403_FORBIDDEN), - (None, "DELETE", status.HTTP_403_FORBIDDEN), + (None, "POST", status.HTTP_401_UNAUTHORIZED), + (None, "PATCH", status.HTTP_401_UNAUTHORIZED), + (None, "DELETE", status.HTTP_401_UNAUTHORIZED), ("user", "POST", status.HTTP_405_METHOD_NOT_ALLOWED), ("user", "PATCH", status.HTTP_405_METHOD_NOT_ALLOWED), ("user", "DELETE", status.HTTP_405_METHOD_NOT_ALLOWED), @@ -593,27 +593,27 @@ def test_object_tags_remaining_http_methods( @ddt.data( # Users and staff can add tags to a taxonomy - (None, "language_taxonomy", ["Portuguese"], status.HTTP_403_FORBIDDEN), + (None, "language_taxonomy", ["Portuguese"], status.HTTP_401_UNAUTHORIZED), ("user", "language_taxonomy", ["Portuguese"], status.HTTP_200_OK), ("staff", "language_taxonomy", ["Portuguese"], status.HTTP_200_OK), # Users and staff can clear add tags to a taxonomy - (None, "enabled_taxonomy", ["Tag 1"], status.HTTP_403_FORBIDDEN), + (None, "enabled_taxonomy", ["Tag 1"], status.HTTP_401_UNAUTHORIZED), ("user", "enabled_taxonomy", ["Tag 1"], status.HTTP_200_OK), ("staff", "enabled_taxonomy", ["Tag 1"], status.HTTP_200_OK), # Only staff can add tag to a disabled taxonomy - (None, "disabled_taxonomy", ["Tag 1"], status.HTTP_403_FORBIDDEN), + (None, "disabled_taxonomy", ["Tag 1"], status.HTTP_401_UNAUTHORIZED), ("user", "disabled_taxonomy", ["Tag 1"], status.HTTP_403_FORBIDDEN), ("staff", "disabled_taxonomy", ["Tag 1"], status.HTTP_200_OK), # Users and staff can add a single tag to a allow_multiple=True taxonomy - (None, "multiple_taxonomy", ["Tag 1"], status.HTTP_403_FORBIDDEN), + (None, "multiple_taxonomy", ["Tag 1"], status.HTTP_401_UNAUTHORIZED), ("user", "multiple_taxonomy", ["Tag 1"], status.HTTP_200_OK), ("staff", "multiple_taxonomy", ["Tag 1"], status.HTTP_200_OK), # Users and staff can add tags to an open taxonomy - (None, "open_taxonomy_enabled", ["tag1"], status.HTTP_403_FORBIDDEN), + (None, "open_taxonomy_enabled", ["tag1"], status.HTTP_401_UNAUTHORIZED), ("user", "open_taxonomy_enabled", ["tag1"], status.HTTP_200_OK), ("staff", "open_taxonomy_enabled", ["tag1"], status.HTTP_200_OK), # Only staff can add tags to a disabled open taxonomy - (None, "open_taxonomy_disabled", ["tag1"], status.HTTP_403_FORBIDDEN), + (None, "open_taxonomy_disabled", ["tag1"], status.HTTP_401_UNAUTHORIZED), ("user", "open_taxonomy_disabled", ["tag1"], status.HTTP_403_FORBIDDEN), ("staff", "open_taxonomy_disabled", ["tag1"], status.HTTP_200_OK), ) @@ -635,17 +635,17 @@ def test_tag_object(self, user_attr, taxonomy_attr, tag_values, expected_status) @ddt.data( # Can't add invalid tags to a closed taxonomy - (None, "language_taxonomy", ["Invalid"], status.HTTP_403_FORBIDDEN), + (None, "language_taxonomy", ["Invalid"], status.HTTP_401_UNAUTHORIZED), ("user", "language_taxonomy", ["Invalid"], status.HTTP_400_BAD_REQUEST), ("staff", "language_taxonomy", ["Invalid"], status.HTTP_400_BAD_REQUEST), - (None, "enabled_taxonomy", ["invalid"], status.HTTP_403_FORBIDDEN), + (None, "enabled_taxonomy", ["invalid"], status.HTTP_401_UNAUTHORIZED), ("user", "enabled_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), ("staff", "enabled_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), - (None, "multiple_taxonomy", ["invalid"], status.HTTP_403_FORBIDDEN), + (None, "multiple_taxonomy", ["invalid"], status.HTTP_401_UNAUTHORIZED), ("user", "multiple_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), ("staff", "multiple_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), # Users can't edit tags from a disabled taxonomy. Staff can't add invalid tags to a closed taxonomy - (None, "disabled_taxonomy", ["invalid"], status.HTTP_403_FORBIDDEN), + (None, "disabled_taxonomy", ["invalid"], status.HTTP_401_UNAUTHORIZED), ("user", "disabled_taxonomy", ["invalid"], status.HTTP_403_FORBIDDEN), ("staff", "disabled_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), ) @@ -665,18 +665,18 @@ def test_tag_object_invalid(self, user_attr, taxonomy_attr, tag_values, expected @ddt.data( # Users and staff can clear tags from a taxonomy - (None, "enabled_taxonomy", [], status.HTTP_403_FORBIDDEN), + (None, "enabled_taxonomy", [], status.HTTP_401_UNAUTHORIZED), ("user", "enabled_taxonomy", [], status.HTTP_200_OK), ("staff", "enabled_taxonomy", [], status.HTTP_200_OK), # Users and staff can clear tags from a allow_multiple=True taxonomy - (None, "multiple_taxonomy", [], status.HTTP_403_FORBIDDEN), + (None, "multiple_taxonomy", [], status.HTTP_401_UNAUTHORIZED), ("user", "multiple_taxonomy", [], status.HTTP_200_OK), ("staff", "multiple_taxonomy", [], status.HTTP_200_OK), # Only staff can clear tags from a disabled taxonomy - (None, "disabled_taxonomy", [], status.HTTP_403_FORBIDDEN), + (None, "disabled_taxonomy", [], status.HTTP_401_UNAUTHORIZED), ("user", "disabled_taxonomy", [], status.HTTP_403_FORBIDDEN), ("staff", "disabled_taxonomy", [], status.HTTP_200_OK), - (None, "open_taxonomy_disabled", [], status.HTTP_403_FORBIDDEN), + (None, "open_taxonomy_disabled", [], status.HTTP_401_UNAUTHORIZED), ("user", "open_taxonomy_disabled", [], status.HTTP_403_FORBIDDEN), ("staff", "open_taxonomy_disabled", [], status.HTTP_200_OK), ) @@ -698,22 +698,22 @@ def test_tag_object_clear(self, user_attr, taxonomy_attr, tag_values, expected_s @ddt.data( # Users and staff can add multiple tags to a allow_multiple=True taxonomy - (None, "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN), + (None, "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_401_UNAUTHORIZED), ("user", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), ("staff", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), - (None, "open_taxonomy_enabled", ["tag1", "tag2"], status.HTTP_403_FORBIDDEN), + (None, "open_taxonomy_enabled", ["tag1", "tag2"], status.HTTP_401_UNAUTHORIZED), ("user", "open_taxonomy_enabled", ["tag1", "tag2"], status.HTTP_400_BAD_REQUEST), ("staff", "open_taxonomy_enabled", ["tag1", "tag2"], status.HTTP_400_BAD_REQUEST), # Users and staff can't add multple tags to a allow_multiple=False taxonomy - (None, "enabled_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN), + (None, "enabled_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_401_UNAUTHORIZED), ("user", "enabled_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_400_BAD_REQUEST), ("staff", "enabled_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_400_BAD_REQUEST), - (None, "language_taxonomy", ["Portuguese", "English"], status.HTTP_403_FORBIDDEN), + (None, "language_taxonomy", ["Portuguese", "English"], status.HTTP_401_UNAUTHORIZED), ("user", "language_taxonomy", ["Portuguese", "English"], status.HTTP_400_BAD_REQUEST), ("staff", "language_taxonomy", ["Portuguese", "English"], status.HTTP_400_BAD_REQUEST), # Users can't edit tags from a disabled taxonomy. Staff can't add multiple tags to # a taxonomy with allow_multiple=False - (None, "disabled_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN), + (None, "disabled_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_401_UNAUTHORIZED), ("user", "disabled_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN), ("staff", "disabled_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_400_BAD_REQUEST), ) @@ -734,7 +734,7 @@ def test_tag_object_multiple(self, user_attr, taxonomy_attr, tag_values, expecte assert set(t["value"] for t in response.data) == set(tag_values) @ddt.data( - (None, status.HTTP_403_FORBIDDEN), + (None, status.HTTP_401_UNAUTHORIZED), ("user", status.HTTP_403_FORBIDDEN), ("staff", status.HTTP_403_FORBIDDEN), ) @@ -828,7 +828,7 @@ def test_invalid_taxonomy(self): def test_not_authorized_user(self): # Not authenticated user response = self.client.get(self.small_taxonomy_url) - assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.status_code == status.HTTP_401_UNAUTHORIZED self.small_taxonomy.enabled = False self.small_taxonomy.save()