diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 147f4ddfee..e4b94cdbb6 100644 --- a/frappe/core/doctype/user/user.py +++ b/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' diff --git a/frappe/rate_limiter.py b/frappe/rate_limiter.py index 2772d4b0e0..b49c1e500e 100644 --- a/frappe/rate_limiter.py +++ b/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: diff --git a/frappe/website/doctype/web_form/test_web_form.py b/frappe/website/doctype/web_form/test_web_form.py index befad6a783..91ee4195df 100644 --- a/frappe/website/doctype/web_form/test_web_form.py +++ b/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", diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index bd14125403..eb7ee90826 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/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))