Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: showing captcha only for learners and not other roles (#37061)

Browse Source 
* feat: showing captcha only for learners and not other roles

* test: added test cases

* fix: fixed pylint errors

* fix: fixed a bug with comment creation

* refactor: refactored code

* fix: fixed lint errors

* fix: fixed bug with utils

* test: added test case
This commit is contained in:
Eemaan Amir
2025-07-28 17:07:26 +05:00
committed by GitHub
parent 7af4b644d0
commit 24181468ec
4 changed files with 279 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
lms/djangoapps/discussion/rest_api/tests/test_utils.py
View File

@@ -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):
"""

238
lms/djangoapps/discussion/rest_api/tests/test_views.py
View File

@@ -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})

14
lms/djangoapps/discussion/rest_api/utils.py
View File

@@ -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)

11
lms/djangoapps/discussion/rest_api/views.py
View File

@@ -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.'})