Skip to content

Commit abd305d

Browse files
feat: added rate limit on one click unsubscribe api (#37272)
* feat: added rate limit on one click unsubscribe api * fix: fixed failing test * chore: raise 400 error on invalid username * fix: fixed pylint
1 parent 58d0839 commit abd305d

4 files changed

Lines changed: 42 additions & 2 deletions

File tree

openedx/core/djangoapps/notifications/email/utils.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
from bs4 import BeautifulSoup
77
from django.conf import settings
88
from django.contrib.auth import get_user_model
9+
from django.core.exceptions import BadRequest
910
from django.shortcuts import get_object_or_404
1011
from django.utils.translation import gettext as _
1112
from pytz import utc
1213
from waffle import get_waffle_flag_model # pylint: disable=invalid-django-waffle-import
1314

1415
from lms.djangoapps.branding.api import get_logo_url_for_email
15-
from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher
16+
from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher, UsernameDecryptionException
1617
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
1718
from openedx.core.djangoapps.notifications.base_notification import COURSE_NOTIFICATION_APPS, COURSE_NOTIFICATION_TYPES
1819
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_EMAIL_NOTIFICATIONS
@@ -384,6 +385,19 @@ def decrypt_string(string):
384385
return UsernameCipher.decrypt(string).decode()
385386

386387

388+
def username_from_hash(group, request):
389+
"""
390+
Django ratelimit key to return username from hash
391+
"""
392+
username = request.resolver_match.kwargs.get("username")
393+
if username:
394+
try:
395+
return decrypt_string(username)
396+
except UsernameDecryptionException as exc:
397+
raise BadRequest("Bad request") from exc
398+
return None
399+
400+
387401
def update_user_preferences_from_patch(encrypted_username):
388402
"""
389403
Decrypt username and patch and updates user preferences

openedx/core/djangoapps/notifications/tests/test_views.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import ddt
88
from django.conf import settings
99
from django.contrib.auth import get_user_model
10+
from django.core.cache import cache
1011
from django.test.utils import override_settings
1112
from django.urls import reverse
1213
from edx_toggles.toggles.testutils import override_waffle_flag
@@ -481,13 +482,30 @@ def setUp(self):
481482
"""
482483
Setup test case
483484
"""
485+
cache.clear()
484486
super().setUp()
485487
password = 'password'
486488
self.user = UserFactory(password=password)
487489
self.client.login(username=self.user.username, password=password)
488490
self.course = CourseFactory.create(display_name='test course 1', run="Testing_course_1")
489491
CourseNotificationPreference(course_id=self.course.id, user=self.user).save()
490492

493+
@override_settings(LMS_BASE="example.com", ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT='1/d')
494+
def test_rate_limit_on_unsub(self):
495+
"""
496+
Test rate limit on unsub
497+
"""
498+
self.client.logout()
499+
user_hash = encrypt_string(self.user.username)
500+
url_params = {
501+
"username": user_hash,
502+
}
503+
url = reverse("preference_update_view", kwargs=url_params)
504+
response = self.client.get(url)
505+
assert response.status_code == status.HTTP_200_OK
506+
response = self.client.get(url)
507+
assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS
508+
491509
@override_settings(LMS_BASE="")
492510
@ddt.data('get', 'post')
493511
def test_if_preference_is_updated(self, request_type):

openedx/core/djangoapps/notifications/views.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from django.conf import settings
77
from django.db.models import Count
8+
from django_ratelimit.core import is_ratelimited
89
from django.shortcuts import get_object_or_404
910
from django.utils.translation import gettext as _
1011
from pytz import UTC
@@ -14,7 +15,7 @@
1415
from rest_framework.response import Response
1516
from rest_framework.views import APIView
1617

17-
from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch
18+
from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch, username_from_hash
1819
from openedx.core.djangoapps.notifications.models import NotificationPreference
1920
from openedx.core.djangoapps.notifications.permissions import allow_any_authenticated_user
2021

@@ -241,6 +242,11 @@ def preference_update_from_encrypted_username_view(request, username, patch=""):
241242
View to update user preferences from encrypted username and patch.
242243
username and patch must be string
243244
"""
245+
if is_ratelimited(
246+
request=request, group="unsubscribe", key=username_from_hash,
247+
rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, increment=True,
248+
):
249+
return Response({"error": "Too many requests"}, status=status.HTTP_429_TOO_MANY_REQUESTS)
244250
update_user_preferences_from_patch(username)
245251
return Response({"result": "success"}, status=status.HTTP_200_OK)
246252

openedx/envs/common.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,8 @@ def _make_locale_paths(settings):
828828
DISCUSSION_RATELIMIT = '100/m'
829829
SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS = 0
830830

831+
ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT = '100/m'
832+
831833
LMS_ROOT_URL = None
832834
LMS_INTERNAL_ROOT_URL = Derived(lambda settings: settings.LMS_ROOT_URL)
833835

0 commit comments

Comments
 (0)