From f4f6d97d06e1a8e3f9301c8d80298339e8494f8f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 17 Oct 2023 16:20:32 +0530 Subject: [PATCH] refactor: make login tracker support arbitrary keys --- frappe/auth.py | 28 ++++++++++++++++++---------- frappe/tests/test_auth.py | 6 ++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/frappe/auth.py b/frappe/auth.py index 15dd2f6cae..35e07236bb 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -19,6 +19,7 @@ from frappe.twofactor import ( should_run_2fa, ) from frappe.utils import cint, date_diff, datetime, get_datetime, today +from frappe.utils.deprecations import deprecation_warning from frappe.utils.password import check_password from frappe.website.utils import get_home_page @@ -440,7 +441,7 @@ def validate_ip_address(user): frappe.throw(_("Access not allowed from this IP Address"), frappe.AuthenticationError) -def get_login_attempt_tracker(user_name: str, raise_locked_exception: bool = True): +def get_login_attempt_tracker(key: str, raise_locked_exception: bool = True): """Get login attempt tracker instance. :param user_name: Name of the loggedin user @@ -454,7 +455,7 @@ def get_login_attempt_tracker(user_name: str, raise_locked_exception: bool = Tru tracker_kwargs["lock_interval"] = sys_settings.allow_login_after_fail tracker_kwargs["max_consecutive_login_attempts"] = sys_settings.allow_consecutive_login_attempts - tracker = LoginAttemptTracker(user_name, **tracker_kwargs) + tracker = LoginAttemptTracker(key, **tracker_kwargs) if raise_locked_exception and track_login_attempts and not tracker.is_user_allowed(): frappe.throw( @@ -473,7 +474,12 @@ class LoginAttemptTracker: """ def __init__( - self, user_name: str, max_consecutive_login_attempts: int = 3, lock_interval: int = 5 * 60 + self, + key: str, + max_consecutive_login_attempts: int = 3, + lock_interval: int = 5 * 60, + *, + user_name: str = None, ): """Initialize the tracker. @@ -481,21 +487,23 @@ class LoginAttemptTracker: :param max_consecutive_login_attempts: Maximum allowed consecutive failed login attempts :param lock_interval: Locking interval incase of maximum failed attempts """ - self.user_name = user_name + if user_name: + deprecation_warning("`username` parameter is deprecated, use `key` instead.") + self.key = key or user_name self.lock_interval = datetime.timedelta(seconds=lock_interval) self.max_failed_logins = max_consecutive_login_attempts @property def login_failed_count(self): - return frappe.cache.hget("login_failed_count", self.user_name) + return frappe.cache.hget("login_failed_count", self.key) @login_failed_count.setter def login_failed_count(self, count): - frappe.cache.hset("login_failed_count", self.user_name, count) + frappe.cache.hset("login_failed_count", self.key, count) @login_failed_count.deleter def login_failed_count(self): - frappe.cache.hdel("login_failed_count", self.user_name) + frappe.cache.hdel("login_failed_count", self.key) @property def login_failed_time(self): @@ -503,15 +511,15 @@ class LoginAttemptTracker: For every user we track only First failed login attempt time within lock interval of time. """ - return frappe.cache.hget("login_failed_time", self.user_name) + return frappe.cache.hget("login_failed_time", self.key) @login_failed_time.setter def login_failed_time(self, timestamp): - frappe.cache.hset("login_failed_time", self.user_name, timestamp) + frappe.cache.hset("login_failed_time", self.key, timestamp) @login_failed_time.deleter def login_failed_time(self): - frappe.cache.hdel("login_failed_time", self.user_name) + frappe.cache.hdel("login_failed_time", self.key) def add_failure_attempt(self): """Log user failure attempts into the system. diff --git a/frappe/tests/test_auth.py b/frappe/tests/test_auth.py index 6aa8d2cc2f..adff36c8f3 100644 --- a/frappe/tests/test_auth.py +++ b/frappe/tests/test_auth.py @@ -161,9 +161,7 @@ class TestAuth(FrappeTestCase): class TestLoginAttemptTracker(FrappeTestCase): def test_account_lock(self): """Make sure that account locks after `n consecutive failures""" - tracker = LoginAttemptTracker( - user_name="tester", max_consecutive_login_attempts=3, lock_interval=60 - ) + tracker = LoginAttemptTracker("tester", max_consecutive_login_attempts=3, lock_interval=60) # Clear the cache by setting attempt as success tracker.add_success_attempt() @@ -183,7 +181,7 @@ class TestLoginAttemptTracker(FrappeTestCase): """Make sure that locked account gets unlocked after lock_interval of time.""" lock_interval = 2 # In sec tracker = LoginAttemptTracker( - user_name="tester", max_consecutive_login_attempts=1, lock_interval=lock_interval + "tester", max_consecutive_login_attempts=1, lock_interval=lock_interval ) # Clear the cache by setting attempt as success tracker.add_success_attempt()