Skip to content

Commit

Permalink
🛂(backend) use playlist token in classroom apis
Browse files Browse the repository at this point in the history
As we are now using playlist tokens, it has to be allowed on apis.
Also, old routes can't be used anymore, as token resource is now a
playlist.
  • Loading branch information
kernicPanel committed Aug 7, 2023
1 parent 542f086 commit ac2d778
Show file tree
Hide file tree
Showing 19 changed files with 261 additions and 171 deletions.
70 changes: 8 additions & 62 deletions src/backend/marsha/bbb/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
from marsha.core.api import APIViewMixin, BulkDestroyModelMixin, ObjectPkMixin

from . import permissions, serializers
from ..core.api.base import ResourceDoesNotMatchParametersException
from ..core.defaults import VOD_CONVERT
from ..core.models import ADMINISTRATOR, INSTRUCTOR, Video
from ..core.utils.convert_lambda_utils import invoke_lambda_convert
Expand All @@ -52,23 +51,14 @@ class ObjectClassroomRelatedMixin:
def get_related_classroom_id(self):
"""Get the related classroom ID from the request."""

# The video ID in the URL will be mandatory when old routes are deleted.
classroom_id = (
# The video ID in the URL is mandatory.
return (
self.kwargs.get("classroom_id")
# Backward compatibility with old routes
or self.request.data.get("classroom")
or self.request.query_params.get("classroom")
)

# Backward compatibility with old routes for LTI context
resource = self.request.resource
if resource is not None:
if resource.id and classroom_id and str(classroom_id) != str(resource.id):
raise ResourceDoesNotMatchParametersException()
return self.request.resource.id

return classroom_id


class InviteTokenThrottle(AnonRateThrottle):
"""Throttling class dedicated to classroom invite token endpoint"""
Expand Down Expand Up @@ -109,7 +99,7 @@ class ClassroomViewSet(

permission_classes = [
(
core_permissions.IsTokenResourceRouteObject
core_permissions.IsPlaylistToken
& (core_permissions.IsTokenInstructor | core_permissions.IsTokenAdmin)
)
| (
Expand Down Expand Up @@ -157,7 +147,8 @@ def get_permissions(self):
]
elif self.action in ["retrieve", "service_join"]:
permission_classes = [
core_permissions.IsTokenResourceRouteObject
core_permissions.IsPlaylistToken
| core_permissions.IsTokenResourceRouteObject # needed for invite links
| (
core_permissions.UserIsAuthenticated # asserts request.resource is None
& (
Expand Down Expand Up @@ -203,7 +194,7 @@ def get_serializer_context(self):
# For LTI
| (
core_permissions.ResourceIsAuthenticated
& core_permissions.IsTokenResourceRouteObject
& core_permissions.IsPlaylistToken
& (
core_permissions.IsTokenInstructor
| core_permissions.IsTokenAdmin
Expand Down Expand Up @@ -289,51 +280,6 @@ def lti_select(self, request):
}
)

@action(
methods=["get"],
detail=True,
url_path="classroomdocuments",
permission_classes=[
core_permissions.IsTokenInstructor
| core_permissions.IsTokenAdmin
| (
core_permissions.UserIsAuthenticated # asserts request.resource is None
& (
core_permissions.IsObjectPlaylistAdminOrInstructor
| core_permissions.IsObjectPlaylistOrganizationAdmin
)
)
],
)
# pylint: disable=unused-argument
def classroomdocuments(self, request, pk=None):
"""Get documents from a classroom.
Calling the endpoint returns a list of classroom documents.
Parameters
----------
request : Type[django.http.request.HttpRequest]
The request on the API endpoint
pk : int
The primary key of the classroom
Returns
-------
Type[rest_framework.response.Response]
HttpResponse carrying deposited files as a JSON object.
"""
classroom = self.get_object()
queryset = classroom.classroom_documents.all().order_by("-created_on")
page = self.paginate_queryset(queryset)
serializer = serializers.ClassroomDocumentSerializer(
page,
many=True,
context={"request": self.request},
)
return self.get_paginated_response(serializer.data)

@action(
methods=["patch"],
detail=True,
Expand Down Expand Up @@ -546,6 +492,7 @@ class ClassroomDocumentViewSet(
APIViewMixin,
ObjectPkMixin,
ObjectClassroomRelatedMixin,
mixins.ListModelMixin,
mixins.CreateModelMixin,
mixins.UpdateModelMixin,
mixins.DestroyModelMixin,
Expand Down Expand Up @@ -627,7 +574,7 @@ def create(self, request, *args, **kwargs):

@action(methods=["post"], detail=True, url_path="initiate-upload")
# pylint: disable=unused-argument
def initiate_upload(self, request, pk=None):
def initiate_upload(self, request, pk=None, classroom_id=None):
"""Get an upload policy for a classroom document.
Calling the endpoint resets the upload state to `pending` and returns an upload policy to
Expand All @@ -651,7 +598,6 @@ def initiate_upload(self, request, pk=None):
serializer = serializers.ClassroomDocumentInitiateUploadSerializer(
data=request.data
)

if serializer.is_valid() is not True:
return Response(serializer.errors, status=400)

Expand Down
5 changes: 4 additions & 1 deletion src/backend/marsha/bbb/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from rest_framework import permissions

from marsha.bbb.models import Classroom
from marsha.core import models, permissions as core_permissions


Expand Down Expand Up @@ -33,7 +34,9 @@ def has_permission(self, request, view):
if not request.resource:
return False

return str(view.get_related_classroom_id()) == request.resource.id
return Classroom.objects.filter(
pk=view.get_related_classroom_id(), playlist_id=request.resource.id
).exists()


class BaseIsRelatedClassroomPlaylistRoleMixin:
Expand Down
8 changes: 2 additions & 6 deletions src/backend/marsha/bbb/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,9 @@ def create(self, validated_data):
The "validated_data" dictionary is returned after modification.
"""
resource = self.context["request"].resource
classroom_id = self.context["request"].data.get("classroom")
classroom_id = self.context["view"].get_related_classroom_id()
if not validated_data.get("classroom_id"):
if resource:
validated_data["classroom_id"] = resource.id
elif classroom_id:
validated_data["classroom_id"] = classroom_id
validated_data["classroom_id"] = classroom_id

if not ClassroomDocument.objects.filter(
classroom_id=validated_data["classroom_id"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_api_classroom_bulk_delete_student(self):
classroom2 = ClassroomFactory()

jwt_token = StudentLtiTokenFactory(
resource=classroom1,
resource=classroom1.playlist,
permissions__can_update=True,
)

Expand Down Expand Up @@ -93,7 +93,7 @@ def test_api_classroom_bulk_delete_instructor(self):
"""LTI Token can't delete a list of classroom."""
classroom = ClassroomFactory()

jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom)
jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom.playlist)

response = self.client.delete(
self._api_url(),
Expand All @@ -113,7 +113,7 @@ def test_api_classroom_bulk_delete_instructor_with_playlist_token(self):
playlist = PlaylistFactory()
classroom = ClassroomFactory(playlist=playlist)

jwt_token = PlaylistLtiTokenFactory(resource=classroom)
jwt_token = PlaylistLtiTokenFactory(resource=classroom.playlist)

self.assertEqual(Classroom.objects.count(), 1)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def test_api_list_classroom_documents_student(self):
"""A student should not be able to fetch a list of classroom documents."""
classroom = ClassroomFactory()
ClassroomDocumentFactory.create_batch(3, classroom=classroom)
jwt_token = StudentLtiTokenFactory(resource=classroom)
jwt_token = StudentLtiTokenFactory(resource=classroom.playlist)

response = self.client.get(
f"/api/classrooms/{classroom.id}/classroomdocuments/",
Expand All @@ -63,7 +63,7 @@ def test_api_list_classroom_documents_instructor(self):
classroom_documents = ClassroomDocumentFactory.create_batch(
3, classroom=classroom
)
jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom)
jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom.playlist)

response = self.client.get(
f"/api/classrooms/{classroom.id}/classroomdocuments/?limit=2",
Expand Down Expand Up @@ -118,7 +118,7 @@ def test_api_list_classroom_documents_instructor_urls(self):
uploaded_on=now,
)
)
jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom)
jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom.playlist)

response = self.client.get(
f"/api/classrooms/{classroom.id}/classroomdocuments/",
Expand Down
4 changes: 2 additions & 2 deletions src/backend/marsha/bbb/tests/api/classroom/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def test_api_classroom_create_student(self):
classroom = ClassroomFactory()

jwt_token = StudentLtiTokenFactory(
resource=classroom,
resource=classroom.playlist,
permissions__can_update=True,
)

Expand All @@ -77,7 +77,7 @@ def test_api_classroom_create_instructor(self):
"""An instructor without playlist token should not be able to create a classroom."""
classroom = ClassroomFactory()

jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom)
jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom.playlist)

response = self.client.post(
"/api/classrooms/", HTTP_AUTHORIZATION=f"Bearer {jwt_token}"
Expand Down
6 changes: 3 additions & 3 deletions src/backend/marsha/bbb/tests/api/classroom/test_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_api_classroom_delete_student(self):
classroom = ClassroomFactory()

jwt_token = StudentLtiTokenFactory(
resource=classroom,
resource=classroom.playlist,
permissions__can_update=True,
)

Expand All @@ -72,7 +72,7 @@ def test_api_classroom_delete_instructor(self):
"""An instructor without playlist token should not be able to delete a classroom."""
classroom = ClassroomFactory()

jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom)
jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom.playlist)

response = self.client.delete(
f"/api/classrooms/{classroom.id}/", HTTP_AUTHORIZATION=f"Bearer {jwt_token}"
Expand All @@ -86,7 +86,7 @@ def test_api_classroom_delete_instructor_with_playlist_token(self):
playlist = PlaylistFactory()
classroom = ClassroomFactory(playlist=playlist)

jwt_token = PlaylistLtiTokenFactory(resource=classroom)
jwt_token = PlaylistLtiTokenFactory(resource=classroom.playlist)

self.assertEqual(Classroom.objects.count(), 1)

Expand Down
4 changes: 2 additions & 2 deletions src/backend/marsha/bbb/tests/api/classroom/test_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_api_classroom_fetch_list_student(self):
classroom = ClassroomFactory()

jwt_token = StudentLtiTokenFactory(
resource=classroom,
resource=classroom.playlist,
permissions__can_update=True,
)

Expand All @@ -59,7 +59,7 @@ def test_api_fetch_list_instructor(self):
"""An instructor should not be able to fetch a classroom list."""
classroom = ClassroomFactory()

jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom)
jwt_token = InstructorOrAdminLtiTokenFactory(resource=classroom.playlist)

response = self.client.get(
"/api/classrooms/", HTTP_AUTHORIZATION=f"Bearer {jwt_token}"
Expand Down
Loading

0 comments on commit ac2d778

Please sign in to comment.