feat: added configurable ratelimit feature in discussions (#37094)
This commit is contained in:
committed by
GitHub
parent
02c45c5325
commit
b658470f8b
71
lms/djangoapps/discussion/rate_limit.py
Normal file
71
lms/djangoapps/discussion/rate_limit.py
Normal file
@@ -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)
|
||||
@@ -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),
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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})
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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__)
|
||||
|
||||
@@ -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>{USERNAME_REGEX_PARTIAL})'
|
||||
|
||||
DISCUSSION_RATELIMIT = '100/m'
|
||||
SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS = 0
|
||||
|
||||
Reference in New Issue
Block a user