🛂(backend) use playlist token in markdown apis

As we are now using playlist tokens, it has to be allowed on apis.
openfun · Aug 7, 2023 · 4676cca · 4676cca
1 parent fe2f1cc
commit 4676cca
Show file tree

Hide file tree

Showing 14 changed files with 96 additions and 75 deletions.
diff --git a/src/backend/marsha/markdown/api.py b/src/backend/marsha/markdown/api.py
@@ -23,6 +23,22 @@
 from .utils.converter import LatexConversionException, render_latex_to_image
 
 
+class ObjectMarkdownDocumentRelatedMixin:
+    """
+    Get the related markdown document id contained in resource.
+
+    It exposes a function used to get the related markdown document.
+    It is also useful to avoid URL crafting (when the url markdown_document_id doesn't
+    match token resource markdown document id).
+    """
+
+    def get_related_markdown_document_id(self):
+        """Get the related markdown document ID from the request."""
+
+        # The video ID in the URL is mandatory.
+        return self.kwargs.get("markdown_document_id")
+
+
 class MarkdownDocumentFilter(django_filters.FilterSet):
     """Filter for file depository."""
 
@@ -57,7 +73,7 @@ class MarkdownDocumentViewSet(
 
     permission_classes = [
         (
-            core_permissions.IsTokenResourceRouteObject
+            core_permissions.IsPlaylistToken
             & (core_permissions.IsTokenInstructor | core_permissions.IsTokenAdmin)
         )
         | markdown_permissions.IsMarkdownDocumentPlaylistOrOrganizationAdmin
@@ -272,6 +288,7 @@ class MarkdownImageViewSet(
     APIViewMixin,
     ObjectPkMixin,
     ObjectRelatedMixin,
+    ObjectMarkdownDocumentRelatedMixin,
     mixins.CreateModelMixin,
     mixins.DestroyModelMixin,
     mixins.RetrieveModelMixin,
@@ -293,9 +310,7 @@ def get_permissions(self):
         else:
             permission_classes = [
                 markdown_permissions.IsTokenResourceRouteObjectRelatedMarkdownDocument
-                & core_permissions.IsTokenInstructor
-                | markdown_permissions.IsTokenResourceRouteObjectRelatedMarkdownDocument
-                & core_permissions.IsTokenAdmin
+                & (core_permissions.IsTokenInstructor | core_permissions.IsTokenAdmin)
                 | IsRelatedMarkdownDocumentPlaylistOrOrganizationAdmin
             ]
         return [permission() for permission in permission_classes]
@@ -306,7 +321,8 @@ def get_queryset(self):
         """
         if self.request.resource:
             return MarkdownImage.objects.filter(
-                markdown_document__id=self.request.resource.id,
+                markdown_document__id=self.get_related_markdown_document_id(),
+                markdown_document__playlist__id=self.request.resource.id,
             )
         return MarkdownImage.objects.all()
 

diff --git a/src/backend/marsha/markdown/permissions.py b/src/backend/marsha/markdown/permissions.py
@@ -4,6 +4,7 @@
 from rest_framework import permissions
 
 from marsha.core import models
+from marsha.markdown.models import MarkdownDocument
 
 
 def _is_organization_admin(user_id, markdown_document_id):
@@ -32,24 +33,23 @@ def _is_playlist_admin(user_id, markdown_document_id):
     ).exists()
 
 
-class IsTokenResourceRouteObjectRelatedResource(permissions.BasePermission):
+class IsTokenResourceRouteObjectRelatedMarkdownDocument(permissions.BasePermission):
     """
-    Base permission class for JWT Tokens related to a resource object linked to a resource.
+    Base permission class for JWT Tokens related to a resource object linked to a
+    Markdown document.
 
-    These permissions grant access to users authenticated with a resource JWT token built from a
-    resource.
+    These permissions grants access to users authenticated with a JWT token built from a
+    resource ie related to a TokenUser as defined in `rest_framework_simplejwt`.
     """
 
-    linked_resource_attribute = ""
-
     def has_permission(self, request, view):
         """
-        Allow the request if the JWT resource matches the resource
-        related to the object in the url.
+        Allow the request if the JWT resource matches the Markdown document related to the object
+        in the url.
 
         Parameters
         ----------
-        request : Type[rest_framework.request.Request]
+        request : Type[django.http.request.HttpRequest]
             The request that holds the authenticated user
         view : Type[rest_framework.viewsets or rest_framework.views]
             The API view for which permissions are being checked
@@ -62,29 +62,9 @@ def has_permission(self, request, view):
         if not request.resource:
             return False
 
-        try:
-            return (
-                str(
-                    getattr(
-                        view.get_related_object(),
-                        self.linked_resource_attribute,
-                    ).id
-                )
-                == request.resource.id
-            )
-        except ObjectDoesNotExist:
-            return False
-
-
-class IsTokenResourceRouteObjectRelatedMarkdownDocument(
-    IsTokenResourceRouteObjectRelatedResource
-):
-    """
-    Base permission class for JWT Tokens related to a resource object
-    linked to a Markdown document.
-    """
-
-    linked_resource_attribute = "markdown_document"
+        return MarkdownDocument.objects.filter(
+            pk=view.get_related_markdown_document_id(), playlist_id=request.resource.id
+        ).exists()
 
 
 class IsMarkdownDocumentPlaylistOrOrganizationAdmin(permissions.BasePermission):

diff --git a/src/backend/marsha/markdown/serializers.py b/src/backend/marsha/markdown/serializers.py
@@ -64,14 +64,9 @@ def create(self, validated_data):
             The "validated_data" dictionary is returned after modification.
 
         """
-        # resource here is a Markdown document
-        resource = self.context["request"].resource
-        markdown_document_id = self.context["request"].data.get("markdown_document")
+        markdown_document_id = self.context["view"].get_related_markdown_document_id()
         if not validated_data.get("markdown_document_id"):
-            if resource:
-                validated_data["markdown_document_id"] = resource.id
-            elif markdown_document_id:
-                validated_data["markdown_document_id"] = markdown_document_id
+            validated_data["markdown_document_id"] = markdown_document_id
 
         return super().create(validated_data)
 

diff --git a/src/backend/marsha/markdown/tests/api/markdown_documents/test_create.py b/src/backend/marsha/markdown/tests/api/markdown_documents/test_create.py
@@ -38,7 +38,7 @@ def test_api_document_create_student(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = StudentLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=True,
         )
 
@@ -60,7 +60,9 @@ def test_api_document_create_instructor(self):
         """An instructor should not be able to create a Markdown document."""
         markdown_document = MarkdownDocumentFactory()
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         response = self.client.post(
             "/api/markdown-documents/",

diff --git a/src/backend/marsha/markdown/tests/api/markdown_documents/test_delete.py b/src/backend/marsha/markdown/tests/api/markdown_documents/test_delete.py
@@ -38,7 +38,7 @@ def test_api_document_delete_student(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = StudentLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=True,
         )
 
@@ -52,7 +52,9 @@ def test_api_document_delete_instructor(self):
         """An instructor should not be able to create a Markdown document."""
         markdown_document = MarkdownDocumentFactory()
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         response = self.client.delete(
             f"/api/markdown-documents/{markdown_document.pk}/",

diff --git a/src/backend/marsha/markdown/tests/api/markdown_documents/test_list.py b/src/backend/marsha/markdown/tests/api/markdown_documents/test_list.py
@@ -36,7 +36,7 @@ def test_api_document_fetch_list_student(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = StudentLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=True,
         )
 
@@ -49,7 +49,9 @@ def test_api_document_fetch_list_instructor(self):
         """An instrustor should not be able to fetch a Markdown document list."""
         markdown_document = MarkdownDocumentFactory()
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         response = self.client.get(
             "/api/markdown-documents/", HTTP_AUTHORIZATION=f"Bearer {jwt_token}"

diff --git a/src/backend/marsha/markdown/tests/api/markdown_documents/test_render_latex.py b/src/backend/marsha/markdown/tests/api/markdown_documents/test_render_latex.py
@@ -32,7 +32,7 @@ def test_api_document_render_latex_student(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = StudentLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=True,
         )
 
@@ -48,7 +48,9 @@ def test_api_document_render_latex_instructor(self):
         """An instructor should be able to render LaTeX content."""
         markdown_document = MarkdownDocumentFactory(is_draft=True)
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         response = self.client.post(
             f"/api/markdown-documents/{markdown_document.pk}/latex-rendering/",

diff --git a/src/backend/marsha/markdown/tests/api/markdown_documents/test_retrieve.py b/src/backend/marsha/markdown/tests/api/markdown_documents/test_retrieve.py
@@ -43,7 +43,7 @@ def test_api_document_fetch_student(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = StudentLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=True,
         )
 
@@ -69,7 +69,9 @@ def test_api_document_fetch_instructor(self):
             translations__rendered_content="<h1>Heading1</h1>\n<p>Some content</p>",
         )
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         response = self.client.get(
             f"/api/markdown-documents/{markdown_document.pk}/",
@@ -130,7 +132,7 @@ def test_api_document_fetch_instructor_read_only(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = InstructorOrAdminLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=False,
         )
 

diff --git a/src/backend/marsha/markdown/tests/api/markdown_documents/test_update.py b/src/backend/marsha/markdown/tests/api/markdown_documents/test_update.py
@@ -48,7 +48,7 @@ def test_api_document_update_student(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = StudentLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=True,
         )
         data = {"title": "new title"}
@@ -66,7 +66,7 @@ def test_api_document_update_instructor_read_only(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = InstructorOrAdminLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=False,
         )
         data = {"title": "new title"}
@@ -99,7 +99,9 @@ def test_api_document_update_instructor(self):
         """An instructor should be able to update a Markdown document."""
         markdown_document = MarkdownDocumentFactory(is_draft=True)
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         data = {"is_draft": False}
 

diff --git a/src/backend/marsha/markdown/tests/api/markdown_documents/test_update_translations.py b/src/backend/marsha/markdown/tests/api/markdown_documents/test_update_translations.py
@@ -30,7 +30,7 @@ def test_api_document_translation_update_student(self):
         markdown_document = MarkdownDocumentFactory()
 
         jwt_token = StudentLtiTokenFactory(
-            resource=markdown_document,
+            resource=markdown_document.playlist,
             permissions__can_update=True,
         )
 
@@ -53,7 +53,9 @@ def test_api_document_translation_update_instructor(self):
         """An instructor should be able to update a Markdown document translated content."""
         markdown_document = MarkdownDocumentFactory(is_draft=True)
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         data = {
             "language_code": "en",

diff --git a/src/backend/marsha/markdown/tests/api/markdown_images/test_create.py b/src/backend/marsha/markdown/tests/api/markdown_images/test_create.py
@@ -35,7 +35,7 @@ def test_api_markdown_image_create_student(self):
         """Student users should not be able to create a Markdown image."""
         markdown_document = MarkdownDocumentFactory()
 
-        jwt_token = StudentLtiTokenFactory(resource=markdown_document)
+        jwt_token = StudentLtiTokenFactory(resource=markdown_document.playlist)
 
         response = self.client.post(
             f"/api/markdown-documents/{markdown_document.id}/markdown-images/",
@@ -48,7 +48,9 @@ def test_api_markdown_image_create_instructor(self):
         """Instructors users should be able to create a Markdown image."""
         markdown_document = MarkdownDocumentFactory()
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         response = self.client.post(
             f"/api/markdown-documents/{markdown_document.id}/markdown-images/",
@@ -80,7 +82,9 @@ def test_api_markdown_image_create_already_existing_instructor(self):
         markdown_document = MarkdownDocumentFactory()
         MarkdownImageFactory(markdown_document=markdown_document)
 
-        jwt_token = InstructorOrAdminLtiTokenFactory(resource=markdown_document)
+        jwt_token = InstructorOrAdminLtiTokenFactory(
+            resource=markdown_document.playlist
+        )
 
         response = self.client.post(
             f"/api/markdown-documents/{markdown_document.id}/markdown-images/",

diff --git a/src/backend/marsha/markdown/tests/api/markdown_images/test_delete.py b/src/backend/marsha/markdown/tests/api/markdown_images/test_delete.py
@@ -35,7 +35,9 @@ def test_api_markdown_image_delete_student(self):
         """Student users should not be able to delete a Markdown image."""
         markdown_image = MarkdownImageFactory()
 
-        jwt_token = StudentLtiTokenFactory(resource=markdown_image.markdown_document)
+        jwt_token = StudentLtiTokenFactory(
+            resource=markdown_image.markdown_document.playlist
+        )
 
         response = self.client.delete(
             f"/api/markdown-documents/{markdown_image.markdown_document.id}"
@@ -49,7 +51,7 @@ def test_api_markdown_image_delete_instructor(self):
         markdown_image = MarkdownImageFactory()
 
         jwt_token = InstructorOrAdminLtiTokenFactory(
-            resource=markdown_image.markdown_document,
+            resource=markdown_image.markdown_document.playlist,
         )
 
         self.assertEqual(MarkdownImage.objects.count(), 1)
@@ -84,7 +86,7 @@ def test_api_markdown_image_delete_instructor_in_read_only(self):
         markdown_image = MarkdownImageFactory()
 
         jwt_token = InstructorOrAdminLtiTokenFactory(
-            resource=markdown_image.markdown_document,
+            resource=markdown_image.markdown_document.playlist,
             permissions__can_update=False,
         )