From 64c5684c129d367761fe3477a5f62a640cd26648 Mon Sep 17 00:00:00 2001 From: Ken Cochrane Date: Wed, 21 Oct 2015 16:33:08 -0400 Subject: [PATCH] Added so that you can disable IP lockouts if you want --- .travis.yml | 12 ++++++--- CHANGES | 4 +++ README.md | 2 ++ defender/config.py | 5 ++++ defender/tests.py | 63 ++++++++++++++++++++++++++++++++++++++++++++++ defender/utils.py | 56 ++++++++++++++++++++++++++++++++--------- setup.py | 3 ++- 7 files changed, 128 insertions(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index ec4cd1c..ecb4a57 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,12 +5,14 @@ python: - "2.7" - "3.3" - "3.4" + - "3.5" - "pypy" env: - DJANGO=Django==1.6.11 - - DJANGO=Django==1.7.8 - - DJANGO=Django==1.8.2 + - DJANGO=Django==1.7.10 + - DJANGO=Django==1.8.5 + - DJANGO=Django==1.9b1 services: - redis-server @@ -29,9 +31,11 @@ script: matrix: exclude: - python: "2.6" - env: DJANGO=Django==1.7.8 + env: DJANGO=Django==1.7.10 - python: "2.6" - env: DJANGO=Django==1.8.2 + env: DJANGO=Django==1.8.5 + - python: "2.6" + env: DJANGO=Django==1.9b1 after_success: - coveralls --verbose diff --git a/CHANGES b/CHANGES index 036c62a..93786f7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +0.4.0 +===== +added ``DEFENDER_DISABLE_IP_LOCKOUT`` and added support for Python 3.5 + 0.3.2 ===== added ``DEFENDER_LOCK_OUT_BY_IP_AND_USERNAME``, and changed settings to support diff --git a/README.md b/README.md index 324683b..098fc61 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ Sites using Defender: Versions ======== +- 0.4.0 - added ``DEFENDER_DISABLE_IP_LOCKOUT`` and added support for Python 3.5 - 0.3.2 - added ``DEFENDER_LOCK_OUT_BY_IP_AND_USERNAME``, and changed settings to support django 1.8. - 0.3.1 - fixed the management command name @@ -306,6 +307,7 @@ record is created for the failed logins. [Default: ``3``] * ``DEFENDER_BEHIND_REVERSE_PROXY``: Boolean: Is defender behind a reverse proxy? [Default: ``False``] * ``DEFENDER_LOCK_OUT_BY_IP_AND_USERNAME``: Boolean: Locks a user out based on a combination of IP and Username. This stops a user denying access to the application for all other users accessing the app from behind the same IP address. [Default: ``False``] +* ``DEFENDER_DISABLE_IP_LOCKOUT``: Boolean: If this is True, it will not lockout the users IP address, it will only lockout the username. [Default: False] * ``DEFENDER_COOLOFF_TIME``: Int: If set, defines a period of inactivity after which old failed login attempts will be forgotten. An integer, will be interpreted as a number of seconds. If ``0``, the locks will not expire. [Default: ``300``] diff --git a/defender/config.py b/defender/config.py index a0c66a3..e43a570 100644 --- a/defender/config.py +++ b/defender/config.py @@ -15,9 +15,14 @@ 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) +# 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. LOCKOUT_BY_IP_USERNAME = get_setting( 'DEFENDER_LOCK_OUT_BY_IP_AND_USERNAME', False) +# if this is True, The users IP address will not get locked when there are too many login attempts. +DISABLE_IP_LOCKOUT = get_setting('DEFENDER_DISABLE_IP_LOCKOUT', False) + # use a specific username field to retrieve from login POST data USERNAME_FORM_FIELD = get_setting('DEFENDER_USERNAME_FORM_FIELD', 'username') diff --git a/defender/tests.py b/defender/tests.py index 34cd4cb..d362359 100644 --- a/defender/tests.py +++ b/defender/tests.py @@ -85,6 +85,11 @@ class AccessAttemptTest(DefenderTestCase): data_out = utils.get_blocked_ips() self.assertEqual(sorted(data_in), sorted(data_out)) + # send in None, should have same values. + utils.block_ip(None) + data_out = utils.get_blocked_ips() + self.assertEqual(sorted(data_in), sorted(data_out)) + def test_data_integrity_of_get_blocked_usernames(self): """ Test whether data retrieved from redis via get_blocked_usernames() is the same as the data saved @@ -95,6 +100,11 @@ class AccessAttemptTest(DefenderTestCase): data_out = utils.get_blocked_usernames() self.assertEqual(sorted(data_in), sorted(data_out)) + # send in None, should have same values. + utils.block_username(None) + data_out = utils.get_blocked_usernames() + self.assertEqual(sorted(data_in), sorted(data_out)) + def test_login_get(self): """ visit the login page """ response = self.client.get(ADMIN_LOGIN_URL) @@ -594,6 +604,59 @@ class AccessAttemptTest(DefenderTestCase): # Check if we are in the same login page self.assertContains(response, LOGIN_FORM_KEY) + @patch('defender.config.DISABLE_IP_LOCKOUT', True) + def test_disable_ip_lockout(self): + """Check that lockout still works when we disable IP Lock out""" + + username = 'testy' + + # try logging in with the same IP, but different username + # we shouldn't be blocked. + # same IP different, usernames + ip = '74.125.239.60' + for i in range(0, config.FAILURE_LIMIT+10): + login_username = u"{}{}".format(username, i) + response = self._login(username=login_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. + # same username with same IP + for i in range(0, config.FAILURE_LIMIT): + response = self._login(username=username) + # Check if we are in the same login page + self.assertContains(response, LOGIN_FORM_KEY) + + # But we should get one now + # same username and Ip, over the limit for username. + response = self._login(username=username) + self.assertContains(response, self.LOCKED_MESSAGE) + + # We shouldn't get a lockout message when attempting to use no username + response = self.client.get(ADMIN_LOGIN_URL) + self.assertContains(response, LOGIN_FORM_KEY) + + # We shouldn't get a lockout message when attempting to use a different username + response = self._login() + self.assertContains(response, LOGIN_FORM_KEY) + + # We shouldn't get a lockout message when attempting to use a different ip address + second_ip = '74.125.239.99' + response = self._login(username=VALID_USERNAME, remote_addr=second_ip) + # Check if we are in the same login page + self.assertContains(response, LOGIN_FORM_KEY) + + # we should have no ip's blocked + data_out = utils.get_blocked_ips() + self.assertEqual(data_out, []) + + # even if we try to manually block one it still won't be in there. + utils.block_ip(second_ip) + + # we should still have no ip's blocked + data_out = utils.get_blocked_ips() + self.assertEqual(data_out, []) + class DefenderTestCaseTest(DefenderTestCase): """Make sure that we're cleaning the cache between tests""" diff --git a/defender/utils.py b/defender/utils.py index dbd40b8..5355f98 100644 --- a/defender/utils.py +++ b/defender/utils.py @@ -87,6 +87,9 @@ def strip_keys(key_list): def get_blocked_ips(): """ get a list of blocked ips from redis """ + if config.DISABLE_IP_LOCKOUT: + # There are no blocked IP's since we disabled them. + return [] key = get_ip_blocked_cache_key("*") key_list = [redis_key.decode('utf-8') for redis_key in REDIS_SERVER.keys(key)] @@ -139,6 +142,9 @@ def block_ip(ip_address): if not ip_address: # no reason to continue when there is no ip return + if config.DISABLE_IP_LOCKOUT: + # no need to block, we disabled it. + return key = get_ip_blocked_cache_key(ip_address) if config.COOLOFF_TIME: REDIS_SERVER.set(key, 'blocked', config.COOLOFF_TIME) @@ -162,23 +168,40 @@ def record_failed_attempt(ip_address, username): """ record the failed login attempt, if over limit return False, if not over limit return True """ # increment the failed count, and get current number - ip_count = increment_key(get_ip_attempt_cache_key(ip_address)) + ip_block = False + if not config.DISABLE_IP_LOCKOUT: + # 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: + block_ip(ip_address) + ip_block = True + + user_block = False user_count = increment_key(get_username_attempt_cache_key(username)) - ip_block = False - user_block = False - # if either are over the limit, add to block - if ip_count > config.FAILURE_LIMIT: - block_ip(ip_address) - ip_block = True + # if over the limit, add to block if user_count > config.FAILURE_LIMIT: block_username(username) user_block = True + # if we have this turned on, then there is no reason to look at ip_block + # we will just look at user_block, and short circut the result since + # we don't need to continue. + if config.DISABLE_IP_LOCKOUT: + # if user_block is True, it means it was blocked + # we need to return False + return not user_block + + # we want to make sure both the IP and user is blocked before we return False + # this is mostly used when a lot of your users are using proxies, and you + # don't want one user to block everyone on that one IP. if config.LOCKOUT_BY_IP_USERNAME: + # both ip_block and user_block need to be True in order + # to return a False. return not (ip_block and user_block) - # if any blocks return False, no blocks return True + # if any blocks return False, no blocks. return True return not (ip_block or user_block) @@ -243,15 +266,24 @@ def lockout_response(request): def is_already_locked(request): """ Is this IP/username already locked? """ - ip_address = get_ip(request) - username = request.POST.get(config.USERNAME_FORM_FIELD, None) - # ip blocked? - ip_blocked = REDIS_SERVER.get(get_ip_blocked_cache_key(ip_address)) + if not config.DISABLE_IP_LOCKOUT: + # ip blocked? + ip_address = get_ip(request) + ip_blocked = REDIS_SERVER.get(get_ip_blocked_cache_key(ip_address)) + else: + # we disabled ip lockout, so it will never be blocked. + ip_blocked = False # username blocked? + username = request.POST.get(config.USERNAME_FORM_FIELD, None) user_blocked = REDIS_SERVER.get(get_username_blocked_cache_key(username)) + if config.DISABLE_IP_LOCKOUT: + # if DISABLE_IP_LOCKOUT is turned on, then return user_blocked + # result, since we don't care about ip at this point. + return user_blocked + if config.LOCKOUT_BY_IP_USERNAME: LOG.info("Block by ip & username") if ip_blocked and user_blocked: diff --git a/setup.py b/setup.py index 5a26aa4..f9d558e 100644 --- a/setup.py +++ b/setup.py @@ -9,7 +9,7 @@ except ImportError: from distutils.core import setup -version = '0.3.2' +version = '0.4.0' def get_packages(package): @@ -54,6 +54,7 @@ setup(name='django-defender', 'Programming Language :: Python :: 2.7', 'Programming Language :: Python :: 3.3', 'Programming Language :: Python :: 3.4', + 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: Implementation :: PyPy', 'Programming Language :: Python :: Implementation :: CPython', 'Topic :: Internet :: WWW/HTTP :: Dynamic Content',