Skip to content

Commit 51a254a

Browse files
feat: added feature to rate limit secondary email change (#37356)
1 parent 8126142 commit 51a254a

File tree

3 files changed

+43
-6
lines changed

3 files changed

+43
-6
lines changed

openedx/core/djangoapps/user_api/accounts/tests/test_views.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,37 @@ def test_patch_invalid_email(self, bad_email):
11101110
field_errors['email']['developer_message']
11111111
assert 'Valid e-mail address required.' == field_errors['email']['user_message']
11121112

1113+
@override_settings(SECONDARY_EMAIL_RATE_LIMIT='1/m')
1114+
def test_patch_secondary_email_ratelimit(self):
1115+
"""
1116+
Tests if rate limit is applied on secondary_email patch
1117+
"""
1118+
client = self.login_client("client", "user")
1119+
self.send_patch(client, {"secondary_email": "new_email_01@example.com"},
1120+
expected_status=status.HTTP_200_OK)
1121+
self.send_patch(client, {"secondary_email": "new_email_02@example.com"},
1122+
expected_status=status.HTTP_429_TOO_MANY_REQUESTS)
1123+
1124+
@override_settings(SECONDARY_EMAIL_RATE_LIMIT='')
1125+
def test_ratelimit_is_disabled_on_secondary_email_patch_if_settings_is_empty(self):
1126+
"""
1127+
Tests rate limit doesn't applied on secondary_email patch if SECONDARY_EMAIL_RATE_LIMIT is empty string or None
1128+
"""
1129+
client = self.login_client("client", "user")
1130+
self.send_patch(client, {"secondary_email": "email_new_01@example.com"},
1131+
expected_status=status.HTTP_200_OK)
1132+
self.send_patch(client, {"secondary_email": "email_new_02@example.com"},
1133+
expected_status=status.HTTP_200_OK)
1134+
1135+
@override_settings(SECONDARY_EMAIL_RATE_LIMIT='1/d')
1136+
def test_ratelimit_is_only_on_secondary_email_change(self):
1137+
"""
1138+
Tests if rate limit is only applied for secondary_email attribute i.e. when user changes recovery email
1139+
"""
1140+
client = self.login_client("client", "user")
1141+
for i in range(5):
1142+
self.send_patch(client, {"name": f"new_name_{i}"}, expected_status=status.HTTP_200_OK)
1143+
11131144
@mock.patch('common.djangoapps.student.views.management.do_email_change_request')
11141145
def test_patch_duplicate_email(self, do_email_change_request):
11151146
"""

openedx/core/djangoapps/user_api/accounts/views.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -396,12 +396,17 @@ def partial_update(self, request, username):
396396
"""
397397
if request.content_type != MergePatchParser.media_type:
398398
raise UnsupportedMediaType(request.content_type)
399-
if request.data.get("email") and settings.EMAIL_CHANGE_RATE_LIMIT:
400-
if is_ratelimited(
401-
request=request, group="email_change_rate_limit", key="user",
402-
rate=settings.EMAIL_CHANGE_RATE_LIMIT, increment=True,
403-
):
404-
return Response({"error": "Too many requests"}, status=status.HTTP_429_TOO_MANY_REQUESTS)
399+
400+
for key, limit in [
401+
('email', settings.EMAIL_CHANGE_RATE_LIMIT),
402+
('secondary_email', settings.SECONDARY_EMAIL_RATE_LIMIT)
403+
]:
404+
if request.data.get(key) and limit:
405+
if is_ratelimited(
406+
request=request, group=f"{key}_change_rate_limit", key="user",
407+
rate=limit, increment=True,
408+
):
409+
return Response({"error": "Too many requests"}, status=status.HTTP_429_TOO_MANY_REQUESTS)
405410

406411
try:
407412
with transaction.atomic():

openedx/envs/common.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,7 @@ def _make_locale_paths(settings):
830830

831831
ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT = '100/m'
832832
EMAIL_CHANGE_RATE_LIMIT = ''
833+
SECONDARY_EMAIL_RATE_LIMIT = ''
833834

834835
LMS_ROOT_URL = None
835836
LMS_INTERNAL_ROOT_URL = Derived(lambda settings: settings.LMS_ROOT_URL)

0 commit comments

Comments
 (0)