Merge pull request #19253 from open-craft/agrendalath/xblock-handle-oauth
Add OAuth2 and JWT support to XBlock handlers
This commit is contained in:
@@ -14,6 +14,7 @@ from completion import waffle as completion_waffle
|
||||
from django.conf import settings
|
||||
from django.contrib.auth.models import User
|
||||
from django.core.cache import cache
|
||||
from django.middleware.csrf import CsrfViewMiddleware
|
||||
from django.template.context_processors import csrf
|
||||
from django.urls import reverse
|
||||
from django.http import Http404, HttpResponse, HttpResponseForbidden
|
||||
@@ -23,10 +24,12 @@ from django.views.decorators.csrf import csrf_exempt
|
||||
from edx_django_utils.cache import RequestCache
|
||||
from edx_django_utils.monitoring import set_custom_metrics_for_course_key, set_monitoring_transaction_name
|
||||
from edx_proctoring.services import ProctoringService
|
||||
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from requests.auth import HTTPBasicAuth
|
||||
from rest_framework.decorators import api_view
|
||||
from rest_framework.exceptions import APIException
|
||||
from six import text_type
|
||||
from xblock.core import XBlock
|
||||
from xblock.django.request import django_to_webob_request, webob_to_django_response
|
||||
@@ -58,6 +61,7 @@ from openedx.core.djangoapps.bookmarks.services import BookmarksService
|
||||
from openedx.core.djangoapps.crawlers.models import CrawlersConfig
|
||||
from openedx.core.djangoapps.credit.services import CreditService
|
||||
from openedx.core.djangoapps.util.user_utils import SystemUser
|
||||
from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser
|
||||
from openedx.core.djangolib.markup import HTML
|
||||
from openedx.core.lib.api.view_utils import view_auth_classes
|
||||
from openedx.core.lib.gating.services import GatingService
|
||||
@@ -994,6 +998,7 @@ 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)
|
||||
|
||||
|
||||
@csrf_exempt
|
||||
@xframe_options_exempt
|
||||
def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None):
|
||||
"""
|
||||
@@ -1007,11 +1012,37 @@ def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None):
|
||||
suffix (str)
|
||||
|
||||
Raises:
|
||||
HttpResponseForbidden: If the request method is not `GET` and user is not authenticated.
|
||||
Http404: If the course is not found in the modulestore.
|
||||
"""
|
||||
# In this case, we are using Session based authentication, so we need to check CSRF token.
|
||||
if request.user.is_authenticated:
|
||||
error = CsrfViewMiddleware().process_view(request, None, (), {})
|
||||
if error:
|
||||
return error
|
||||
|
||||
# We are reusing DRF logic to provide support for JWT and Oauth2. We abandoned the idea of using DRF view here
|
||||
# to avoid introducing backwards-incompatible changes.
|
||||
# You can see https://github.com/edx/XBlock/pull/383 for more details.
|
||||
else:
|
||||
authentication_classes = (JwtAuthentication, OAuth2AuthenticationAllowInactiveUser)
|
||||
authenticators = [auth() for auth in authentication_classes]
|
||||
|
||||
for authenticator in authenticators:
|
||||
try:
|
||||
user_auth_tuple = authenticator.authenticate(request)
|
||||
except APIException:
|
||||
log.exception(
|
||||
"XBlock handler %r failed to authenticate with %s", handler, authenticator.__class__.__name__
|
||||
)
|
||||
else:
|
||||
if user_auth_tuple is not None:
|
||||
request.user, _ = user_auth_tuple
|
||||
break
|
||||
|
||||
# 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:
|
||||
# the XBlocks to use `handle_xblock_callback_noauth`, which is practically identical to this view.
|
||||
if request.method != 'GET' and not (request.user and request.user.is_authenticated):
|
||||
return HttpResponseForbidden()
|
||||
|
||||
request.user.known = request.user.is_authenticated
|
||||
|
||||
@@ -4,6 +4,7 @@ import json
|
||||
from functools import partial
|
||||
|
||||
import factory
|
||||
from django.test.client import RequestFactory
|
||||
from factory.django import DjangoModelFactory
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from opaque_keys.edx.locator import CourseLocator
|
||||
@@ -162,3 +163,13 @@ class StudentInfoFactory(DjangoModelFactory):
|
||||
field_name = 'existing_field'
|
||||
value = json.dumps('old_value')
|
||||
student = factory.SubFactory(UserFactory)
|
||||
|
||||
|
||||
class RequestFactoryNoCsrf(RequestFactory):
|
||||
"""
|
||||
RequestFactory, which disables csrf checks.
|
||||
"""
|
||||
def request(self, **kwargs):
|
||||
request = super(RequestFactoryNoCsrf, self).request(**kwargs)
|
||||
setattr(request, '_dont_enforce_csrf_checks', True) # pylint: disable=literal-used-as-attribute
|
||||
return request
|
||||
|
||||
@@ -2,7 +2,6 @@
|
||||
Tests use cases related to LMS Entrance Exam behavior, such as gated content access (TOC)
|
||||
"""
|
||||
from django.urls import reverse
|
||||
from django.test.client import RequestFactory
|
||||
from mock import Mock, patch
|
||||
from crum import set_current_request
|
||||
|
||||
@@ -15,7 +14,7 @@ from courseware.entrance_exams import (
|
||||
)
|
||||
from courseware.model_data import FieldDataCache
|
||||
from courseware.module_render import get_module, handle_xblock_callback, toc_for_course
|
||||
from courseware.tests.factories import InstructorFactory, StaffFactory, UserFactory
|
||||
from courseware.tests.factories import InstructorFactory, StaffFactory, UserFactory, RequestFactoryNoCsrf
|
||||
from courseware.tests.helpers import LoginEnrollmentTestCase
|
||||
from milestones.tests.utils import MilestonesTestCaseMixin
|
||||
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag
|
||||
@@ -536,7 +535,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest
|
||||
"""
|
||||
Tests entrance exam xblock has `entrance_exam_passed` key in json response.
|
||||
"""
|
||||
request_factory = RequestFactory()
|
||||
request_factory = RequestFactoryNoCsrf()
|
||||
data = {'input_{}_2_1'.format(unicode(self.problem_1.location.html_id())): 'choice_2'}
|
||||
request = request_factory.post(
|
||||
'problem_check',
|
||||
|
||||
@@ -14,10 +14,12 @@ from completion.models import BlockCompletion
|
||||
from completion import waffle as completion_waffle
|
||||
from django.conf import settings
|
||||
from django.contrib.auth.models import AnonymousUser
|
||||
from django.middleware.csrf import get_token
|
||||
from django.test.client import RequestFactory
|
||||
from django.urls import reverse
|
||||
from django.http import Http404, HttpResponse
|
||||
from django.test.client import RequestFactory
|
||||
from django.test.utils import override_settings
|
||||
from edx_oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory
|
||||
from edx_proctoring.api import create_exam, create_exam_attempt, update_attempt_status
|
||||
from edx_proctoring.runtime import set_runtime_service
|
||||
from edx_proctoring.tests.test_services import MockCreditService, MockGradesService, MockCertificateService
|
||||
@@ -26,6 +28,7 @@ from milestones.tests.utils import MilestonesTestCaseMixin
|
||||
from mock import MagicMock, Mock, patch
|
||||
from opaque_keys.edx.asides import AsideUsageKeyV2
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user
|
||||
from pyquery import PyQuery
|
||||
from six import text_type
|
||||
from web_fragments.fragment import Fragment
|
||||
@@ -49,7 +52,7 @@ 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.tests.factories import GlobalStaffFactory, StudentModuleFactory, UserFactory
|
||||
from courseware.tests.factories import GlobalStaffFactory, StudentModuleFactory, UserFactory, RequestFactoryNoCsrf
|
||||
from courseware.tests.test_submitting_problems import TestSubmittingProblems
|
||||
from courseware.tests.tests import LoginEnrollmentTestCase
|
||||
from lms.djangoapps.lms_xblock.field_data import LmsFieldData
|
||||
@@ -191,7 +194,7 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
|
||||
|
||||
self.mock_user = UserFactory()
|
||||
self.mock_user.id = 1
|
||||
self.request_factory = RequestFactory()
|
||||
self.request_factory = RequestFactoryNoCsrf()
|
||||
|
||||
# Construct a mock module for the modulestore to return
|
||||
self.mock_module = MagicMock()
|
||||
@@ -304,8 +307,9 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
|
||||
self.dispatch
|
||||
)
|
||||
|
||||
def test_anonymous_handle_xblock_callback(self):
|
||||
dispatch_url = reverse(
|
||||
def _get_dispatch_url(self):
|
||||
"""Helper to get dispatch URL for testing xblock callback."""
|
||||
return reverse(
|
||||
'xblock_handler',
|
||||
args=[
|
||||
text_type(self.course_key),
|
||||
@@ -314,23 +318,48 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
|
||||
'goto_position'
|
||||
]
|
||||
)
|
||||
|
||||
def test_anonymous_get_xblock_callback(self):
|
||||
"""Test that anonymous GET is allowed."""
|
||||
dispatch_url = self._get_dispatch_url()
|
||||
response = self.client.get(dispatch_url)
|
||||
self.assertEquals(200, response.status_code)
|
||||
|
||||
def test_anonymous_post_xblock_callback(self):
|
||||
"""Test that anonymous POST is not allowed."""
|
||||
dispatch_url = self._get_dispatch_url()
|
||||
response = self.client.post(dispatch_url, {'position': 2})
|
||||
self.assertEquals(403, 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 = self._get_dispatch_url()
|
||||
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 = self._get_dispatch_url()
|
||||
access_token = AccessTokenFactory(user=self.mock_user, client=ClientFactory()).token
|
||||
headers = {'HTTP_AUTHORIZATION': 'Bearer ' + access_token}
|
||||
response = self.client.post(dispatch_url, {}, **headers)
|
||||
self.assertEqual(200, response.status_code)
|
||||
|
||||
def test_jwt_authentication(self):
|
||||
""" Test that the xblock endpoint supports JWT authentication."""
|
||||
dispatch_url = self._get_dispatch_url()
|
||||
token = create_jwt_for_user(self.mock_user)
|
||||
headers = {'HTTP_AUTHORIZATION': 'JWT ' + token}
|
||||
response = self.client.post(dispatch_url, {}, **headers)
|
||||
self.assertEqual(200, response.status_code)
|
||||
|
||||
def test_missing_position_handler(self):
|
||||
"""
|
||||
Test that sending POST request without or invalid position argument don't raise server error
|
||||
"""
|
||||
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'
|
||||
]
|
||||
)
|
||||
dispatch_url = self._get_dispatch_url()
|
||||
response = self.client.post(dispatch_url)
|
||||
self.assertEqual(200, response.status_code)
|
||||
self.assertEqual(json.loads(response.content), {'success': True})
|
||||
@@ -493,7 +522,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.request_factory = RequestFactoryNoCsrf()
|
||||
|
||||
# Construct a mock module for the modulestore to return
|
||||
self.mock_module = MagicMock()
|
||||
@@ -541,6 +570,40 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas
|
||||
|
||||
return response
|
||||
|
||||
def test_invalid_csrf_token(self):
|
||||
"""
|
||||
Verify that invalid CSRF token is rejected.
|
||||
"""
|
||||
request = RequestFactory().post('dummy_url', data={'position': 1})
|
||||
csrf_token = get_token(request)
|
||||
request._post = {'csrfmiddlewaretoken': '{}-dummy'.format(csrf_token)} # pylint: disable=protected-access
|
||||
request.user = self.mock_user
|
||||
response = render.handle_xblock_callback(
|
||||
request,
|
||||
text_type(self.course_key),
|
||||
quote_slashes(text_type(self.location)),
|
||||
'xmodule_handler',
|
||||
'goto_position',
|
||||
)
|
||||
self.assertEqual(403, response.status_code)
|
||||
|
||||
def test_valid_csrf_token(self):
|
||||
"""
|
||||
Verify that valid CSRF token is accepted.
|
||||
"""
|
||||
request = RequestFactory().post('dummy_url', data={'position': 1})
|
||||
csrf_token = get_token(request)
|
||||
request._post = {'csrfmiddlewaretoken': csrf_token} # pylint: disable=protected-access
|
||||
request.user = self.mock_user
|
||||
response = render.handle_xblock_callback(
|
||||
request,
|
||||
text_type(self.course_key),
|
||||
quote_slashes(text_type(self.location)),
|
||||
'xmodule_handler',
|
||||
'goto_position',
|
||||
)
|
||||
self.assertEqual(200, response.status_code)
|
||||
|
||||
def test_invalid_location(self):
|
||||
request = self.request_factory.post('dummy_url', data={'position': 1})
|
||||
request.user = self.mock_user
|
||||
@@ -912,7 +975,7 @@ class TestTOC(ModuleStoreTestCase):
|
||||
self.course_key = ToyCourseFactory.create().id # pylint: disable=attribute-defined-outside-init
|
||||
self.chapter = 'Overview'
|
||||
chapter_url = '%s/%s/%s' % ('/courses', self.course_key, self.chapter)
|
||||
factory = RequestFactory()
|
||||
factory = RequestFactoryNoCsrf()
|
||||
self.request = factory.get(chapter_url)
|
||||
self.request.user = UserFactory()
|
||||
self.modulestore = self.store._get_modulestore_for_courselike(self.course_key) # pylint: disable=protected-access, attribute-defined-outside-init
|
||||
@@ -1024,7 +1087,7 @@ class TestProctoringRendering(SharedModuleStoreTestCase):
|
||||
super(TestProctoringRendering, self).setUp()
|
||||
self.chapter = 'Overview'
|
||||
chapter_url = '%s/%s/%s' % ('/courses', self.course_key, self.chapter)
|
||||
factory = RequestFactory()
|
||||
factory = RequestFactoryNoCsrf()
|
||||
self.request = factory.get(chapter_url)
|
||||
self.request.user = UserFactory.create()
|
||||
self.user = UserFactory.create()
|
||||
@@ -1379,7 +1442,7 @@ class TestGatedSubsectionRendering(SharedModuleStoreTestCase, MilestonesTestCase
|
||||
category='sequential',
|
||||
display_name="Gated Sequential"
|
||||
)
|
||||
self.request = RequestFactory().get('%s/%s/%s' % ('/courses', self.course.id, self.chapter.display_name))
|
||||
self.request = RequestFactoryNoCsrf().get('%s/%s/%s' % ('/courses', self.course.id, self.chapter.display_name))
|
||||
self.request.user = UserFactory()
|
||||
self.field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
|
||||
self.course.id, self.request.user, self.course, depth=2
|
||||
@@ -1438,7 +1501,7 @@ class TestHtmlModifiers(ModuleStoreTestCase):
|
||||
def setUp(self):
|
||||
super(TestHtmlModifiers, self).setUp()
|
||||
self.course = CourseFactory.create()
|
||||
self.request = RequestFactory().get('/')
|
||||
self.request = RequestFactoryNoCsrf().get('/')
|
||||
self.request.user = self.user
|
||||
self.request.session = {}
|
||||
self.content_string = '<p>This is the content<p>'
|
||||
@@ -1632,7 +1695,7 @@ class ViewInStudioTest(ModuleStoreTestCase):
|
||||
""" Set up the user and request that will be used. """
|
||||
super(ViewInStudioTest, self).setUp()
|
||||
self.staff_user = GlobalStaffFactory.create()
|
||||
self.request = RequestFactory().get('/')
|
||||
self.request = RequestFactoryNoCsrf().get('/')
|
||||
self.request.user = self.staff_user
|
||||
self.request.session = {}
|
||||
self.module = None
|
||||
@@ -1752,7 +1815,7 @@ class TestStaffDebugInfo(SharedModuleStoreTestCase):
|
||||
def setUp(self):
|
||||
super(TestStaffDebugInfo, self).setUp()
|
||||
self.user = UserFactory.create()
|
||||
self.request = RequestFactory().get('/')
|
||||
self.request = RequestFactoryNoCsrf().get('/')
|
||||
self.request.user = self.user
|
||||
self.request.session = {}
|
||||
|
||||
@@ -1984,7 +2047,7 @@ class TestModuleTrackingContext(SharedModuleStoreTestCase):
|
||||
super(TestModuleTrackingContext, self).setUp()
|
||||
|
||||
self.user = UserFactory.create()
|
||||
self.request = RequestFactory().get('/')
|
||||
self.request = RequestFactoryNoCsrf().get('/')
|
||||
self.request.user = self.user
|
||||
self.request.session = {}
|
||||
self.course = CourseFactory.create()
|
||||
@@ -2249,7 +2312,7 @@ class TestEventPublishing(ModuleStoreTestCase, LoginEnrollmentTestCase):
|
||||
|
||||
self.mock_user = UserFactory()
|
||||
self.mock_user.id = 1
|
||||
self.request_factory = RequestFactory()
|
||||
self.request_factory = RequestFactoryNoCsrf()
|
||||
|
||||
@ddt.data('xblock', 'xmodule')
|
||||
@XBlock.register_temp_plugin(PureXBlock, identifier='xblock')
|
||||
|
||||
@@ -19,7 +19,7 @@ from django.contrib.auth.models import AnonymousUser
|
||||
from django.urls import reverse, reverse_lazy
|
||||
from django.http import Http404, HttpResponseBadRequest
|
||||
from django.test import TestCase
|
||||
from django.test.client import Client, RequestFactory
|
||||
from django.test.client import Client
|
||||
from django.test.utils import override_settings
|
||||
from freezegun import freeze_time
|
||||
from milestones.tests.utils import MilestonesTestCaseMixin
|
||||
@@ -40,7 +40,7 @@ from course_modes.tests.factories import CourseModeFactory
|
||||
from courseware.access_utils import check_course_open_for_learner
|
||||
from courseware.model_data import FieldDataCache, set_score
|
||||
from courseware.module_render import get_module, handle_xblock_callback
|
||||
from courseware.tests.factories import GlobalStaffFactory, StudentModuleFactory
|
||||
from courseware.tests.factories import GlobalStaffFactory, StudentModuleFactory, RequestFactoryNoCsrf
|
||||
from courseware.tests.helpers import get_expiration_banner_text
|
||||
from courseware.testutils import RenderXBlockTestMixin
|
||||
from courseware.url_helpers import get_redirect_url
|
||||
@@ -2156,7 +2156,7 @@ class VerifyCourseKeyDecoratorTests(TestCase):
|
||||
def setUp(self):
|
||||
super(VerifyCourseKeyDecoratorTests, self).setUp()
|
||||
|
||||
self.request = RequestFactory().get("foo")
|
||||
self.request = RequestFactoryNoCsrf().get("foo")
|
||||
self.valid_course_id = "edX/test/1"
|
||||
self.invalid_course_id = "edX/"
|
||||
|
||||
@@ -2494,7 +2494,7 @@ class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin
|
||||
"""
|
||||
# pylint:disable=attribute-defined-outside-init
|
||||
|
||||
self.request_factory = RequestFactory()
|
||||
self.request_factory = RequestFactoryNoCsrf()
|
||||
self.user = UserFactory()
|
||||
|
||||
with modulestore().default_store(default_store):
|
||||
@@ -2860,7 +2860,7 @@ class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase, CompletionWaf
|
||||
self.assertIn('data-enable-completion-on-view-service="true"', response.content)
|
||||
self.assertIn('data-mark-completed-on-view-after-delay', response.content)
|
||||
|
||||
request = RequestFactory().post(
|
||||
request = RequestFactoryNoCsrf().post(
|
||||
'/',
|
||||
data=json.dumps({"completion": 1}),
|
||||
content_type='application/json',
|
||||
|
||||
Reference in New Issue
Block a user