diff --git a/lms/djangoapps/discussion/rest_api/tests/test_utils.py b/lms/djangoapps/discussion/rest_api/tests/test_utils.py index db24847a82..49c1e3889d 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_utils.py @@ -20,13 +20,15 @@ from lms.djangoapps.discussion.rest_api.utils import ( get_course_ta_users_list, get_moderator_users_list, is_posting_allowed, - remove_empty_sequentials + remove_empty_sequentials, + is_only_student ) from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, PostingRestriction from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +@ddt.ddt class DiscussionAPIUtilsTestCase(ModuleStoreTestCase): """ Base test-case class for utils for Discussion REST API. @@ -104,6 +106,24 @@ class DiscussionAPIUtilsTestCase(ModuleStoreTestCase): # Assert that the output matches the expected output assert output == expected_output + @ddt.data( + ('student', False, True), + ('student', True, False), + ('moderator', False, False), + ('course_staff_user', False, False), + ('course_instructor_user', False, False), + ('community_ta', False, False), + ('group_community_ta', False, False), + ) + @ddt.unpack + def test_is_only_student(self, user_attr, is_user_admin, expected_result): + """Test is_only_student for various user role combinations.""" + user = getattr(self, user_attr) + user.is_staff = is_user_admin + user.save() + result = is_only_student(self.course.id, user) + assert result == expected_result + class TestRemoveEmptySequentials(unittest.TestCase): """ diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index dce16d0d32..c67326581f 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -57,7 +57,10 @@ from openedx.core.djangoapps.course_groups.tests.helpers import config_course_co from openedx.core.djangoapps.discussions.config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, DiscussionTopicLink, Provider from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task -from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role +from openedx.core.djangoapps.django_comment_common.models import ( + CourseDiscussionSettings, + Role, +) from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.djangoapps.oauth_dispatch.tests.factories import AccessTokenFactory, ApplicationFactory @@ -1081,7 +1084,21 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): patcher.start() self.addCleanup(patcher.stop) + self.is_captcha_enabled_patcher = mock.patch( + 'lms.djangoapps.discussion.rest_api.views.is_captcha_enabled' + ) + self.mock_is_captcha_enabled = self.is_captcha_enabled_patcher.start() + self.mock_is_captcha_enabled.side_effect = lambda course_key: str(course_key) == str(self.course.id) + self.addCleanup(self.is_captcha_enabled_patcher.stop) + + self.verify_recaptcha_token_patcher = mock.patch( + 'lms.djangoapps.discussion.rest_api.views.verify_recaptcha_token' + ) + self.mock_verify_recaptcha_token = self.verify_recaptcha_token_patcher.start() + self.addCleanup(self.verify_recaptcha_token_patcher.stop) + def test_basic(self): + self.mock_is_captcha_enabled.side_effect = lambda course_key: False self.register_get_user_response(self.user) cs_thread = make_minimal_cs_thread({ "id": "test_thread", @@ -1122,6 +1139,7 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): } def test_error(self): + self.mock_is_captcha_enabled.side_effect = lambda course_key: False request_data = { "topic_id": "dummy", "type": "discussion", @@ -1151,6 +1169,7 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): """ Tests posts cannot be created if ONLY_VERIFIED_USERS_CAN_POST is enabled and user email is unverified. """ + self.mock_is_captcha_enabled.side_effect = lambda course_key: False with override_waffle_flag(ONLY_VERIFIED_USERS_CAN_POST, only_verified_user_can_post): self.user.is_active = email_verified self.user.save() @@ -1175,6 +1194,115 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ) assert response.status_code == response_status + def test_captcha_enabled_privileged_user(self): + """Test that CAPTCHA is skipped for users with privileged roles when CAPTCHA is enabled.""" + self.mock_is_captcha_enabled.side_effect = lambda course_key: True + + self.register_get_user_response(self.user) + cs_thread = make_minimal_cs_thread({ + "id": "test_thread", + "username": self.user.username, + "read": True, + }) + self.register_post_thread_response(cs_thread) + with mock.patch( + 'lms.djangoapps.discussion.rest_api.views.is_only_student', return_value=False + ): + request_data = { + "course_id": str(self.course.id), + "topic_id": "test_topic", + "type": "discussion", + "title": "Test Title", + "raw_body": "Test body", + } + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == 200 + assert "captcha_token" not in parsed_body(httpretty.last_request()) + + @ddt.data( + (False, False, status.HTTP_400_BAD_REQUEST, + {'captcha_token': {'developer_message': 'This field is required.'}}), + (True, False, status.HTTP_400_BAD_REQUEST, {'error': 'CAPTCHA verification failed.'}), + (True, True, status.HTTP_200_OK, None), + ) + @ddt.unpack + def test_captcha_enabled_learner( + self, provide_token, valid_token, expected_status, expected_error + ): + """Test CAPTCHA behavior for a learner when CAPTCHA is enabled.""" + self.mock_is_captcha_enabled.side_effect = lambda course_key: True + self.mock_verify_recaptcha_token.return_value = valid_token + + with mock.patch( + 'lms.djangoapps.discussion.rest_api.views.is_only_student', return_value=True + ): + self.register_get_user_response(self.user) + cs_thread = make_minimal_cs_thread({ + "id": "test_thread", + "username": self.user.username, + "read": True, + }) + self.register_post_thread_response(cs_thread) + request_data = { + "course_id": str(self.course.id), + "topic_id": "test_topic", + "type": "discussion", + "title": "Test Title", + "raw_body": "Test body", + } + if provide_token: + request_data["captcha_token"] = "test_token" + + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + + assert response.status_code == expected_status + if expected_error: + response_data = json.loads(response.content.decode('utf-8')) + if expected_status == 400: + if response_data.get("field_errors"): + assert response_data["field_errors"] == expected_error + else: + assert response_data == expected_error + if expected_status == 200: + assert "captcha_token" not in parsed_body(httpretty.last_request()) + + def test_captcha_disabled(self): + """Test that CAPTCHA is skipped when CAPTCHA is disabled.""" + self.mock_is_captcha_enabled.side_effect = lambda course_key: False + + with mock.patch( + 'lms.djangoapps.discussion.rest_api.views.is_only_student', return_value=True + ): + self.register_get_user_response(self.user) + cs_thread = make_minimal_cs_thread({ + "id": "test_thread", + "username": self.user.username, + "read": True, + }) + self.register_post_thread_response(cs_thread) + request_data = { + "course_id": str(self.course.id), + "topic_id": "test_topic", + "type": "discussion", + "title": "Test Title", + "raw_body": "Test body", + } + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == 200 + assert "captcha_token" not in parsed_body(httpretty.last_request()) + @httpretty.activate @disable_signal(api, 'thread_deleted') @@ -2072,7 +2200,6 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ) patcher.start() self.addCleanup(patcher.stop) - patcher = mock.patch( "openedx.core.djangoapps.django_comment_common.comment_client.models.forum_api.get_course_id_by_comment" ) @@ -2084,7 +2211,27 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): self.mock_get_course_id_by_thread = patcher.start() self.addCleanup(patcher.stop) + self.is_captcha_enabled_patcher = mock.patch( + 'lms.djangoapps.discussion.rest_api.views.is_captcha_enabled' + ) + self.mock_is_captcha_enabled = self.is_captcha_enabled_patcher.start() + self.mock_is_captcha_enabled.side_effect = lambda course_key: str(course_key) == str(self.course.id) + self.addCleanup(self.is_captcha_enabled_patcher.stop) + + self.verify_recaptcha_token_patcher = mock.patch( + 'lms.djangoapps.discussion.rest_api.views.verify_recaptcha_token' + ) + self.mock_verify_recaptcha_token = self.verify_recaptcha_token_patcher.start() + self.addCleanup(self.verify_recaptcha_token_patcher.stop) + + self.get_course_id_patcher = mock.patch( + 'lms.djangoapps.discussion.rest_api.views.get_course_id_from_thread_id', return_value=str(self.course.id) + ) + self.mock_get_course_id_from_thread_id = self.get_course_id_patcher.start() + self.addCleanup(self.get_course_id_patcher.stop) + def test_basic(self): + self.mock_is_captcha_enabled.side_effect = lambda course_key: False self.register_get_user_response(self.user) self.register_thread() self.register_comment() @@ -2144,6 +2291,7 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): } def test_error(self): + self.mock_is_captcha_enabled.side_effect = lambda course_key: False response = self.client.post( self.url, json.dumps({}), @@ -2157,6 +2305,7 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): assert response_data == expected_response_data def test_closed_thread(self): + self.mock_is_captcha_enabled.side_effect = lambda course_key: False self.register_get_user_response(self.user) self.register_thread({"closed": True}) self.register_comment() @@ -2183,6 +2332,7 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): Tests comments/replies cannot be created if ONLY_VERIFIED_USERS_CAN_POST is enabled and user email is unverified. """ + self.mock_is_captcha_enabled.side_effect = lambda course_key: False with override_waffle_flag(ONLY_VERIFIED_USERS_CAN_POST, only_verified_user_can_post): self.user.is_active = email_verified self.user.save() @@ -2234,6 +2384,90 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ) assert response.status_code == response_status + def test_captcha_enabled_privileged_user(self): + """Test that CAPTCHA is skipped for users with privileged roles when CAPTCHA is enabled.""" + self.mock_is_captcha_enabled.side_effect = lambda course_key: True + self.register_get_user_response(self.user) + self.register_thread() + self.register_comment() + with mock.patch( + 'lms.djangoapps.discussion.rest_api.views.is_only_student', return_value=False + ): + request_data = { + "thread_id": "test_thread", + "raw_body": "Test body", + } + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == 200 + assert "captcha_token" not in parsed_body(httpretty.last_request()) + + @ddt.data( + (False, False, status.HTTP_400_BAD_REQUEST, + {'captcha_token': {'developer_message': 'This field is required.'}}), + (True, False, status.HTTP_400_BAD_REQUEST, {'error': 'CAPTCHA verification failed.'}), + (True, True, status.HTTP_200_OK, None), + ) + @ddt.unpack + def test_captcha_enabled_learner( + self, provide_token, valid_token, expected_status, expected_error + ): + """Test CAPTCHA behavior for a learner when CAPTCHA is enabled.""" + self.mock_is_captcha_enabled.side_effect = lambda course_key: True + self.mock_verify_recaptcha_token.return_value = valid_token + + with mock.patch( + 'lms.djangoapps.discussion.rest_api.views.is_only_student', return_value=True + ): + self.register_get_user_response(self.user) + self.register_thread() + self.register_comment() + request_data = { + "thread_id": "test_thread", + "raw_body": "Test body", + } + if provide_token: + request_data["captcha_token"] = "test_token" + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == expected_status + if expected_error: + response_data = json.loads(response.content.decode('utf-8')) + if response_data.get("field_errors"): + assert response_data["field_errors"] == expected_error + else: + assert response_data == expected_error + if expected_status == 200: + assert "captcha_token" not in parsed_body(httpretty.last_request()) + + def test_captcha_disabled(self): + """Test that CAPTCHA is skipped when CAPTCHA is disabled.""" + self.mock_is_captcha_enabled.side_effect = lambda course_key: False + + with mock.patch( + 'lms.djangoapps.discussion.rest_api.views.is_only_student', return_value=True + ): + self.register_get_user_response(self.user) + self.register_thread() + self.register_comment() + request_data = { + "thread_id": "test_thread", + "raw_body": "Test body", + } + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == 200 + assert "captcha_token" not in parsed_body(httpretty.last_request()) + @httpretty.activate @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) diff --git a/lms/djangoapps/discussion/rest_api/utils.py b/lms/djangoapps/discussion/rest_api/utils.py index baadf0bc67..f9928c9eb2 100644 --- a/lms/djangoapps/discussion/rest_api/utils.py +++ b/lms/djangoapps/discussion/rest_api/utils.py @@ -13,6 +13,7 @@ from django.db.models.functions import Length from pytz import UTC from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole +from common.djangoapps.student.models import CourseAccessRole from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread from lms.djangoapps.discussion.config.settings import ENABLE_CAPTCHA_IN_DISCUSSION @@ -24,8 +25,10 @@ from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_MODERATOR, + FORUM_ROLE_STUDENT, Role ) +from ..django_comment_client.utils import get_user_role_names log = logging.getLogger(__name__) @@ -448,3 +451,14 @@ def get_course_id_from_thread_id(thread_id: str) -> str: 'mark_as_read': False }) return thread["course_id"] + + +def is_only_student(course_key, user) -> bool: + """ + Check if the user is only a user and doesn't hold any other roles the given course. + """ + is_course_staff_or_admin = (CourseAccessRole.objects.filter + (user=user, course_id__in=[course_key], role__in=["instructor", "staff"]).exists()) + is_user_admin = user.is_staff + user_roles = get_user_role_names(user, course_key) + return user_roles == {FORUM_ROLE_STUDENT} and not (is_course_staff_or_admin or is_user_admin) diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 71821a8d02..9ec4fb227e 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -86,7 +86,11 @@ from ..rest_api.serializers import ( ) from .utils import ( create_blocks_params, - create_topics_v3_structure, is_captcha_enabled, verify_recaptcha_token, get_course_id_from_thread_id, + create_topics_v3_structure, + is_captcha_enabled, + verify_recaptcha_token, + get_course_id_from_thread_id, + is_only_student, ) log = logging.getLogger(__name__) @@ -674,7 +678,8 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): raise ValidationError({"course_id": ["This field is required."]}) course_key_str = request.data.get("course_id") course_key = CourseKey.from_string(course_key_str) - if is_captcha_enabled(course_key): + + if is_captcha_enabled(course_key) and is_only_student(course_key, request.user): captcha_token = request.data.get('captcha_token') if not captcha_token: raise ValidationError({'captcha_token': 'This field is required.'}) @@ -1047,7 +1052,7 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet): course_key_str = get_course_id_from_thread_id(request.data["thread_id"]) course_key = CourseKey.from_string(course_key_str) - if is_captcha_enabled(course_key): + if is_captcha_enabled(course_key) and is_only_student(course_key, request.user): captcha_token = request.data.get('captcha_token') if not captcha_token: raise ValidationError({'captcha_token': 'This field is required.'})