Merge pull request #14087 from shadrak98/rate-limiting

feat: Introduce rate-limiting for web forms

frappe/core/doctype/user/user.py

@@ -788,7 +788,7 @@ def sign_up(email, full_name, redirect_to):
 			return 2, _("Please ask your administrator to verify your sign-up")
 
 @frappe.whitelist(allow_guest=True)
-@rate_limit(key='user', limit=get_password_reset_limit, seconds = 24*60*60, methods=['POST'])
+@rate_limit(limit=get_password_reset_limit, seconds = 24*60*60, methods=['POST'])
 def reset_password(user):
 	if user=="Administrator":
 		return 'not allowed'

frappe/rate_limiter.py

@@ -82,19 +82,21 @@ class RateLimiter:
 		if self.rejected:
 			return Response(_("Too Many Requests"), status=429)
 
-def rate_limit(key: str, limit: Union[int, Callable] = 5, seconds: int= 24*60*60, methods: Union[str, list]='ALL'):
+def rate_limit(key: str = None, limit: Union[int, Callable] = 5, seconds: int = 24*60*60, methods: Union[str, list] = 'ALL', ip_based: bool = True):
 	"""Decorator to rate limit an endpoint.
 
 	This will limit Number of requests per endpoint to `limit` within `seconds`.
 	Uses redis cache to track request counts.
 
-	:param key: Key is used to identify the requests uniqueness
+	:param key: Key is used to identify the requests uniqueness (Optional)
 	:param limit: Maximum number of requests to allow with in window time
 	:type limit: Callable or Integer
 	:param seconds: window time to allow requests
 	:param methods: Limit the validation for these methods.
 		`ALL` is a wildcard that applies rate limit on all methods.
 	:type methods: string or list or tuple
+	:param ip_based: flag to allow ip based rate-limiting
+	:type ip_based: Boolean
 
 	:returns: a decorator function that limit the number of requests per endpoint
 	"""
@@ -102,17 +104,30 @@ def rate_limit(key: str, limit: Union[int, Callable] = 5, seconds: int= 24*60*60
 		@wraps(fun)
 		def wrapper(*args, **kwargs):
 			# Do not apply rate limits if method is not opted to check
-			if methods != 'ALL' and frappe.request.method.upper() not in methods:
+			if methods != 'ALL' and frappe.request and frappe.request.method and frappe.request.method.upper() not in methods:
 				return frappe.call(fun, **frappe.form_dict or kwargs)
 
 			_limit = limit() if callable(limit) else limit
 
-			identity = frappe.form_dict[key]
+			ip = frappe.local.request_ip if ip_based is True else None
+
+			user_key = frappe.form_dict[key] if key else None
+
+			identity = None
+
+			if key and ip_based:
+				identity = ":".join([ip, user_key])
+
+			identity = identity or ip or user_key
+
+			if not identity:
+				frappe.throw(_('Either key or IP flag is required.'))
+
 			cache_key = f"rl:{frappe.form_dict.cmd}:{identity}"
 
-			value = frappe.cache().get_value(cache_key, expires=True) or 0
+			value = frappe.cache().get(cache_key) or 0
 			if not value:
-				frappe.cache().set_value(cache_key, 0, expires_in_sec=seconds)
+				frappe.cache().setex(cache_key, seconds, 0)
 
 			value = frappe.cache().incrby(cache_key, 1)
 			if value > _limit:

frappe/website/doctype/web_form/test_web_form.py

@@ -16,15 +16,26 @@ class TestWebForm(unittest.TestCase):
 	def tearDown(self):
 		frappe.conf.disable_website_cache = False
 		frappe.local.path = None
+		frappe.local.request_ip = None
+		frappe.form_dict.web_form = None
+		frappe.form_dict.data = None
+		frappe.form_dict.docname = None
 
 	def test_accept(self):
 		frappe.set_user("Administrator")
-		accept(web_form='manage-events', data=json.dumps({
+
+		doc = {
 			'doctype': 'Event',
 			'subject': '_Test Event Web Form',
 			'description': '_Test Event Description',
 			'starts_on': '2014-09-09'
-		}))
+		}
+
+		frappe.form_dict.web_form = "manage-events"
+		frappe.form_dict.data = json.dumps(doc)
+		frappe.local.request_ip = '127.0.0.1'
+
+		accept(web_form='manage-events', data=json.dumps(doc))
 
 		self.event_name = frappe.db.get_value("Event",
 			{"subject": "_Test Event Web Form"})
@@ -32,6 +43,7 @@ class TestWebForm(unittest.TestCase):
 
 	def test_edit(self):
 		self.test_accept()
+
 		doc={
 			'doctype': 'Event',
 			'subject': '_Test Event Web Form',
@@ -43,6 +55,10 @@ class TestWebForm(unittest.TestCase):
 		self.assertNotEqual(frappe.db.get_value("Event",
 			self.event_name, "description"), doc.get('description'))
 
+		frappe.form_dict.web_form = 'manage-events'
+		frappe.form_dict.docname = self.event_name
+		frappe.form_dict.data = json.dumps(doc)
+
 		accept(web_form='manage-events', docname=self.event_name, data=json.dumps(doc))
 
 		self.assertEqual(frappe.db.get_value("Event",

frappe/website/doctype/web_form/web_form.py

@@ -13,7 +13,7 @@ from frappe.modules.utils import export_module_json, get_doc_module
 from frappe.utils import cstr
 from frappe.website.utils import get_comment_list
 from frappe.website.website_generator import WebsiteGenerator
-
+from frappe.rate_limiter import rate_limit
 
 class WebForm(WebsiteGenerator):
 	website = frappe._dict(
@@ -365,6 +365,7 @@ def get_context(context):
 
 
 @frappe.whitelist(allow_guest=True)
+@rate_limit(key='web_form', limit=5, seconds=60, methods=['POST'])
 def accept(web_form, data, docname=None, for_payment=False):
 	'''Save the web form'''
 	data = frappe._dict(json.loads(data))