diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 7366243574..dd38fa8af9 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -17,17 +17,11 @@ from django.template.context_processors import csrf from django.core.exceptions import PermissionDenied from django.urls import reverse from django.http import Http404, HttpResponse, HttpResponseForbidden -from django.utils.text import slugify from django.views.decorators.csrf import csrf_exempt from edx_proctoring.services import ProctoringService -from edx_rest_framework_extensions.authentication import JwtAuthentication from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from requests.auth import HTTPBasicAuth -from rest_framework.authentication import SessionAuthentication -from rest_framework.permissions import IsAuthenticatedOrReadOnly -from rest_framework.views import APIView -from rest_framework_oauth.authentication import OAuth2Authentication from six import text_type from xblock.core import XBlock from xblock.django.request import django_to_webob_request, webob_to_django_response @@ -74,6 +68,7 @@ from student.roles import CourseBetaTesterRole from track import contexts from util import milestones_helpers from util.json_request import JsonResponse +from django.utils.text import slugify from util.sandboxing import can_execute_unsafe_code, get_python_lib_zip from xblock_django.user_service import DjangoXBlockUserService from xmodule.contentstore.django import contentstore @@ -932,31 +927,39 @@ def handle_xblock_callback_noauth(request, course_id, usage_id, handler, suffix= return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course=course) -class XblockCallbackView(APIView): +def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None): """ - Class-based view for extensions. This is where AJAX calls go. - Return 403 error if the user is not logged in. Raises Http404 if - the location and course_id do not identify a valid module, the module is - not accessible by the user, or the module raises NotFoundError. If the - module raises any other error, it will escape this function. + Generic view for extensions. This is where AJAX calls go. + + Arguments: + request (Request): Django request. + course_id (str): Course containing the block + usage_id (str) + handler (str) + suffix (str) + + Raises: + Http404: If the course is not found in the modulestore. """ - authentication_classes = (JwtAuthentication, SessionAuthentication, OAuth2Authentication,) - permission_classes = (IsAuthenticatedOrReadOnly,) + # NOTE (CCB): Allow anonymous GET calls (e.g. for transcripts). Modifying this view is simpler than updating + # the XBlocks to use `handle_xblock_callback_noauth`...which is practically identical to this view. + if request.method != 'GET' and not request.user.is_authenticated: + return HttpResponseForbidden() - def get(self, request, course_id, usage_id, handler, suffix=None): - return _get_course_and_invoke_handler(request, course_id, usage_id, handler, suffix) + request.user.known = request.user.is_authenticated - def post(self, request, course_id, usage_id, handler, suffix=None): - return _get_course_and_invoke_handler(request, course_id, usage_id, handler, suffix) + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError: + raise Http404('{} is not a valid course key'.format(course_id)) - def put(self, request, course_id, usage_id, handler, suffix=None): - return _get_course_and_invoke_handler(request, course_id, usage_id, handler, suffix) + with modulestore().bulk_operations(course_key): + try: + course = modulestore().get_course(course_key) + except ItemNotFoundError: + raise Http404('{} does not exist in the modulestore'.format(course_id)) - def patch(self, request, course_id, usage_id, handler, suffix=None): - return _get_course_and_invoke_handler(request, course_id, usage_id, handler, suffix) - - def delete(self, request, course_id, usage_id, handler, suffix=None): - return _get_course_and_invoke_handler(request, course_id, usage_id, handler, suffix) + return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course=course) def get_module_by_usage_id(request, course_id, usage_id, disable_staff_debug_info=False, course=None): @@ -1034,11 +1037,10 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course """ # Check submitted files - if request.method == 'POST' and request.META.get('CONTENT_TYPE', '').startswith('multipart/form-data'): - files = request.FILES - error_msg = _check_files_limits(files) - if error_msg: - return JsonResponse({'success': error_msg}, status=413) + files = request.FILES or {} + error_msg = _check_files_limits(files) + if error_msg: + return JsonResponse({'success': error_msg}, status=413) # Make a CourseKey from the course_id, raising a 404 upon parse error. try: @@ -1093,27 +1095,6 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course return webob_to_django_response(resp) -def _get_course_and_invoke_handler(request, course_id, usage_id, handler, suffix): - """ - Gets the course object to pass as an additional argument to invoke the xblock handler. - """ - - request.user.known = request.user.is_authenticated - - try: - course_key = CourseKey.from_string(course_id) - except InvalidKeyError: - raise Http404('{} is not a valid course key'.format(course_id)) - - with modulestore().bulk_operations(course_key): - try: - course = modulestore().get_course(course_key) - except ItemNotFoundError: - raise Http404('{} does not exist in the modulestore'.format(course_id)) - - return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course=course) - - def hash_resource(resource): """ Hash a :class:`web_fragments.fragment.FragmentResource diff --git a/lms/djangoapps/courseware/tests/test_entrance_exam.py b/lms/djangoapps/courseware/tests/test_entrance_exam.py index 9a686cd55d..eeeab157a4 100644 --- a/lms/djangoapps/courseware/tests/test_entrance_exam.py +++ b/lms/djangoapps/courseware/tests/test_entrance_exam.py @@ -4,7 +4,6 @@ Tests use cases related to LMS Entrance Exam behavior, such as gated content acc from django.urls import reverse from django.test.client import RequestFactory from mock import Mock, patch -from rest_framework.test import APIRequestFactory from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from courseware.entrance_exams import ( @@ -14,7 +13,7 @@ from courseware.entrance_exams import ( user_has_passed_entrance_exam ) from courseware.model_data import FieldDataCache -from courseware.module_render import get_module, XblockCallbackView, toc_for_course +from courseware.module_render import get_module, handle_xblock_callback, toc_for_course from courseware.tests.factories import InstructorFactory, StaffFactory, UserFactory from courseware.tests.helpers import LoginEnrollmentTestCase from milestones.tests.utils import MilestonesTestCaseMixin @@ -535,15 +534,14 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest """ Tests entrance exam xblock has `entrance_exam_passed` key in json response. """ - request_factory = APIRequestFactory() + request_factory = RequestFactory() data = {'input_{}_2_1'.format(unicode(self.problem_1.location.html_id())): 'choice_2'} request = request_factory.post( 'problem_check', data=data ) request.user = self.user - view = XblockCallbackView.as_view() - response = view( + response = handle_xblock_callback( request, unicode(self.course.id), unicode(self.problem_1.location), diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index d866a41343..ae2eab1c96 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -28,7 +28,6 @@ from mock import MagicMock, Mock, patch from nose.plugins.attrib import attr from opaque_keys.edx.keys import CourseKey, UsageKey from pyquery import PyQuery -from rest_framework.test import APIRequestFactory from six import text_type from web_fragments.fragment import Fragment from xblock.completable import CompletableXBlockMixin @@ -45,12 +44,7 @@ from courseware.field_overrides import OverrideFieldData from courseware.masquerade import CourseMasquerade from courseware.model_data import FieldDataCache from courseware.models import StudentModule -from courseware.module_render import ( - get_module_for_descriptor, - hash_resource, - XblockCallbackView, - handle_xblock_callback_noauth, -) +from courseware.module_render import get_module_for_descriptor, hash_resource from courseware.tests.factories import GlobalStaffFactory, StudentModuleFactory, UserFactory from courseware.tests.test_submitting_problems import TestSubmittingProblems from courseware.tests.tests import LoginEnrollmentTestCase @@ -75,8 +69,6 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, ToyC from xmodule.modulestore.tests.test_asides import AsideTestType from xmodule.x_module import STUDENT_VIEW, CombinedSystem, XModule, XModuleDescriptor -from edx_oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory - TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT @@ -306,8 +298,7 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.dispatch ) - def test_anonymous_post_xblock_callback(self): - """Test that anonymous POST are not allowed.""" + def test_anonymous_handle_xblock_callback(self): dispatch_url = reverse( 'xblock_handler', args=[ @@ -318,55 +309,7 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): ] ) response = self.client.post(dispatch_url, {'position': 2}) - # "401 Unauthorized", despite its name, means authentication error, i.e. no token/key/password supplied - self.assertEquals(401, response.status_code) - - def test_anonymous_get_xblock_callback(self): - """Test that anonymous GET are allowed.""" - dispatch_url = reverse( - 'xblock_handler', - args=[ - text_type(self.course_key), - quote_slashes(text_type(self.course_key.make_usage_key('videosequence', 'Toy_Videos'))), - 'xmodule_handler', - 'goto_position', - ] - ) - # Calling 'goto_position' with GET won't have side effects but it triggers the permission tests - response = self.client.get(dispatch_url) - self.assertEquals(200, response.status_code) - - def test_session_authentication(self): - """ Test that the xblock endpoint supports session authentication. """ - self.client.login(username=self.mock_user.username, password="test") - dispatch_url = reverse( - 'xblock_handler', - args=[ - text_type(self.course_key), - quote_slashes(text_type(self.course_key.make_usage_key('videosequence', 'Toy_Videos'))), - 'xmodule_handler', - 'goto_position' - ] - ) - response = self.client.post(dispatch_url) - self.assertEqual(200, response.status_code) - - def test_oauth_authentication(self): - """ Test that the xblock endpoint supports oauth authentication. """ - dispatch_url = reverse( - 'xblock_handler', - args=[ - text_type(self.course_key), - quote_slashes(text_type(self.course_key.make_usage_key('videosequence', 'Toy_Videos'))), - 'xmodule_handler', - 'goto_position' - ] - ) - access_token = AccessTokenFactory(user=self.mock_user, client=ClientFactory()).token - headers = {} - headers['HTTP_AUTHORIZATION'] = 'Bearer ' + access_token - response = self.client.post(dispatch_url, {}, **headers) - self.assertEqual(200, response.status_code) + self.assertEquals(403, response.status_code) def test_missing_position_handler(self): """ @@ -505,7 +448,7 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @ddt.ddt class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCase): """ - Test the handle_xblock_callback view + Test the handle_xblock_callback function """ @classmethod def setUpClass(cls): @@ -519,7 +462,6 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas self.location = self.course_key.make_usage_key('chapter', 'Overview') self.mock_user = UserFactory.create() self.request_factory = RequestFactory() - self.api_request_factory = APIRequestFactory() # Construct a mock module for the modulestore to return self.mock_module = MagicMock() @@ -557,16 +499,7 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas content_type='application/json', ) request.user = self.mock_user - - # Django tests have CSRF checks disabled by default, but DRF always wants to see a CSRF token if we use - # SessionAuthentication (check http://www.django-rest-framework.org/api-guide/testing/#csrf) - # Instead of getting the CSRF token as per that code (which doesn't work in this case due to RequestFactory), - # we disable CSRF for this request too. - # See https://dammit.nl/20141111-disabling-csrf-checking-in-django-rest-framework-to-enable-replay.html - setattr(request, '_dont_enforce_csrf_checks', True) - - view = XblockCallbackView.as_view() - response = view( + response = render.handle_xblock_callback( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), @@ -577,27 +510,25 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas return response def test_invalid_location(self): - request = self.api_request_factory.post('dummy_url', data={'position': 1}) + request = self.request_factory.post('dummy_url', data={'position': 1}) request.user = self.mock_user - view = XblockCallbackView.as_view() - resp = view( - request, - text_type(self.course_key), - 'invalid Location', - 'dummy_handler' - 'dummy_dispatch' - ) - self.assertEquals(resp.status_code, 404) + with self.assertRaises(Http404): + render.handle_xblock_callback( + request, + text_type(self.course_key), + 'invalid Location', + 'dummy_handler' + 'dummy_dispatch' + ) def test_too_many_files(self): - request = self.api_request_factory.post( + request = self.request_factory.post( 'dummy_url', data={'file_id': (self._mock_file(), ) * (settings.MAX_FILEUPLOADS_PER_INPUT + 1)} ) request.user = self.mock_user - view = XblockCallbackView.as_view() self.assertEquals( - view( + render.handle_xblock_callback( request, text_type(self.course_key), quote_slashes(text_type(self.location)), @@ -611,14 +542,13 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas def test_too_large_file(self): inputfile = self._mock_file(size=1 + settings.STUDENT_FILEUPLOAD_MAX_SIZE) - request = self.api_request_factory.post( + request = self.request_factory.post( 'dummy_url', data={'file_id': inputfile} ) request.user = self.mock_user - view = XblockCallbackView.as_view() self.assertEquals( - view( + render.handle_xblock_callback( request, text_type(self.course_key), quote_slashes(text_type(self.location)), @@ -631,10 +561,9 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas ) def test_xmodule_dispatch(self): - request = self.api_request_factory.post('dummy_url', data={'position': 1}) + request = self.request_factory.post('dummy_url', data={'position': 1}) request.user = self.mock_user - view = XblockCallbackView.as_view() - response = view( + response = render.handle_xblock_callback( request, text_type(self.course_key), quote_slashes(text_type(self.location)), @@ -644,70 +573,66 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas self.assertIsInstance(response, HttpResponse) def test_bad_course_id(self): - request = self.api_request_factory.post('dummy_url') + request = self.request_factory.post('dummy_url') request.user = self.mock_user - view = XblockCallbackView.as_view() - resp = view( - request, - 'bad_course_id', - quote_slashes(text_type(self.location)), - 'xmodule_handler', - 'goto_position', - ) - self.assertEquals(resp.status_code, 404) + with self.assertRaises(Http404): + render.handle_xblock_callback( + request, + 'bad_course_id', + quote_slashes(text_type(self.location)), + 'xmodule_handler', + 'goto_position', + ) def test_bad_location(self): - request = self.api_request_factory.post('dummy_url') + request = self.request_factory.post('dummy_url') request.user = self.mock_user - view = XblockCallbackView.as_view() - resp = view( - request, - text_type(self.course_key), - quote_slashes(text_type(self.course_key.make_usage_key('chapter', 'bad_location'))), - 'xmodule_handler', - 'goto_position', - ) - self.assertEquals(resp.status_code, 404) + with self.assertRaises(Http404): + render.handle_xblock_callback( + request, + text_type(self.course_key), + quote_slashes(text_type(self.course_key.make_usage_key('chapter', 'bad_location'))), + 'xmodule_handler', + 'goto_position', + ) def test_bad_xmodule_dispatch(self): - request = self.api_request_factory.post('dummy_url') + request = self.request_factory.post('dummy_url') request.user = self.mock_user - view = XblockCallbackView.as_view() - resp = view( - request, - text_type(self.course_key), - quote_slashes(text_type(self.location)), - 'xmodule_handler', - 'bad_dispatch', - ) - self.assertEquals(resp.status_code, 404) + with self.assertRaises(Http404): + render.handle_xblock_callback( + request, + text_type(self.course_key), + quote_slashes(text_type(self.location)), + 'xmodule_handler', + 'bad_dispatch', + ) def test_missing_handler(self): - request = self.api_request_factory.post('dummy_url') + request = self.request_factory.post('dummy_url') request.user = self.mock_user - view = XblockCallbackView.as_view() - resp = view( - request, - text_type(self.course_key), - quote_slashes(text_type(self.location)), - 'bad_handler', - 'bad_dispatch', - ) - self.assertEquals(resp.status_code, 404) + with self.assertRaises(Http404): + render.handle_xblock_callback( + request, + text_type(self.course_key), + quote_slashes(text_type(self.location)), + 'bad_handler', + 'bad_dispatch', + ) @XBlock.register_temp_plugin(GradedStatelessXBlock, identifier='stateless_scorer') def test_score_without_student_state(self): course = CourseFactory.create() block = ItemFactory.create(category='stateless_scorer', parent=course) - request = self.api_request_factory.post( + request = self.request_factory.post( 'dummy_url', data=json.dumps({"grade": 0.75}), content_type='application/json' ) request.user = self.mock_user - view = XblockCallbackView.as_view() - response = view( + + response = render.handle_xblock_callback( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), @@ -732,15 +657,14 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas with completion_waffle.waffle().override(completion_waffle.ENABLE_COMPLETION_TRACKING, False): course = CourseFactory.create() block = ItemFactory.create(category='comp', parent=course) - request = self.api_request_factory.post( + request = self.request_factory.post( '/', data=json.dumps(data), content_type='application/json', ) request.user = self.mock_user with patch('completion.models.BlockCompletionManager.submit_completion') as mock_complete: - view = XblockCallbackView.as_view() - view( + render.handle_xblock_callback( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), @@ -807,7 +731,7 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas request.user.real_user.masquerade_settings = CourseMasquerade(course.id, user_name="jem") with patch('courseware.module_render.is_masquerading_as_specific_student') as mock_masq: mock_masq.return_value = True - response = handle_xblock_callback_noauth( + response = render.handle_xblock_callback( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), @@ -1986,8 +1910,7 @@ class TestModuleTrackingContext(SharedModuleStoreTestCase): descriptor = ItemFactory.create(**descriptor_kwargs) - view = XblockCallbackView.as_view() - view( + render.handle_xblock_callback( self.request, text_type(self.course.id), quote_slashes(text_type(descriptor.location)), diff --git a/lms/static/js/ajax-error.js b/lms/static/js/ajax-error.js index 5563093cb9..edab7732c4 100644 --- a/lms/static/js/ajax-error.js +++ b/lms/static/js/ajax-error.js @@ -1,5 +1,5 @@ $(document).ajaxError(function(event, jXHR) { - if (jXHR.status === 403 || jXHR.status === 401) { + if (jXHR.status === 403) { var message = gettext( 'You have been logged out of your edX account. ' + 'Click Okay to log in again now. ' + diff --git a/lms/urls.py b/lms/urls.py index d90d667e50..bd5bad78e7 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -1,6 +1,7 @@ """ URLs for LMS """ + from django.conf import settings from django.conf.urls import include, url from django.conf.urls.static import static @@ -12,7 +13,7 @@ from rest_framework_swagger.views import get_swagger_view from branding import views as branding_views from config_models.views import ConfigurationModelCurrentAPIView from courseware.masquerade import handle_ajax as courseware_masquerade_handle_ajax -from courseware.module_render import handle_xblock_callback_noauth, XblockCallbackView, xblock_view, xqueue_callback +from courseware.module_render import handle_xblock_callback, handle_xblock_callback_noauth, xblock_view, xqueue_callback from courseware.views import views as courseware_views from courseware.views.index import CoursewareIndex from courseware.views.views import CourseTabView, EnrollStaffView, StaticCourseTabView @@ -249,7 +250,7 @@ urlpatterns += [ course_key=settings.COURSE_ID_PATTERN, usage_key=settings.USAGE_ID_PATTERN, ), - XblockCallbackView.as_view(), + handle_xblock_callback, name='xblock_handler', ), url(