From a3df96bf1c4d0d87f63dba3cd6b7e1eafd32a200 Mon Sep 17 00:00:00 2001 From: Chandrakant Gopalan <chandrakant@opencraft.com> Date: Thu, 31 Aug 2017 16:03:17 -0400 Subject: [PATCH] OAuth2 API to call XBlock handlers --- lms/djangoapps/courseware/module_render.py | 83 ++++--- .../courseware/tests/test_entrance_exam.py | 8 +- .../courseware/tests/test_module_render.py | 203 ++++++++++++------ lms/static/js/ajax-error.js | 2 +- lms/urls.py | 5 +- 5 files changed, 199 insertions(+), 102 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index a48307ddd11..480f8b43249 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -17,11 +17,17 @@ from django.template.context_processors import csrf from django.core.exceptions import PermissionDenied from django.core.urlresolvers 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 @@ -68,7 +74,6 @@ 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 @@ -927,39 +932,31 @@ 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) -def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None): +class XblockCallbackView(APIView): """ - 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. + 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. """ - # 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() + authentication_classes = (JwtAuthentication, SessionAuthentication, OAuth2Authentication,) + permission_classes = (IsAuthenticatedOrReadOnly,) - request.user.known = request.user.is_authenticated + def get(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 post(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 put(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 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) def get_module_by_usage_id(request, course_id, usage_id, disable_staff_debug_info=False, course=None): @@ -1037,10 +1034,11 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course """ # Check submitted files - files = request.FILES or {} - error_msg = _check_files_limits(files) - if error_msg: - return JsonResponse({'success': error_msg}, status=413) + 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) # Make a CourseKey from the course_id, raising a 404 upon parse error. try: @@ -1095,6 +1093,27 @@ 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 b56e1e074fa..85ddcc68ec2 100644 --- a/lms/djangoapps/courseware/tests/test_entrance_exam.py +++ b/lms/djangoapps/courseware/tests/test_entrance_exam.py @@ -4,6 +4,7 @@ Tests use cases related to LMS Entrance Exam behavior, such as gated content acc from django.core.urlresolvers 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 ( @@ -13,7 +14,7 @@ from courseware.entrance_exams import ( user_has_passed_entrance_exam ) from courseware.model_data import FieldDataCache -from courseware.module_render import get_module, handle_xblock_callback, toc_for_course +from courseware.module_render import get_module, XblockCallbackView, toc_for_course from courseware.tests.factories import InstructorFactory, StaffFactory, UserFactory from courseware.tests.helpers import LoginEnrollmentTestCase from milestones.tests.utils import MilestonesTestCaseMixin @@ -534,14 +535,15 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest """ Tests entrance exam xblock has `entrance_exam_passed` key in json response. """ - request_factory = RequestFactory() + request_factory = APIRequestFactory() 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 - response = handle_xblock_callback( + view = XblockCallbackView.as_view() + response = view( 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 0a454cf77bb..65a8a3a732d 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -28,6 +28,7 @@ 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 @@ -44,7 +45,12 @@ 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 +from courseware.module_render import ( + get_module_for_descriptor, + hash_resource, + XblockCallbackView, + handle_xblock_callback_noauth, +) from courseware.tests.factories import GlobalStaffFactory, StudentModuleFactory, UserFactory from courseware.tests.test_submitting_problems import TestSubmittingProblems from courseware.tests.tests import LoginEnrollmentTestCase @@ -69,6 +75,8 @@ 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 @@ -299,7 +307,8 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.dispatch ) - def test_anonymous_handle_xblock_callback(self): + def test_anonymous_post_xblock_callback(self): + """Test that anonymous POST are not allowed.""" dispatch_url = reverse( 'xblock_handler', args=[ @@ -310,7 +319,55 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): ] ) response = self.client.post(dispatch_url, {'position': 2}) - self.assertEquals(403, response.status_code) + # "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) def test_missing_position_handler(self): """ @@ -449,7 +506,7 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @ddt.ddt class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCase): """ - Test the handle_xblock_callback function + Test the handle_xblock_callback view """ @classmethod def setUpClass(cls): @@ -463,6 +520,7 @@ 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() @@ -500,7 +558,16 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas content_type='application/json', ) request.user = self.mock_user - response = render.handle_xblock_callback( + + # 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( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), @@ -511,25 +578,27 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas return response def test_invalid_location(self): - request = self.request_factory.post('dummy_url', data={'position': 1}) + request = self.api_request_factory.post('dummy_url', data={'position': 1}) request.user = self.mock_user - with self.assertRaises(Http404): - render.handle_xblock_callback( - request, - text_type(self.course_key), - 'invalid Location', - 'dummy_handler' - 'dummy_dispatch' - ) + view = XblockCallbackView.as_view() + resp = view( + request, + text_type(self.course_key), + 'invalid Location', + 'dummy_handler' + 'dummy_dispatch' + ) + self.assertEquals(resp.status_code, 404) def test_too_many_files(self): - request = self.request_factory.post( + request = self.api_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( - render.handle_xblock_callback( + view( request, text_type(self.course_key), quote_slashes(text_type(self.location)), @@ -543,13 +612,14 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas def test_too_large_file(self): inputfile = self._mock_file(size=1 + settings.STUDENT_FILEUPLOAD_MAX_SIZE) - request = self.request_factory.post( + request = self.api_request_factory.post( 'dummy_url', data={'file_id': inputfile} ) request.user = self.mock_user + view = XblockCallbackView.as_view() self.assertEquals( - render.handle_xblock_callback( + view( request, text_type(self.course_key), quote_slashes(text_type(self.location)), @@ -562,9 +632,10 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas ) def test_xmodule_dispatch(self): - request = self.request_factory.post('dummy_url', data={'position': 1}) + request = self.api_request_factory.post('dummy_url', data={'position': 1}) request.user = self.mock_user - response = render.handle_xblock_callback( + view = XblockCallbackView.as_view() + response = view( request, text_type(self.course_key), quote_slashes(text_type(self.location)), @@ -574,66 +645,70 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas self.assertIsInstance(response, HttpResponse) def test_bad_course_id(self): - request = self.request_factory.post('dummy_url') + request = self.api_request_factory.post('dummy_url') request.user = self.mock_user - with self.assertRaises(Http404): - render.handle_xblock_callback( - request, - 'bad_course_id', - quote_slashes(text_type(self.location)), - 'xmodule_handler', - 'goto_position', - ) + 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) def test_bad_location(self): - request = self.request_factory.post('dummy_url') + request = self.api_request_factory.post('dummy_url') request.user = self.mock_user - 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', - ) + 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) def test_bad_xmodule_dispatch(self): - request = self.request_factory.post('dummy_url') + request = self.api_request_factory.post('dummy_url') request.user = self.mock_user - with self.assertRaises(Http404): - render.handle_xblock_callback( - request, - text_type(self.course_key), - quote_slashes(text_type(self.location)), - 'xmodule_handler', - 'bad_dispatch', - ) + 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) def test_missing_handler(self): - request = self.request_factory.post('dummy_url') + request = self.api_request_factory.post('dummy_url') request.user = self.mock_user - with self.assertRaises(Http404): - render.handle_xblock_callback( - request, - text_type(self.course_key), - quote_slashes(text_type(self.location)), - 'bad_handler', - 'bad_dispatch', - ) + 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) @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.request_factory.post( + request = self.api_request_factory.post( 'dummy_url', data=json.dumps({"grade": 0.75}), content_type='application/json' ) request.user = self.mock_user - - response = render.handle_xblock_callback( + view = XblockCallbackView.as_view() + response = view( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), @@ -658,14 +733,15 @@ 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.request_factory.post( + request = self.api_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: - render.handle_xblock_callback( + view = XblockCallbackView.as_view() + view( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), @@ -732,7 +808,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 = render.handle_xblock_callback( + response = handle_xblock_callback_noauth( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), @@ -1911,7 +1987,8 @@ class TestModuleTrackingContext(SharedModuleStoreTestCase): descriptor = ItemFactory.create(**descriptor_kwargs) - render.handle_xblock_callback( + view = XblockCallbackView.as_view() + view( 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 edab7732c42..5563093cb9a 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) { + if (jXHR.status === 403 || jXHR.status === 401) { 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 bd5bad78e7f..d90d667e503 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -1,7 +1,6 @@ """ URLs for LMS """ - from django.conf import settings from django.conf.urls import include, url from django.conf.urls.static import static @@ -13,7 +12,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, handle_xblock_callback_noauth, xblock_view, xqueue_callback +from courseware.module_render import handle_xblock_callback_noauth, XblockCallbackView, 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 @@ -250,7 +249,7 @@ urlpatterns += [ course_key=settings.COURSE_ID_PATTERN, usage_key=settings.USAGE_ID_PATTERN, ), - handle_xblock_callback, + XblockCallbackView.as_view(), name='xblock_handler', ), url( -- GitLab