From 898ee265c261f147f4988dceb54f5db82adb4a7b Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Fri, 21 Jul 2023 15:27:26 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=82(backend)=20use=20playlist=20token?= =?UTF-8?q?=20in=20deposit=20apis?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As we are now using playlist tokens, it has to be allowed on apis. --- src/backend/marsha/deposit/api.py | 6 +++--- src/backend/marsha/deposit/permissions.py | 13 +++++-------- src/backend/marsha/deposit/serializers.py | 7 ++----- .../deposit/tests/api/depositedfiles/test_create.py | 10 +++++++--- .../api/depositedfiles/test_initiate_upload.py | 12 +++++++++--- .../tests/api/filedepositories/test_create.py | 2 +- .../tests/api/filedepositories/test_delete.py | 6 +++--- .../api/filedepositories/test_depositedfiles.py | 8 ++++---- .../deposit/tests/api/filedepositories/test_list.py | 4 ++-- .../tests/api/filedepositories/test_retrieve.py | 6 +++--- .../tests/api/filedepositories/test_update.py | 6 +++--- .../marsha/deposit/tests/api/test_options.py | 4 ++-- 12 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/backend/marsha/deposit/api.py b/src/backend/marsha/deposit/api.py index e9ff7dbbdf..461982d494 100644 --- a/src/backend/marsha/deposit/api.py +++ b/src/backend/marsha/deposit/api.py @@ -107,7 +107,7 @@ def get_permissions(self): ] elif self.action in ["retrieve"]: permission_classes = [ - core_permissions.IsTokenResourceRouteObject + core_permissions.IsPlaylistToken & ( core_permissions.IsTokenInstructor | core_permissions.IsTokenAdmin @@ -118,7 +118,7 @@ def get_permissions(self): elif self.action in ["update", "partial_update", "destroy"]: permission_classes = [ ( - core_permissions.IsTokenResourceRouteObject + core_permissions.IsPlaylistToken & ( core_permissions.IsTokenInstructor | core_permissions.IsTokenAdmin @@ -243,7 +243,7 @@ def get_permissions(self): | core_permissions.IsTokenStudent | core_permissions.UserIsAuthenticated ] - elif self.action in ["create", "list", "metadata"]: + elif self.action in ["list", "metadata"]: permission_classes = [ core_permissions.IsTokenInstructor | core_permissions.IsTokenAdmin diff --git a/src/backend/marsha/deposit/permissions.py b/src/backend/marsha/deposit/permissions.py index 70f4771c97..6b1f308802 100644 --- a/src/backend/marsha/deposit/permissions.py +++ b/src/backend/marsha/deposit/permissions.py @@ -1,9 +1,8 @@ """Custom permission classes for the Deposit app.""" -from django.core.exceptions import ObjectDoesNotExist - from rest_framework import permissions from marsha.core import models +from marsha.deposit.models import FileDepository def _is_organization_admin(user_id, file_depository_id): @@ -59,12 +58,10 @@ def has_permission(self, request, view): """ if not request.resource: return False - try: - return ( - str(view.get_related_object().file_depository.id) == request.resource.id - ) - except ObjectDoesNotExist: - return False + + return FileDepository.objects.filter( + pk=view.get_related_filedepository_id(), playlist_id=request.resource.id + ).exists() class IsFileDepositoryPlaylistOrOrganizationAdmin(permissions.BasePermission): diff --git a/src/backend/marsha/deposit/serializers.py b/src/backend/marsha/deposit/serializers.py index b41adb6d54..3f0f8b2773 100644 --- a/src/backend/marsha/deposit/serializers.py +++ b/src/backend/marsha/deposit/serializers.py @@ -83,13 +83,10 @@ def create(self, validated_data): """ resource = self.context["request"].resource user = self.context["request"].user - file_depository_id = self.context["request"].data.get("file_depository_id") + file_depository_id = self.context["view"].get_related_filedepository_id() if not validated_data.get("file_depository_id"): - if resource: - validated_data["file_depository_id"] = resource.id - elif file_depository_id: - validated_data["file_depository_id"] = file_depository_id + validated_data["file_depository_id"] = file_depository_id if resource: validated_data["author_id"] = resource.user.get("id") diff --git a/src/backend/marsha/deposit/tests/api/depositedfiles/test_create.py b/src/backend/marsha/deposit/tests/api/depositedfiles/test_create.py index 4357171d8f..ada6fad42b 100644 --- a/src/backend/marsha/deposit/tests/api/depositedfiles/test_create.py +++ b/src/backend/marsha/deposit/tests/api/depositedfiles/test_create.py @@ -42,7 +42,7 @@ def test_api_deposited_file_create_student_with_user_fullname(self): """ file_depository = FileDepositoryFactory() - jwt_token = StudentLtiTokenFactory(resource=file_depository) + jwt_token = StudentLtiTokenFactory(resource=file_depository.playlist) response = self.client.post( f"/api/filedepositories/{file_depository.id}/depositedfiles/", @@ -89,7 +89,9 @@ def test_api_deposited_file_create_student_with_username(self): file_depository = FileDepositoryFactory() jwt_token = StudentLtiTokenFactory( - resource=file_depository, user__user_fullname=None, user__username="student" + resource=file_depository.playlist, + user__user_fullname=None, + user__username="student", ) response = self.client.post( @@ -121,7 +123,9 @@ def test_api_deposited_file_create_student_without_username(self): file_depository = FileDepositoryFactory() jwt_token = StudentLtiTokenFactory( - resource=file_depository, user__user_fullname=None, user__username=None + resource=file_depository.playlist, + user__user_fullname=None, + user__username=None, ) response = self.client.post( diff --git a/src/backend/marsha/deposit/tests/api/depositedfiles/test_initiate_upload.py b/src/backend/marsha/deposit/tests/api/depositedfiles/test_initiate_upload.py index 7f59592b0d..3368264df1 100644 --- a/src/backend/marsha/deposit/tests/api/depositedfiles/test_initiate_upload.py +++ b/src/backend/marsha/deposit/tests/api/depositedfiles/test_initiate_upload.py @@ -49,7 +49,9 @@ def test_api_deposited_file_initiate_upload_student(self): upload_state=random.choice(["ready", "error"]), file_depository__id="ed08da34-7447-4141-96ff-5740315d7b99", ) - jwt_token = StudentLtiTokenFactory(resource=deposited_file.file_depository) + jwt_token = StudentLtiTokenFactory( + resource=deposited_file.file_depository.playlist + ) now = datetime(2018, 8, 8, tzinfo=baseTimezone.utc) with mock.patch.object(timezone, "now", return_value=now), mock.patch( @@ -106,7 +108,9 @@ def test_api_deposited_file_initiate_upload_file_without_size(self): upload_state=random.choice(["ready", "error"]), file_depository__id="ed08da34-7447-4141-96ff-5740315d7b99", ) - jwt_token = StudentLtiTokenFactory(resource=deposited_file.file_depository) + jwt_token = StudentLtiTokenFactory( + resource=deposited_file.file_depository.playlist + ) now = datetime(2018, 8, 8, tzinfo=baseTimezone.utc) with mock.patch.object(timezone, "now", return_value=now), mock.patch( @@ -134,7 +138,9 @@ def test_api_deposited_file_initiate_upload_file_too_large(self): upload_state=random.choice(["ready", "error"]), file_depository__id="ed08da34-7447-4141-96ff-5740315d7b99", ) - jwt_token = StudentLtiTokenFactory(resource=deposited_file.file_depository) + jwt_token = StudentLtiTokenFactory( + resource=deposited_file.file_depository.playlist + ) now = datetime(2018, 8, 8, tzinfo=baseTimezone.utc) with mock.patch.object(timezone, "now", return_value=now), mock.patch( diff --git a/src/backend/marsha/deposit/tests/api/filedepositories/test_create.py b/src/backend/marsha/deposit/tests/api/filedepositories/test_create.py index 972c559237..6d1a299402 100644 --- a/src/backend/marsha/deposit/tests/api/filedepositories/test_create.py +++ b/src/backend/marsha/deposit/tests/api/filedepositories/test_create.py @@ -66,7 +66,7 @@ def test_api_file_depository_create_student_with_playlist_token(self): def test_api_file_depository_create_instructor(self): """An instructor without playlist token should not be able to create a file_depository.""" file_depository = FileDepositoryFactory() - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) response = self.client.post( "/api/filedepositories/", HTTP_AUTHORIZATION=f"Bearer {jwt_token}" diff --git a/src/backend/marsha/deposit/tests/api/filedepositories/test_delete.py b/src/backend/marsha/deposit/tests/api/filedepositories/test_delete.py index c2ba1ec5b4..da261bf98f 100644 --- a/src/backend/marsha/deposit/tests/api/filedepositories/test_delete.py +++ b/src/backend/marsha/deposit/tests/api/filedepositories/test_delete.py @@ -54,7 +54,7 @@ def test_api_file_depository_delete_user_logged_in(self): def test_api_file_depository_delete_student(self): """A student user should not be able to delete a file_depository.""" file_depository = FileDepositoryFactory() - jwt_token = StudentLtiTokenFactory(resource=file_depository) + jwt_token = StudentLtiTokenFactory(resource=file_depository.playlist) self.assertEqual(FileDepository.objects.count(), 1) response = self.client.delete( @@ -69,7 +69,7 @@ def test_api_file_depository_delete_instructor_read_only(self): """An instructor should not be able to delete a file_depository in read_only.""" file_depository = FileDepositoryFactory() jwt_token = InstructorOrAdminLtiTokenFactory( - resource=file_depository, + resource=file_depository.playlist, permissions__can_update=False, ) @@ -85,7 +85,7 @@ def test_api_file_depository_delete_instructor_read_only(self): def test_api_file_depository_delete_instructor(self): """An instructor should be able to delete a file_depository.""" file_depository = FileDepositoryFactory() - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) self.assertEqual(FileDepository.objects.count(), 1) response = self.client.delete( diff --git a/src/backend/marsha/deposit/tests/api/filedepositories/test_depositedfiles.py b/src/backend/marsha/deposit/tests/api/filedepositories/test_depositedfiles.py index 7669825c18..ccd9ddc59b 100644 --- a/src/backend/marsha/deposit/tests/api/filedepositories/test_depositedfiles.py +++ b/src/backend/marsha/deposit/tests/api/filedepositories/test_depositedfiles.py @@ -51,7 +51,7 @@ def test_api_file_depository_list_deposited_files_student(self): DepositedFileFactory.create_batch(3, file_depository=file_depository) owned_deposited_file = DepositedFileFactory(file_depository=file_depository) jwt_token = StudentLtiTokenFactory( - resource=file_depository, + resource=file_depository.playlist, permissions__can_update=True, user__id=owned_deposited_file.author_id, user__full_username=owned_deposited_file.author_name, @@ -90,7 +90,7 @@ def test_api_file_depository_list_deposited_files_instructor(self): deposited_files = DepositedFileFactory.create_batch( 3, file_depository=file_depository ) - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) response = self.client.get( f"/api/filedepositories/{file_depository.id}/depositedfiles/?limit=2", @@ -140,7 +140,7 @@ def test_api_file_depository_list_deposited_files_instructor_filtered(self): deposited_files_new = DepositedFileFactory.create_batch( 2, file_depository=file_depository ) - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) response = self.client.get( f"/api/filedepositories/{file_depository.id}/depositedfiles/?limit=10", @@ -291,7 +291,7 @@ def test_api_file_depository_list_deposited_files_instructor_signed_urls(self): deposited_files = DepositedFileFactory.create_batch( 3, file_depository=file_depository, uploaded_on=now ) - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) now = datetime(2021, 11, 30, tzinfo=baseTimezone.utc) with mock.patch.object(timezone, "now", return_value=now), mock.patch( diff --git a/src/backend/marsha/deposit/tests/api/filedepositories/test_list.py b/src/backend/marsha/deposit/tests/api/filedepositories/test_list.py index f398edb8f2..5c49bf5d99 100644 --- a/src/backend/marsha/deposit/tests/api/filedepositories/test_list.py +++ b/src/backend/marsha/deposit/tests/api/filedepositories/test_list.py @@ -43,7 +43,7 @@ def test_api_file_depository_fetch_list_student(self): """A student should not be able to fetch a list of file_depository.""" file_depository = FileDepositoryFactory() jwt_token = StudentLtiTokenFactory( - resource=file_depository, + resource=file_depository.playlist, permissions__can_update=True, ) @@ -55,7 +55,7 @@ def test_api_file_depository_fetch_list_student(self): def test_api_file_depository_fetch_list_instructor(self): """An instructor should not be able to fetch a file_depository list.""" file_depository = FileDepositoryFactory() - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) response = self.client.get( "/api/filedepositories/", HTTP_AUTHORIZATION=f"Bearer {jwt_token}" diff --git a/src/backend/marsha/deposit/tests/api/filedepositories/test_retrieve.py b/src/backend/marsha/deposit/tests/api/filedepositories/test_retrieve.py index b41e45a6b2..69f95ed228 100644 --- a/src/backend/marsha/deposit/tests/api/filedepositories/test_retrieve.py +++ b/src/backend/marsha/deposit/tests/api/filedepositories/test_retrieve.py @@ -38,7 +38,7 @@ def setUpClass(cls): def test_api_file_depository_fetch_student(self): """A student should be allowed to fetch a file_depository.""" file_depository = FileDepositoryFactory() - jwt_token = StudentLtiTokenFactory(resource=file_depository) + jwt_token = StudentLtiTokenFactory(resource=file_depository.playlist) response = self.client.get( f"/api/filedepositories/{file_depository.id!s}/", @@ -68,7 +68,7 @@ def test_api_file_depository_fetch_from_other_file_depository(self): """ file_depository = FileDepositoryFactory() other_file_depository = FileDepositoryFactory() - jwt_token = StudentLtiTokenFactory(resource=other_file_depository) + jwt_token = StudentLtiTokenFactory(resource=other_file_depository.playlist) response = self.client.get( f"/api/filedepositories/{file_depository.id!s}/", @@ -79,7 +79,7 @@ def test_api_file_depository_fetch_from_other_file_depository(self): def test_api_file_depository_fetch_instructor(self): """An instructor should be able to fetch a file_depository.""" file_depository = FileDepositoryFactory() - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) response = self.client.get( f"/api/filedepositories/{file_depository.id!s}/", diff --git a/src/backend/marsha/deposit/tests/api/filedepositories/test_update.py b/src/backend/marsha/deposit/tests/api/filedepositories/test_update.py index 3b181efdf0..15cf962617 100644 --- a/src/backend/marsha/deposit/tests/api/filedepositories/test_update.py +++ b/src/backend/marsha/deposit/tests/api/filedepositories/test_update.py @@ -55,7 +55,7 @@ def test_api_file_depository_update_user_logged_in(self): def test_api_file_depository_update_student(self): """A student user should not be able to update a file_depository.""" file_depository = FileDepositoryFactory() - jwt_token = StudentLtiTokenFactory(resource=file_depository) + jwt_token = StudentLtiTokenFactory(resource=file_depository.playlist) data = {"title": "new title"} response = self.client.patch( @@ -70,7 +70,7 @@ def test_api_file_depository_update_instructor_read_only(self): """An instructor should not be able to update a file_depository in read_only.""" file_depository = FileDepositoryFactory() jwt_token = InstructorOrAdminLtiTokenFactory( - resource=file_depository, + resource=file_depository.playlist, permissions__can_update=False, ) data = {"title": "new title"} @@ -86,7 +86,7 @@ def test_api_file_depository_update_instructor_read_only(self): def test_api_file_depository_update_instructor(self): """An instructor should be able to update a file_depository.""" file_depository = FileDepositoryFactory() - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) data = {"title": "new title", "description": "Hello"} response = self.client.patch( diff --git a/src/backend/marsha/deposit/tests/api/test_options.py b/src/backend/marsha/deposit/tests/api/test_options.py index 7a91a5e26f..e7e111dbe6 100644 --- a/src/backend/marsha/deposit/tests/api/test_options.py +++ b/src/backend/marsha/deposit/tests/api/test_options.py @@ -29,7 +29,7 @@ def test_api_deposited_files_options_as_student(self): """A student can fetch the deposited files options endpoint""" file_depository = FileDepositoryFactory() - jwt_token = StudentLtiTokenFactory(resource=file_depository) + jwt_token = StudentLtiTokenFactory(resource=file_depository.playlist) response = self.client.options( f"/api/filedepositories/{file_depository.id}/depositedfiles/", HTTP_AUTHORIZATION=f"Bearer {jwt_token}", @@ -42,7 +42,7 @@ def test_api_deposited_files_options_instructor(self): """An instructor can fetch the deposited files options endpoint""" file_depository = FileDepositoryFactory() - jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository) + jwt_token = InstructorOrAdminLtiTokenFactory(resource=file_depository.playlist) response = self.client.options( f"/api/filedepositories/{file_depository.id}/depositedfiles/",