From aab2d3f02832a85cfb4114826d18e563bd30848b Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul Date: Tue, 26 Aug 2025 09:47:25 +0500 Subject: [PATCH 1/4] feat: added rate limit on one click unsubscribe api --- .../djangoapps/notifications/email/utils.py | 15 ++++++++++++++- .../notifications/tests/test_views.py | 18 ++++++++++++++++++ openedx/core/djangoapps/notifications/views.py | 8 +++++++- openedx/envs/common.py | 2 ++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/notifications/email/utils.py b/openedx/core/djangoapps/notifications/email/utils.py index 3ce2590d1859..0bb778a5d681 100644 --- a/openedx/core/djangoapps/notifications/email/utils.py +++ b/openedx/core/djangoapps/notifications/email/utils.py @@ -12,7 +12,7 @@ from waffle import get_waffle_flag_model # pylint: disable=invalid-django-waffle-import from lms.djangoapps.branding.api import get_logo_url_for_email -from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher +from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher, UsernameDecryptionException from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.notifications.base_notification import COURSE_NOTIFICATION_APPS, COURSE_NOTIFICATION_TYPES from openedx.core.djangoapps.notifications.config.waffle import ENABLE_EMAIL_NOTIFICATIONS @@ -384,6 +384,19 @@ def decrypt_string(string): return UsernameCipher.decrypt(string).decode() +def username_from_hash(group, request): + """ + Django ratelimit key to return username from hash + """ + username = request.resolver_match.kwargs.get("username") + if username: + try: + return decrypt_string(username) + except UsernameDecryptionException: + pass + return None + + def update_user_preferences_from_patch(encrypted_username): """ Decrypt username and patch and updates user preferences diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index de02141745e8..7b015b69bbad 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -7,6 +7,7 @@ import ddt from django.conf import settings from django.contrib.auth import get_user_model +from django.core.cache import cache from django.test.utils import override_settings from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag @@ -481,6 +482,7 @@ def setUp(self): """ Setup test case """ + cache.clear() super().setUp() password = 'password' self.user = UserFactory(password=password) @@ -488,6 +490,22 @@ def setUp(self): self.course = CourseFactory.create(display_name='test course 1', run="Testing_course_1") CourseNotificationPreference(course_id=self.course.id, user=self.user).save() + @override_settings(LMS_BASE="example.com", ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT='1/d') + def test_rate_limit_on_unsub(self): + """ + Test rate limit on unsub + """ + self.client.logout() + user_hash = encrypt_string(self.user.username) + url_params = { + "username": user_hash, + } + url = reverse("preference_update_view", kwargs=url_params) + response = self.client.get(url) + assert response.status_code == status.HTTP_200_OK + response = self.client.get(url) + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + @override_settings(LMS_BASE="") @ddt.data('get', 'post') def test_if_preference_is_updated(self, request_type): diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index d3a9dd1f48e2..57ef88cde1ef 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -5,6 +5,7 @@ from django.conf import settings from django.db.models import Count +from django_ratelimit.core import is_ratelimited from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as _ from pytz import UTC @@ -14,7 +15,7 @@ from rest_framework.response import Response from rest_framework.views import APIView -from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch +from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch, username_from_hash from openedx.core.djangoapps.notifications.models import NotificationPreference from openedx.core.djangoapps.notifications.permissions import allow_any_authenticated_user @@ -241,6 +242,11 @@ def preference_update_from_encrypted_username_view(request, username, patch=""): View to update user preferences from encrypted username and patch. username and patch must be string """ + if is_ratelimited( + request=request, group="unsubscribe", key=username_from_hash, + rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, increment=True, + ): + return Response({"error": "Too many requests"}, status=status.HTTP_429_TOO_MANY_REQUESTS) update_user_preferences_from_patch(username) return Response({"result": "success"}, status=status.HTTP_200_OK) diff --git a/openedx/envs/common.py b/openedx/envs/common.py index 767bfdbd3ee9..c888ec8fd73f 100644 --- a/openedx/envs/common.py +++ b/openedx/envs/common.py @@ -828,6 +828,8 @@ def _make_locale_paths(settings): DISCUSSION_RATELIMIT = '100/m' SKIP_RATE_LIMIT_ON_ACCOUNT_AFTER_DAYS = 0 +ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT = '100/m' + LMS_ROOT_URL = None LMS_INTERNAL_ROOT_URL = Derived(lambda settings: settings.LMS_ROOT_URL) From aaf96e427adbe901e7a74204d990fe13486010f5 Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul Date: Tue, 26 Aug 2025 10:23:57 +0500 Subject: [PATCH 2/4] fix: fixed failing test --- openedx/core/djangoapps/notifications/tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index 7b015b69bbad..d5b2237303dc 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -504,7 +504,7 @@ def test_rate_limit_on_unsub(self): response = self.client.get(url) assert response.status_code == status.HTTP_200_OK response = self.client.get(url) - assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS @override_settings(LMS_BASE="") @ddt.data('get', 'post') From a8c6516b3484562f67e405a59c10bfb86038a8ac Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul Date: Tue, 26 Aug 2025 12:36:28 +0500 Subject: [PATCH 3/4] chore: raise 400 error on invalid username --- openedx/core/djangoapps/notifications/email/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/notifications/email/utils.py b/openedx/core/djangoapps/notifications/email/utils.py index 0bb778a5d681..413f4f99facf 100644 --- a/openedx/core/djangoapps/notifications/email/utils.py +++ b/openedx/core/djangoapps/notifications/email/utils.py @@ -6,6 +6,7 @@ from bs4 import BeautifulSoup from django.conf import settings from django.contrib.auth import get_user_model +from django.core.exceptions import BadRequest from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as _ from pytz import utc @@ -393,7 +394,7 @@ def username_from_hash(group, request): try: return decrypt_string(username) except UsernameDecryptionException: - pass + raise BadRequest("Bad request") return None From b3cebbc11f2424b55ecedf7b8e4a40c8b95becdc Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul Date: Tue, 26 Aug 2025 12:59:11 +0500 Subject: [PATCH 4/4] fix: fixed pylint --- openedx/core/djangoapps/notifications/email/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/notifications/email/utils.py b/openedx/core/djangoapps/notifications/email/utils.py index 413f4f99facf..1f761c086095 100644 --- a/openedx/core/djangoapps/notifications/email/utils.py +++ b/openedx/core/djangoapps/notifications/email/utils.py @@ -393,8 +393,8 @@ def username_from_hash(group, request): if username: try: return decrypt_string(username) - except UsernameDecryptionException: - raise BadRequest("Bad request") + except UsernameDecryptionException as exc: + raise BadRequest("Bad request") from exc return None