fix: render_xblock was denying access to staff masquerading as learners (#27017)
The render_xblock view, which powers the Learning MFE (among other things) returned a 404 when un- enrolled course staff users tried to load it while masquerading as learners. This was because we checked course access after enabling the masquerading context, which triggered a redirect- to-enrollment exception. The fix is simply to enable the masquerading context after checking course access. Content-level behavior and access is still calculated within the masquerading context, as intended. TNL-7989
This commit is contained in:
@@ -10,7 +10,6 @@ from urllib.parse import urlencode
|
||||
|
||||
import ddt
|
||||
|
||||
from lms.djangoapps.courseware.field_overrides import OverrideModulestoreFieldData
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.features.course_experience.url_helpers import get_legacy_courseware_url
|
||||
from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
|
||||
@@ -18,9 +17,12 @@ from xmodule.modulestore import ModuleStoreEnum
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
|
||||
|
||||
from .field_overrides import OverrideModulestoreFieldData
|
||||
from .tests.helpers import MasqueradeMixin
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class RenderXBlockTestMixin(metaclass=ABCMeta):
|
||||
class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
|
||||
"""
|
||||
Mixin for testing the courseware.render_xblock function.
|
||||
It can be used for testing any higher-level endpoint that calls this method.
|
||||
@@ -220,6 +222,12 @@ class RenderXBlockTestMixin(metaclass=ABCMeta):
|
||||
self.setup_user(admin=True, enroll=False, login=True)
|
||||
self.verify_response()
|
||||
|
||||
def test_success_unenrolled_staff_masquerading_as_student(self):
|
||||
self.setup_course()
|
||||
self.setup_user(admin=True, enroll=False, login=True)
|
||||
self.update_masquerade(role='student')
|
||||
self.verify_response()
|
||||
|
||||
def test_success_enrolled_student(self):
|
||||
self.setup_course()
|
||||
self.setup_user(admin=False, enroll=True, login=True)
|
||||
|
||||
@@ -1700,7 +1700,6 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True):
|
||||
)
|
||||
|
||||
staff_access = has_access(request.user, 'staff', course_key)
|
||||
_course_masquerade, request.user = setup_masquerade(request, course_key, staff_access)
|
||||
|
||||
with modulestore().bulk_operations(course_key):
|
||||
# verify the user has access to the course, including enrollment check
|
||||
@@ -1709,6 +1708,18 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True):
|
||||
except CourseAccessRedirect:
|
||||
raise Http404("Course not found.") # lint-amnesty, pylint: disable=raise-missing-from
|
||||
|
||||
# with course access now verified:
|
||||
# assume masquerading role, if applicable.
|
||||
# (if we did this *before* the course access check, then course staff
|
||||
# masquerading as learners would often be denied access, since course
|
||||
# staff are generally not enrolled, and viewing a course generally
|
||||
# requires enrollment.)
|
||||
_course_masquerade, request.user = setup_masquerade(
|
||||
request,
|
||||
course_key,
|
||||
staff_access,
|
||||
)
|
||||
|
||||
# get the block, which verifies whether the user has access to the block.
|
||||
recheck_access = request.GET.get('recheck_access') == '1'
|
||||
block, _ = get_module_by_usage_id(
|
||||
|
||||
Reference in New Issue
Block a user