diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index e1080fd924..7969d335b7 100644 --- a/cms/envs/devstack.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 2f23b2cd9e..87ff101cf2 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 173c00f288..6bd3cde360 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/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="

Test HTML Content

" + ) + 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): """ diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 23d2fe071c..e611adb799 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/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, diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 3a0f04fb7d..a0f4907cd1 100644 --- a/lms/envs/devstack.py +++ b/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