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

Playlist token #2342

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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")
kernicPanel marked this conversation as resolved.
Show resolved Hide resolved
)

# 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
20 changes: 8 additions & 12 deletions src/backend/marsha/bbb/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,23 @@ class Meta: # noqa
model = ClassroomRecording
fields = (
"id",
"classroom",
"classroom_id",
"record_id",
"started_at",
"video_file_url",
"vod",
)
read_only_fields = (
"id",
"classroom",
"classroom_id",
"record_id",
"started_at",
"video_file_url",
"vod",
)

# Make sure classroom and vod UUIDs are converted to a string during serialization
classroom = serializers.PrimaryKeyRelatedField(
classroom_id = serializers.PrimaryKeyRelatedField(
read_only=True, pk_field=serializers.CharField()
)
vod = VideoFromRecordingSerializer(read_only=True)
Expand Down Expand Up @@ -249,7 +249,7 @@ class ClassroomDocumentSerializer(
class Meta: # noqa
model = ClassroomDocument
fields = (
"classroom",
"classroom_id",
"filename",
"id",
"is_default",
Expand All @@ -258,7 +258,7 @@ class Meta: # noqa
"url",
)
read_only_fields = (
"classroom",
"classroom_id",
"id",
"upload_state",
"uploaded_on",
Expand All @@ -267,7 +267,7 @@ class Meta: # noqa

url = serializers.SerializerMethodField()
# Make sure classroom UUID is converted to a string during serialization
classroom = serializers.PrimaryKeyRelatedField(
classroom_id = serializers.PrimaryKeyRelatedField(
read_only=True, pk_field=serializers.CharField()
)

Expand Down 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 @@ -125,7 +125,7 @@ def test_api_classroom_recording_create_vod_instructor_or_admin(self):
self.assertDictEqual(
response.json(),
{
"classroom": str(recording.classroom.id),
"classroom_id": str(recording.classroom.id),
"id": str(recording.id),
"record_id": str(recording.record_id),
"started_at": "2019-08-21T15:00:02Z",
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 All @@ -79,7 +79,7 @@ def test_api_list_classroom_documents_instructor(self):
"previous": None,
"results": [
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[2].filename,
"id": str(classroom_documents[2].id),
"is_default": False,
Expand All @@ -88,7 +88,7 @@ def test_api_list_classroom_documents_instructor(self):
"url": None,
},
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[1].filename,
"id": str(classroom_documents[1].id),
"is_default": False,
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 All @@ -134,7 +134,7 @@ def test_api_list_classroom_documents_instructor_urls(self):
"previous": None,
"results": [
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[3].filename,
"id": str(classroom_documents[3].id),
"is_default": False,
Expand All @@ -148,7 +148,7 @@ def test_api_list_classroom_documents_instructor_urls(self):
),
},
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[2].filename,
"id": str(classroom_documents[2].id),
"is_default": False,
Expand All @@ -163,7 +163,7 @@ def test_api_list_classroom_documents_instructor_urls(self):
),
},
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[1].filename,
"id": str(classroom_documents[1].id),
"is_default": False,
Expand All @@ -178,7 +178,7 @@ def test_api_list_classroom_documents_instructor_urls(self):
),
},
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[0].filename,
"id": str(classroom_documents[0].id),
"is_default": False,
Expand Down Expand Up @@ -236,7 +236,7 @@ def test_api_list_classroom_documents_user_access_token_organization_admin(self)
"previous": None,
"results": [
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[2].filename,
"id": str(classroom_documents[2].id),
"is_default": False,
Expand All @@ -245,7 +245,7 @@ def test_api_list_classroom_documents_user_access_token_organization_admin(self)
"url": None,
},
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[1].filename,
"id": str(classroom_documents[1].id),
"is_default": False,
Expand Down Expand Up @@ -281,7 +281,7 @@ def test_api_list_classroom_documents_user_access_token_playlist_admin(self):
"previous": None,
"results": [
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[2].filename,
"id": str(classroom_documents[2].id),
"is_default": False,
Expand All @@ -290,7 +290,7 @@ def test_api_list_classroom_documents_user_access_token_playlist_admin(self):
"url": None,
},
{
"classroom": str(classroom.id),
"classroom_id": str(classroom.id),
"filename": classroom_documents[1].filename,
"id": str(classroom_documents[1].id),
"is_default": False,
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
Loading