CMS API serializers and validations (#33316)

Internal ticket: https://2u-internal.atlassian.net/browse/TNL-11077

This adds validations to all new CMS API endpoints via serializers.

Validations follow the "strong parameter" concept:
https://www.mintbit.com/blog/securing-ruby-on-rails-applications-part-3-use-strong-parameters#:~:text=Strong%20parameters%20are%20a%20feature,parameters%20to%20a%20controller%20action.
This commit is contained in:
Jesper Hodge
2023-09-22 17:49:49 -04:00
committed by GitHub
parent f9148f032f
commit cd2ce32d73
15 changed files with 239 additions and 39 deletions

View File

@@ -13,3 +13,7 @@ from .proctoring import (
ProctoringErrorsSerializer
)
from .settings import CourseSettingsSerializer
from .xblock import XblockSerializer
from .videos import VideoUploadSerializer, VideoImageSerializer
from .transcripts import TranscriptSerializer
from .assets import AssetSerializer

View File

@@ -0,0 +1,13 @@
"""
API Serializers for assets
"""
from rest_framework import serializers
from .common import StrictSerializer
class AssetSerializer(StrictSerializer):
"""
Strict Serializer for file assets.
"""
file = serializers.FileField(required=False, allow_null=True)
locked = serializers.BooleanField(required=False, allow_null=True)

View File

@@ -17,3 +17,36 @@ class CourseCommonSerializer(serializers.Serializer):
rerun_link = serializers.CharField()
run = serializers.CharField()
url = serializers.CharField()
class StrictSerializer(serializers.Serializer):
"""
Serializers that validates strong parameters, i.e. that no extra fields are passed in.
The serializer inheriting from this may throw a ValidationError and can be called in a try/catch
block that will return a 400 response on ValidationError.
"""
def to_internal_value(self, data):
"""
raise validation error if there are any unexpected fields.
"""
# Transform and validate the expected fields
ret = super().to_internal_value(data)
# Get the list of valid fields from the serializer
valid_fields = set(self.fields.keys())
# Check for unexpected fields
extra_fields = set(data.keys()) - valid_fields
if extra_fields:
# Check if these unexpected fields are due to nested serializers
for field_name in list(extra_fields):
if isinstance(self.fields.get(field_name), serializers.BaseSerializer):
extra_fields.remove(field_name)
# If there are still unexpected fields left, raise an error
if extra_fields:
raise serializers.ValidationError(
{field: ["This field is not expected."] for field in extra_fields}
)
return ret

View File

@@ -0,0 +1,15 @@
"""
API Serializers for transcripts
"""
from rest_framework import serializers
from .common import StrictSerializer
class TranscriptSerializer(StrictSerializer):
"""
Strict Serializer for video transcripts.
"""
file = serializers.FileField()
edx_video_id = serializers.CharField()
language_code = serializers.CharField(required=False, allow_null=True)
new_language_code = serializers.CharField(required=False, allow_null=True)

View File

@@ -0,0 +1,29 @@
"""
API Serializers for videos
"""
from rest_framework import serializers
from .common import StrictSerializer
class FileSpecSerializer(StrictSerializer):
""" Strict Serializer for file specs """
file_name = serializers.CharField()
content_type = serializers.ChoiceField(choices=['video/mp4', 'video/webm', 'video/ogg'])
class VideoUploadSerializer(StrictSerializer):
"""
Strict Serializer for video upload urls.
Note that these are not actually video uploads but endpoints to generate an upload url for AWS
and generating a video placeholder without performing an actual upload.
"""
files = serializers.ListField(
child=FileSpecSerializer()
)
class VideoImageSerializer(StrictSerializer):
"""
Strict Serializer for video imgage files.
"""
file = serializers.ImageField()

View File

@@ -1,15 +1,79 @@
"""
API Serializers for xblocks
"""
from rest_framework import serializers
from .common import StrictSerializer
# The XblockSerializer is designed to be scalable and generic. As such, its structure
# should remain as general as possible. Avoid indiscriminately adding fields to it,
# especially those that are xblock-specific. In the future, we aim to develop a solution
# that can generate serializer fields dynamically based on the xblock definitions.
# TODO: implement and use serializer
# from rest_framework import serializers
class XblockSerializer(StrictSerializer):
"""
A serializer for xblocks that enforces strict validation.
# class XblockSerializer(serializers.Serializer):
# """Serializer for xblocks"""
# id=serializers.CharField(required=False)
# display_name=serializers.CharField(required=False)
# category=serializers.CharField(required=False)
# data=serializers.CharField(required=False)
# metadata=serializers.DictField(required=False)
The serializer ensures:
1. All top-level fields have the expected data types.
2. No unexpected fields are passed in.
Note: The current list of fields is not exhaustive. It is primarily designed
to support the CMS API demo. While optional fields have been added, they were
chosen based on ease of discovery, not comprehensiveness.
"""
id = serializers.CharField(required=False, allow_null=True)
parent_locator = serializers.CharField(required=False, allow_null=True)
display_name = serializers.CharField(required=False, allow_null=True)
category = serializers.CharField(required=False, allow_null=True)
data = serializers.CharField(required=False, allow_null=True)
metadata = serializers.DictField(required=False, allow_null=True)
has_changes = serializers.BooleanField(required=False, allow_null=True)
children = serializers.ListField(required=False, allow_null=True)
fields = serializers.DictField(required=False, allow_null=True)
has_children = serializers.BooleanField(required=False, allow_null=True)
video_sharing_enabled = serializers.BooleanField(required=False, allow_null=True)
video_sharing_options = serializers.CharField(required=False, allow_null=True)
video_sharing_doc_url = serializers.CharField(required=False, allow_null=True)
edited_on = serializers.CharField(required=False, allow_null=True)
published = serializers.BooleanField(required=False, allow_null=True)
published_on = serializers.JSONField(required=False, allow_null=True)
studio_url = serializers.CharField(required=False, allow_null=True)
released_to_students = serializers.BooleanField(required=False, allow_null=True)
release_date = serializers.JSONField(required=False, allow_null=True)
nullout = serializers.JSONField(required=False, allow_null=True)
graderType = serializers.JSONField(required=False, allow_null=True)
visibility_state = serializers.CharField(required=False, allow_null=True)
has_explicit_staff_lock = serializers.BooleanField(
required=False, allow_null=True
)
start = serializers.CharField(required=False, allow_null=True)
graded = serializers.BooleanField(required=False, allow_null=True)
due_date = serializers.CharField(required=False, allow_null=True)
due = serializers.JSONField(required=False, allow_null=True)
relative_weeks_due = serializers.JSONField(required=False, allow_null=True)
format = serializers.JSONField(required=False, allow_null=True)
course_graders = serializers.ListField(required=False, allow_null=True)
actions = serializers.DictField(required=False, allow_null=True)
explanatory_message = serializers.Field(required=False, allow_null=True)
group_access = serializers.DictField(required=False, allow_null=True)
user_partitions = serializers.ListField(required=False, allow_null=True)
show_correctness = serializers.CharField(required=False, allow_null=True)
discussion_enabled = serializers.BooleanField(required=False, allow_null=True)
ancestor_has_staff_lock = serializers.BooleanField(required=False, allow_null=True)
user_partition_info = serializers.DictField(required=False, allow_null=True)
summary_configuration_enabled = serializers.JSONField(required=False, allow_null=True)
isPrereq = serializers.BooleanField(required=False, allow_null=True)
prereqUsageKey = serializers.CharField(required=False, allow_null=True)
prereqMinScore = serializers.IntegerField(required=False, allow_null=True)
prereqMinCompletion = serializers.IntegerField(required=False, allow_null=True)
publish = serializers.ChoiceField(
required=False,
allow_null=True,
choices=['make_public', 'republish', 'discard_changes']
)
duplicate_source_locator = serializers.CharField(required=False, allow_null=True)
move_source_locator = serializers.CharField(required=False, allow_null=True)
target_index = serializers.IntegerField(required=False, allow_null=True)
boilerplate = serializers.JSONField(required=False, allow_null=True)
staged_content = serializers.CharField(required=False, allow_null=True)

View File

@@ -74,7 +74,7 @@ urlpatterns = [
videos.VideosView.as_view(), name='studio_content_videos_uploads'
),
re_path(
fr'^videos/images/{settings.COURSE_ID_PATTERN}/{VIDEO_ID_PATTERN}?$',
fr'^videos/images/{settings.COURSE_ID_PATTERN}/{VIDEO_ID_PATTERN}$',
videos.VideoImagesView.as_view(), name='studio_content_videos_images'
),
re_path(

View File

@@ -14,6 +14,11 @@ from ....api import course_author_access_required
from cms.djangoapps.contentstore.asset_storage_handlers import handle_assets
import cms.djangoapps.contentstore.toggles as contentstore_toggles
from cms.djangoapps.contentstore.rest_api.v1.serializers import AssetSerializer
from .utils import validate_request_with_serializer
from rest_framework.parsers import (MultiPartParser, FormParser, JSONParser)
from openedx.core.lib.api.parsers import TypedFileUploadParser
log = logging.getLogger(__name__)
toggles = contentstore_toggles
@@ -25,6 +30,8 @@ class AssetsView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP
course_key: required argument, needed to authorize course authors and identify the asset.
asset_key_string: required argument, needed to identify the asset.
"""
serializer_class = AssetSerializer
parser_classes = (JSONParser, MultiPartParser, FormParser, TypedFileUploadParser)
def dispatch(self, request, *args, **kwargs):
# TODO: probably want to refactor this to a decorator.
@@ -44,11 +51,13 @@ class AssetsView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP
@csrf_exempt
@course_author_access_required
@validate_request_with_serializer
def create(self, request, course_key): # pylint: disable=arguments-differ
return handle_assets(request, course_key.html_id())
@course_author_access_required
@expect_json_in_class_view
@validate_request_with_serializer
def update(self, request, course_key, asset_key_string): # pylint: disable=arguments-differ
return handle_assets(request, course_key.html_id(), asset_key_string)

View File

@@ -6,9 +6,11 @@ not the underlying Xblock service.
It checks that the assets_handler method of the Xblock service is called with the expected parameters.
"""
from unittest.mock import patch
from django.core.files import File
from django.http import JsonResponse
from django.urls import reverse
from mock import MagicMock
from rest_framework import status
from rest_framework.test import APITestCase
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -17,6 +19,8 @@ from cms.djangoapps.contentstore.tests.test_utils import AuthorizeStaffTestCase
ASSET_KEY_STRING = "asset-v1:dede+aba+weagi+type@asset+block@_0e37192a-42c4-441e-a3e1-8e40ec304e2e.jpg"
mock_image = MagicMock(file=File)
mock_image.name = "test.jpg"
class AssetsViewTestCase(AuthorizeStaffTestCase):
@@ -146,7 +150,7 @@ class AssetsViewPostTest(AssetsViewTestCase, ModuleStoreTestCase, APITestCase):
def get_test_data(self):
return {
"file": ASSET_KEY_STRING,
"file": mock_image,
}
def assert_assets_handler_called(self, *, mock_handle_assets, response):
@@ -159,7 +163,7 @@ class AssetsViewPostTest(AssetsViewTestCase, ModuleStoreTestCase, APITestCase):
course_id = self.get_course_key_string()
assert passed_args.data.get("file") == ASSET_KEY_STRING
assert passed_args.data.get("file").name == mock_image.name
assert passed_args.method == "POST"
assert passed_args.path == self.get_url()

View File

@@ -50,7 +50,7 @@ class XBlockViewTestCase(AuthorizeStaffTestCase):
return_value=JsonResponse(
{
"locator": TEST_LOCATOR,
"courseKey": AuthorizeStaffTestCase.get_course_key_string(),
"course_key": AuthorizeStaffTestCase.get_course_key_string(),
}
),
)
@@ -128,7 +128,6 @@ class XBlockViewGetTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
assert response.status_code == status.HTTP_200_OK
data = response.json()
assert data["locator"] == TEST_LOCATOR
assert data["courseKey"] == self.get_course_key_string()
class XBlockViewPostTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
@@ -150,7 +149,6 @@ class XBlockViewPostTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
return {
"parent_locator": course_id,
"category": "html",
"courseKey": course_id,
}
def assert_xblock_handler_called(self, *, mock_handle_xblock, response):
@@ -161,9 +159,6 @@ class XBlockViewPostTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
mock_handle_xblock.assert_called_once()
passed_args = mock_handle_xblock.call_args[0][0]
course_id = self.get_course_key_string()
assert passed_args.data.get("courseKey") == course_id
assert passed_args.method == "POST"
assert passed_args.path == self.get_url()
@@ -187,7 +182,6 @@ class XBlockViewPostTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
assert response.status_code == status.HTTP_200_OK
data = response.json()
assert data["locator"] == TEST_LOCATOR
assert data["courseKey"] == self.get_course_key_string()
class XBlockViewPutTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
@@ -196,10 +190,8 @@ class XBlockViewPutTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
"""
def get_test_data(self):
course_id = self.get_course_key_string()
return {
"category": "html",
"courseKey": course_id,
"data": "<p>Updated block!</p>",
"has_changes": True,
"id": TEST_LOCATOR,
@@ -216,9 +208,6 @@ class XBlockViewPutTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
mock_handle_xblock.assert_called_once()
passed_args = mock_handle_xblock.call_args[0][0]
course_id = self.get_course_key_string()
assert passed_args.data.get("courseKey") == course_id
assert passed_args.data.get("data") == "<p>Updated block!</p>"
assert passed_args.data.get("id") == TEST_LOCATOR
assert passed_args.method == "PUT"
@@ -244,7 +233,6 @@ class XBlockViewPutTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
assert response.status_code == status.HTTP_200_OK
data = response.json()
assert data["locator"] == TEST_LOCATOR
assert data["courseKey"] == self.get_course_key_string()
class XBlockViewPatchTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
@@ -253,10 +241,8 @@ class XBlockViewPatchTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
"""
def get_test_data(self):
course_id = self.get_course_key_string()
return {
"category": "html",
"courseKey": course_id,
"data": "<p>Patched block!</p>",
"has_changes": True,
"id": TEST_LOCATOR,
@@ -273,9 +259,6 @@ class XBlockViewPatchTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
mock_handle_xblock.assert_called_once()
passed_args = mock_handle_xblock.call_args[0][0]
course_id = self.get_course_key_string()
assert passed_args.data.get("courseKey") == course_id
assert passed_args.data.get("data") == "<p>Patched block!</p>"
assert passed_args.data.get("id") == TEST_LOCATOR
assert passed_args.method == "PATCH"
@@ -301,7 +284,6 @@ class XBlockViewPatchTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
assert response.status_code == status.HTTP_200_OK
data = response.json()
assert data["locator"] == TEST_LOCATOR
assert data["courseKey"] == self.get_course_key_string()
class XBlockViewDeleteTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase):
@@ -343,4 +325,3 @@ class XBlockViewDeleteTest(XBlockViewTestCase, ModuleStoreTestCase, APITestCase)
assert response.status_code == status.HTTP_200_OK
data = response.json()
assert data["locator"] == TEST_LOCATOR
assert data["courseKey"] == self.get_course_key_string()

View File

@@ -21,6 +21,11 @@ from cms.djangoapps.contentstore.transcript_storage_handlers import (
handle_transcript_download,
)
import cms.djangoapps.contentstore.toggles as contentstore_toggles
from cms.djangoapps.contentstore.rest_api.v1.serializers import TranscriptSerializer
from rest_framework.parsers import (MultiPartParser, FormParser)
from openedx.core.lib.api.parsers import TypedFileUploadParser
from .utils import validate_request_with_serializer
log = logging.getLogger(__name__)
toggles = contentstore_toggles
@@ -34,6 +39,8 @@ class TranscriptView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, De
edx_video_id: optional query parameter, needed to identify the transcript.
language_code: optional query parameter, needed to identify the transcript.
"""
serializer_class = TranscriptSerializer
parser_classes = (MultiPartParser, FormParser, TypedFileUploadParser)
def dispatch(self, request, *args, **kwargs):
if not toggles.use_studio_content_api():
@@ -43,6 +50,7 @@ class TranscriptView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, De
@csrf_exempt
@course_author_access_required
@expect_json_in_class_view
@validate_request_with_serializer
def create(self, request, course_key_string): # pylint: disable=arguments-differ
return upload_transcript(request)

View File

@@ -0,0 +1,23 @@
"""
Utilities for the REST API views.
"""
from functools import wraps
from django.http import HttpResponseBadRequest
def validate_request_with_serializer(view_func):
"""
A decorator to validate request data using the view's serializer.
Usage:
@validate_request_with_serializer
def my_view_function(self, request, ...):
...
"""
@wraps(view_func)
def _wrapped_view(instance, request, *args, **kwargs):
serializer = instance.serializer_class(data=request.data)
if not serializer.is_valid():
return HttpResponseBadRequest(reason=serializer.errors)
return view_func(instance, request, *args, **kwargs)
return _wrapped_view

View File

@@ -7,10 +7,12 @@ from rest_framework.generics import (
RetrieveAPIView,
DestroyAPIView
)
from rest_framework.parsers import (MultiPartParser, FormParser)
from django.views.decorators.csrf import csrf_exempt
from django.http import Http404
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
from openedx.core.lib.api.parsers import TypedFileUploadParser
from common.djangoapps.util.json_request import expect_json_in_class_view
from ....api import course_author_access_required
@@ -19,10 +21,12 @@ from cms.djangoapps.contentstore.video_storage_handlers import (
handle_videos,
get_video_encodings_download,
handle_video_images,
enabled_video_features,
handle_generate_video_upload_link
enabled_video_features
)
from cms.djangoapps.contentstore.rest_api.v1.serializers import VideoUploadSerializer, VideoImageSerializer
import cms.djangoapps.contentstore.toggles as contentstore_toggles
from .utils import validate_request_with_serializer
log = logging.getLogger(__name__)
toggles = contentstore_toggles
@@ -35,6 +39,7 @@ class VideosView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, Destro
course_key: required argument, needed to authorize course authors and identify the video.
video_id: required argument, needed to identify the video.
"""
serializer_class = VideoUploadSerializer
def dispatch(self, request, *args, **kwargs):
# TODO: probably want to refactor this to a decorator.
@@ -50,7 +55,9 @@ class VideosView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, Destro
@csrf_exempt
@course_author_access_required
@expect_json_in_class_view
@validate_request_with_serializer
def create(self, request, course_key): # pylint: disable=arguments-differ
"""Deprecated. Use the upload_link endpoint instead."""
return handle_videos(request, course_key.html_id())
@course_author_access_required
@@ -70,6 +77,8 @@ class VideoImagesView(DeveloperErrorViewMixin, CreateAPIView):
course_key: required argument, needed to authorize course authors and identify the video.
video_id: required argument, needed to identify the video.
"""
serializer_class = VideoImageSerializer
parser_classes = (MultiPartParser, FormParser, TypedFileUploadParser)
def dispatch(self, request, *args, **kwargs):
# TODO: probably want to refactor this to a decorator.
@@ -85,6 +94,7 @@ class VideoImagesView(DeveloperErrorViewMixin, CreateAPIView):
@csrf_exempt
@course_author_access_required
@expect_json_in_class_view
@validate_request_with_serializer
def create(self, request, course_key, edx_video_id=None): # pylint: disable=arguments-differ
return handle_video_images(request, course_key.html_id(), edx_video_id)
@@ -140,6 +150,7 @@ class UploadLinkView(DeveloperErrorViewMixin, CreateAPIView):
"""
public rest API endpoint providing a list of enabled video features.
"""
serializer_class = VideoUploadSerializer
def dispatch(self, request, *args, **kwargs):
# TODO: probably want to refactor this to a decorator.
@@ -155,5 +166,6 @@ class UploadLinkView(DeveloperErrorViewMixin, CreateAPIView):
@csrf_exempt
@course_author_access_required
@expect_json_in_class_view
@validate_request_with_serializer
def create(self, request, course_key): # pylint: disable=arguments-differ
return handle_generate_video_upload_link(request, course_key.html_id())
return handle_videos(request, course_key.html_id())

View File

@@ -13,7 +13,8 @@ from cms.djangoapps.contentstore.api import course_author_access_required
from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers
import cms.djangoapps.contentstore.toggles as contentstore_toggles
# from cms.djangoapps.contentstore.rest_api.v1.serializers import XblockSerializer
from cms.djangoapps.contentstore.rest_api.v1.serializers import XblockSerializer
from .utils import validate_request_with_serializer
log = logging.getLogger(__name__)
@@ -29,8 +30,7 @@ class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP
usage_key_string (optional):
xblock identifier, for example in the form of "block-v1:<course id>+type@<type>+block@<block id>"
"""
# TODO: uncomment next line after XblockSerializer is implemented
# serializer_class = XblockSerializer
serializer_class = XblockSerializer
def dispatch(self, request, *args, **kwargs):
# TODO: probably want to refactor this to a decorator.
@@ -51,11 +51,13 @@ class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP
@course_author_access_required
@expect_json_in_class_view
@validate_request_with_serializer
def update(self, request, course_key, usage_key_string=None):
return handle_xblock(request, usage_key_string)
@course_author_access_required
@expect_json_in_class_view
@validate_request_with_serializer
def partial_update(self, request, course_key, usage_key_string=None):
return handle_xblock(request, usage_key_string)
@@ -67,5 +69,6 @@ class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView, CreateAP
@csrf_exempt
@course_author_access_required
@expect_json_in_class_view
@validate_request_with_serializer
def create(self, request, course_key, usage_key_string=None):
return handle_xblock(request, usage_key_string)

View File

@@ -729,7 +729,9 @@ def videos_post(course, request):
"""
if use_mock_video_uploads():
return {'files': [{'file_name': 'video.mp4', 'upload_url': 'http://example.com/put_video'}]}, 200
return {'files': [{
'file_name': 'video.mp4', 'upload_url': 'http://example.com/put_video', 'edx_video_id': '1234'
}]}, 200
error = None
data = request.json