From 1f392bdc3e10ecd87a0b960fe010ab06a569de8c Mon Sep 17 00:00:00 2001
From: Kyle McCormick <kmccormick@edx.org>
Date: Mon, 15 Mar 2021 16:50:33 -0400
Subject: [PATCH] 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
---
lms/djangoapps/courseware/testutils.py | 12 ++++++++++--
lms/djangoapps/courseware/views/views.py | 13 ++++++++++++-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py
index 91e8cf89e5a..35a44d7e9f1 100644
--- a/lms/djangoapps/courseware/testutils.py
+++ b/lms/djangoapps/courseware/testutils.py
@@ -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)
diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py
index 895c74253cc..4e3ebeab718 100644
--- a/lms/djangoapps/courseware/views/views.py
+++ b/lms/djangoapps/courseware/views/views.py
@@ -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(
--
GitLab