From 250c4d53880d5309d2bd6f0376c1e22b4a1ddb68 Mon Sep 17 00:00:00 2001 From: William Boman Date: Tue, 10 Apr 2018 15:22:51 +0200 Subject: [PATCH] add 2 new setting variables for more granular failure limit control (#113) --- README.md | 4 ++++ defender/config.py | 2 ++ defender/tests.py | 47 +++++++++++++++++++++++++++++++++++++++++++--- defender/utils.py | 4 ++-- 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 2ae3c37..755ee95 100644 --- a/README.md +++ b/README.md @@ -323,6 +323,10 @@ These should be defined in your ``settings.py`` file. * ``DEFENDER_LOGIN_FAILURE_LIMIT``: Int: The number of login attempts allowed before a record is created for the failed logins. [Default: ``3``] +* ``DEFENDER_LOGIN_FAILURE_LIMIT_USERNAME``: Int: The number of login attempts allowed +on a username before a record is created for the failed logins. [Default: ``DEFENDER_LOGIN_FAILURE_LIMIT``] +* ``DEFENDER_LOGIN_FAILURE_LIMIT_IP``: Int: The number of login attempts allowed +from an IP before a record is created for the failed logins. [Default: ``DEFENDER_LOGIN_FAILURE_LIMIT``] * ``DEFENDER_BEHIND_REVERSE_PROXY``: Boolean: Is defender behind a reverse proxy? [Default: ``False``] * ``DEFENDER_REVERSE_PROXY_HEADER``: String: the name of the http header with your diff --git a/defender/config.py b/defender/config.py index a612d03..9e59c16 100644 --- a/defender/config.py +++ b/defender/config.py @@ -18,6 +18,8 @@ MOCK_REDIS = get_setting('DEFENDER_MOCK_REDIS', False) # see if the user has overridden the failure limit FAILURE_LIMIT = get_setting('DEFENDER_LOGIN_FAILURE_LIMIT', 3) +USERNAME_FAILURE_LIMIT = get_setting('DEFENDER_LOGIN_FAILURE_LIMIT_USERNAME', FAILURE_LIMIT) +IP_FAILURE_LIMIT = get_setting('DEFENDER_LOGIN_FAILURE_LIMIT_IP', FAILURE_LIMIT) # If this is True, the lockout checks to evaluate if the IP failure limit and # the username failure limit has been reached before issuing the lockout. diff --git a/defender/tests.py b/defender/tests.py index 11f2d81..b9127ed 100644 --- a/defender/tests.py +++ b/defender/tests.py @@ -173,6 +173,47 @@ class AccessAttemptTest(DefenderTestCase): response = self.client.get(ADMIN_LOGIN_URL) self.assertContains(response, self.LOCKED_MESSAGE) + @patch('defender.config.USERNAME_FAILURE_LIMIT', 3) + def test_username_failure_limit(self): + """ Tests that the username failure limit setting is + respected when trying to login one more time than failure limit + """ + for i in range(0, config.USERNAME_FAILURE_LIMIT): + ip = '74.125.239.{0}.'.format(i) + response = self._login(username=VALID_USERNAME, remote_addr=ip) + # Check if we are in the same login page + self.assertContains(response, LOGIN_FORM_KEY) + + # So, we shouldn't have gotten a lock-out yet. + # But we should get one now + response = self._login(username=VALID_USERNAME, remote_addr=ip) + self.assertContains(response, self.LOCKED_MESSAGE) + + # doing a get should not get locked out message + response = self.client.get(ADMIN_LOGIN_URL) + self.assertContains(response, LOGIN_FORM_KEY) + + @patch('defender.config.IP_FAILURE_LIMIT', 3) + def test_ip_failure_limit(self): + """ Tests that the IP failure limit setting is + respected when trying to login one more time than failure limit + """ + for i in range(0, config.IP_FAILURE_LIMIT): + username = 'john-doe__%d' % i + response = self._login(username=username) + # Check if we are in the same login page + self.assertContains(response, LOGIN_FORM_KEY) + + # So, we shouldn't have gotten a lock-out yet. + # But we should get one now + response = self._login(username=VALID_USERNAME) + self.assertContains(response, self.LOCKED_MESSAGE) + + # doing a get should also get locked out message + response = self.client.get(ADMIN_LOGIN_URL) + self.assertContains(response, self.LOCKED_MESSAGE) + + def test_valid_login(self): """ Tests a valid login for a real username """ @@ -787,7 +828,7 @@ class AccessAttemptTest(DefenderTestCase): self.assertEqual(data_out, []) @patch('defender.config.BEHIND_REVERSE_PROXY', True) - @patch('defender.config.FAILURE_LIMIT', 3) + @patch('defender.config.IP_FAILURE_LIMIT', 3) def test_login_blocked_for_non_standard_login_views_without_msg(self): """ Check that a view wich returns the expected status code is causing @@ -819,7 +860,7 @@ class AccessAttemptTest(DefenderTestCase): self.assertEqual(data_out, ['192.168.24.24']) @patch('defender.config.BEHIND_REVERSE_PROXY', True) - @patch('defender.config.FAILURE_LIMIT', 3) + @patch('defender.config.IP_FAILURE_LIMIT', 3) def test_login_blocked_for_non_standard_login_views_with_msg(self): """ Check that a view wich returns the expected status code and the @@ -850,7 +891,7 @@ class AccessAttemptTest(DefenderTestCase): self.assertEqual(data_out, ['192.168.24.24']) @patch('defender.config.BEHIND_REVERSE_PROXY', True) - @patch('defender.config.FAILURE_LIMIT', 3) + @patch('defender.config.IP_FAILURE_LIMIT', 3) def test_login_non_blocked_for_non_standard_login_views_different_msg(self): """ Check that a view wich returns the expected status code but not the diff --git a/defender/utils.py b/defender/utils.py index b94dba8..3299862 100644 --- a/defender/utils.py +++ b/defender/utils.py @@ -200,7 +200,7 @@ def record_failed_attempt(ip_address, username): # we only want to increment the IP if this is disabled. ip_count = increment_key(get_ip_attempt_cache_key(ip_address)) # if over the limit, add to block - if ip_count > config.FAILURE_LIMIT: + if ip_count > config.IP_FAILURE_LIMIT: block_ip(ip_address) ip_block = True @@ -208,7 +208,7 @@ def record_failed_attempt(ip_address, username): if username and not config.DISABLE_USERNAME_LOCKOUT: user_count = increment_key(get_username_attempt_cache_key(username)) # if over the limit, add to block - if user_count > config.FAILURE_LIMIT: + if user_count > config.USERNAME_FAILURE_LIMIT: block_username(username) user_block = True