Merge pull request #25987 from edx/ormsbee/tnl-7636-xblock-access

Check sequence-level gating in render_xblock (TNL-7636)

cms/envs/devstack.py

@@ -236,3 +236,7 @@ CORS_ORIGIN_ALLOW_ALL = True
 CORS_ALLOW_HEADERS = corsheaders_default_headers + (
     'use-jwt-cookie',
 )
+
+################### Special Exams (Proctoring) and Prereqs ###################
+FEATURES['ENABLE_SPECIAL_EXAMS'] = True
+FEATURES['ENABLE_PREREQUISITE_COURSES'] = True

common/lib/xmodule/xmodule/seq_module.py

@@ -560,6 +560,50 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule):
 
         return None
 
+    def descendants_are_gated(self):
+        """
+        Sequences do their own access gating logic as to whether their content
+        should be viewable, based on things like pre-reqs and time exam starts.
+        Ideally, this information would be passed down to all descendants so
+        that they would know if it's safe to render themselves, but the least
+        invasive patch to this is to make a method that rendering Django views
+        can use to verify before rendering descendants.
+
+        This does _NOT_ check for the content types of children because the
+        performing that traversal undoes a lot of the performance gains made in
+        large sequences when hitting the render_xblock endpoint directly. This
+        method is here mostly to help render_xblock figure out if it's okay to
+        render a descendant of a sequence to guard against malicious actors. So
+        the "let's check all descendants to not let people start an exam they
+        can't finish" reasoning of doing the full traversal does not apply.
+
+        Returns:
+            True if this sequence and its descendants are gated by what are
+                currently sequence-level checks.
+            False if the sequence is and its decendants are not gated.
+
+            Note that this gating logic is only a part of the equation when it
+            comes to determining whether a student is allowed to access this,
+            with other checks being done in has_access calls.
+        """
+        if self.runtime.user_is_staff:
+            return False
+
+        # We're not allowed to see it because of pre-reqs that haven't been
+        # fullfilled.
+        if self._required_prereq():
+            prereq_met, _prereq_meta_info = self._compute_is_prereq_met(True)
+            if not prereq_met:
+                return True
+
+        # Are we a time limited test that hasn't started yet?
+        if self.is_time_limited:
+            if self._time_limited_student_view() or self._hidden_content_student_view({}):
+                return True
+
+        # Otherwise, nothing is blocking us.
+        return False
+
     def _compute_is_prereq_met(self, recalc_on_unmet):
         """
         Evaluate if the user has completed the prerequisite

lms/djangoapps/courseware/tests/test_views.py

@@ -2878,7 +2878,7 @@ class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase, CompletionWaf
         """
         Overridable method to get the response from the endpoint that is being tested.
         """
-        url = reverse('render_xblock', kwargs={'usage_key_string': six.text_type(usage_key)})
+        url = reverse('render_xblock', kwargs={'usage_key_string': str(usage_key)})
         if url_encoded_params:
             url += '?' + url_encoded_params
         return self.client.get(url)
@@ -2934,6 +2934,53 @@ class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase, CompletionWaf
         self.assertContains(response, 'data-enable-completion-on-view-service="false"')
         self.assertNotContains(response, 'data-mark-completed-on-view-after-delay')
 
+    def test_rendering_descendant_of_gated_sequence(self):
+        """
+        Test that we redirect instead of rendering what should be gated content,
+        for things that are gated at the sequence level.
+        """
+        with self.store.default_store(ModuleStoreEnum.Type.split):
+            # pylint:disable=attribute-defined-outside-init
+            self.course = CourseFactory.create(**self.course_options())
+            self.chapter = ItemFactory.create(parent=self.course, category='chapter')
+            self.sequence = ItemFactory.create(
+                parent=self.chapter,
+                category='sequential',
+                display_name='Sequence',
+                is_time_limited=True,
+            )
+            self.vertical_block = ItemFactory.create(
+                parent=self.sequence,
+                category='vertical',
+                display_name="Vertical",
+            )
+            self.html_block = ItemFactory.create(
+                parent=self.vertical_block,
+                category='html',
+                data="<p>Test HTML Content<p>"
+            )
+            self.problem_block = ItemFactory.create(
+                parent=self.vertical_block,
+                category='problem',
+                display_name='Problem'
+            )
+        CourseOverview.load_from_module_store(self.course.id)
+        self.setup_user(admin=False, enroll=True, login=True)
+
+        # Problem and Vertical response should both redirect to the Sequential
+        # (where useful messaging would be).
+        seq_url = reverse('render_xblock', kwargs={'usage_key_string': str(self.sequence.location)})
+        for block in [self.problem_block, self.vertical_block]:
+            response = self.get_response(usage_key=block.location)
+            self.assertEqual(response.status_code, 302)
+            self.assertEqual(response.get('Location'), seq_url)
+
+        # The Sequence itself 200s (or we risk infinite redirect loops).
+        self.assertEqual(
+            self.get_response(usage_key=self.sequence.location).status_code,
+            200
+        )
+
 
 class TestRenderXBlockSelfPaced(TestRenderXBlock):
     """

lms/djangoapps/courseware/views/views.py

@@ -1612,6 +1612,43 @@ def _track_successful_certificate_generation(user_id, course_id):
     })
 
 
+def enclosing_sequence_for_gating_checks(block):
+    """
+    Return the first ancestor of this block that is a SequenceDescriptor.
+
+    Returns None if there is no such ancestor. Returns None if you call it on a
+    SequenceDescriptor directly.
+
+    We explicitly test against the three known tag types that map to sequences
+    (even though two of them have been long since deprecated and are never
+    used). We _don't_ test against SequentialDescriptor directly because:
+
+    1. A direct comparison on the type fails because we magically mix it into a
+       SequenceDescriptorWithMixins object.
+    2. An isinstance check doesn't give us the right behavior because Courses
+       and Sections both subclass SequenceDescriptor. >_<
+
+    Also important to note that some content isn't contained in Sequences at
+    all. LabXchange uses learning pathways, but even content inside courses like
+    `static_tab`, `book`, and `about` live outside the sequence hierarchy.
+    """
+    seq_tags = ['sequential', 'problemset', 'videosequence']
+
+    # If it's being called on a Sequence itself, then don't bother crawling the
+    # ancestor tree, because all the sequence metadata we need for gating checks
+    # will happen automatically when rendering the render_xblock view anyway,
+    # and we don't want weird, weird edge cases where you have nested Sequences
+    # (which would probably "work" in terms of OLX import).
+    if block.location.block_type in seq_tags:
+        return None
+
+    ancestor = block
+    while ancestor and ancestor.location.block_type not in seq_tags:
+        ancestor = ancestor.get_parent()  # Note: CourseDescriptor's parent is None
+
+    return ancestor
+
+
 @require_http_methods(["GET", "POST"])
 @ensure_valid_usage_key
 @xframe_options_exempt
@@ -1675,6 +1712,39 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True):
 
         missed_deadlines, missed_gated_content = dates_banner_should_display(course_key, request.user)
 
+        # Some content gating happens only at the Sequence level (e.g. "has this
+        # timed exam started?").
+        ancestor_seq = enclosing_sequence_for_gating_checks(block)
+        if ancestor_seq:
+            seq_usage_key = ancestor_seq.location
+            # We have a Descriptor, but I had trouble getting a SequenceModule
+            # from it (even using ._xmodule to force the conversion) because the
+            # runtime wasn't properly initialized. This view uses multiple
+            # runtimes (including Blockstore), so I'm pulling it from scratch
+            # based on the usage_key. We'll have to watch the performance impact
+            # of this. :(
+            seq_module_descriptor, _ = get_module_by_usage_id(
+                request, str(course_key), str(seq_usage_key), disable_staff_debug_info=True, course=course
+            )
+
+            # I'm not at all clear why get_module_by_usage_id returns the
+            # descriptor or why I need to manually force it to load the module
+            # like this manually instead of the proxying working, but trial and
+            # error has led me here. Hopefully all this weirdness goes away when
+            # SequenceModule gets converted to an XBlock in:
+            #     https://github.com/edx/edx-platform/pull/25965
+            seq_module = seq_module_descriptor._xmodule  # pylint: disable=protected-access
+
+            # If the SequenceModule feels that gating is necessary, redirect
+            # there so we can have some kind of error message at any rate.
+            if seq_module.descendants_are_gated():
+                return redirect(
+                    reverse(
+                        'render_xblock',
+                        kwargs={'usage_key_string': str(seq_module.location)}
+                    )
+                )
+
         context = {
             'fragment': block.render(requested_view, context=student_view_context),
             'course': course,

lms/envs/devstack.py

@@ -427,3 +427,7 @@ if os.path.isfile(join(dirname(abspath(__file__)), 'private.py')):
 # Uncomment the lines below if you'd like to see SQL statements in your devstack LMS log.
 # LOGGING['handlers']['console']['level'] = 'DEBUG'
 # LOGGING['loggers']['django.db.backends'] = {'handlers': ['console'], 'level': 'DEBUG', 'propagate': False}
+
+################### Special Exams (Proctoring) and Prereqs ###################
+FEATURES['ENABLE_SPECIAL_EXAMS'] = True
+FEATURES['ENABLE_PREREQUISITE_COURSES'] = True