From b658470f8bc57fc0f4a7a9a6af6ec07ec9a90eaf Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Thu, 31 Jul 2025 17:35:10 +0500 Subject: [PATCH] feat: added configurable ratelimit feature in discussions (#37094) --- lms/djangoapps/discussion/rate_limit.py | 71 ++++++ lms/djangoapps/discussion/rest_api/api.py | 2 + .../discussion/rest_api/tests/test_api.py | 3 +- .../discussion/rest_api/tests/test_views.py | 222 +++++++++++++++++- lms/djangoapps/discussion/rest_api/views.py | 7 + lms/djangoapps/discussion/toggles.py | 10 + openedx/envs/common.py | 3 + 7 files changed, 313 insertions(+), 5 deletions(-) create mode 100644 lms/djangoapps/discussion/rate_limit.py diff --git a/lms/djangoapps/discussion/rate_limit.py b/lms/djangoapps/discussion/rate_limit.py new file mode 100644 index 0000000000..bd810bbca6 --- /dev/null +++ b/lms/djangoapps/discussion/rate_limit.py @@ -0,0 +1,71 @@ +""" +Contains all code related to rate limit +""" +from datetime import timedelta +from django.conf import settings +from django_ratelimit import ALL +from django_ratelimit.core import is_ratelimited as _ratelimit +from django.utils import timezone +from opaque_keys.edx.keys import CourseKey + +from common.djangoapps.student.models import CourseAccessRole +from lms.djangoapps.discussion.toggles import ENABLE_RATE_LIMIT_IN_DISCUSSION +from openedx.core.djangoapps.django_comment_common.models import ( + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_MODERATOR, + Role +) + + +CONTENT_CREATION_GROUP = "content_creation" + + +def is_rate_limited(request, group, key="user", rate='10/m', method=ALL, increment=True, course_key=None): + """ + Check if the request is rate limited + Args: + request: The HTTP request object. + group: The group to which the rate limit applies + key: The key to identify (user or ip). + rate: The rate limit (default is '10/m' - 10 requests per minute). + method: The HTTP method to check (default is 'POST'). + increment: Whether to increment the rate limit counter (default is True). + course_key: The course key for which the rate limit is applied (optional). + Returns: + bool: True if the request is rate limited, False otherwise. + """ + if ENABLE_RATE_LIMIT_IN_DISCUSSION.is_enabled(course_key): + org_method = request.method + if increment is False and method != ALL: + request.method = method + rate_limited = _ratelimit(request, group=group, key=key, rate=rate, method=method, increment=increment) + request.method = org_method + return rate_limited + return False + + +def is_content_creation_rate_limited(request, course_key=None, increment=True): + """ + Check if the request is rate limited for content creation in discussions. + """ + if course_key and isinstance(course_key, str): + course_key = CourseKey.from_string(course_key) + + user = request.user + num_days = settings.SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS + if user.is_staff or (user.date_joined < (timezone.now() - timedelta(days=num_days))): + return False + + course_roles = ["instructor", "staff", "limited_staff"] + if CourseAccessRole.objects.filter(user=user, course_id=course_key, role__in=course_roles).exists(): + return False + + forum_roles = {FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_COMMUNITY_TA} + user_roles = set(Role.objects.filter(users=user, course_id=course_key).values_list('name', flat=True).distinct()) + if bool(user_roles & forum_roles): + return False + + return is_rate_limited(request, CONTENT_CREATION_GROUP, key='user', rate=settings.DISCUSSION_RATELIMIT, + method='POST', increment=increment, course_key=course_key) diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index d0e88f0092..7a91bb96a3 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -35,6 +35,7 @@ from common.djangoapps.student.roles import ( from lms.djangoapps.course_api.blocks.api import get_blocks from lms.djangoapps.courseware.courses import get_course_with_access from lms.djangoapps.courseware.exceptions import CourseAccessRedirect +from lms.djangoapps.discussion.rate_limit import is_content_creation_rate_limited from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE, ONLY_VERIFIED_USERS_CAN_POST from lms.djangoapps.discussion.views import is_privileged_user from openedx.core.djangoapps.discussions.models import ( @@ -386,6 +387,7 @@ def get_course(request, course_key, check_tab=True): }, "is_email_verified": request.user.is_active, "only_verified_users_can_post": ONLY_VERIFIED_USERS_CAN_POST.is_enabled(course_key), + "content_creation_rate_limited": is_content_creation_rate_limited(request, course_key, increment=False), } diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index ab565ee846..6b6fdb6ced 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -221,7 +221,8 @@ class GetCourseTest(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase) 'site_key': '', }, "is_email_verified": True, - "only_verified_users_can_post": False + "only_verified_users_can_post": False, + "content_creation_rate_limited": False } @ddt.data( diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index c67326581f..96ee730f9b 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -5,7 +5,7 @@ Tests for Discussion API views import json import random -from datetime import datetime +from datetime import datetime, timedelta from unittest import mock from urllib.parse import parse_qs, urlencode, urlparse @@ -20,7 +20,9 @@ from pytz import UTC from rest_framework import status from rest_framework.test import APIClient, APITestCase -from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE, ONLY_VERIFIED_USERS_CAN_POST +from lms.djangoapps.discussion.toggles import ( + ENABLE_DISCUSSIONS_MFE, ENABLE_RATE_LIMIT_IN_DISCUSSION, ONLY_VERIFIED_USERS_CAN_POST, +) from lms.djangoapps.discussion.rest_api.utils import get_usernames_from_search_string from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -29,7 +31,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, che from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory -from common.djangoapps.student.models import get_retired_username_by_username, CourseEnrollment +from common.djangoapps.student.models import get_retired_username_by_username, CourseEnrollment, CourseAccessRole from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff from common.djangoapps.student.tests.factories import ( AdminFactory, @@ -59,6 +61,10 @@ from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task from openedx.core.djangoapps.django_comment_common.models import ( CourseDiscussionSettings, + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_MODERATOR, Role, ) from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles @@ -567,7 +573,8 @@ class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): 'site_key': '', }, "is_email_verified": True, - "only_verified_users_can_post": False + "only_verified_users_can_post": False, + "content_creation_rate_limited": False } ) @@ -1303,6 +1310,119 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): assert response.status_code == 200 assert "captcha_token" not in parsed_body(httpretty.last_request()) + @override_settings(DISCUSSION_RATELIMIT='1/d') + @override_settings(SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS=1) + @override_waffle_flag(ENABLE_RATE_LIMIT_IN_DISCUSSION, True) + def test_rate_limit_on_learners(self): + """ + Test rate limit is applied on learners when creating posts + """ + self.user.date_joined = datetime.now(UTC) + self.user.save() + 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", + "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 \n This is a very long body but will not be truncated for the preview.", + } + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == status.HTTP_200_OK + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + + @override_settings(DISCUSSION_RATELIMIT='1/d') + @override_settings(SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS=1) + @override_waffle_flag(ENABLE_RATE_LIMIT_IN_DISCUSSION, True) + @ddt.data('staff', 'instructor', 'limited_staff', 'global_staff', FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_COMMUNITY_TA) + def test_rate_limit_not_applied_to_privileged_user(self, role): + """ + Test rate limit is not applied on privileged roles when creating posts + """ + self.user.date_joined = datetime.now(UTC) - timedelta(days=4) + self.user.save() + self.mock_is_captcha_enabled.side_effect = lambda course_key: False + + if role in ['staff', 'instructor', 'limited_staff']: + CourseAccessRole.objects.get_or_create(user=self.user, course_id=self.course.id, role=role) + elif role == 'global_staff': + GlobalStaff.add_users(self.user) + else: + role_obj, _ = Role.objects.get_or_create(course_id=self.course.id, name=role) + role_obj.users.add(self.user) + + 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 \n This is a very long body but will not be truncated for the preview.", + } + for _ in range(4): + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == status.HTTP_200_OK + + @override_settings(DISCUSSION_RATELIMIT='1/d') + @override_settings(SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS=1) + @override_waffle_flag(ENABLE_RATE_LIMIT_IN_DISCUSSION, True) + def test_rate_limit_not_applied_to_aged_account(self): + """ + Test rate limit is not applied on aged accounts when creating posts + """ + self.user.date_joined = datetime.now(UTC) - timedelta(days=2) + self.user.save() + 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", + "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 \n This is a very long body but will not be truncated for the preview.", + } + for _ in range(4): + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == status.HTTP_200_OK + @httpretty.activate @disable_signal(api, 'thread_deleted') @@ -2468,6 +2588,100 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): assert response.status_code == 200 assert "captcha_token" not in parsed_body(httpretty.last_request()) + @override_settings(DISCUSSION_RATELIMIT='1/d') + @override_settings(SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS=1) + @override_waffle_flag(ENABLE_RATE_LIMIT_IN_DISCUSSION, True) + def test_rate_limit_on_learners(self): + """ + Tests rate limit is applied on learners when creating comments + """ + self.user.date_joined = datetime.now(UTC) + self.user.save() + self.mock_is_captcha_enabled.side_effect = lambda course_key: False + + 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 == status.HTTP_200_OK + + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + + @override_settings(DISCUSSION_RATELIMIT='1/d') + @override_settings(SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS=1) + @override_waffle_flag(ENABLE_RATE_LIMIT_IN_DISCUSSION, True) + @ddt.data('staff', 'instructor', 'limited_staff', 'global_staff', FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_COMMUNITY_TA) + def test_rate_limit_not_applied_to_privileged_user(self, role): + """ + Test rate limit is not applied on privileged roles when creating comments + """ + self.user.date_joined = datetime.now(UTC) - timedelta(days=4) + self.user.save() + self.mock_is_captcha_enabled.side_effect = lambda course_key: False + + if role in ['staff', 'instructor', 'limited_staff']: + CourseAccessRole.objects.get_or_create(user=self.user, course_id=self.course.id, role=role) + elif role == 'global_staff': + GlobalStaff.add_users(self.user) + else: + role_obj, _ = Role.objects.get_or_create(course_id=self.course.id, name=role) + role_obj.users.add(self.user) + + self.register_get_user_response(self.user) + self.register_thread() + self.register_comment() + request_data = { + "thread_id": "test_thread", + "raw_body": "Test body", + } + for _ in range(4): + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == status.HTTP_200_OK + + @override_settings(DISCUSSION_RATELIMIT='1/d') + @override_settings(SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS=1) + @override_waffle_flag(ENABLE_RATE_LIMIT_IN_DISCUSSION, True) + def test_rate_limit_not_applied_to_aged_account(self): + """ + Test rate limit on applied on aged accounts when creating comments + """ + self.user.date_joined = datetime.now(UTC) - timedelta(days=2) + self.user.save() + self.mock_is_captcha_enabled.side_effect = lambda course_key: False + + self.register_get_user_response(self.user) + self.register_thread() + self.register_comment() + request_data = { + "thread_id": "test_thread", + "raw_body": "Test body", + } + for _ in range(4): + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + assert response.status_code == status.HTTP_200_OK + @httpretty.activate @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 9ec4fb227e..43aded1df8 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -27,6 +27,7 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.util.file import store_uploaded_file from lms.djangoapps.course_api.blocks.api import get_blocks from lms.djangoapps.course_goals.models import UserActivity +from lms.djangoapps.discussion.rate_limit import is_content_creation_rate_limited from lms.djangoapps.discussion.rest_api.permissions import IsAllowedToBulkDelete from lms.djangoapps.discussion.rest_api.tasks import delete_course_post_for_user from lms.djangoapps.discussion.toggles import ONLY_VERIFIED_USERS_CAN_POST @@ -679,6 +680,9 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): course_key_str = request.data.get("course_id") course_key = CourseKey.from_string(course_key_str) + if is_content_creation_rate_limited(request, course_key=course_key): + return Response("Too many requests", status=status.HTTP_429_TOO_MANY_REQUESTS) + 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: @@ -1052,6 +1056,9 @@ 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_content_creation_rate_limited(request, course_key=course_key): + return Response("Too many requests", status=status.HTTP_429_TOO_MANY_REQUESTS) + 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: diff --git a/lms/djangoapps/discussion/toggles.py b/lms/djangoapps/discussion/toggles.py index 6965f462f9..61286686b7 100644 --- a/lms/djangoapps/discussion/toggles.py +++ b/lms/djangoapps/discussion/toggles.py @@ -27,3 +27,13 @@ ENABLE_DISCUSSIONS_MFE = CourseWaffleFlag( ONLY_VERIFIED_USERS_CAN_POST = CourseWaffleFlag( f"{WAFFLE_FLAG_NAMESPACE}.only_verified_users_can_post", __name__ ) + + +# .. toggle_name: discussions.enable_rate_limit +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to enable rate limit on discussions +# .. toggle_use_cases: temporary, open_edx +# .. toggle_creation_date: 2025-07-29 +# .. toggle_target_removal_date: 2026-07-29 +ENABLE_RATE_LIMIT_IN_DISCUSSION = CourseWaffleFlag(f'{WAFFLE_FLAG_NAMESPACE}.enable_rate_limit', __name__) diff --git a/openedx/envs/common.py b/openedx/envs/common.py index ceccce3342..07c5384648 100644 --- a/openedx/envs/common.py +++ b/openedx/envs/common.py @@ -780,3 +780,6 @@ GENERATE_PROFILE_SCORES = False # in the AccountCreationForm and the user_api through the ENABLE_UNICODE_USERNAME feature flag. USERNAME_REGEX_PARTIAL = r'[\w .@_+-]+' USERNAME_PATTERN = fr'(?P{USERNAME_REGEX_PARTIAL})' + +DISCUSSION_RATELIMIT = '100/m' +SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS = 0