diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/unknown_route.py b/cms/djangoapps/contentstore/rest_api/v1/views/unknown_route.py new file mode 100644 index 000000000000..e551794a8507 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/unknown_route.py @@ -0,0 +1,25 @@ +"""API view answering addresses that no endpoint serves.""" + +from drf_spectacular.utils import extend_schema +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from rest_framework.exceptions import NotFound +from rest_framework.permissions import AllowAny +from rest_framework.views import APIView + + +@extend_schema(exclude=True) +class UnknownRouteView(StandardizedErrorMixin, APIView): + """ + Answer a video address whose course key the conforming routes refuse. + + A key that is malformed or in the deprecated slash-separated form fails to + resolve, and would otherwise be answered by the site's HTML error page, which + an API client cannot read. Nothing here needs the caller's identity, so the + answer is the same for everyone: the address holds no resource. + """ + + permission_classes = (AllowAny,) + + def initial(self, request, *args, **kwargs): + """Refuse the request before any handler runs; the address holds nothing.""" + raise NotFound() diff --git a/cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py b/cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py index 1c0d693dec4f..56e2a3e9cd30 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py +++ b/cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py @@ -1,9 +1,12 @@ """Contentstore API v2 authoring URLs.""" -from django.urls import path +from django.urls import path, re_path +from cms.djangoapps.contentstore.rest_api.v1.views.unknown_route import UnknownRouteView from cms.djangoapps.contentstore.rest_api.v2.views.textbooks import CourseTextbooksViewSet +from cms.djangoapps.contentstore.rest_api.v2.views.video_archives import CourseVideoArchiveView from cms.djangoapps.contentstore.rest_api.v2.views.video_settings import CourseVideoSettingsViewSet +from cms.djangoapps.contentstore.rest_api.v2.views.video_usages import CourseVideoUsageViewSet app_name = "authoring_v2" @@ -18,4 +21,44 @@ CourseVideoSettingsViewSet.as_view({"get": "retrieve"}), name="course_video_settings", ), + path( + "courses//video_usages//", + CourseVideoUsageViewSet.as_view({"get": "retrieve"}), + name="course_video_usage_detail", + ), + path( + "courses//video_archives/", + CourseVideoArchiveView.as_view(), + name="course_video_archive_list", + ), + # Every other address under these two collections holds nothing: a course + # key the routes above refuse (malformed, or in the deprecated + # slash-separated form), the usage collection without a video id, an empty + # video id, or a path below a usage or below the archives. They answer with + # the API error body rather than the site's HTML error page. They match + # addresses under these two collections and nothing else, so a resource + # added under courses/ is never shadowed, and only slash-terminated ones, so + # the redirect to the slash-terminated form still happens first. A path + # converter cannot match an empty segment, hence the patterns for the + # addresses below each collection. + path( + "courses//video_usages/", + UnknownRouteView.as_view(), + name="course_video_usage_list_unmatched", + ), + re_path( + r"^courses/(?P.+)/video_usages/(?P.*)/$", + UnknownRouteView.as_view(), + name="course_video_usage_detail_unmatched", + ), + path( + "courses//video_archives/", + UnknownRouteView.as_view(), + name="course_video_archive_list_unmatched", + ), + re_path( + r"^courses/(?P.+)/video_archives/(?P.*)/$", + UnknownRouteView.as_view(), + name="course_video_archive_detail_unmatched", + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/video_archives.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/video_archives.py new file mode 100644 index 000000000000..2885d39b3c5a --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/video_archives.py @@ -0,0 +1,35 @@ +"""Serializers for zip archives of a course's videos.""" + +from rest_framework import serializers + + +class VideoArchiveFileSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One video to put in the archive.""" + + # Values are used exactly as sent: the URL is matched character for + # character against the course's video URLs, and the name becomes the entry + # name in the archive. + url = serializers.CharField( + trim_whitespace=False, + help_text=( + "Address of one of the course's encoded video files, exactly as the course's video " + "listing reports it. Any other address is refused." + ), + ) + name = serializers.CharField( + trim_whitespace=False, + allow_blank=True, + help_text=( + "File name of the video inside the archive. When it does not already end with the " + "extension of the video's content type, that extension is appended." + ), + ) + + +class VideoArchiveRequestSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The videos to bundle into one archive.""" + + files = VideoArchiveFileSerializer( + many=True, + help_text="The videos to include, in archive order. An empty list gives an empty archive.", + ) diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/video_usages.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/video_usages.py new file mode 100644 index 000000000000..c4862d114463 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/video_usages.py @@ -0,0 +1,34 @@ +"""Serializers for where a video is used in a course.""" + +from rest_framework import serializers + + +class VideoUsageLocationSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One video component of the course that plays the video.""" + + display_location = serializers.CharField( + help_text=( + "Where the component sits, as ' - / ' display names. " + "A subsection or unit with no display name set appears as 'None', and a component with " + "none as 'Video'. The subsection part is empty for a unit that is in no subsection." + ), + ) + url = serializers.CharField( + help_text=( + "Studio address of the unit holding the component, anchored at the component: " + "'/container/#'." + ), + ) + + +class CourseVideoUsageSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The places in a course where one video is used.""" + + usage_locations = VideoUsageLocationSerializer( + many=True, + help_text=( + "One entry per video component that references the video, published or not. A " + "component with no parent in the course outline is left out. Empty when no component " + "is listed." + ), + ) diff --git a/cms/djangoapps/contentstore/rest_api/v2/video_management_service.py b/cms/djangoapps/contentstore/rest_api/v2/video_management_service.py new file mode 100644 index 000000000000..32a49b65b3d0 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/video_management_service.py @@ -0,0 +1,41 @@ +"""Service layer for the video usage and video archive resources of a course.""" + +from rest_framework.exceptions import NotFound + +from cms.djangoapps.contentstore.video_storage_handlers import create_video_zip, get_video_usage_path +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + + +def ensure_course_exists(course_key): + """ + Raise ``NotFound`` unless a course run exists under ``course_key``. + + Only reads: the course overview table first, then the content store. The + overview row is never created here, so a read request writes nothing. + """ + if not CourseOverview.course_exists(course_key): + raise NotFound("The course does not exist.") + + +def get_course_video_usages(course_key, edx_video_id): + """ + Return where the video ``edx_video_id`` is placed in the course. + + The result is ``{"usage_locations": [{"display_location", "url"}, ...]}``, + one entry per video component that references the id, in content-store + order. Unpublished components are included; a component with no parent is + left out. An id that nothing references gives an empty list. + """ + ensure_course_exists(course_key) + return get_video_usage_path(course_key, edx_video_id) + + +def create_course_video_archive(course_key, files): + """ + Return a streaming zip response holding the requested videos of the course. + + Every ``files[].url`` must be one of the course's own encoded-video URLs; + otherwise ``ValidationError`` is raised before anything is fetched. The + archive is assembled while it is sent and is not stored. + """ + return create_video_zip(str(course_key), files) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_archives.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_archives.py new file mode 100644 index 000000000000..cf743ace2b08 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_archives.py @@ -0,0 +1,1023 @@ +""" +Unit tests for the course video archives API. +""" +import io +import json +import zipfile +from datetime import UTC, datetime +from unittest.mock import MagicMock, patch +from urllib.parse import urlencode + +import ddt +import pytest +import requests +from django.core.signals import got_request_exception +from django.urls import Resolver404, resolve, reverse +from edx_rest_framework_extensions.auth.jwt.tests.utils import generate_jwt +from edx_rest_framework_extensions.testing import assert_error_envelope +from edxval.api import create_profile, create_video, remove_video_for_course +from rest_framework import status +from rest_framework.test import APIClient + +from cms.djangoapps.contentstore.rest_api.v1.views.unknown_route import UnknownRouteView +from cms.djangoapps.contentstore.rest_api.v1.views.videos import VideoDownloadThrottle +from cms.djangoapps.contentstore.rest_api.v2.views.video_archives import CourseVideoArchiveView +from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole, +) +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.oauth_dispatch.tests.factories import AccessTokenFactory, ApplicationFactory +from xmodule.modulestore.tests.factories import CourseFactory + +LIST_URL_NAME = "authoring_v2:course_video_archive_list" +LEGACY_URL_NAME = "cms.djangoapps.contentstore:v1:video_usage" + +MISSING_COURSE_KEY = "course-v1:Org+Missing+Run" +CCX_COURSE_KEY = "ccx-v1:edX+DemoX+Demo_Course+ccx@1" + +# Keys the course_key path converter accepts that carry only a content version, +# with no organization, course or run. +VERSION_ONLY_COURSE_KEYS = ( + "course-v1:version@" + "a" * 24, + "course-v1:branch@published-branch+version@" + "a" * 24, + "ccx-v1:version@" + "a" * 24 + "+ccx@1", + "library-v1:version@" + "a" * 24, +) + +FIRST_URL = "http://video-store.example.com/profile1/first.mp4" +SECOND_URL = "http://video-store.example.com/profile2/second.webm" +OTHER_COURSE_URL = "http://video-store.example.com/profile1/other-course.mp4" +SHARED_URL = "http://video-store.example.com/profile1/shared.mp4" +# An internal address an attacker might try to make the server fetch. +SSRF_URL = "http://169.254.169.254/latest/meta-data/" + +FETCH = "cms.djangoapps.contentstore.video_storage_handlers.requests.get" + +# The archive builder each address calls. +SERVICE_ARCHIVE_BUILDER = "cms.djangoapps.contentstore.rest_api.v2.video_management_service.create_video_zip" +LEGACY_ARCHIVE_BUILDER = "cms.djangoapps.contentstore.rest_api.v1.views.videos.create_video_zip" + +# How the error handling publishes a required member missing from a list entry. +MISSING_MEMBER_ERROR = "{{'{member}': [ErrorDetail(string='This field is required.', code='required')]}}" + +# The size of a zip archive with no entries: the end-of-central-directory record. +EMPTY_ARCHIVE_SIZE = 22 +# The signature opening each record of a zip archive's central directory, which +# follows the last entry. +CENTRAL_DIRECTORY_SIGNATURE = b"PK\x01\x02" + + +def upstream_response(payload=b"video-bytes", content_type="video/mp4"): + """A stand-in for a streamed video fetch answering with ``payload``.""" + response = MagicMock() + response.iter_content = lambda chunk_size=None: iter([payload]) + response.headers = {"Content-Type": content_type} + return response + + +def failing_upstream_response(): + """A stand-in for a video fetch that the video store answers with a server error.""" + response = upstream_response() + response.raise_for_status.side_effect = requests.HTTPError("500 Server Error") + return response + + +def read_into(received, stream): + """Append each chunk of ``stream`` to ``received`` as it arrives.""" + for chunk in stream: + received.append(chunk) + + +def archive_entries(response): + """Return {entry name: bytes} of the streamed zip ``response``.""" + with zipfile.ZipFile(io.BytesIO(b"".join(response.streaming_content))) as archive: + return {name: archive.read(name) for name in archive.namelist()} + + +class VideoArchiveTestBase(CourseTestCase): + """A course with two encoded videos, and a second course with one of its own.""" + + # Isolates the throttle counters between tests. + ENABLED_CACHES = ["default"] + + def setUp(self): + super().setUp() + create_profile("profile1") + create_profile("profile2") + self.create_course_video("first", self.course.id, [("profile1", FIRST_URL), ("profile2", SECOND_URL)]) + self.other_course = CourseFactory.create() + self.create_course_video("other", self.other_course.id, [("profile1", OTHER_COURSE_URL)]) + + def create_course_video(self, edx_video_id, course_key, encodings, also_in=()): + create_video({ + "edx_video_id": edx_video_id, + "client_video_id": f"{edx_video_id}.mp4", + "duration": 42.0, + "status": "file_complete", + "courses": [str(key) for key in (course_key, *also_in)], + "created": datetime.now(UTC), + "encoded_videos": [ + {"profile": profile, "url": url, "file_size": 1600, "bitrate": 100} + for profile, url in encodings + ], + }) + + def url(self, course_key=None): + return reverse(LIST_URL_NAME, kwargs={"course_key": course_key or self.course.id}) + + def legacy_url(self, course_key=None): + # The legacy download route shares its name with the usage route and is + # the one reverse() picks when no video id is given. + return reverse(LEGACY_URL_NAME, kwargs={"course_id": course_key or self.course.id}) + + def client_for(self, user): + client = APIClient() + if user is not None: + client.force_authenticate(user=user) + return client + + def user_with_role(self, role_class, course_key=None): + user = UserFactory.create() + role_class(course_key or self.course.id).add_users(user) + return user + + def post(self, client, files, course_key=None, **extra): + return client.post(self.url(course_key), {"files": files}, format="json", **extra) + + def put_legacy(self, client, files, course_key=None, **extra): + return client.put(self.legacy_url(course_key), {"files": files}, format="json", **extra) + + +@patch(FETCH) +class VideoArchiveResponseTest(VideoArchiveTestBase): + """The archive holds the requested videos under the requested names.""" + + def test_the_archive_is_sent_as_a_zip_attachment(self, fetch): + fetch.return_value = upstream_response() + + response = self.post(self.client_for(self.user), [{"url": FIRST_URL, "name": "Lecture 1.mp4"}]) + + assert response.status_code == status.HTTP_200_OK + assert response["Content-Type"] == "application/zip" + assert response["Content-Disposition"] == f"attachment; filename={self.course.id}_videos.zip" + assert archive_entries(response) == {"Lecture 1.mp4": b"video-bytes"} + fetch.assert_called_once_with(FIRST_URL, stream=True, allow_redirects=True) + + def test_a_name_without_an_extension_gets_one_from_the_content_type(self, fetch): + fetch.return_value = upstream_response(content_type="video/mp4") + + response = self.post(self.client_for(self.user), [{"url": FIRST_URL, "name": "Lecture 1"}]) + + assert list(archive_entries(response)) == ["Lecture 1.mp4"] + + def test_an_unknown_content_type_leaves_the_name_alone(self, fetch): + fetch.return_value = upstream_response(content_type="application/x-not-a-real-type") + + response = self.post(self.client_for(self.user), [{"url": FIRST_URL, "name": "Lecture 1"}]) + + assert list(archive_entries(response)) == ["Lecture 1"] + + def test_several_videos_are_archived_in_request_order(self, fetch): + fetch.side_effect = [ + upstream_response(b"first-bytes", "video/mp4"), + upstream_response(b"second-bytes", "video/webm"), + ] + + response = self.post(self.client_for(self.user), [ + {"url": FIRST_URL, "name": "one.mp4"}, + {"url": SECOND_URL, "name": "two"}, + ]) + + entries = archive_entries(response) + assert list(entries) == ["one.mp4", "two.webm"] + assert entries == {"one.mp4": b"first-bytes", "two.webm": b"second-bytes"} + + def test_an_empty_request_gives_an_empty_archive(self, fetch): + response = self.post(self.client_for(self.user), []) + + body = b"".join(response.streaming_content) + assert response.status_code == status.HTTP_200_OK + assert len(body) == EMPTY_ARCHIVE_SIZE + assert zipfile.ZipFile(io.BytesIO(body)).namelist() == [] + fetch.assert_not_called() + + def test_the_name_is_used_exactly_as_sent(self, fetch): + fetch.return_value = upstream_response() + + response = self.post(self.client_for(self.user), [{"url": FIRST_URL, "name": " spaced.mp4 "}]) + + assert list(archive_entries(response)) == [" spaced.mp4 .mp4"] + + +@patch(FETCH) +class VideoArchiveAllowlistTest(VideoArchiveTestBase): + """Only the course's own video addresses are ever fetched.""" + + def assert_refused_without_fetching(self, response, fetch, address): + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"non_field_errors": [f"Invalid video download url: {address}"]} + fetch.assert_not_called() + + def test_an_address_outside_the_course_is_refused(self, fetch): + response = self.post(self.client_for(self.user), [{"url": SSRF_URL, "name": "x"}]) + + self.assert_refused_without_fetching(response, fetch, SSRF_URL) + + def test_a_video_of_another_course_is_refused(self, fetch): + response = self.post(self.client_for(self.user), [{"url": OTHER_COURSE_URL, "name": "x.mp4"}]) + + self.assert_refused_without_fetching(response, fetch, OTHER_COURSE_URL) + + def test_one_bad_address_refuses_the_whole_request(self, fetch): + response = self.post(self.client_for(self.user), [ + {"url": FIRST_URL, "name": "one.mp4"}, + {"url": SSRF_URL, "name": "x"}, + ]) + + self.assert_refused_without_fetching(response, fetch, SSRF_URL) + + def test_an_address_is_matched_exactly(self, fetch): + padded = f" {FIRST_URL}" + + response = self.post(self.client_for(self.user), [{"url": padded, "name": "x"}]) + + self.assert_refused_without_fetching(response, fetch, padded) + + def test_the_legacy_address_refuses_the_same_request(self, fetch): + response = self.put_legacy(self.client_for(self.user), [{"url": SSRF_URL, "name": "x"}]) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == {"developer_message": [f"Invalid video download url: {SSRF_URL}"]} + fetch.assert_not_called() + + def share_a_video_then_remove_it_from_this_course(self): + """Give both courses one video, then remove it from this course only.""" + self.create_course_video( + "shared", self.course.id, [("profile1", SHARED_URL)], also_in=[self.other_course.id], + ) + remove_video_for_course(str(self.course.id), "shared") + + def test_a_video_removed_from_the_course_is_refused(self, fetch): + self.share_a_video_then_remove_it_from_this_course() + + response = self.post(self.client_for(self.user), [{"url": SHARED_URL, "name": "x.mp4"}]) + + self.assert_refused_without_fetching(response, fetch, SHARED_URL) + + def test_the_legacy_address_refuses_a_video_removed_from_the_course(self, fetch): + self.share_a_video_then_remove_it_from_this_course() + + response = self.put_legacy(self.client_for(self.user), [{"url": SHARED_URL, "name": "x.mp4"}]) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == {"developer_message": [f"Invalid video download url: {SHARED_URL}"]} + fetch.assert_not_called() + + def test_a_video_removed_from_one_course_is_still_archived_by_a_course_that_keeps_it(self, fetch): + self.share_a_video_then_remove_it_from_this_course() + fetch.return_value = upstream_response() + + response = self.post( + self.client_for(self.user), [{"url": SHARED_URL, "name": "x.mp4"}], course_key=self.other_course.id, + ) + + assert archive_entries(response) == {"x.mp4": b"video-bytes"} + fetch.assert_called_once_with(SHARED_URL, stream=True, allow_redirects=True) + + +@ddt.ddt +@patch(FETCH) +class VideoArchiveRequestValidationTest(VideoArchiveTestBase): + """ + A malformed body is refused with a 400 before anything is fetched. + + The error handling publishes the error of a list entry as the Python text of + that entry's errors, with "{}" for each valid entry, rather than mapping the + entry's path to its messages; and the detail of every validation error is + the Python text of all the errors. The exact text is pinned, so a change in + that formatting shows up here. + """ + + def assert_invalid(self, response, fetch, field="files"): + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert list(envelope["errors"]) == [field] + fetch.assert_not_called() + return envelope + + def test_a_missing_list_is_refused(self, fetch): + response = self.client_for(self.user).post(self.url(), {}, format="json") + + envelope = self.assert_invalid(response, fetch) + assert envelope["errors"]["files"] == ["This field is required."] + + def test_the_detail_of_a_validation_error_is_the_text_of_all_the_errors(self, fetch): + response = self.client_for(self.user).post(self.url(), {}, format="json") + + envelope = self.assert_invalid(response, fetch) + assert envelope["detail"] == "{'files': [ErrorDetail(string='This field is required.', code='required')]}" + + def test_a_list_of_the_wrong_type_is_refused(self, fetch): + response = self.client_for(self.user).post(self.url(), {"files": "x"}, format="json") + + envelope = self.assert_invalid(response, fetch) + assert envelope["errors"]["files"] == [ + "{'non_field_errors': [ErrorDetail(string='Expected a list of items but got type \"str\".', " + "code='not_a_list')]}" + ] + + @ddt.data("url", "name") + def test_an_entry_missing_a_member_is_refused(self, member, fetch): + entry = {"url": FIRST_URL, "name": "one.mp4"} + del entry[member] + + response = self.post(self.client_for(self.user), [entry]) + + envelope = self.assert_invalid(response, fetch) + assert envelope["errors"]["files"] == [MISSING_MEMBER_ERROR.format(member=member)] + + def test_a_valid_entry_beside_an_invalid_one_is_published_as_an_empty_map(self, fetch): + response = self.post(self.client_for(self.user), [{"url": FIRST_URL, "name": "one.mp4"}, {"name": "two.mp4"}]) + + envelope = self.assert_invalid(response, fetch) + assert envelope["errors"]["files"] == ["{}", MISSING_MEMBER_ERROR.format(member="url")] + + def test_a_null_entry_is_refused(self, fetch): + response = self.post(self.client_for(self.user), [None]) + + envelope = self.assert_invalid(response, fetch) + assert envelope["errors"]["files"] == ["[ErrorDetail(string='This field may not be null.', code='null')]"] + + def test_a_null_name_is_refused(self, fetch): + response = self.post(self.client_for(self.user), [{"url": FIRST_URL, "name": None}]) + + envelope = self.assert_invalid(response, fetch) + assert envelope["errors"]["files"] == [ + "{'name': [ErrorDetail(string='This field may not be null.', code='null')]}" + ] + + def test_malformed_json_is_refused(self, fetch): + response = self.client_for(self.user).post(self.url(), "{not json", content_type="application/json") + + assert_error_envelope(response, expected_status=400) + fetch.assert_not_called() + + +@patch(FETCH) +class VideoArchiveMediaTypeTest(VideoArchiveTestBase): + """A body that is not JSON is refused before anything is fetched.""" + + # The indexed syntax the default form parsers would turn back into the list. + FORM_FIELDS = {"files[0]url": FIRST_URL, "files[0]name": "one.mp4"} + + def assert_unsupported(self, response, fetch): + # The status and the envelope are pinned, not the error-type slug, which + # the shared error catalog decides for every API. + assert_error_envelope(response, expected_status=status.HTTP_415_UNSUPPORTED_MEDIA_TYPE) + fetch.assert_not_called() + + def test_a_form_encoded_body_is_refused(self, fetch): + fetch.return_value = upstream_response() + + response = self.client_for(self.user).post( + self.url(), urlencode(self.FORM_FIELDS), content_type="application/x-www-form-urlencoded" + ) + + self.assert_unsupported(response, fetch) + + def test_a_plain_text_body_is_refused(self, fetch): + fetch.return_value = upstream_response() + + response = self.client_for(self.user).post( + self.url(), json.dumps({"files": [{"url": FIRST_URL, "name": "one.mp4"}]}), content_type="text/plain" + ) + + self.assert_unsupported(response, fetch) + + def test_a_multipart_body_is_refused(self, fetch): + fetch.return_value = upstream_response() + + response = self.client_for(self.user).post(self.url(), self.FORM_FIELDS, format="multipart") + + # Studio's request-tracking middleware reads the form fields of a + # multipart request before the view runs, which leaves the JSON parser + # an empty body: the request is refused as one without files. + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"files": ["This field is required."]} + fetch.assert_not_called() + + def test_the_view_parses_json_only(self, fetch): + assert [parser.media_type for parser in CourseVideoArchiveView().get_parsers()] == ["application/json"] + + def test_the_legacy_address_fails_on_a_form_encoded_body(self, fetch): + client = self.client_for(self.user) + client.raise_request_exception = False + + response = client.put( + self.legacy_url(), urlencode(self.FORM_FIELDS), content_type="application/x-www-form-urlencoded" + ) + + assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + fetch.assert_not_called() + + +@patch(FETCH) +class VideoArchiveAcceptHeaderTest(VideoArchiveTestBase): + """ + The archive is sent to a request that also accepts JSON, in which errors are + sent; a request that accepts the archive's type alone is refused. + """ + + def test_a_request_accepting_the_archive_and_json_gets_the_archive(self, fetch): + fetch.return_value = upstream_response() + + response = self.post( + self.client_for(self.user), + [{"url": FIRST_URL, "name": "one.mp4"}], + HTTP_ACCEPT="application/zip, application/json", + ) + + assert response.status_code == status.HTTP_200_OK + assert response["Content-Type"] == "application/zip" + assert archive_entries(response) == {"one.mp4": b"video-bytes"} + + def test_a_request_accepting_only_the_archive_is_refused_before_anything_is_fetched(self, fetch): + fetch.return_value = upstream_response() + + response = self.post( + self.client_for(self.user), [{"url": FIRST_URL, "name": "one.mp4"}], HTTP_ACCEPT="application/zip" + ) + + # The status and the envelope are pinned, not the error-type slug, which + # the shared error catalog decides for every API. + assert_error_envelope(response, expected_status=status.HTTP_406_NOT_ACCEPTABLE) + fetch.assert_not_called() + + +@ddt.ddt +@patch(FETCH) +class VideoArchiveAuthorizationTest(VideoArchiveTestBase): + """Studio read access to the course is required; everyone else is refused before any fetch.""" + + def assert_allowed(self, user, fetch): + """Assert ``user`` receives the archive of one requested video.""" + fetch.return_value = upstream_response() + response = self.post(self.client_for(user), [{"url": FIRST_URL, "name": "one.mp4"}]) + assert response.status_code == status.HTTP_200_OK + assert response["Content-Type"] == "application/zip" + assert archive_entries(response) == {"one.mp4": b"video-bytes"} + + def test_global_staff_is_allowed(self, fetch): + self.assert_allowed(self.user, fetch) + + def test_course_staff_is_allowed(self, fetch): + self.assert_allowed(self.user_with_role(CourseStaffRole), fetch) + + def test_course_instructor_is_allowed(self, fetch): + self.assert_allowed(self.user_with_role(CourseInstructorRole), fetch) + + def assert_refused(self, response, fetch, expected_status, slug): + assert_error_envelope(response, expected_status=expected_status, expected_type_slug=slug) + fetch.assert_not_called() + + def test_anonymous_is_refused(self, fetch): + response = self.post(self.client_for(None), [{"url": FIRST_URL, "name": "one.mp4"}]) + + self.assert_refused(response, fetch, 401, "authn") + assert response["WWW-Authenticate"] == 'JWT realm="api"' + + def test_user_without_a_role_is_refused(self, fetch): + response = self.post(self.client_for(UserFactory.create()), [{"url": FIRST_URL, "name": "one.mp4"}]) + + self.assert_refused(response, fetch, 403, "authz") + + def test_limited_staff_is_refused(self, fetch): + user = self.user_with_role(CourseLimitedStaffRole) + + response = self.post(self.client_for(user), [{"url": FIRST_URL, "name": "one.mp4"}]) + + self.assert_refused(response, fetch, 403, "authz") + + def test_staff_of_another_course_is_refused(self, fetch): + user = self.user_with_role(CourseStaffRole, course_key=self.other_course.id) + + response = self.post(self.client_for(user), [{"url": FIRST_URL, "name": "one.mp4"}]) + + self.assert_refused(response, fetch, 403, "authz") + + def test_ccx_course_is_refused_even_to_global_staff(self, fetch): + response = self.post(self.client_for(self.user), [], course_key=CCX_COURSE_KEY) + + self.assert_refused(response, fetch, 403, "authz") + + def test_missing_course_is_refused_to_a_caller_without_access(self, fetch): + response = self.post(self.client_for(UserFactory.create()), [], course_key=MISSING_COURSE_KEY) + + self.assert_refused(response, fetch, 403, "authz") + + def test_missing_course_is_reported_to_global_staff(self, fetch): + response = self.post(self.client_for(self.user), [], course_key=MISSING_COURSE_KEY) + + self.assert_refused(response, fetch, 404, "not-found") + + def test_a_missing_course_is_reported_before_the_body_is_read(self, fetch): + response = self.client_for(self.user).post(self.url(MISSING_COURSE_KEY), {}, format="json") + + self.assert_refused(response, fetch, 404, "not-found") + + @ddt.data(*VERSION_ONLY_COURSE_KEYS) + def test_a_key_carrying_only_a_version_is_not_found_by_global_staff(self, key, fetch): + response = self.post(self.client_for(self.user), [{"url": FIRST_URL, "name": "one.mp4"}], course_key=key) + + self.assert_refused(response, fetch, 404, "not-found") + assert response.json()["detail"] == "Not found." + + @ddt.data(*VERSION_ONLY_COURSE_KEYS) + def test_a_key_carrying_only_a_version_is_not_found_by_a_caller_without_a_role(self, key, fetch): + response = self.post( + self.client_for(UserFactory.create()), [{"url": FIRST_URL, "name": "one.mp4"}], course_key=key, + ) + + self.assert_refused(response, fetch, 404, "not-found") + assert response.json()["detail"] == "Not found." + + @ddt.data(*VERSION_ONLY_COURSE_KEYS) + def test_a_key_carrying_only_a_version_still_needs_authentication(self, key, fetch): + response = self.post(self.client_for(None), [{"url": FIRST_URL, "name": "one.mp4"}], course_key=key) + + self.assert_refused(response, fetch, 401, "authn") + + def test_the_legacy_address_has_no_route_for_a_version_only_key(self, fetch): + # The legacy pattern needs a separator inside the key. + with pytest.raises(Resolver404): + resolve(f"/api/contentstore/v1/videos/{VERSION_ONLY_COURSE_KEYS[0]}/download") + + def test_the_view_declares_its_checks_in_order(self, fetch): + # The JWT middleware appends its own restricted-application check to + # views that accept JWTs, so only the declared head of the list is fixed. + assert [klass.__name__ for klass in CourseVideoArchiveView.permission_classes[:3]] == [ + "IsAuthenticated", "HasFullCourseKey", "HasStudioReadAccess", + ] + + +class VideoArchiveUnexpectedFailureTest(VideoArchiveTestBase): + """ + An unexpected failure while the archive is set up, at both addresses. + + The new address answers with the generic 500 body from inside the view, so + the exception never reaches Django: ``got_request_exception`` is not sent, + and the only log record is Django's one-line 500 notice, without the + traceback. The legacy address lets the exception reach Django, which sends + the signal and logs the traceback. Both sides are pinned, so a change in how + the error handling reports failures shows up here. + """ + + BODY = {"files": [{"url": FIRST_URL, "name": "one.mp4"}]} + + def request_while_failing(self, method, address, target): + """Send the body as global staff while ``target`` raises; return the response, signals and log records.""" + signals = [] + + def receiver(sender, **kwargs): + signals.append(sender) + + got_request_exception.connect(receiver) + self.addCleanup(got_request_exception.disconnect, receiver) + client = APIClient(raise_request_exception=False) + client.force_authenticate(user=self.user) + with patch(target, side_effect=RuntimeError("video store unreachable at edxval-3")): + with self.assertLogs(level="ERROR") as logs: + response = getattr(client, method)(address, self.BODY, format="json") + return response, signals, logs.records + + def test_the_failure_is_answered_with_the_generic_body(self): + response, __, __ = self.request_while_failing("post", self.url(), SERVICE_ARCHIVE_BUILDER) + + envelope = assert_error_envelope(response, expected_status=500, expected_type_slug="internal") + assert envelope["detail"] == "An unexpected error occurred. Please try again later." + assert "edxval-3" not in response.content.decode() + + def test_the_failure_does_not_reach_django(self): + __, signals, records = self.request_while_failing("post", self.url(), SERVICE_ARCHIVE_BUILDER) + + assert not signals + assert [(record.name, record.getMessage(), record.exc_info) for record in records] == [ + ("django.request", f"Internal Server Error: {self.url()}", None), + ] + + def test_the_legacy_address_hands_the_failure_to_django(self): + __, signals, records = self.request_while_failing("put", self.legacy_url(), LEGACY_ARCHIVE_BUILDER) + + assert len(signals) == 1 + assert "django.request" in { + record.name for record in records if record.exc_info and record.exc_info[0] is RuntimeError + } + + +@patch(FETCH) +class VideoArchiveAuthenticationTest(VideoArchiveTestBase): + """The endpoint uses the platform's default authentication.""" + + def test_session_caller_succeeds(self, fetch): + fetch.return_value = upstream_response() + client = APIClient() + assert client.login(username=self.user.username, password=self.user_password) + + response = self.post(client, [{"url": FIRST_URL, "name": "one.mp4"}]) + + assert archive_entries(response) == {"one.mp4": b"video-bytes"} + + def test_session_caller_needs_a_csrf_token(self, fetch): + client = APIClient(enforce_csrf_checks=True) + assert client.login(username=self.user.username, password=self.user_password) + + response = self.post(client, [{"url": FIRST_URL, "name": "one.mp4"}]) + legacy = self.put_legacy(client, [{"url": FIRST_URL, "name": "one.mp4"}]) + + envelope = assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + assert envelope["detail"].startswith("CSRF Failed") + assert legacy.status_code == status.HTTP_403_FORBIDDEN + fetch.assert_not_called() + + def test_jwt_caller_succeeds(self, fetch): + fetch.return_value = upstream_response() + + response = self.post( + APIClient(), [{"url": FIRST_URL, "name": "one.mp4"}], + HTTP_AUTHORIZATION=f"JWT {generate_jwt(self.user)}", + ) + + assert archive_entries(response) == {"one.mp4": b"video-bytes"} + + def bearer_header(self, user): + token = AccessTokenFactory(user=user, application=ApplicationFactory()).token + return f"Bearer {token}" + + def test_bearer_token_is_refused(self, fetch): + response = self.post( + APIClient(), [{"url": FIRST_URL, "name": "one.mp4"}], HTTP_AUTHORIZATION=self.bearer_header(self.user), + ) + + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + fetch.assert_not_called() + + def test_the_legacy_address_still_accepts_a_bearer_token(self, fetch): + response = self.put_legacy(APIClient(), [], HTTP_AUTHORIZATION=self.bearer_header(self.user)) + + assert response.status_code == status.HTTP_200_OK + + def deactivated_global_staff(self): + """Log a global staff member in, then deactivate the account behind the live session.""" + password = "password-12345" + staff = UserFactory.create(is_staff=True, password=password) + session_client = APIClient() + assert session_client.login(username=staff.username, password=password) + staff.is_active = False + staff.save() + return staff, session_client + + def test_inactive_global_staff_is_refused_over_a_session(self, fetch): + __, session_client = self.deactivated_global_staff() + + response = self.post(session_client, [{"url": FIRST_URL, "name": "one.mp4"}]) + + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + fetch.assert_not_called() + + def test_inactive_global_staff_still_reaches_the_legacy_address_over_a_session(self, fetch): + __, session_client = self.deactivated_global_staff() + + response = self.put_legacy(session_client, []) + + assert response.status_code == status.HTTP_200_OK + + def test_inactive_global_staff_is_admitted_over_jwt(self, fetch): + staff, __ = self.deactivated_global_staff() + + response = self.post(APIClient(), [], HTTP_AUTHORIZATION=f"JWT {generate_jwt(staff)}") + + assert response.status_code == status.HTTP_200_OK + assert len(b"".join(response.streaming_content)) == EMPTY_ARCHIVE_SIZE + + def test_the_view_does_not_declare_authentication_classes(self, fetch): + assert "authentication_classes" not in CourseVideoArchiveView.__dict__ + assert [klass.__name__ for klass in CourseVideoArchiveView.authentication_classes] == [ + "DefaultJwtAuthentication", "DefaultSessionAuthentication", + ] + + +@patch(FETCH) +class VideoArchiveThrottleTest(VideoArchiveTestBase): + """Archive requests are rate-limited per user, in one budget with the legacy download address.""" + + def budget(self): + return VideoDownloadThrottle().num_requests + + def test_the_view_uses_the_download_throttle(self, fetch): + assert CourseVideoArchiveView.throttle_classes == (VideoDownloadThrottle,) + assert self.budget() == 12 + + def test_requests_past_the_budget_are_throttled(self, fetch): + client = self.client_for(self.user) + for __ in range(self.budget()): + assert self.post(client, []).status_code == status.HTTP_200_OK + + response = self.post(client, []) + + assert_error_envelope(response, expected_status=429, expected_type_slug="rate-limited") + assert response["Retry-After"] == "3600" + + def test_the_budget_is_shared_with_the_legacy_address(self, fetch): + client = self.client_for(self.user) + for __ in range(self.budget()): + assert self.put_legacy(client, []).status_code == status.HTTP_200_OK + + response = self.post(client, []) + + assert_error_envelope(response, expected_status=429, expected_type_slug="rate-limited") + + def test_the_budget_is_per_user(self, fetch): + client = self.client_for(self.user) + for __ in range(self.budget()): + self.post(client, []) + + response = self.post(self.client_for(self.user_with_role(CourseStaffRole)), []) + + assert response.status_code == status.HTTP_200_OK + + def test_a_refused_caller_spends_no_budget(self, fetch): + client = self.client_for(UserFactory.create()) + responses = [self.post(client, []) for __ in range(self.budget() + 2)] + + assert {response.status_code for response in responses} == {status.HTTP_403_FORBIDDEN} + + def test_the_legacy_address_spends_budget_on_a_refused_caller(self, fetch): + client = self.client_for(UserFactory.create()) + responses = [self.put_legacy(client, []) for __ in range(self.budget() + 1)] + + assert responses[-2].status_code == status.HTTP_403_FORBIDDEN + assert responses[-1].status_code == status.HTTP_429_TOO_MANY_REQUESTS + + +@ddt.ddt +@patch(FETCH) +class VideoArchiveParityTest(VideoArchiveTestBase): + """For the same request and caller, both addresses send the same archive.""" + + def pair(self, files, responses_for_each, course_key=None, user=None): + """POST to the new address and PUT to the legacy one, each fetch answered alike.""" + client = self.client_for(user or self.user) + with patch(FETCH, side_effect=list(responses_for_each())): + new = self.post(client, files, course_key) + new_body = b"".join(new.streaming_content) if new.status_code == 200 else None + with patch(FETCH, side_effect=list(responses_for_each())): + legacy = self.put_legacy(client, files, course_key) + legacy_body = b"".join(legacy.streaming_content) if legacy.status_code == 200 else None + return legacy, legacy_body, new, new_body + + def assert_same_archive(self, files, responses_for_each, expected_entries): + """Assert both addresses send byte-identical archives holding ``expected_entries``.""" + legacy, legacy_body, new, new_body = self.pair(files, responses_for_each) + assert (legacy.status_code, new.status_code) == (status.HTTP_200_OK, status.HTTP_200_OK) + for header in ("Content-Type", "Content-Disposition"): + assert new[header] == legacy[header] + assert new_body == legacy_body + with zipfile.ZipFile(io.BytesIO(new_body)) as archive: + assert {name: archive.read(name) for name in archive.namelist()} == expected_entries + + def test_a_name_with_an_extension(self, fetch): + self.assert_same_archive( + [{"url": FIRST_URL, "name": "one.mp4"}], + lambda: [upstream_response(b"a", "video/mp4")], + {"one.mp4": b"a"}, + ) + + def test_an_extension_from_the_content_type(self, fetch): + self.assert_same_archive( + [{"url": FIRST_URL, "name": "one"}], + lambda: [upstream_response(b"a", "video/mp4")], + {"one.mp4": b"a"}, + ) + + def test_an_unknown_content_type(self, fetch): + self.assert_same_archive( + [{"url": FIRST_URL, "name": "one"}], + lambda: [upstream_response(b"a", "application/x-not-a-real-type")], + {"one": b"a"}, + ) + + def test_several_files(self, fetch): + self.assert_same_archive( + [{"url": FIRST_URL, "name": "one.mp4"}, {"url": SECOND_URL, "name": "two"}], + lambda: [upstream_response(b"a", "video/mp4"), upstream_response(b"b", "video/webm")], + {"one.mp4": b"a", "two.webm": b"b"}, + ) + + def test_an_empty_list(self, fetch): + legacy, legacy_body, new, new_body = self.pair([], lambda: []) + + assert new_body == legacy_body + assert len(new_body) == EMPTY_ARCHIVE_SIZE + assert new["Content-Disposition"] == legacy["Content-Disposition"] == ( + f"attachment; filename={self.course.id}_videos.zip" + ) + + def received_until_the_failure(self, send, second_fetch, failure): + """ + Request two videos whose second fetch fails; return the bytes streamed before the failure. + + The status is sent before the archive, so the response is a 200 and the + failure surfaces while the body is being read. + """ + with patch(FETCH, side_effect=[upstream_response(b"first-bytes", "video/mp4"), second_fetch]) as fetch: + response = send( + self.client_for(self.user), + [{"url": FIRST_URL, "name": "one.mp4"}, {"url": SECOND_URL, "name": "two.webm"}], + ) + assert response.status_code == status.HTTP_200_OK + received = [] + with pytest.raises(failure): + read_into(received, response.streaming_content) + assert [call.args[0] for call in fetch.call_args_list] == [FIRST_URL, SECOND_URL] + return b"".join(received) + + @ddt.data( + # The video store answers the second fetch with a server error. + (failing_upstream_response, requests.HTTPError), + # The connection to the video store fails on the second fetch. + (lambda: requests.ConnectionError("Connection reset"), requests.ConnectionError), + ) + @ddt.unpack + def test_a_fetch_failing_part_way_stops_the_archive(self, second_fetch, failure, fetch): + new = self.received_until_the_failure(self.post, second_fetch(), failure) + legacy = self.received_until_the_failure(self.put_legacy, second_fetch(), failure) + with patch(FETCH, return_value=upstream_response(b"first-bytes", "video/mp4")): + response = self.post(self.client_for(self.user), [{"url": FIRST_URL, "name": "one.mp4"}]) + first_video_only = b"".join(response.streaming_content) + + # Both addresses stop after the first video's entry, before the + # archive's directory: what arrives is the archive of the first video + # alone, cut short, and no zip reader can open it. + assert new == legacy + assert first_video_only[:len(new)] == new + assert first_video_only[len(new):].startswith(CENTRAL_DIRECTORY_SIGNATURE) + with pytest.raises(zipfile.BadZipFile): + zipfile.ZipFile(io.BytesIO(new)) + + @ddt.data( + (None, status.HTTP_401_UNAUTHORIZED, {"developer_message"}), + ("no-role", status.HTTP_403_FORBIDDEN, {"developer_message"}), + ("missing-course", status.HTTP_404_NOT_FOUND, {"developer_message", "error_code"}), + ("bad-url", status.HTTP_400_BAD_REQUEST, {"developer_message"}), + ) + @ddt.unpack + def test_error_bodies(self, case, expected_status, legacy_keys, fetch): + client = { + None: self.client_for(None), + "no-role": self.client_for(UserFactory.create()), + }.get(case, self.client_for(self.user)) + course_key = MISSING_COURSE_KEY if case == "missing-course" else None + files = [{"url": SSRF_URL if case == "bad-url" else FIRST_URL, "name": "x"}] + + legacy = self.put_legacy(client, files, course_key) + new = self.post(client, files, course_key) + + assert legacy.status_code == new.status_code == expected_status + assert set(legacy.json()) == legacy_keys + expected_new_keys = {"type", "title", "status", "detail", "instance"} + if case == "bad-url": + expected_new_keys.add("errors") + assert set(new.json()) == expected_new_keys + fetch.assert_not_called() + + +class VideoArchiveQueryCountTest(VideoArchiveTestBase): + """ + The new address costs no more queries than the legacy one for the same caller. + + Each error answered by the new address ends with one extra statement, a + rollback of the request's savepoint, issued by the standard error handling. + """ + + #: Set from the pytest fixture below, which class-based tests cannot request directly. + django_assert_num_queries = None + + @pytest.fixture(autouse=True) + def _assert_num_queries(self, django_assert_num_queries): + self.django_assert_num_queries = django_assert_num_queries + + def measure(self, client, files, legacy_count, new_count, course_key=None): + """Warm the per-process caches, then count the queries of each address.""" + with patch(FETCH, return_value=upstream_response()): + self.put_legacy(client, files, course_key) + self.post(client, files, course_key) + with self.django_assert_num_queries(legacy_count): + self.put_legacy(client, files, course_key) + with self.django_assert_num_queries(new_count): + self.post(client, files, course_key) + + def test_global_staff(self): + self.measure(self.client_for(self.user), [{"url": FIRST_URL, "name": "a"}], 11, 11) + + def test_refused_address(self): + self.measure(self.client_for(self.user), [{"url": SSRF_URL, "name": "a"}], 11, 12) + + def test_refused_caller(self): + self.measure(self.client_for(UserFactory.create()), [], 8, 7) + + def test_missing_course(self): + self.measure(self.client_for(self.user), [], 8, 9, course_key=MISSING_COURSE_KEY) + + def test_anonymous(self): + self.measure(self.client_for(None), [], 5, 6) + + +@ddt.ddt +class VideoArchiveUrlContractTest(VideoArchiveTestBase): + """The archive address, and how unusable addresses and requests are answered.""" + + def test_conforming_address(self): + assert self.url() == f"/api/authoring/v2/courses/{self.course.id}/video_archives/" + + def test_the_address_resolves_to_the_view(self): + match = resolve(self.url()) + + assert match.func.cls is CourseVideoArchiveView + assert match.kwargs == {"course_key": self.course.id} + + def test_the_legacy_address_is_unchanged(self): + assert self.legacy_url() == f"/api/contentstore/v1/videos/{self.course.id}/download" + assert resolve(self.legacy_url()).func.view_class.__name__ == "VideoDownloadView" + + def assert_json_not_found(self, address): + """POST to ``address`` as a JSON client; assert the 404 error envelope and return it.""" + response = self.client_for(self.user).post( + address, {"files": []}, format="json", HTTP_ACCEPT="application/json", + ) + assert response["Content-Type"] == "application/json" + return assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_malformed_course_key_is_answered_with_the_error_envelope(self): + self.assert_json_not_found("/api/authoring/v2/courses/not-a-key/video_archives/") + + def test_deprecated_course_key_is_answered_with_the_error_envelope(self): + self.assert_json_not_found("/api/authoring/v2/courses/Org/Course/Run/video_archives/") + + def test_a_block_key_in_place_of_a_course_key_is_answered_with_the_error_envelope(self): + self.assert_json_not_found("/api/authoring/v2/courses/i4x://Org/Course/video/abc/video_archives/") + + def test_an_unusable_course_key_is_claimed_by_the_fallback_route(self): + address = "/api/authoring/v2/courses/Org/Course/Run/video_archives/" + + assert resolve(address).func.cls is UnknownRouteView + assert reverse( + "authoring_v2:course_video_archive_list_unmatched", kwargs={"course_key": "Org/Course/Run"}, + ) == address + assert reverse( + "authoring_v2:course_video_archive_detail_unmatched", + kwargs={"course_key": "Org/Course/Run", "subpath": "abc"}, + ) == f"{address}abc/" + + def test_the_legacy_address_still_accepts_a_deprecated_course_key(self): + match = resolve("/api/contentstore/v1/videos/Org/Course/Run/download") + + assert match.func.view_class.__name__ == "VideoDownloadView" + assert match.kwargs == {"course_id": "Org/Course/Run"} + + @ddt.data("video_archives/abc/", "video_archives//", "video_archives/abc/def/") + def test_an_address_below_the_archives_is_answered_with_the_error_envelope(self, rest): + address = f"/api/authoring/v2/courses/{self.course.id}/{rest}" + + assert resolve(address).func.cls is UnknownRouteView + self.assert_json_not_found(address) + + def test_an_address_below_the_archives_without_the_trailing_slash_is_redirected(self): + address = f"/api/authoring/v2/courses/{self.course.id}/video_archives/abc" + + response = self.client_for(self.user).post(address, {"files": []}, format="json") + + assert response.status_code == status.HTTP_301_MOVED_PERMANENTLY + assert response["Location"] == f"{address}/" + + def test_the_address_requires_the_trailing_slash(self): + with pytest.raises(Resolver404): + resolve(self.url().rstrip("/")) + + def test_a_post_without_the_trailing_slash_is_redirected(self): + response = self.client_for(self.user).post(self.url().rstrip("/"), {"files": []}, format="json") + + assert response.status_code == status.HTTP_301_MOVED_PERMANENTLY + assert response["Location"] == self.url() + + def test_other_methods_are_refused(self): + client = self.client_for(self.user) + for response in (client.get(self.url()), client.put(self.url(), {"files": []}, format="json")): + assert_error_envelope(response, expected_status=405) + assert response["Allow"] == "POST, OPTIONS" + + def test_an_accept_header_admitting_only_zip_is_refused_before_authentication(self): + response = self.post(self.client_for(None), [], HTTP_ACCEPT="application/zip") + + assert_error_envelope(response, expected_status=406) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_management_contract.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_management_contract.py new file mode 100644 index 000000000000..322ad4d88c86 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_management_contract.py @@ -0,0 +1,632 @@ +""" +Contract tests shared by the course video usage and video archive APIs. +""" +import functools +import json +import os +import subprocess +import sys +from collections import defaultdict +from pathlib import Path +from types import SimpleNamespace + +import ddt +import pytest +from django.contrib.auth.models import AnonymousUser +from django.test import RequestFactory, TestCase +from django.urls import Resolver404, get_resolver, resolve, reverse +from drf_spectacular.generators import SchemaGenerator +from drf_spectacular.settings import patched_settings +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from rest_framework.exceptions import NotFound + +import cms.envs +from cms.djangoapps.contentstore.rest_api.v2.serializers.video_usages import VideoUsageLocationSerializer +from cms.djangoapps.contentstore.rest_api.v2.views.video_archives import CourseVideoArchiveView +from cms.djangoapps.contentstore.rest_api.v2.views.video_usages import CourseVideoUsageViewSet +from cms.djangoapps.contentstore.tests.utils import CourseTestCase, routed_actions +from cms.djangoapps.contentstore.views.permissions import HasFullCourseKey, HasStudioReadAccess +from cms.lib.spectacular import SUPERSEDED_PATHS, cms_api_filter, cms_mark_superseded_paths +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole, +) +from common.djangoapps.student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import CourseFactory + +USAGE_PATH = "/api/authoring/v2/courses/{course_key}/video_usages/{edx_video_id}/" +ARCHIVE_PATH = "/api/authoring/v2/courses/{course_key}/video_archives/" +LEGACY_USAGE_PATH = "/api/contentstore/v1/videos/{course_id}/{edx_video_id}/usage" +LEGACY_DOWNLOAD_PATH = "/api/contentstore/v1/videos/{course_id}/download" + +ENUM_POSTPROCESSING_HOOK = "drf_spectacular.hooks.postprocess_schema_enums" +SUPERSEDED_PATHS_HOOK = "cms.lib.spectacular.cms_mark_superseded_paths" +MIGRATED_PATHS_HOOK = "cms.lib.spectacular.cms_mark_migrated_paths" + +PRODUCTION_SETTINGS = "cms.envs.production" +DEVSTACK_SETTINGS = "cms.envs.devstack" +SETTINGS_DIRECTORY = Path(cms.envs.__file__).resolve().parent +REPO_ROOT = SETTINGS_DIRECTORY.parents[1] +MOCK_CONFIG = SETTINGS_DIRECTORY / "mock.yml" + +# The schema settings a document generated in this process has to be given for +# its addresses to come out the way the deployment publishes them. +GENERATION_SETTING_KEYS = ("PREPROCESSING_HOOKS", "POSTPROCESSING_HOOKS", "SCHEMA_PATH_PREFIX") +SETTINGS_MARKER = "schema settings: " + +# Keys the course_key path converter accepts that carry only a content version, +# with no organization, course or run. +VERSION_ONLY_COURSE_KEYS = ( + "course-v1:version@" + "a" * 24, + "course-v1:branch@published-branch+version@" + "a" * 24, + "ccx-v1:version@" + "a" * 24 + "+ccx@1", + "library-v1:version@" + "a" * 24, +) + + +@functools.lru_cache +def schema_settings(module): + """ + Return ``SPECTACULAR_SETTINGS`` of the deployment settings module ``module``. + + Deployment settings share their mutable defaults with the settings the test + process runs under and would alter them on import, so they are read in a + separate process. + """ + script = ( + "import importlib, json, sys\n" + "values = importlib.import_module(sys.argv[1]).SPECTACULAR_SETTINGS\n" + "print(sys.argv[2] + json.dumps(values))\n" + ) + completed = subprocess.run( + [sys.executable, "-c", script, module, SETTINGS_MARKER], + capture_output=True, + check=True, + cwd=REPO_ROOT, + env={**os.environ, "CMS_CFG": str(MOCK_CONFIG), "SERVICE_VARIANT": "cms"}, + text=True, + ) + reported = next(line for line in completed.stdout.splitlines() if line.startswith(SETTINGS_MARKER)) + return json.loads(reported[len(SETTINGS_MARKER):]) + + +def generate_document(postprocessing_hooks=None, generator=None): + """Generate the Studio document with the production schema settings.""" + configured = schema_settings(PRODUCTION_SETTINGS) + overrides = {key: configured[key] for key in GENERATION_SETTING_KEYS} + if postprocessing_hooks is not None: + overrides["POSTPROCESSING_HOOKS"] = postprocessing_hooks + with patched_settings(overrides): + return (generator or SchemaGenerator()).get_schema(request=None, public=True) + + +@functools.lru_cache +def production_document(): + return generate_document() + + +def operations(document): + """Yield (path, method, operation) for every operation of ``document``.""" + for path, item in document["paths"].items(): + for method, operation in item.items(): + if isinstance(operation, dict) and "operationId" in operation: + yield path, method, operation + + +def record_assigned_operation_ids(result, generator, request, public): # pylint: disable=unused-argument + """ + Post-processing hook: keep every operation's id on the generator, unchanged. + + Post-processing runs before the generator renames colliding ids with numeric + suffixes, so the ids kept here are the ones each path was given. + """ + generator.assigned_operation_ids = [ + (operation["operationId"], path, method) for path, method, operation in operations(result) + ] + return result + + +RECORDING_HOOK = f"{record_assigned_operation_ids.__module__}.{record_assigned_operation_ids.__name__}" + + +class VideoRoutesTest(TestCase): + """Route names: the legacy ones stay as they are; the new ones are unique.""" + + COURSE_KEY = "course-v1:edX+DemoX+Demo_Course" + + # The routes of these two resources, in order. Other resources share the + # namespace, so only these are pinned. + ROUTE_NAMES = ( + "course_video_usage_detail", + "course_video_archive_list", + "course_video_usage_list_unmatched", + "course_video_usage_detail_unmatched", + "course_video_archive_list_unmatched", + "course_video_archive_detail_unmatched", + ) + + # Addresses outside the two collections, some named like them, that another + # resource may serve. Neither fallback may claim them. + ADDRESSES_OUTSIDE_THE_COLLECTIONS = ( + f"/api/authoring/v2/courses/{COURSE_KEY}/", + f"/api/authoring/v2/courses/{COURSE_KEY}/textbooks/", + "/api/authoring/v2/courses/Org/Course/Run/textbooks/", + f"/api/authoring/v2/courses/{COURSE_KEY}/textbooks/video_usages_report/", + f"/api/authoring/v2/courses/{COURSE_KEY}/video_usage/abc-123/", + f"/api/authoring/v2/courses/{COURSE_KEY}/all_video_archives/", + "/api/authoring/v2/video_usages/", + ) + + def test_the_shared_legacy_name_without_a_video_id_reverses_to_the_download_route(self): + address = reverse("cms.djangoapps.contentstore:v1:video_usage", kwargs={"course_id": self.COURSE_KEY}) + + assert address == f"/api/contentstore/v1/videos/{self.COURSE_KEY}/download" + assert resolve(address).func.view_class.__name__ == "VideoDownloadView" + + def test_the_shared_legacy_name_with_a_video_id_reverses_to_the_usage_route(self): + address = reverse( + "cms.djangoapps.contentstore:v1:video_usage", + kwargs={"course_id": self.COURSE_KEY, "edx_video_id": "abc-123"}, + ) + + assert address == f"/api/contentstore/v1/videos/{self.COURSE_KEY}/abc-123/usage" + assert resolve(address).func.view_class.__name__ == "VideoUsageView" + + def test_the_new_route_names_are_unique_and_carry_no_version(self): + __, resolver = get_resolver().namespace_dict["authoring_v2"] + + names = [pattern.name for pattern in resolver.url_patterns if pattern.name in self.ROUTE_NAMES] + + # Each fallback route has to come after the route it backs, or it would + # answer valid course keys too. + assert names == list(self.ROUTE_NAMES) + assert resolver.app_name == "authoring_v2" + assert not [name for name in names if "v1" in name or "v2" in name] + + def test_the_fallbacks_claim_nothing_outside_the_two_collections(self): + fallbacks = {f"authoring_v2:{name}" for name in self.ROUTE_NAMES if name.endswith("_unmatched")} + claimed = [] + for address in self.ADDRESSES_OUTSIDE_THE_COLLECTIONS: + try: + match = resolve(address) + except Resolver404: + continue + if match.view_name in fallbacks: + claimed.append(address) + + assert not claimed, claimed + + +class HasStudioReadAccessTest(CourseTestCase): + """The permission grants exactly the callers with Studio read access to the course in the path.""" + + def allows(self, user, course_key=None, kwargs=None): + request = RequestFactory().get("/") + request.user = user + view = SimpleNamespace(kwargs={"course_key": course_key or self.course.id} if kwargs is None else kwargs) + return HasStudioReadAccess().has_permission(request, view) + + def user_with_role(self, role_class, course_key=None): + user = UserFactory.create() + role_class(course_key or self.course.id).add_users(user) + return user + + def test_global_staff_is_allowed(self): + assert self.allows(self.user) is True + + def test_course_staff_is_allowed(self): + assert self.allows(self.user_with_role(CourseStaffRole)) is True + + def test_course_instructor_is_allowed(self): + assert self.allows(self.user_with_role(CourseInstructorRole)) is True + + def test_user_without_a_role_is_refused(self): + assert self.allows(UserFactory.create()) is False + + def test_anonymous_is_refused(self): + assert self.allows(AnonymousUser()) is False + + def test_limited_staff_is_refused(self): + assert self.allows(self.user_with_role(CourseLimitedStaffRole)) is False + + def test_staff_of_another_course_is_refused(self): + other_course = CourseFactory.create() + + assert self.allows(self.user_with_role(CourseStaffRole, course_key=other_course.id)) is False + + def test_ccx_course_is_refused_to_global_staff(self): + assert self.allows(self.user, course_key=CourseKey.from_string("ccx-v1:edX+DemoX+Demo_Course+ccx@1")) is False + + def test_a_view_without_a_course_key_is_refused(self): + assert self.allows(self.user, kwargs={}) is False + + # The role checks behind this class cannot evaluate a key that carries only + # a content version, which is why the views list HasFullCourseKey first. + def test_a_key_without_an_organization_cannot_be_evaluated(self): + with pytest.raises(InvalidKeyError): + self.allows(self.user, course_key=CourseKey.from_string(VERSION_ONLY_COURSE_KEYS[0])) + + +@ddt.ddt +class HasFullCourseKeyTest(TestCase): + """The permission answers 404 for a course key with no organization, course or run, and passes any other.""" + + def check(self, kwargs): + request = RequestFactory().get("/") + request.user = AnonymousUser() + return HasFullCourseKey().has_permission(request, SimpleNamespace(kwargs=kwargs)) + + @ddt.data(*VERSION_ONLY_COURSE_KEYS) + def test_a_key_carrying_only_a_version_is_not_found(self, key): + with pytest.raises(NotFound): + self.check({"course_key": CourseKey.from_string(key)}) + + @ddt.data( + "course-v1:edX+DemoX+Demo_Course", + "course-v1:edX+DemoX+Demo_Course+version@" + "a" * 24, + "ccx-v1:edX+DemoX+Demo_Course+ccx@1", + "library-v1:edX+DemoLib", + ) + def test_a_key_naming_an_organization_passes(self, key): + assert self.check({"course_key": CourseKey.from_string(key)}) is True + + def test_a_view_without_a_course_key_passes(self): + assert self.check({}) is True + + +class CmsSchemaHookTest(TestCase): + """The schema hooks admit the new addresses and deprecate exactly the two superseded ones.""" + + def test_filter_admits_the_authoring_prefix(self): + admitted = cms_api_filter([ + ("/api/authoring/v2/courses/course-v1:a+b+c/video_archives/", None, "post", None), + ("/api/contentstore/v1/videos/course-v1:a+b+c/download", None, "put", None), + ("/api/authoring/v2", None, "get", None), + ("/login", None, "get", None), + ]) + + assert [entry[0] for entry in admitted] == [ + "/api/authoring/v2/courses/course-v1:a+b+c/video_archives/", + "/api/contentstore/v1/videos/course-v1:a+b+c/download", + ] + + def test_filter_refuses_the_versioned_paths_of_other_apis(self): + admitted = cms_api_filter([ + ("/api/xblock/v2/xblocks/lb:Org:lib:html:abc/", None, "get", None), + ("/api/content_tagging/v1/taxonomies/", None, "get", None), + ("/api/libraries/v2/", None, "get", None), + ("/api/courses/v1/courses/", None, "get", None), + ("/api/authoring_tools/v1/", None, "get", None), + ("/api/v1/authoring/", None, "get", None), + ]) + + assert not admitted, admitted + + def test_filter_admits_the_course_discussions_switch_and_nothing_else_under_courses(self): + admitted = cms_api_filter([ + ("/api/courses/{course_key_string}/bulk_enable_disable_discussions", None, "put", None), + ("/api/courses/{course_key_string}/updates", None, "get", None), + ("/api/discussions/v0/bulk_enable_disable_discussions", None, "get", None), + ]) + + assert [entry[0] for entry in admitted] == [ + "/api/courses/{course_key_string}/bulk_enable_disable_discussions", + ] + + # Listed as exact paths rather than prefixes: the operations beside them + # under /api/contentstore/v1/videos/ have no successor, and a prefix would + # also reach any sibling later added below one of these two addresses. + def test_the_superseded_paths_include_the_two_video_operations(self): + assert {LEGACY_USAGE_PATH, LEGACY_DOWNLOAD_PATH} <= set(SUPERSEDED_PATHS) + + # Operations of other resources that nothing supersedes, next to the two + # superseded ones or elsewhere in Studio, and the new addresses. Marking any + # of them would tell clients to leave an operation that has no successor. + # Resources superseded in their own right are not listed: whether they are + # marked is decided with their successors, not here. + OTHER_RESOURCE_PATHS = ( + "/api/contentstore/v1/videos/{course_id}", + "/api/contentstore/v0/videos/encodings/{course_id}", + "/api/contentstore/v0/videos/features", + "/api/contentstore/v0/videos/images/{course_id}/{edx_video_id}", + "/api/contentstore/v2/home/courses", + "/api/contentstore/v2/downstreams/{usage_key_string}", + USAGE_PATH, + ARCHIVE_PATH, + ) + + def marked_schema(self): + """Run the post-processing hook over the superseded paths and those of other resources.""" + paths = { + LEGACY_USAGE_PATH: {"get": {}}, + LEGACY_DOWNLOAD_PATH: {"put": {}}, + } + paths.update({path: {"get": {}, "post": {}} for path in self.OTHER_RESOURCE_PATHS}) + return cms_mark_superseded_paths({"paths": paths}, None, None, True) + + def test_post_processing_marks_the_two_superseded_operations_deprecated(self): + paths = self.marked_schema()["paths"] + + assert paths[LEGACY_USAGE_PATH] == {"get": {"deprecated": True}} + assert paths[LEGACY_DOWNLOAD_PATH] == {"put": {"deprecated": True}} + + def test_post_processing_leaves_other_resources_alone(self): + paths = self.marked_schema()["paths"] + + assert {path: paths[path] for path in self.OTHER_RESOURCE_PATHS} == { + path: {"get": {}, "post": {}} for path in self.OTHER_RESOURCE_PATHS + } + + def test_post_processing_leaves_other_members_of_a_path_alone(self): + result = cms_mark_superseded_paths( + {"paths": {LEGACY_DOWNLOAD_PATH: {"put": {}, "parameters": []}}}, None, None, True, + ) + + assert result["paths"][LEGACY_DOWNLOAD_PATH] == {"put": {"deprecated": True}, "parameters": []} + + +@ddt.ddt +class CmsSchemaDocumentTest(TestCase): + """The published Studio document, generated with the production schema settings.""" + + def test_both_new_operations_are_published_at_their_full_address(self): + document = production_document() + + assert [method for method in document["paths"][USAGE_PATH] if method != "parameters"] == ["get"] + assert [method for method in document["paths"][ARCHIVE_PATH] if method != "parameters"] == ["post"] + + def test_only_the_studio_apis_are_published(self): + paths = list(production_document()["paths"]) + + assert {path.split("/")[2] for path in paths} == {"contentstore", "authoring", "courses"} + assert [path for path in paths if path.startswith("/api/courses/")] == [ + "/api/courses/{course_key_string}/bulk_enable_disable_discussions", + ] + + def test_no_other_address_of_these_resources_is_published(self): + published = { + path + for path in production_document()["paths"] + if path.startswith("/api/authoring/") and {"video_usages", "video_archives"} & set(path.split("/")) + } + + assert published == {USAGE_PATH, ARCHIVE_PATH} + + # The usage address is a viewset mount, whose handlers come from the action + # map it was registered with rather than from the class, so that map is + # pinned too. The archive address is a plain view: its class is the whole + # contract. + def test_the_published_addresses_resolve_to_the_new_views(self): + usage = resolve(USAGE_PATH.format(course_key="course-v1:a+b+c", edx_video_id="v1")) + archive = resolve(ARCHIVE_PATH.format(course_key="course-v1:a+b+c")) + + assert (usage.func.cls, archive.func.cls) == (CourseVideoUsageViewSet, CourseVideoArchiveView) + assert routed_actions(usage.func) == {"get": "retrieve"} + + def test_operation_ids(self): + document = production_document() + + assert document["paths"][USAGE_PATH]["get"]["operationId"] == "v2_courses_video_usages_retrieve" + assert document["paths"][ARCHIVE_PATH]["post"]["operationId"] == "v2_courses_video_archives_create" + assert document["paths"][LEGACY_USAGE_PATH]["get"]["operationId"] == "v1_videos_usage_retrieve" + assert document["paths"][LEGACY_DOWNLOAD_PATH]["put"]["operationId"] == "v1_videos_download_update" + + def test_no_authoring_operation_is_given_the_id_of_another(self): + # Ids are derived from the path after its API prefix, so an authoring + # path can be given the id of a contentstore one. The generator then + # renames one of the two with a numeric suffix, possibly the existing + # operation, so a collision is checked on the ids as first assigned. + generator = SchemaGenerator() + hooks = [*schema_settings(PRODUCTION_SETTINGS)["POSTPROCESSING_HOOKS"], RECORDING_HOOK] + generate_document(postprocessing_hooks=hooks, generator=generator) + holders = defaultdict(list) + for operation_id, path, method in generator.assigned_operation_ids: + holders[operation_id].append((path, method)) + + shared = { + operation_id: places + for operation_id, places in holders.items() + if len(places) > 1 and any(path.startswith("/api/authoring/") for path, __ in places) + } + assert shared == {} + assert sorted((path, method) for __, path, method in generator.assigned_operation_ids) == sorted( + (path, method) for path, method, __ in operations(production_document()) + ) + + # Published operations nothing supersedes: the neighbours of the two + # superseded ones, other Studio resources, and the new addresses. Resources + # superseded in their own right are not listed. + UNMARKED_OPERATIONS = ( + ("/api/contentstore/v1/videos/{course_id}", "get"), + ("/api/contentstore/v0/videos/encodings/{course_id}", "get"), + ("/api/contentstore/v0/videos/features", "get"), + ("/api/contentstore/v0/videos/images/{course_id}/{edx_video_id}", "post"), + ("/api/contentstore/v2/home/courses", "get"), + ("/api/contentstore/v2/downstreams/{usage_key_string}", "get"), + (USAGE_PATH, "get"), + (ARCHIVE_PATH, "post"), + ) + + @staticmethod + def marks(document, path, method): + operation = document["paths"][path][method] + return operation.get("deprecated"), operation.get("x-internal") + + def test_the_two_legacy_operations_are_deprecated(self): + document = production_document() + + assert self.marks(document, LEGACY_USAGE_PATH, "get") == (True, None) + assert self.marks(document, LEGACY_DOWNLOAD_PATH, "put") == (True, None) + + def test_other_operations_are_left_unmarked(self): + document = production_document() + + assert {key: self.marks(document, *key) for key in self.UNMARKED_OPERATIONS} == { + key: (None, None) for key in self.UNMARKED_OPERATIONS + } + + # The document declares no security schemes for the platform's default + # authentication, so the refusal text is where a client learns what to send. + @ddt.data( + (USAGE_PATH, "get", ( + "The request carries no accepted credentials. Send a JSON Web Token in the Authorization " + "header as 'JWT ' (the OAuth2 access token endpoint issues one when the token request " + "sets token_type=jwt), or the session cookie of a signed-in Studio user whose account is " + "active. OAuth2 access tokens sent as 'Bearer ' are not accepted." + )), + (ARCHIVE_PATH, "post", ( + "The request carries no accepted credentials. Send a JSON Web Token in the Authorization " + "header as 'JWT ' (the OAuth2 access token endpoint issues one when the token request " + "sets token_type=jwt), or the session cookie of a signed-in Studio user whose account is " + "active, together with the CSRF token in the X-CSRFToken header. OAuth2 access tokens sent " + "as 'Bearer ' are not accepted." + )), + ) + @ddt.unpack + def test_the_accepted_credentials_are_named(self, path, method, description): + operation = production_document()["paths"][path][method] + + assert operation["responses"]["401"]["description"] == description + + @ddt.data( + (USAGE_PATH, "get", "403", "The requester does not have Studio access to the course."), + (ARCHIVE_PATH, "post", "403", ( + "The requester does not have Studio access to the course, or a request made with a session " + "did not send the CSRF token." + )), + (ARCHIVE_PATH, "post", "406", ( + "The Accept header does not admit application/json. Errors are sent as JSON, so a request " + "for the archive sends 'Accept: application/zip, application/json', or no Accept header. " + "Nothing is fetched." + )), + ) + @ddt.unpack + def test_each_refusal_names_its_reasons(self, path, method, code, description): + operation = production_document()["paths"][path][method] + + assert operation["responses"][code]["description"] == description + + def test_both_new_operations_are_tagged_for_the_sdk(self): + document = production_document() + + assert document["paths"][USAGE_PATH]["get"]["tags"] == ["openedx-platform-sdk"] + assert document["paths"][ARCHIVE_PATH]["post"]["tags"] == ["openedx-platform-sdk"] + + def component(self, document, reference): + return document["components"]["schemas"][reference["$ref"].rsplit("/", 1)[-1]] + + def test_the_usage_response_is_declared(self): + document = production_document() + response = document["paths"][USAGE_PATH]["get"]["responses"]["200"] + + body = self.component(document, response["content"]["application/json"]["schema"]) + entry = self.component(document, body["properties"]["usage_locations"]["items"]) + assert body["required"] == ["usage_locations"] + assert sorted(entry["properties"]) == ["display_location", "url"] + fields = VideoUsageLocationSerializer().fields + assert {name: entry["properties"][name]["description"] for name in fields} == { + name: field.help_text for name, field in fields.items() + } + + def test_the_archive_request_body_is_declared(self): + document = production_document() + request_body = document["paths"][ARCHIVE_PATH]["post"]["requestBody"] + + body = self.component(document, request_body["content"]["application/json"]["schema"]) + entry = self.component(document, body["properties"]["files"]["items"]) + assert list(request_body["content"]) == ["application/json"] + assert request_body["required"] is True + assert body["required"] == ["files"] + assert sorted(entry["required"]) == ["name", "url"] + + def test_the_archive_response_is_a_binary_zip(self): + response = production_document()["paths"][ARCHIVE_PATH]["post"]["responses"]["200"] + + assert response["content"] == {"application/zip": {"schema": {"type": "string", "format": "binary"}}} + assert "_videos.zip" in response["description"] + + @ddt.data( + (USAGE_PATH, "get", ["401", "403", "404"]), + (ARCHIVE_PATH, "post", ["400", "401", "403", "404", "406", "415", "429"]), + ) + @ddt.unpack + def test_every_error_response_is_the_error_envelope(self, path, method, codes): + responses = production_document()["paths"][path][method]["responses"] + + assert sorted(code for code in responses if code != "200") == codes + for code in codes: + assert responses[code]["content"]["application/json"]["schema"] == { + "$ref": "#/components/schemas/ErrorResponse" + } + + def test_enum_components_survive_the_registered_post_processing(self): + without_enum_hook = generate_document(postprocessing_hooks=[SUPERSEDED_PATHS_HOOK]) + + assert not [name for name in without_enum_hook["components"]["schemas"] if name.endswith("Enum")] + assert "ContentTypeEnum" in production_document()["components"]["schemas"] + + +@ddt.ddt +class CmsSchemaSettingsTest(TestCase): + """The deployment settings publish full paths, and no server adds an API prefix of its own.""" + + @ddt.data(PRODUCTION_SETTINGS, DEVSTACK_SETTINGS) + def test_paths_are_published_in_full(self, module): + configured = schema_settings(module) + + assert "SCHEMA_PATH_PREFIX_TRIM" not in configured + assert configured["SCHEMA_PATH_PREFIX"] == r"/api/(contentstore|authoring)" + + @ddt.data(PRODUCTION_SETTINGS, DEVSTACK_SETTINGS) + # These modules replace SPECTACULAR_SETTINGS wholesale, so a hook left out + # here is silently disabled for the deployment that uses them. + def test_the_post_processing_hooks_are_all_registered(self, module): + hooks = schema_settings(module)["POSTPROCESSING_HOOKS"] + + assert hooks == [ENUM_POSTPROCESSING_HOOK, SUPERSEDED_PATHS_HOOK, MIGRATED_PATHS_HOOK] + + # The Public server is AUTHORING_API_URL as configured. Whether a gateway + # behind it serves the full paths is not decided by these settings and is + # not checked here. + @ddt.data(PRODUCTION_SETTINGS, DEVSTACK_SETTINGS) + def test_no_server_url_carries_an_api_prefix(self, module): + servers = schema_settings(module)["SERVERS"] + + assert [server["description"] for server in servers] == ["Public", "Local"] + assert [server for server in servers if "/api/" in server["url"]] == [] + + +class StudioApiDocsTest(TestCase): + """ + The service-wide documentation site, which documents every endpoint of Studio. + + It applies none of the Authoring API's filtering or prefix stripping, so it + carries both new operations at their full addresses. + """ + + ARCHIVE_DOCS_PATH = "/api/authoring/v2/courses/{course_key}/video_archives/" + USAGE_DOCS_PATH = "/api/authoring/v2/courses/{course_key}/video_usages/{edx_video_id}/" + + def document(self): + response = self.client.get(reverse("apidocs-data", kwargs={"format": "json"})) + assert response.status_code == 200 + return json.loads(response.content) + + def test_both_operations_are_documented_at_their_full_addresses(self): + paths = self.document()["paths"] + + assert list(paths[self.ARCHIVE_DOCS_PATH]) == ["post"] + assert list(paths[self.USAGE_DOCS_PATH]) == ["get"] + + # A documentation generator that infers the response body from the view's + # serializer would describe the archive as JSON. The view declares the + # response itself, so the only representation offered is the archive. + def test_the_archive_response_is_offered_only_as_a_zip(self): + operation = self.document()["paths"][self.ARCHIVE_DOCS_PATH]["post"] + + assert operation["responses"]["200"]["content"] == { + "application/zip": {"schema": {"type": "string", "format": "binary"}} + } + assert list(operation["requestBody"]["content"]) == ["application/json"] diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_settings.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_settings.py index 41aef38e3e5a..ed575012a281 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_settings.py @@ -458,13 +458,20 @@ def test_every_error_references_the_shared_envelope(self): "$ref": "#/components/schemas/ErrorResponse" }, code - def test_the_legacy_video_operations_are_unchanged(self): + def test_the_legacy_videos_listing_is_unchanged(self): operation = self.schema["paths"][LEGACY_VIDEOS_PATH]["get"] assert operation["operationId"] == "v1_videos_retrieve" assert "deprecated" not in operation + + # The usage and download operations beside the listing have conforming + # successors of their own, so they carry the mark. The listing does not, + # which is what the assertion above guards: a change that marked the whole + # legacy videos prefix rather than the two exact paths would sweep it up. + def test_the_legacy_siblings_of_the_listing_are_deprecated(self): for path, view in self._legacy_sibling_paths(): for operation in self.schema["paths"][path].values(): - assert "deprecated" not in operation, (path, view) + if isinstance(operation, dict): + assert operation["deprecated"] is True, (path, view) def _legacy_sibling_paths(self): """Return the legacy usage and download paths published beside the legacy videos path.""" diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_usages.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_usages.py new file mode 100644 index 000000000000..40d597af3640 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_video_usages.py @@ -0,0 +1,847 @@ +""" +Unit tests for the course video usages API. +""" +import json +from unittest.mock import patch + +import ddt +import pytest +from crum.signals import current_user_getter, current_user_setter +from django.core.signals import got_request_exception, request_finished, request_started +from django.db import connection +from django.db.models.signals import post_init, pre_init +from django.dispatch import Signal +from django.test.utils import CaptureQueriesContext +from django.urls import Resolver404, resolve, reverse +from edx_rest_framework_extensions.auth.jwt.tests.utils import generate_jwt +from edx_rest_framework_extensions.testing import assert_error_envelope +from openedx_authz.models.engine import PolicyCacheControl +from rest_framework import status +from rest_framework.test import APIClient + +from cms.djangoapps.contentstore.rest_api.v1.views.unknown_route import UnknownRouteView +from cms.djangoapps.contentstore.rest_api.v2.views.video_usages import CourseVideoUsageViewSet +from cms.djangoapps.contentstore.tests.utils import CourseTestCase, routed_actions +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole, +) +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.oauth_dispatch.tests.factories import AccessTokenFactory, ApplicationFactory +from xmodule.modulestore.django import SwitchedSignal +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory + +DETAIL_URL_NAME = "authoring_v2:course_video_usage_detail" +LEGACY_URL_NAME = "cms.djangoapps.contentstore:v1:video_usage" + +MISSING_COURSE_KEY = "course-v1:Org+Missing+Run" +CCX_COURSE_KEY = "ccx-v1:edX+DemoX+Demo_Course+ccx@1" + +# Keys the course_key path converter accepts that carry only a content version, +# with no organization, course or run. +VERSION_ONLY_COURSE_KEYS = ( + "course-v1:version@" + "a" * 24, + "course-v1:branch@published-branch+version@" + "a" * 24, + "ccx-v1:version@" + "a" * 24 + "+ccx@1", + "library-v1:version@" + "a" * 24, +) + +# Fields that legitimately differ per request and are ignored when diffing. +VOLATILE_FIELDS = {"instance"} + +# Every way a successful body of this version may differ from the legacy one, +# as (json_path, reason). The usage body is carried over unchanged, so there are +# none; the parity tests fail on any difference at all. +INTENTIONAL_DIFFERENCES = [] + +# The members of an error body at each address. The legacy address answers with +# its own error format, this version with the standard envelope. +LEGACY_ERROR_KEYS = { + 401: {"developer_message"}, + 403: {"developer_message"}, + 404: {"developer_message", "error_code"}, +} +ENVELOPE_KEYS = {"type", "title", "status", "detail", "instance"} + +# Signals sent while serving any request, whatever it reads: the request +# starting and finishing, model instances being built, and the current user +# being recorded and looked up. +ROUTINE_SIGNALS = ( + request_started, + request_finished, + pre_init, + post_init, + current_user_setter, + current_user_getter, +) + +# The usage lookup each address calls. +SERVICE_USAGE_LOOKUP = "cms.djangoapps.contentstore.rest_api.v2.video_management_service.get_video_usage_path" +LEGACY_USAGE_LOOKUP = "cms.djangoapps.contentstore.rest_api.v1.views.videos.get_video_usage_path" + + +def _normalize(obj, path=""): + """Sort keys, drop volatile fields, and collect a {json_path: value} map for diffing.""" + flat = {} + if isinstance(obj, dict): + for key in sorted(obj): + if key in VOLATILE_FIELDS: + continue + flat.update(_normalize(obj[key], f"{path}.{key}" if path else key)) + elif isinstance(obj, list): + for index, item in enumerate(obj): + flat.update(_normalize(item, f"{path}[{index}]")) + else: + flat[path] = obj + return flat + + +def _matches(json_path, declared): + return json_path == declared or json_path.startswith((declared + ".", declared + "[")) + + +def _diff(legacy, new): + """Return every JSON path that differs, and those of them not declared intentional.""" + flat_legacy, flat_new = _normalize(legacy), _normalize(new) + changed = { + key for key in set(flat_legacy) | set(flat_new) + if flat_legacy.get(key, "") != flat_new.get(key, "") + } + unexpected = { + key for key in changed + if not any(_matches(key, declared) for declared, __ in INTENTIONAL_DIFFERENCES) + } + return changed, unexpected + + +class VideoUsageTestBase(CourseTestCase): + """ + A course whose content exercises every path of the usage lookup. + + ``single-use`` is placed once, ``triple-use`` three times, ``draft-use`` in a + unit that was never published, and ``orphan-use`` once in a unit and once in + a component that has no parent. ``unused`` is referenced by nothing. + """ + + ENABLED_CACHES = ["default"] + + def setUp(self): + super().setUp() + chapter = BlockFactory.create(parent=self.course, category="chapter", display_name="Week 1") + self.subsection = BlockFactory.create(parent=chapter, category="sequential", display_name="Welcome") + self.placements = { + "single-use": [self.place_video("Unit A", "Clip A", "single-use")], + "triple-use": [ + self.place_video(f"Unit T{number}", f"Clip T{number}", "triple-use") + for number in (1, 2, 3) + ], + "orphan-use": [self.place_video("Unit O", "Clip O", "orphan-use")], + "unused": [], + } + # Publishing a unit publishes its whole subsection, so the unpublished + # unit is added after every published one. + self.placements["draft-use"] = [self.place_video("Draft unit", "Draft clip", "draft-use", publish=False)] + self.orphan = self.store.create_item( + self.user.id, + self.course.id, + "video", + block_id="orphan_clip", + fields={"edx_video_id": "orphan-use", "display_name": "Orphan clip"}, + ) + + def place_video(self, unit_name, video_name, edx_video_id, publish=True): + """Add a unit holding one video component to the subsection; return both.""" + unit = BlockFactory.create( + parent=self.subsection, category="vertical", display_name=unit_name, publish_item=publish, + ) + video = BlockFactory.create( + parent=unit, category="video", display_name=video_name, edx_video_id=edx_video_id, + publish_item=publish, + ) + return unit, video + + def expected_body(self, edx_video_id): + """The usage body the lookup must produce for ``edx_video_id``.""" + return { + "usage_locations": [ + { + "display_location": f"Welcome - {unit.display_name} / {video.display_name}", + "url": f"/container/{unit.location}#{video.location}", + } + for unit, video in self.placements[edx_video_id] + ] + } + + def url(self, edx_video_id, course_key=None): + return reverse(DETAIL_URL_NAME, kwargs={ + "course_key": course_key or self.course.id, + "edx_video_id": edx_video_id, + }) + + def legacy_url(self, edx_video_id, course_key=None): + return reverse(LEGACY_URL_NAME, kwargs={ + "course_id": course_key or self.course.id, + "edx_video_id": edx_video_id, + }) + + def client_for(self, user): + client = APIClient() + if user is not None: + client.force_authenticate(user=user) + return client + + def user_with_role(self, role_class, course_key=None): + user = UserFactory.create() + role_class(course_key or self.course.id).add_users(user) + return user + + +@ddt.ddt +class VideoUsageResponseTest(VideoUsageTestBase): + """The body names every component that plays the video, and nothing else.""" + + @ddt.data("single-use", "triple-use", "draft-use", "orphan-use", "unused") + def test_body_lists_each_placement(self, edx_video_id): + response = self.client_for(self.user).get(self.url(edx_video_id)) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == self.expected_body(edx_video_id) + + def test_a_video_used_three_times_is_listed_three_times_in_content_order(self): + response = self.client_for(self.user).get(self.url("triple-use")) + + assert [entry["display_location"] for entry in response.json()["usage_locations"]] == [ + "Welcome - Unit T1 / Clip T1", + "Welcome - Unit T2 / Clip T2", + "Welcome - Unit T3 / Clip T3", + ] + + def test_a_component_in_an_unpublished_unit_is_listed(self): + unit, __ = self.placements["draft-use"][0] + assert not self.store.has_published_version(unit) + + response = self.client_for(self.user).get(self.url("draft-use")) + + assert response.json()["usage_locations"] == [{ + "display_location": "Welcome - Draft unit / Draft clip", + "url": f"/container/{unit.location}#{self.placements['draft-use'][0][1].location}", + }] + + def test_a_component_without_a_parent_is_skipped(self): + referencing = [ + block.location.block_id + for block in self.store.get_items(self.course.id, qualifiers={"category": "video"}) + if block.edx_video_id == "orphan-use" + ] + assert sorted(referencing) == sorted(["orphan_clip", self.placements["orphan-use"][0][1].location.block_id]) + + response = self.client_for(self.user).get(self.url("orphan-use")) + + assert response.json() == self.expected_body("orphan-use") + assert len(response.json()["usage_locations"]) == 1 + + @ddt.data("url", "legacy_url") + def test_names_the_content_does_not_set_appear_as_their_field_defaults(self, address): + chapter = BlockFactory.create(parent=self.course, category="chapter", display_name="Week 2") + subsection = BlockFactory.create(parent=chapter, category="sequential", display_name=None) + unit = BlockFactory.create(parent=subsection, category="vertical", display_name=None) + BlockFactory.create(parent=unit, category="video", display_name=None, edx_video_id="unnamed-use") + + response = self.client_for(self.user).get(getattr(self, address)("unnamed-use")) + + assert [entry["display_location"] for entry in response.json()["usage_locations"]] == [ + "None - None / Video", + ] + + @ddt.data("url", "legacy_url") + def test_a_unit_in_no_subsection_leaves_the_subsection_part_empty(self, address): + unit = self.store.create_item( + self.user.id, self.course.id, "vertical", block_id="loose_unit", fields={"display_name": "Loose unit"}, + ) + BlockFactory.create(parent=unit, category="video", display_name="Loose clip", edx_video_id="loose-use") + + response = self.client_for(self.user).get(getattr(self, address)("loose-use")) + + assert [entry["display_location"] for entry in response.json()["usage_locations"]] == [ + " - Loose unit / Loose clip", + ] + + def test_an_unreferenced_video_answers_with_an_empty_list(self): + response = self.client_for(self.user).get(self.url("unused")) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == {"usage_locations": []} + + def test_head_answers_like_get_without_a_body(self): + response = self.client_for(self.user).head(self.url("single-use")) + + assert response.status_code == status.HTTP_200_OK + assert response.content == b"" + + +@ddt.ddt +class VideoUsageParityTest(VideoUsageTestBase): + """For the same course, video and caller, both addresses answer alike.""" + + def pair(self, edx_video_id, user, course_key=None, method="get"): + client = self.client_for(user) + call = getattr(client, method) + return ( + call(self.legacy_url(edx_video_id, course_key)), + call(self.url(edx_video_id, course_key)), + ) + + def assert_parity(self, legacy, new, expected_status): + """Assert both answered ``expected_status`` and differ only where declared.""" + assert legacy.status_code == expected_status, legacy.content + assert new.status_code == expected_status, new.content + legacy_json = json.loads(legacy.content or b"null") + new_json = json.loads(new.content or b"null") + changed, unexpected = _diff(legacy_json, new_json) + assert not unexpected, f"unexpected differences: {sorted(unexpected)}\nlegacy={legacy_json}\nnew={new_json}" + for declared, reason in INTENTIONAL_DIFFERENCES: + assert any(_matches(key, declared) for key in changed), ( + f"declared difference did not occur: {declared} ({reason})" + ) + + @ddt.data("single-use", "triple-use", "draft-use", "orphan-use", "unused") + def test_success_bodies_are_identical(self, edx_video_id): + legacy, new = self.pair(edx_video_id, self.user) + + self.assert_parity(legacy, new, status.HTTP_200_OK) + assert new.json() == self.expected_body(edx_video_id) + assert new.content == legacy.content + + def test_course_staff_sees_the_same_body_at_both_addresses(self): + legacy, new = self.pair("triple-use", self.user_with_role(CourseStaffRole)) + + self.assert_parity(legacy, new, status.HTTP_200_OK) + assert new.json() == self.expected_body("triple-use") + + def test_head_answers_alike(self): + legacy, new = self.pair("single-use", self.user, method="head") + + assert (legacy.status_code, legacy.content) == (status.HTTP_200_OK, b"") + assert (new.status_code, new.content) == (status.HTTP_200_OK, b"") + + def assert_error_keys(self, legacy, new, expected_status): + assert legacy.status_code == new.status_code == expected_status + assert set(legacy.json()) == LEGACY_ERROR_KEYS[expected_status] + assert set(new.json()) == ENVELOPE_KEYS + + def test_unauthenticated_error_bodies(self): + legacy, new = self.pair("single-use", None) + + self.assert_error_keys(legacy, new, status.HTTP_401_UNAUTHORIZED) + assert_error_envelope(new, expected_status=401, expected_type_slug="authn") + + def test_forbidden_error_bodies(self): + legacy, new = self.pair("single-use", UserFactory.create()) + + self.assert_error_keys(legacy, new, status.HTTP_403_FORBIDDEN) + assert_error_envelope(new, expected_status=403, expected_type_slug="authz") + + def test_missing_course_error_bodies(self): + legacy, new = self.pair("single-use", self.user, course_key=MISSING_COURSE_KEY) + + self.assert_error_keys(legacy, new, status.HTTP_404_NOT_FOUND) + assert legacy.json()["error_code"] == "course_does_not_exist" + assert_error_envelope(new, expected_status=404, expected_type_slug="not-found") + + +@ddt.ddt +class VideoUsageAuthorizationTest(VideoUsageTestBase): + """Studio read access to the course is required; everyone else is refused.""" + + def assert_allowed(self, user): + response = self.client_for(user).get(self.url("triple-use")) + assert response.status_code == status.HTTP_200_OK + assert response.json() == self.expected_body("triple-use") + + def test_global_staff_is_allowed(self): + self.assert_allowed(self.user) + + def test_course_staff_is_allowed(self): + self.assert_allowed(self.user_with_role(CourseStaffRole)) + + def test_course_instructor_is_allowed(self): + self.assert_allowed(self.user_with_role(CourseInstructorRole)) + + @ddt.data("get", "head") + def test_anonymous_is_refused(self, method): + response = getattr(self.client_for(None), method)(self.url("single-use")) + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response["WWW-Authenticate"] == 'JWT realm="api"' + if method == "get": + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + + @ddt.data("get", "head") + def test_user_without_a_role_is_refused(self, method): + response = getattr(self.client_for(UserFactory.create()), method)(self.url("single-use")) + + assert response.status_code == status.HTTP_403_FORBIDDEN + if method == "get": + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + def test_limited_staff_is_refused(self): + response = self.client_for(self.user_with_role(CourseLimitedStaffRole)).get(self.url("single-use")) + + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + def test_staff_of_another_course_is_refused(self): + other_course = CourseFactory.create() + author = self.user_with_role(CourseStaffRole, course_key=other_course.id) + + response = self.client_for(author).get(self.url("single-use")) + + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + def test_ccx_course_is_refused_even_to_global_staff(self): + response = self.client_for(self.user).get(self.url("single-use", course_key=CCX_COURSE_KEY)) + + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + def test_missing_course_is_refused_to_a_caller_without_access(self): + response = self.client_for(UserFactory.create()).get(self.url("single-use", course_key=MISSING_COURSE_KEY)) + + assert_error_envelope(response, expected_status=403, expected_type_slug="authz") + + def test_missing_course_is_reported_to_global_staff(self): + response = self.client_for(self.user).get(self.url("single-use", course_key=MISSING_COURSE_KEY)) + + envelope = assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + assert envelope["detail"] == "The course does not exist." + + @ddt.data(*VERSION_ONLY_COURSE_KEYS) + def test_a_key_carrying_only_a_version_is_not_found_by_global_staff(self, key): + response = self.client_for(self.user).get(self.url("single-use", course_key=key)) + + envelope = assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + assert envelope["detail"] == "Not found." + + @ddt.data(*VERSION_ONLY_COURSE_KEYS) + def test_a_key_carrying_only_a_version_is_not_found_by_a_caller_without_a_role(self, key): + response = self.client_for(UserFactory.create()).get(self.url("single-use", course_key=key)) + + envelope = assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + assert envelope["detail"] == "Not found." + + @ddt.data(*VERSION_ONLY_COURSE_KEYS) + def test_a_key_carrying_only_a_version_still_needs_authentication(self, key): + response = self.client_for(None).get(self.url("single-use", course_key=key)) + + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + + def test_the_legacy_address_answers_404_for_a_version_only_key(self): + # The legacy pattern needs a separator inside the key, so the address + # falls through to the course videos listing, which cannot parse the + # rest of the path as a course key. + address = f"/api/contentstore/v1/videos/{VERSION_ONLY_COURSE_KEYS[0]}/single-use/usage" + + response = self.client_for(self.user).get(address) + + assert resolve(address).func.view_class.__name__ == "CourseVideosView" + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json()["error_code"] == "invalid_course_key" + + def test_the_legacy_address_reports_a_missing_course_to_anyone(self): + response = self.client_for(UserFactory.create()).get( + self.legacy_url("single-use", course_key=MISSING_COURSE_KEY) + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json()["error_code"] == "course_does_not_exist" + + def test_the_view_declares_its_checks_in_order(self): + # The JWT middleware appends its own restricted-application check to + # views that accept JWTs, so only the declared head of the list is fixed. + assert [klass.__name__ for klass in CourseVideoUsageViewSet.permission_classes[:3]] == [ + "IsAuthenticated", "HasFullCourseKey", "HasStudioReadAccess", + ] + + +class VideoUsageUnexpectedFailureTest(VideoUsageTestBase): + """ + An unexpected failure behind the lookup, at both addresses. + + The new address answers with the generic 500 body from inside the view, so + the exception never reaches Django: ``got_request_exception`` is not sent, + and the only log record is Django's one-line 500 notice, without the + traceback. The legacy address lets the exception reach Django, which sends + the signal and logs the traceback. Both sides are pinned, so a change in how + the error handling reports failures shows up here. + """ + + def request_while_failing(self, address, target): + """GET ``address`` as global staff while ``target`` raises; return the response, signals and log records.""" + signals = [] + + def receiver(sender, **kwargs): + signals.append(sender) + + got_request_exception.connect(receiver) + self.addCleanup(got_request_exception.disconnect, receiver) + client = APIClient(raise_request_exception=False) + client.force_authenticate(user=self.user) + with patch(target, side_effect=RuntimeError("content store unreachable at mongo-7")): + with self.assertLogs(level="ERROR") as logs: + response = client.get(address) + return response, signals, logs.records + + def test_the_failure_is_answered_with_the_generic_body(self): + response, __, __ = self.request_while_failing(self.url("single-use"), SERVICE_USAGE_LOOKUP) + + envelope = assert_error_envelope(response, expected_status=500, expected_type_slug="internal") + assert envelope["detail"] == "An unexpected error occurred. Please try again later." + assert "mongo-7" not in response.content.decode() + + def test_the_failure_does_not_reach_django(self): + address = self.url("single-use") + + __, signals, records = self.request_while_failing(address, SERVICE_USAGE_LOOKUP) + + assert not signals + assert [(record.name, record.getMessage(), record.exc_info) for record in records] == [ + ("django.request", f"Internal Server Error: {address}", None), + ] + + def test_the_legacy_address_hands_the_failure_to_django(self): + __, signals, records = self.request_while_failing(self.legacy_url("single-use"), LEGACY_USAGE_LOOKUP) + + assert len(signals) == 1 + assert "django.request" in { + record.name for record in records if record.exc_info and record.exc_info[0] is RuntimeError + } + + +class VideoUsageAuthenticationTest(VideoUsageTestBase): + """The endpoint uses the platform's default authentication.""" + + def test_session_caller_succeeds(self): + client = APIClient() + assert client.login(username=self.user.username, password=self.user_password) + + response = client.get(self.url("single-use")) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == self.expected_body("single-use") + + def test_jwt_caller_succeeds(self): + response = APIClient().get(self.url("single-use"), HTTP_AUTHORIZATION=f"JWT {generate_jwt(self.user)}") + + assert response.status_code == status.HTTP_200_OK + assert response.json() == self.expected_body("single-use") + + def bearer_header(self, user): + token = AccessTokenFactory(user=user, application=ApplicationFactory()).token + return f"Bearer {token}" + + def test_bearer_token_is_refused(self): + response = APIClient().get(self.url("single-use"), HTTP_AUTHORIZATION=self.bearer_header(self.user)) + + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + + def test_the_legacy_address_still_accepts_a_bearer_token(self): + response = APIClient().get(self.legacy_url("single-use"), HTTP_AUTHORIZATION=self.bearer_header(self.user)) + + assert response.status_code == status.HTTP_200_OK + + def deactivated_global_staff(self): + """Log a global staff member in, then deactivate the account behind the live session.""" + password = "password-12345" + staff = UserFactory.create(is_staff=True, password=password) + session_client = APIClient() + assert session_client.login(username=staff.username, password=password) + staff.is_active = False + staff.save() + return staff, session_client + + def test_inactive_global_staff_is_refused_over_a_session(self): + __, session_client = self.deactivated_global_staff() + + response = session_client.get(self.url("single-use")) + + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + + def test_inactive_global_staff_still_reaches_the_legacy_address_over_a_session(self): + __, session_client = self.deactivated_global_staff() + + response = session_client.get(self.legacy_url("single-use")) + + assert response.status_code == status.HTTP_200_OK + + def test_inactive_global_staff_is_admitted_over_jwt_at_both_addresses(self): + staff, __ = self.deactivated_global_staff() + header = f"JWT {generate_jwt(staff)}" + + new = APIClient().get(self.url("single-use"), HTTP_AUTHORIZATION=header) + legacy = APIClient().get(self.legacy_url("single-use"), HTTP_AUTHORIZATION=header) + + assert (new.status_code, legacy.status_code) == (status.HTTP_200_OK, status.HTTP_200_OK) + assert new.json() == self.expected_body("single-use") + + def test_the_viewset_does_not_declare_authentication_classes(self): + assert "authentication_classes" not in CourseVideoUsageViewSet.__dict__ + assert [klass.__name__ for klass in CourseVideoUsageViewSet.authentication_classes] == [ + "DefaultJwtAuthentication", "DefaultSessionAuthentication", + ] + + +@ddt.ddt +class VideoUsageReadOnlyTest(VideoUsageTestBase): + """ + Reading usages, at either address, writes nothing to the database and sends + no signal or event beyond the ones every request sends. + """ + + def setUp(self): + super().setUp() + # The authorization engine stores a one-row policy cache marker the + # first time any role is checked against an empty database. That is the + # engine starting up, not this endpoint, so it is created up front. + PolicyCacheControl.get_version() + + def session_client(self, role_class=None): + """Return a client logged in over a session as a new user holding ``role_class``, if any.""" + password = "password-12345" + user = UserFactory.create(password=password) + if role_class: + role_class(self.course.id).add_users(user) + client = APIClient() + assert client.login(username=user.username, password=password) + return client + + def side_effects_during(self, request): + """ + Run ``request``; return its response, every INSERT, UPDATE or DELETE it + issued, and every signal it sent beyond ``ROUTINE_SIGNALS``, as + (signal, sender) pairs. + + Signals are recorded as they are sent, whether or not anything receives + them, and Open edX events are sent as signals, so an event published with + no receiver writing to the database is still seen. + """ + sent = [] + + def recording(send): + def record(signal, *args, **named): + if signal not in ROUTINE_SIGNALS: + sent.append((signal, args[0] if args else named.get("sender"))) + return send(signal, *args, **named) + return record + + with ( + patch.object(Signal, "send", recording(Signal.send)), + patch.object(Signal, "send_robust", recording(Signal.send_robust)), + # The test case switches the content store's signals off, and a + # switched-off signal returns before it reaches Signal.send. + patch.object(SwitchedSignal, "send", recording(SwitchedSignal.send)), + patch.object(SwitchedSignal, "send_robust", recording(SwitchedSignal.send_robust)), + CaptureQueriesContext(connection) as captured, + ): + response = request() + writes = [ + query["sql"] for query in captured.captured_queries + if query["sql"].lstrip().split(" ", 1)[0].upper() in ("INSERT", "UPDATE", "DELETE") + ] + return response, writes, sent + + @ddt.data( + *[ + (edx_video_id, method, address) + for edx_video_id in ("single-use", "triple-use", "draft-use", "orphan-use", "unused") + for method in ("get", "head") + for address in ("url", "legacy_url") + ] + ) + @ddt.unpack + def test_course_staff_over_a_session_writes_nothing(self, edx_video_id, method, address): + client = self.session_client(CourseStaffRole) + + response, writes, signals = self.side_effects_during( + lambda: getattr(client, method)(getattr(self, address)(edx_video_id)) + ) + + assert response.status_code == status.HTTP_200_OK + assert writes == [] + assert not signals + + @ddt.data( + *[(method, address) for method in ("get", "head") for address in ("url", "legacy_url")] + ) + @ddt.unpack + def test_global_staff_writes_nothing(self, method, address): + client = self.client_for(self.user) + + response, writes, signals = self.side_effects_during( + lambda: getattr(client, method)(getattr(self, address)("triple-use")) + ) + + assert response.status_code == status.HTTP_200_OK + assert writes == [] + assert not signals + + @ddt.data("url", "legacy_url") + def test_a_missing_course_is_not_created_by_looking_it_up(self, address): + response, writes, signals = self.side_effects_during( + lambda: self.client_for(self.user).get(getattr(self, address)("single-use", MISSING_COURSE_KEY)) + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert writes == [] + assert not signals + + def test_a_refused_caller_writes_nothing(self): + client = self.session_client() + + response, writes, signals = self.side_effects_during(lambda: client.get(self.url("single-use"))) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert writes == [] + assert not signals + + +class VideoUsageQueryCountTest(VideoUsageTestBase): + """ + The new address costs no more queries than the legacy one for the same caller. + + Each error answered by the new address ends with one extra statement, a + rollback of the request's savepoint, issued by the standard error handling. + """ + + #: Set from the pytest fixture below, which class-based tests cannot request directly. + django_assert_num_queries = None + + @pytest.fixture(autouse=True) + def _assert_num_queries(self, django_assert_num_queries): + self.django_assert_num_queries = django_assert_num_queries + + def measure(self, client, legacy_count, new_count, course_key=None): + """Warm the per-process caches, then count the queries of each address.""" + for __ in range(2): + client.get(self.legacy_url("triple-use", course_key)) + client.get(self.url("triple-use", course_key)) + with self.django_assert_num_queries(legacy_count): + client.get(self.legacy_url("triple-use", course_key)) + with self.django_assert_num_queries(new_count): + client.get(self.url("triple-use", course_key)) + + def test_global_staff(self): + self.measure(self.client_for(self.user), legacy_count=11, new_count=11) + + def test_global_staff_over_a_session(self): + self.measure(self.client, legacy_count=13, new_count=13) + + def test_course_staff(self): + self.measure(self.client_for(self.user_with_role(CourseStaffRole)), legacy_count=11, new_count=11) + + def test_refused_caller(self): + self.measure(self.client_for(UserFactory.create()), legacy_count=8, new_count=7) + + def test_missing_course(self): + self.measure(self.client_for(self.user), legacy_count=8, new_count=9, course_key=MISSING_COURSE_KEY) + + def test_anonymous(self): + self.measure(self.client_for(None), legacy_count=5, new_count=6) + + +@ddt.ddt +class VideoUsageUrlContractTest(VideoUsageTestBase): + """The address of a video's usages, and how unusable addresses are answered.""" + + def test_conforming_address(self): + assert self.url("abc-123") == f"/api/authoring/v2/courses/{self.course.id}/video_usages/abc-123/" + + # The class alone says nothing about which actions this mount wires up, so + # the method-to-action map is pinned as well. + def test_the_address_resolves_to_the_viewset(self): + match = resolve(self.url("abc-123")) + + assert match.func.cls is CourseVideoUsageViewSet + assert routed_actions(match.func) == {"get": "retrieve"} + assert match.kwargs == {"course_key": self.course.id, "edx_video_id": "abc-123"} + + def test_the_legacy_address_is_unchanged(self): + assert self.legacy_url("abc-123") == f"/api/contentstore/v1/videos/{self.course.id}/abc-123/usage" + assert resolve(self.legacy_url("abc-123")).func.view_class.__name__ == "VideoUsageView" + + def assert_json_not_found(self, address): + response = self.client_for(self.user).get(address, HTTP_ACCEPT="application/json") + assert response["Content-Type"] == "application/json" + return assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_malformed_course_key_is_answered_with_the_error_envelope(self): + self.assert_json_not_found("/api/authoring/v2/courses/not-a-key/video_usages/abc-123/") + + def test_deprecated_course_key_is_answered_with_the_error_envelope(self): + self.assert_json_not_found("/api/authoring/v2/courses/Org/Course/Run/video_usages/abc-123/") + + def test_a_block_key_in_place_of_a_course_key_is_answered_with_the_error_envelope(self): + self.assert_json_not_found("/api/authoring/v2/courses/i4x://Org/Course/video/abc/video_usages/abc-123/") + + def test_an_unusable_course_key_is_claimed_by_the_fallback_route(self): + address = "/api/authoring/v2/courses/Org/Course/Run/video_usages/abc-123/" + + assert resolve(address).func.cls is UnknownRouteView + assert reverse( + "authoring_v2:course_video_usage_detail_unmatched", + kwargs={"course_key": "Org/Course/Run", "subpath": "abc-123"}, + ) == address + assert reverse( + "authoring_v2:course_video_usage_list_unmatched", kwargs={"course_key": "Org/Course/Run"}, + ) == "/api/authoring/v2/courses/Org/Course/Run/video_usages/" + + def test_the_fallback_needs_no_authentication(self): + response = self.client_for(None).get("/api/authoring/v2/courses/not-a-key/video_usages/abc-123/") + + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_the_legacy_address_still_accepts_a_deprecated_course_key(self): + match = resolve("/api/contentstore/v1/videos/Org/Course/Run/abc-123/usage") + + assert match.func.view_class.__name__ == "VideoUsageView" + assert match.kwargs == {"course_id": "Org/Course/Run", "edx_video_id": "abc-123"} + + @ddt.data( + "video_usages/", + "video_usages//", + "video_usages/abc-123/extra/", + "video_usages/abc-123/extra/more/", + ) + def test_other_addresses_under_the_collection_are_answered_with_the_error_envelope(self, rest): + address = f"/api/authoring/v2/courses/{self.course.id}/{rest}" + + assert resolve(address).func.cls is UnknownRouteView + self.assert_json_not_found(address) + + @ddt.data("video_usages", "video_usages/abc-123/extra") + def test_an_address_under_the_collection_without_the_trailing_slash_is_redirected(self, rest): + address = f"/api/authoring/v2/courses/{self.course.id}/{rest}" + + response = self.client_for(self.user).get(address, HTTP_ACCEPT="application/json") + + assert response.status_code == status.HTTP_301_MOVED_PERMANENTLY + assert response["Location"] == f"{address}/" + + def test_the_address_requires_the_trailing_slash(self): + with pytest.raises(Resolver404): + resolve(self.url("abc-123").rstrip("/")) + + def test_the_trailing_slash_is_appended_by_redirect(self): + response = self.client_for(self.user).get(self.url("abc-123").rstrip("/")) + + assert response.status_code == status.HTTP_301_MOVED_PERMANENTLY + assert response["Location"] == self.url("abc-123") + + def test_an_unsupported_method_is_refused(self): + response = self.client_for(self.user).post(self.url("single-use"), {}, format="json") + + assert_error_envelope(response, expected_status=405) + assert response["Allow"] == "GET, HEAD, OPTIONS" + + def test_an_unsatisfiable_accept_header_is_refused(self): + response = self.client_for(self.user).get(self.url("single-use"), HTTP_ACCEPT="application/zip") + + assert_error_envelope(response, expected_status=406) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/video_archives.py b/cms/djangoapps/contentstore/rest_api/v2/views/video_archives.py new file mode 100644 index 000000000000..02051a029497 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/video_archives.py @@ -0,0 +1,127 @@ +"""API Views for zip archives of a course's videos.""" + +from drf_spectacular.types import OpenApiTypes +from drf_spectacular.utils import OpenApiExample, OpenApiResponse, extend_schema +from edx_rest_framework_extensions.errors import ErrorResponseSerializer +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from rest_framework.parsers import JSONParser +from rest_framework.permissions import IsAuthenticated +from rest_framework.views import APIView + +from cms.djangoapps.contentstore.rest_api.v1.views.videos import VideoDownloadThrottle +from cms.djangoapps.contentstore.rest_api.v2.serializers.video_archives import VideoArchiveRequestSerializer +from cms.djangoapps.contentstore.rest_api.v2.video_management_service import ( + create_course_video_archive, + ensure_course_exists, +) +from cms.djangoapps.contentstore.rest_api.v2.views.video_schema import ( + COURSE_KEY_PARAMETER, + COURSE_NOT_FOUND_RESPONSE, + forbidden_response, + unauthenticated_response, +) +from cms.djangoapps.contentstore.views.permissions import HasFullCourseKey, HasStudioReadAccess + +_REQUEST_EXAMPLE = OpenApiExample( + "Two videos", + value={ + "files": [ + {"url": "https://video-store.example.com/course-videos/lecture-01.mp4", "name": "Lecture 1.mp4"}, + {"url": "https://video-store.example.com/course-videos/lecture-02.mp4", "name": "Lecture 2"}, + ] + }, + request_only=True, +) + +_RESPONSE_ARCHIVE = OpenApiResponse( + response=OpenApiTypes.BINARY, + description=( + "A zip archive holding one entry per requested video, in request order, sent as an " + "attachment named '_videos.zip'. The archive is streamed while the videos " + "are fetched; if fetching a video fails part way, the transfer stops and the archive " + "received is incomplete." + ), +) +_RESPONSE_BAD_REQUEST = OpenApiResponse( + response=ErrorResponseSerializer, + description=( + "The request body is malformed, or names an address that is not one of the course's " + "video files. Nothing is fetched." + ), +) +_RESPONSE_NOT_ACCEPTABLE = OpenApiResponse( + response=ErrorResponseSerializer, + description=( + "The Accept header does not admit application/json. Errors are sent as JSON, so a request " + "for the archive sends 'Accept: application/zip, application/json', or no Accept header. " + "Nothing is fetched." + ), +) +_RESPONSE_UNSUPPORTED_MEDIA_TYPE = OpenApiResponse( + response=ErrorResponseSerializer, + description="The request body is not JSON. Nothing is fetched.", +) +_RESPONSE_THROTTLED = OpenApiResponse( + response=ErrorResponseSerializer, + description=( + "The requester has made too many archive requests recently. The Retry-After header gives " + "the number of seconds to wait." + ), +) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseVideoArchiveView(StandardizedErrorMixin, APIView): + """ + Build a zip archive of chosen videos of a course. + + The archive is assembled while it is sent and is never stored, so there is + no collection to list and no member to fetch or delete afterwards; that is + why this is a single POST handler rather than a ViewSet. Each requested + address must be one of the course's own encoded-video URLs and is checked + before any video is fetched, so the server only ever fetches addresses the + course already holds. + + The body is accepted as JSON only: a list of objects has no form encoding + that both the published schema and the parser agree on. + + Callers need Studio read access to the course. Requests are rate-limited + per user with the throttle of the v1 video download endpoint, so both + endpoints draw on one budget. + """ + + permission_classes = (IsAuthenticated, HasFullCourseKey, HasStudioReadAccess) + parser_classes = (JSONParser,) + throttle_classes = (VideoDownloadThrottle,) + + @extend_schema( + summary="Download chosen videos of a course as one zip archive", + description=( + "Fetches the requested videos of the course and streams them back as a single zip " + "archive. The archive is built for this request only and is not stored. Every " + "address must be one of the course's encoded video files, as the course's video " + "listing reports them; a request naming any other address is refused as a whole " + "before anything is fetched. Requests are rate-limited per user." + ), + parameters=[COURSE_KEY_PARAMETER], + request=VideoArchiveRequestSerializer, + examples=[_REQUEST_EXAMPLE], + responses={ + (200, "application/zip"): _RESPONSE_ARCHIVE, + 400: _RESPONSE_BAD_REQUEST, + 401: unauthenticated_response(requires_csrf_token=True), + 403: forbidden_response(requires_csrf_token=True), + 404: COURSE_NOT_FOUND_RESPONSE, + 406: _RESPONSE_NOT_ACCEPTABLE, + 415: _RESPONSE_UNSUPPORTED_MEDIA_TYPE, + 429: _RESPONSE_THROTTLED, + }, + ) + def post(self, request, course_key): + """Stream an archive of the requested videos of the course.""" + ensure_course_exists(course_key) + # The view declares no serializer_class: documentation generators read + # it as the response body as well, and the response is a binary archive. + body = VideoArchiveRequestSerializer(data=request.data) + body.is_valid(raise_exception=True) + return create_course_video_archive(course_key, body.validated_data["files"]) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/video_schema.py b/cms/djangoapps/contentstore/rest_api/v2/views/video_schema.py new file mode 100644 index 000000000000..f7583fc8b121 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/video_schema.py @@ -0,0 +1,47 @@ +"""Schema parameters and responses shared by the API views of a course's videos.""" + +from drf_spectacular.utils import OpenApiParameter, OpenApiResponse +from edx_rest_framework_extensions.errors import ErrorResponseSerializer + +COURSE_KEY_PARAMETER = OpenApiParameter( + name="course_key", + description="Key of the course, for example course-v1:edX+DemoX+Demo_Course.", + required=True, + type=str, + location=OpenApiParameter.PATH, +) + +COURSE_NOT_FOUND_RESPONSE = OpenApiResponse( + response=ErrorResponseSerializer, + description="The course does not exist, or the course key is malformed.", +) + + +def unauthenticated_response(requires_csrf_token): + """ + Return the 401 response, naming the credentials the platform accepts. + + ``requires_csrf_token`` is true for an operation whose session callers must + also send the CSRF token, as for any method other than GET, HEAD, OPTIONS + and TRACE. + """ + session = "the session cookie of a signed-in Studio user whose account is active" + if requires_csrf_token: + session += ", together with the CSRF token in the X-CSRFToken header" + return OpenApiResponse( + response=ErrorResponseSerializer, + description=( + "The request carries no accepted credentials. Send a JSON Web Token in the Authorization " + "header as 'JWT ' (the OAuth2 access token endpoint issues one when the token request " + f"sets token_type=jwt), or {session}. OAuth2 access tokens sent as 'Bearer ' are not " + "accepted." + ), + ) + + +def forbidden_response(requires_csrf_token): + """Return the 403 response; ``requires_csrf_token`` as for ``unauthenticated_response``.""" + description = "The requester does not have Studio access to the course" + if requires_csrf_token: + description += ", or a request made with a session did not send the CSRF token" + return OpenApiResponse(response=ErrorResponseSerializer, description=f"{description}.") diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/video_usages.py b/cms/djangoapps/contentstore/rest_api/v2/views/video_usages.py new file mode 100644 index 000000000000..f7617dc61041 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/video_usages.py @@ -0,0 +1,97 @@ +"""API Views for where a video is used in a course.""" + +from drf_spectacular.utils import OpenApiExample, OpenApiParameter, OpenApiResponse, extend_schema +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from rest_framework import viewsets +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.v2.serializers.video_usages import CourseVideoUsageSerializer +from cms.djangoapps.contentstore.rest_api.v2.video_management_service import get_course_video_usages +from cms.djangoapps.contentstore.rest_api.v2.views.video_schema import ( + COURSE_KEY_PARAMETER, + COURSE_NOT_FOUND_RESPONSE, + forbidden_response, + unauthenticated_response, +) +from cms.djangoapps.contentstore.views.permissions import HasFullCourseKey, HasStudioReadAccess + +_EDX_VIDEO_ID_PARAMETER = OpenApiParameter( + name="edx_video_id", + description="Identifier of the video in the video store, as set on the video components that play it.", + required=True, + type=str, + location=OpenApiParameter.PATH, +) + +_USAGE_EXAMPLE = OpenApiExample( + "A video placed in two units", + value={ + "usage_locations": [ + { + "display_location": "Week 1 - Welcome / Introduction video", + "url": ( + "/container/block-v1:edX+DemoX+Demo_Course+type@vertical+block@welcome" + "#block-v1:edX+DemoX+Demo_Course+type@video+block@intro" + ), + }, + { + "display_location": "Week 3 - Recap / Introduction video", + "url": ( + "/container/block-v1:edX+DemoX+Demo_Course+type@vertical+block@recap" + "#block-v1:edX+DemoX+Demo_Course+type@video+block@intro_again" + ), + }, + ] + }, + response_only=True, +) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseVideoUsageViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + Where one video of a course is used. + + Answers with every video component in the course that references the video, + including components in unpublished units. Callers need Studio read access + to the course; a caller without it is refused before the course is looked + up, so the refusal does not reveal whether the course exists. + + The usages come from walking the course's video components in the content + store, not from a database query, so this is a plain ViewSet with no + queryset. An identifier that no component references answers with an empty + list rather than 404, because components may reference videos that the + video store does not hold. + """ + + permission_classes = (IsAuthenticated, HasFullCourseKey, HasStudioReadAccess) + serializer_class = CourseVideoUsageSerializer + + def get_serializer(self, *args, **kwargs): + """Instantiate and return the configured serializer class.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="Get where a video is used in a course", + description=( + "Returns one entry for every video component in the course that plays the video, " + "including components in units that are not published yet. Each entry names the " + "subsection, unit and component, and gives the Studio address of the unit." + ), + parameters=[COURSE_KEY_PARAMETER, _EDX_VIDEO_ID_PARAMETER], + responses={ + 200: OpenApiResponse( + response=CourseVideoUsageSerializer, + description="The places the video is used; an empty list when it is used nowhere.", + examples=[_USAGE_EXAMPLE], + ), + 401: unauthenticated_response(requires_csrf_token=False), + 403: forbidden_response(requires_csrf_token=False), + 404: COURSE_NOT_FOUND_RESPONSE, + }, + ) + def retrieve(self, request, course_key, edx_video_id): + """Return the places in the course where the video is used.""" + usages = get_course_video_usages(course_key, edx_video_id) + return Response(self.get_serializer(usages).data) diff --git a/cms/djangoapps/contentstore/views/permissions.py b/cms/djangoapps/contentstore/views/permissions.py index 6450a3bfcc3e..51019dad061e 100644 --- a/cms/djangoapps/contentstore/views/permissions.py +++ b/cms/djangoapps/contentstore/views/permissions.py @@ -3,6 +3,7 @@ """ from openedx_authz.constants.permissions import COURSES_VIEW_PAGES_AND_RESOURCES +from rest_framework.exceptions import NotFound from rest_framework.permissions import BasePermission from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access @@ -58,3 +59,22 @@ def has_permission(self, request, view): if course_key is None: return False return has_studio_read_access(request.user, course_key) + + +class HasFullCourseKey(BasePermission): + """ + Answer 404 for a course key in the path that carries only a content version. + + Reads the parsed ``course_key`` URL argument. A key such as + ``course-v1:version@`` parses, but names no organization, course or + run: no course can be found by it, and Studio's role checks raise an error + on it instead of refusing it. The request is answered as an address that + holds nothing, as for a malformed key. List this class after the + authentication check and before any class that checks roles on the course. + """ + + def has_permission(self, request, view): + course_key = view.kwargs.get("course_key") + if course_key is not None and course_key.org is None: + raise NotFound() + return True diff --git a/cms/envs/common.py b/cms/envs/common.py index 87ee13f3e0a6..06ac9c6c8823 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -667,9 +667,11 @@ def make_lms_template_path(settings): # .. setting_name: VIDEO_DOWNLOAD_RATE_LIMIT # .. setting_default: '12/hour' # .. setting_description: Per-user rate limit applied to the Studio -# video-download endpoint -# (``PUT /api/contentstore/v1/videos/{course_id}/download``). Bounds how -# often a single course author can initiate a multi-video zip download. +# video-download endpoints +# (``PUT /api/contentstore/v1/videos/{course_id}/download`` and +# ``POST /api/authoring/v2/courses/{course_key}/video_archives/``, which +# share one budget). Bounds how often a single course author can initiate a +# multi-video zip download. # Rate format: DRF ``UserRateThrottle`` rate string. See # https://www.django-rest-framework.org/api-guide/throttling/#setting-the-throttling-policy VIDEO_DOWNLOAD_RATE_LIMIT = '12/hour' diff --git a/cms/lib/spectacular.py b/cms/lib/spectacular.py index 662cf80c65f0..189e664a0d72 100644 --- a/cms/lib/spectacular.py +++ b/cms/lib/spectacular.py @@ -5,14 +5,21 @@ from drf_spectacular.openapi import AutoSchema # Legacy operations superseded by a conforming address under /api/authoring/. -# Each is listed in the truncated spelling as well as the full one: the routes -# make the final character optional, so both addresses resolve, and which one -# the generator emits depends on how it renders that regex. +# Listed as exact paths, not prefixes: their neighbours under the same legacy +# collection are not superseded, and a prefix would also match them. +# +# The youtube_transcripts routes make the final character optional, so both +# addresses resolve and which one the generator emits depends on how it renders +# that regex; each is therefore listed in the truncated spelling as well. SUPERSEDED_PATHS = ( "/api/contentstore/v0/youtube_transcripts/{course_id}/chec", "/api/contentstore/v0/youtube_transcripts/{course_id}/check", "/api/contentstore/v0/youtube_transcripts/{course_id}/uploa", "/api/contentstore/v0/youtube_transcripts/{course_id}/upload", + # → /api/authoring/v2/courses/{course_key}/video_usages/{edx_video_id}/ + "/api/contentstore/v1/videos/{course_id}/{edx_video_id}/usage", + # → /api/authoring/v2/courses/{course_key}/video_archives/ + "/api/contentstore/v1/videos/{course_id}/download", ) # Legacy addresses of APIs migrated to /api/authoring/, marked deprecated for diff --git a/cms/lib/tests/test_spectacular.py b/cms/lib/tests/test_spectacular.py index 4581f23a6148..0d847bd7ddb6 100644 --- a/cms/lib/tests/test_spectacular.py +++ b/cms/lib/tests/test_spectacular.py @@ -70,12 +70,16 @@ def test_every_operation_of_a_superseded_path_is_marked(self): for path in SUPERSEDED_PATHS: assert result["paths"][path]["get"]["deprecated"] is True - def test_both_spellings_of_each_superseded_path_are_listed(self): + def test_every_superseded_path_is_listed(self): assert set(SUPERSEDED_PATHS) == { + # The youtube_transcripts routes make the final character optional, + # so each of them is listed in both spellings. "/api/contentstore/v0/youtube_transcripts/{course_id}/chec", "/api/contentstore/v0/youtube_transcripts/{course_id}/check", "/api/contentstore/v0/youtube_transcripts/{course_id}/uploa", "/api/contentstore/v0/youtube_transcripts/{course_id}/upload", + "/api/contentstore/v1/videos/{course_id}/{edx_video_id}/usage", + "/api/contentstore/v1/videos/{course_id}/download", } def test_the_conforming_paths_are_left_unmarked(self):