diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 2dc54201d1..54b5bb3114 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -176,6 +176,7 @@ def make_mock_request_impl( thread_id=thread_id, num_children=num_thread_responses, group_id=group_id, + commentable_id=commentable_id ) elif "/users/" in url: data = { @@ -336,8 +337,8 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase): @ddt.data( # old mongo with cache - (ModuleStoreEnum.Type.mongo, 1, 7, 5, 14, 8), - (ModuleStoreEnum.Type.mongo, 50, 7, 5, 14, 8), + (ModuleStoreEnum.Type.mongo, 1, 6, 4, 14, 8), + (ModuleStoreEnum.Type.mongo, 50, 6, 4, 14, 8), # split mongo: 3 queries, regardless of thread response size. (ModuleStoreEnum.Type.split, 1, 3, 3, 14, 8), (ModuleStoreEnum.Type.split, 50, 3, 3, 14, 8), @@ -668,6 +669,40 @@ class SingleThreadContentGroupTestCase(ContentGroupTestCase): self.assert_can_access(self.non_cohorted_user, self.beta_module.discussion_id, thread_id, False) + def test_course_context_respected(self, mock_request): + """ + Verify that course threads go through discussion_category_id_access method. + """ + thread_id = "test_thread_id" + mock_request.side_effect = make_mock_request_impl( + course=self.course, text="dummy content", thread_id=thread_id + ) + + # Beta user does not have access to alpha_module. + self.assert_can_access(self.beta_user, self.alpha_module.discussion_id, thread_id, False) + + def test_standalone_context_respected(self, mock_request): + """ + Verify that standalone threads don't go through discussion_category_id_access method. + """ + # For this rather pathological test, we are assigning the alpha module discussion_id (commentable_id) + # to a team so that we can verify that standalone threads don't go through discussion_category_id_access. + thread_id = "test_thread_id" + CourseTeamFactory( + name="A team", + course_id=self.course.id, + topic_id='topic_id', + discussion_topic_id=self.alpha_module.discussion_id + ) + mock_request.side_effect = make_mock_request_impl( + course=self.course, text="dummy content", thread_id=thread_id, + commentable_id=self.alpha_module.discussion_id + ) + + # If a thread returns context other than "course", the access check is not done, and the beta user + # can see the alpha discussion module. + self.assert_can_access(self.beta_user, self.alpha_module.discussion_id, thread_id, True) + @patch('lms.lib.comment_client.utils.requests.request') class InlineDiscussionContextTestCase(ModuleStoreTestCase): diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 6d57b0f983..3ff0a9a8c3 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -320,10 +320,6 @@ def single_thread(request, course_key, discussion_id, thread_id): user_info = cc_user.to_dict() is_moderator = has_permission(request.user, "see_all_cohorts", course_key) - # Verify that the student has access to this thread if belongs to a discussion module - if discussion_id not in utils.get_discussion_categories_ids(course, request.user): - raise Http404 - # Currently, the front end always loads responses via AJAX, even for this # page; it would be a nice optimization to avoid that extra round trip to # the comments service. @@ -340,8 +336,7 @@ def single_thread(request, course_key, discussion_id, thread_id): raise # Verify that the student has access to this thread if belongs to a course discussion module - thread_context = getattr(thread, "context", "course") - if thread_context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): + if thread.context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): raise Http404 # verify that the thread belongs to the requesting student's cohort