From 1b10e546110b990becced6d63af851b5548b56d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksi=20H=C3=A4kli?= Date: Thu, 6 Apr 2017 19:50:52 +0300 Subject: [PATCH 01/10] Fixed #224 -- Add AXES_NUM_PROXIES setting This enables secure calculation of client IP value by allowing the end users to set the number of proxies they have in their current setups --- axes/decorators.py | 48 +++++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/axes/decorators.py b/axes/decorators.py index e26df06..f72872a 100644 --- a/axes/decorators.py +++ b/axes/decorators.py @@ -51,15 +51,15 @@ def get_ip(request): if BEHIND_REVERSE_PROXY: # For requests originating from behind a reverse proxy, # resolve the IP address from the given AXES_REVERSE_PROXY_HEADER. - # AXES_REVERSE_PROXY_HEADER defaults to HTTP_X_FORWARDED_FOR if not given, - # which is the Django calling format for the HTTP X-Forwarder-For header. + # AXES_REVERSE_PROXY_HEADER defaults to HTTP_X_FORWARDED_FOR, + # which is the Django name for the HTTP X-Forwarder-For header. # Please see RFC7239 for additional information: # https://tools.ietf.org/html/rfc7239#section-5 # The REVERSE_PROXY_HEADER HTTP header is a list # of potentionally unsecure IPs, for example: # X-Forwarded-For: 1.1.1.1, 11.11.11.11:8080, 111.111.111.111 - ip = request.META.get(REVERSE_PROXY_HEADER, '') + ip_str = request.META.get(REVERSE_PROXY_HEADER, '') # We need to know the number of proxies present in the request chain # in order to securely calculate the one IP that is the real client IP. @@ -68,23 +68,45 @@ def get_ip(request): # configurations, with e.g. the X-Forwarded-For header containing # the originating client IP, proxies and possibly spoofed values. # - # If you are using a special header for client calculation such as - # the X-Real-IP or the like with nginx, please check this configuration. + # If you are using a special header for client calculation such as the + # X-Real-IP or the like with nginx, please check this configuration. # # Please see discussion for more information: # https://github.com/jazzband/django-axes/issues/224 - ip = [ip.strip() for ip in ip.split(',')][-NUM_PROXIES] + ip_list = [ip.strip() for ip in ip_str.split(',')] - # Fix IIS adding client port number to 'X-Forwarded-For' header (strip port) - if not is_ipv6(ip): - ip = ip.split(':', 1)[0] + # Pick the nth last IP in the given list of addresses after parsing + if len(ip_list) >= NUM_PROXIES: + ip = ip_list[-NUM_PROXIES] + + # Fix IIS adding client port number to the + # 'X-Forwarded-For' header (strip port) + if not is_ipv6(ip): + ip = ip.split(':', 1)[0] + + # If nth last is not found, default to no IP and raise a warning + else: + ip = '' + raise Warning( + 'AXES: Axes is configured for operation behind a ' + 'reverse proxy but received too few IPs in the HTTP ' + 'AXES_REVERSE_PROXY_HEADER. Check your ' + 'AXES_NUM_PROXIES configuration. ' + 'Header name: {0}, value: {1}'.format( + REVERSE_PROXY_HEADER, ip_str + ) + ) if not ip: raise Warning( - 'AXES: Axes is configured for operation behind a reverse proxy ' - 'but could not find an HTTP header value. Check your proxy ' - 'server settings to make sure this header value is being ' - 'passed. Header name {0}'.format(REVERSE_PROXY_HEADER) + 'AXES: Axes is configured for operation behind a reverse ' + 'proxy but could not find a suitable IP in the specified ' + 'HTTP header. Check your proxy server settings to make ' + 'sure correct headers are being passed to Django in ' + 'AXES_REVERSE_PROXY_HEADER. ' + 'Header name: {0}, value: {1}'.format( + REVERSE_PROXY_HEADER, ip_str + ) ) return ip From 919df8ebf77e3745f86560394997271aa5121771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksi=20H=C3=A4kli?= Date: Thu, 6 Apr 2017 20:03:10 +0300 Subject: [PATCH 02/10] Add tests for proxy number parametrization --- .travis.yml | 1 + axes/test_settings_num_proxies.py | 5 +++++ axes/tests.py | 30 ++++++++++++++++++++++++++++++ runtests_num_proxies.py | 8 ++++++++ 4 files changed, 44 insertions(+) create mode 100644 axes/test_settings_num_proxies.py create mode 100644 runtests_num_proxies.py diff --git a/.travis.yml b/.travis.yml index 5df10d4..f488e0e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,7 @@ install: script: - coverage run -a --source=axes runtests.py - coverage run -a --source=axes runtests_proxy.py +- coverage run -a --source=axes runtests_num_proxies.py - coverage run -a --source=axes runtests_proxy_custom_header.py - coverage report after_success: diff --git a/axes/test_settings_num_proxies.py b/axes/test_settings_num_proxies.py new file mode 100644 index 0000000..53f3166 --- /dev/null +++ b/axes/test_settings_num_proxies.py @@ -0,0 +1,5 @@ +from .test_settings import * + +AXES_BEHIND_REVERSE_PROXY = True +AXES_REVERSE_PROXY_HEADER = 'HTTP_X_FORWARDED_FOR' +AXES_NUM_PROXIES = 2 diff --git a/axes/tests.py b/axes/tests.py index 1c32c6d..4849b2c 100644 --- a/axes/tests.py +++ b/axes/tests.py @@ -538,3 +538,33 @@ class GetIPProxyCustomHeaderTest(TestCase): for header in valid_headers: self.request.META[settings.AXES_REVERSE_PROXY_HEADER] = header self.assertEqual(self.ip, get_ip(self.request)) + +class GetIPNumProxiesTest(TestCase): + """Test that get_ip returns the correct last IP when NUM_PROXIES is configured + """ + + def setUp(self): + self.request = MockRequest() + + def test_header_ordering(self): + self.ip = '2.2.2.2' + + valid_headers = [ + '4.4.4.4, 3.3.3.3, 2.2.2.2, 1.1.1.1', + ' 3.3.3.3, 2.2.2.2, 1.1.1.1', + ' 2.2.2.2, 1.1.1.1', + ] + + for header in valid_headers: + self.request.META[settings.AXES_REVERSE_PROXY_HEADER] = header + self.assertEqual(self.ip, get_ip(self.request)) + + def test_invalid_headers_too_few(self): + self.request.META[settings.AXES_REVERSE_PROXY_HEADER] = '1.1.1.1' + with self.assertRaises(Warning): + get_ip(self.request) + + def test_invalid_headers_no_ip(self): + self.request.META[settings.AXES_REVERSE_PROXY_HEADER] = '' + with self.assertRaises(Warning): + get_ip(self.request) diff --git a/runtests_num_proxies.py b/runtests_num_proxies.py new file mode 100644 index 0000000..a3dbba8 --- /dev/null +++ b/runtests_num_proxies.py @@ -0,0 +1,8 @@ +#!/usr/bin/env python + +from runtests import run_tests + +if __name__ == '__main__': + run_tests('axes.test_settings_num_proxies', [ + 'axes.tests.GetIPNumProxiesTest', + ]) From 9de8b356a656ee97081d139db013ef7e6c956b5c Mon Sep 17 00:00:00 2001 From: Jack Sullivan Date: Sat, 22 Apr 2017 18:15:28 -0700 Subject: [PATCH 03/10] Using @patch instead of @override_settings Axes configuration values are pulled from axes.settings, into axes.decorators. Using @override_settings wasn't setting AXES_ONLY_USER_FAILURES. Patching the decorator in the test set the value correctly. --- axes/tests.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/axes/tests.py b/axes/tests.py index 1c32c6d..b45cdf7 100644 --- a/axes/tests.py +++ b/axes/tests.py @@ -9,7 +9,6 @@ from mock import patch from django.conf import settings from django.test import TestCase -from django.test.utils import override_settings from django.contrib.auth.models import User from django.core.urlresolvers import NoReverseMatch from django.core.urlresolvers import reverse @@ -286,7 +285,7 @@ class AccessAttemptTest(TestCase): response = self._login(is_valid_username=True, is_valid_password=False) self.assertContains(response, self.LOCKED_MESSAGE, status_code=403) - @override_settings(AXES_ONLY_USER_FAILURES=True) + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) @patch('axes.decorators.cache.set', return_value=None) @patch('axes.decorators.cache.get', return_value=None) def test_lockout_by_user_only(self, cache_set_mock, cache_get_mock): From fb205cc95c3cf22bf99ad51523b459d51d9b14f5 Mon Sep 17 00:00:00 2001 From: Jack Sullivan Date: Sat, 22 Apr 2017 18:48:31 -0700 Subject: [PATCH 04/10] Test blocking configs, without the cache enabled. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added 12 tests that verify lockouts for default, AXES_ONLY_USER_FAILURES, and LOCK_OUT_BY_COMBINATION_USER_AND_IP settings, under four conditions each: same/different user, and same/different IP address. Truth Table: ¦ ¦ ¦ ¦ ¦ ¦ ¦User IP Action ¦ ¦ ¦ ¦ ¦ ¦|-------------------------------- IP Only | Same Same Block (Default) | Same Different Allow ¦ ¦ ¦ ¦ ¦ ¦| Different Same Block ¦ ¦ ¦ ¦ ¦ ¦| Different Different Allow ¦ ¦ ¦ ¦ ¦ ¦|-------------------------------- User Only | Same Same Block ¦ ¦ ¦ ¦ ¦ ¦| Same Different Block ¦ ¦ ¦ ¦ ¦ ¦| Different Same Allow ¦ ¦ ¦ ¦ ¦ ¦| Different Different Allow ¦ ¦ ¦ ¦ ¦ ¦|-------------------------------- User and IP | Same Same Block ¦ ¦ ¦ ¦ ¦ ¦| Same Different Allow ¦ ¦ ¦ ¦ ¦ ¦| Different Same Allow ¦ ¦ ¦ ¦ ¦ ¦| Different Different Allow --- axes/tests.py | 281 ++++++++++++++++++++++++++++++++++++++++++++++++++ runtests.py | 1 + 2 files changed, 282 insertions(+) diff --git a/axes/tests.py b/axes/tests.py index b45cdf7..0840858 100644 --- a/axes/tests.py +++ b/axes/tests.py @@ -443,6 +443,287 @@ class AccessAttemptTest(TestCase): self.assertEqual(response.status_code, 200) +class AccessAttemptConfigTest(TestCase): + """ This set of tests checks for lockouts under different configurations + and circumstances to prevent false positives and false negatives. + Always block attempted logins for the same user from the same IP. + Always allow attempted logins for a different user from a different IP. + """ + + IP_1 = '10.1.1.1' + IP_2 = '10.2.2.2' + USER_1 = 'valid-user-1' + USER_2 = 'valid-user-2' + VALID_PASSWORD = 'valid-password' + WRONG_PASSWORD = 'wrong-password' + LOCKED_MESSAGE = 'Account locked: too many login attempts.' + LOGIN_FORM_KEY = '' + ALLOWED = 302 + BLOCKED = 403 + + def _login(self, username, password, ip_addr='127.0.0.1', + is_json=False, **kwargs): + """Login a user and get the response. + IP address can be configured to test IP blocking functionality. + """ + try: + admin_login = reverse('admin:login') + except NoReverseMatch: + admin_login = reverse('admin:index') + + headers = { + 'user_agent': 'test-browser' + } + post_data = { + 'username': username, + 'password': password, + 'this_is_the_login_form': 1, + } + post_data.update(kwargs) + + if is_json: + headers.update({ + 'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest', + 'content_type': 'application/json', + }) + post_data = json.dumps(post_data) + + response = self.client.post( + admin_login, post_data, REMOTE_ADDR=ip_addr, **headers + ) + return response + + def _lockout_user1_from_ip1(self): + for i in range(1, FAILURE_LIMIT+1): + response = self._login( + username=self.USER_1, + password=self.WRONG_PASSWORD, + ip_addr=self.IP_1 + ) + return response + + def setUp(self): + """Create two valid users for authentication. + """ + + self.user = User.objects.create_superuser( + username=self.USER_1, + email='test_1@example.com', + password=self.VALID_PASSWORD, + ) + self.user = User.objects.create_superuser( + username=self.USER_2, + email='test_2@example.com', + password=self.VALID_PASSWORD, + ) + + # Test for true and false positives when blocking by IP *OR* user (default). + # Cache disabled. Default settings. + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_ip_blocks_when_same_user_same_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 is still blocked from IP 1. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_ip_allows_when_same_user_diff_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 can still login from IP 2. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_ip_blocks_when_diff_user_same_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 is also locked out from IP 1. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_ip_allows_when_diff_user_diff_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 2. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + # Test for true and false positives when blocking by user only. + # Cache disabled. When AXES_ONLY_USER_FAILURES = True + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_user_blocks_when_same_user_same_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 is still blocked from IP 1. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_user_blocks_when_same_user_diff_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 is also locked out from IP 2. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_user_allows_when_diff_user_same_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 1. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_user_allows_when_diff_user_diff_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 2. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + # Test for true and false positives when blocking by user and IP together. + # Cache disabled. When LOCK_OUT_BY_COMBINATION_USER_AND_IP = True + @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_user_and_ip_blocks_when_same_user_same_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 is still blocked from IP 1. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_user_and_ip_allows_when_same_user_diff_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 can still login from IP 2. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_user_and_ip_allows_when_diff_user_same_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 1. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_user_and_ip_allows_when_diff_user_diff_ip_without_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 2. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + class UtilsTest(TestCase): def test_iso8601(self): """Tests iso8601 correctly translates datetime.timdelta to ISO 8601 diff --git a/runtests.py b/runtests.py index f76e943..085472a 100755 --- a/runtests.py +++ b/runtests.py @@ -20,5 +20,6 @@ def run_tests(settings_module, *modules): if __name__ == '__main__': run_tests('axes.test_settings', [ 'axes.tests.AccessAttemptTest', + 'axes.tests.AccessAttemptConfigTest', 'axes.tests.UtilsTest', ]) From ad170dabcbb54b1a8c51c0dcfa57e07c4ae2c1c7 Mon Sep 17 00:00:00 2001 From: Jack Sullivan Date: Sat, 22 Apr 2017 18:53:59 -0700 Subject: [PATCH 05/10] ONLY_USER works when cache is disabled The _get_user_attempts function now checks for AXES_ONLY_USER_FAILURES, and only includes the IP when AXES_ONLY_USER_FAILURES = False. --- axes/decorators.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/axes/decorators.py b/axes/decorators.py index e26df06..2099013 100644 --- a/axes/decorators.py +++ b/axes/decorators.py @@ -171,11 +171,18 @@ def _get_user_attempts(request): ) if not attempts: - params = {'ip_address': ip, 'trusted': False} + params = {'trusted': False} + + if AXES_ONLY_USER_FAILURES: + params['username'] = username + elif LOCK_OUT_BY_COMBINATION_USER_AND_IP: + params['username'] = username + params['ip_address'] = ip + else: + params['ip_address'] = ip + if USE_USER_AGENT: params['user_agent'] = ua - if LOCK_OUT_BY_COMBINATION_USER_AND_IP: - params['username'] = username attempts = AccessAttempt.objects.filter(**params) From 1ed448d02f8427e5fee0ef05e60241c660d0880a Mon Sep 17 00:00:00 2001 From: Jack Sullivan Date: Sat, 22 Apr 2017 18:59:32 -0700 Subject: [PATCH 06/10] Test blocking configs, using the cache. Added 12 tests that verify lockouts for default, AXES_ONLY_USER_FAILURES, and LOCK_OUT_BY_COMBINATION_USER_AND_IP settings, under four conditions each: same/different user, and same/different IP address. These tests verify the cache functionality. --- axes/tests.py | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) diff --git a/axes/tests.py b/axes/tests.py index 0840858..f0e995b 100644 --- a/axes/tests.py +++ b/axes/tests.py @@ -723,6 +723,188 @@ class AccessAttemptConfigTest(TestCase): ) self.assertEqual(response.status_code, self.ALLOWED) + # Test for true and false positives when blocking by IP *OR* user (default). + # With cache enabled. Default criteria. + def test_lockout_by_ip_blocks_when_same_user_same_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 is still blocked from IP 1. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + def test_lockout_by_ip_allows_when_same_user_diff_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 can still login from IP 2. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + def test_lockout_by_ip_blocks_when_diff_user_same_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 is also locked out from IP 1. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + def test_lockout_by_ip_allows_when_diff_user_diff_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 2. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + # Test for true and false positives when blocking by user only. + # With cache enabled. When AXES_ONLY_USER_FAILURES = True + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) + def test_lockout_by_user_blocks_when_same_user_same_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 is still blocked from IP 1. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) + def test_lockout_by_user_blocks_when_same_user_diff_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 is also locked out from IP 2. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) + def test_lockout_by_user_allows_when_diff_user_same_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 1. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + @patch('axes.decorators.AXES_ONLY_USER_FAILURES', True) + def test_lockout_by_user_allows_when_diff_user_diff_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 2. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + # Test for true and false positives when blocking by user and IP together. + # With cache enabled. When LOCK_OUT_BY_COMBINATION_USER_AND_IP = True + @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) + def test_lockout_by_user_and_ip_blocks_when_same_user_same_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 is still blocked from IP 1. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.BLOCKED) + + @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) + def test_lockout_by_user_and_ip_allows_when_same_user_diff_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 1 can still login from IP 2. + response = self._login( + self.USER_1, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) + def test_lockout_by_user_and_ip_allows_when_diff_user_same_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 1. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_1 + ) + self.assertEqual(response.status_code, self.ALLOWED) + + @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) + def test_lockout_by_user_and_ip_allows_when_diff_user_diff_ip_using_cache( + self, cache_get_mock=None, cache_set_mock=None + ): + # User 1 is locked out from IP 1. + self._lockout_user1_from_ip1() + + # User 2 can still login from IP 2. + response = self._login( + self.USER_2, + self.VALID_PASSWORD, + ip_addr=self.IP_2 + ) + self.assertEqual(response.status_code, self.ALLOWED) + class UtilsTest(TestCase): def test_iso8601(self): From c86ad06d9d82314b42c9429249bc5d6e9c166d41 Mon Sep 17 00:00:00 2001 From: Jack Sullivan Date: Sat, 22 Apr 2017 19:19:48 -0700 Subject: [PATCH 07/10] Fixed #222, cache blocks by user only and ip+user Cache hash keys now include usernames. The axes settings AXES_ONLY_USER_FAILURES and LOCK_OUT_BY_COMBINATION_USER_AND_IP are checked to decide which request attributes to include in generated cache hash keys. --- axes/decorators.py | 23 +++++++++++++++-------- axes/tests.py | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/axes/decorators.py b/axes/decorators.py index 2099013..7169ad6 100644 --- a/axes/decorators.py +++ b/axes/decorators.py @@ -499,23 +499,30 @@ def get_cache_key(request_or_object): :param request_or_object: Request or AccessAttempt object :return cache-key: String, key to be used in cache system """ - ua = None - ip = None - if isinstance(request_or_object, AccessAttempt): ip = request_or_object.ip_address + un = request_or_object.username ua = request_or_object.user_agent else: ip = get_ip(request_or_object) + un = request_or_object.POST.get(USERNAME_FORM_FIELD, None) ua = request_or_object.META.get('HTTP_USER_AGENT', '')[:255] - ip = ip.encode('utf-8') + ip = ip.encode('utf-8') if ip else '' + un = un.encode('utf-8') if un else '' + ua = ua.encode('utf-8') if ua else '' - if ua: - ua = ua.encode('utf-8') - cache_hash_key = 'axes-{}'.format(md5(ip+ua).hexdigest()) + if AXES_ONLY_USER_FAILURES: + attributes = un + elif LOCK_OUT_BY_COMBINATION_USER_AND_IP: + attributes = ip+un else: - cache_hash_key = 'axes-{}'.format(md5(ip).hexdigest()) + attributes = ip + + if USE_USER_AGENT: + attributes += ua + + cache_hash_key = 'axes-{}'.format(md5(attributes).hexdigest()) return cache_hash_key diff --git a/axes/tests.py b/axes/tests.py index f0e995b..1ce899e 100644 --- a/axes/tests.py +++ b/axes/tests.py @@ -213,7 +213,7 @@ class AccessAttemptTest(TestCase): ip = '127.0.0.1'.encode('utf-8') ua = ''.encode('utf-8') - cache_hash_key_checker = 'axes-{}'.format(md5((ip+ua)).hexdigest()) + cache_hash_key_checker = 'axes-{}'.format(md5((ip)).hexdigest()) request_factory = RequestFactory() request = request_factory.post('/admin/login/', From 4783787c6dbb1f021f9c04da928745dec7682d12 Mon Sep 17 00:00:00 2001 From: Jack Sullivan Date: Wed, 26 Apr 2017 09:11:11 -0700 Subject: [PATCH 08/10] Fixed UTF-8 encoding bug. --- axes/decorators.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/axes/decorators.py b/axes/decorators.py index 7169ad6..c89aa38 100644 --- a/axes/decorators.py +++ b/axes/decorators.py @@ -508,9 +508,9 @@ def get_cache_key(request_or_object): un = request_or_object.POST.get(USERNAME_FORM_FIELD, None) ua = request_or_object.META.get('HTTP_USER_AGENT', '')[:255] - ip = ip.encode('utf-8') if ip else '' - un = un.encode('utf-8') if un else '' - ua = ua.encode('utf-8') if ua else '' + ip = ip.encode('utf-8') if ip else ''.encode('utf-8') + un = un.encode('utf-8') if un else ''.encode('utf-8') + ua = ua.encode('utf-8') if ua else ''.encode('utf-8') if AXES_ONLY_USER_FAILURES: attributes = un From 95917a951ea6a506b34c79d3bd3ff9eaa02b70dc Mon Sep 17 00:00:00 2001 From: Jack Sullivan Date: Wed, 26 Apr 2017 12:49:44 -0700 Subject: [PATCH 09/10] In tests, only set cooldown if testing it The results for the cache unit tests were inconsistent, sometimes blocking and other time allowing. The source of the non-determinism was the COOLDOWN_TIME set to 2 seconds in the test. If a test took slightly longer than the cooldown time, it would fail. Testing times on Travis CI vary with each build, and would produce unreliable results. Now all tests have no cooldown period, except when the cooldown itself is being tested. This ensures accurate and predicable test results. --- axes/test_settings.py | 3 --- axes/tests.py | 8 ++++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/axes/test_settings.py b/axes/test_settings.py index b8a47d4..46ba0b5 100644 --- a/axes/test_settings.py +++ b/axes/test_settings.py @@ -1,5 +1,3 @@ -from datetime import timedelta - DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', @@ -55,4 +53,3 @@ USE_TZ = False LOGIN_REDIRECT_URL = '/admin/' AXES_LOGIN_FAILURE_LIMIT = 10 -AXES_COOLOFF_TIME = timedelta(seconds=2) diff --git a/axes/tests.py b/axes/tests.py index 1ce899e..f12702f 100644 --- a/axes/tests.py +++ b/axes/tests.py @@ -16,13 +16,15 @@ from django.utils import six from django.test.client import RequestFactory from axes.decorators import get_ip, get_cache_key -from axes.settings import COOLOFF_TIME from axes.settings import FAILURE_LIMIT from axes.models import AccessAttempt, AccessLog from axes.signals import user_locked_out from axes.utils import reset, iso8601 +TEST_COOLOFF_TIME = datetime.timedelta(seconds=2) + + class MockRequest: def __init__(self): self.META = dict() @@ -140,17 +142,19 @@ class AccessAttemptTest(TestCase): self.assertNotEquals(AccessLog.objects.latest('id').logout_time, None) self.assertContains(response, 'Logged out') + @patch('axes.decorators.COOLOFF_TIME', TEST_COOLOFF_TIME) def test_cooling_off(self): """Tests if the cooling time allows a user to login """ self.test_failure_limit_once() # Wait for the cooling off period - time.sleep(COOLOFF_TIME.total_seconds()) + time.sleep(TEST_COOLOFF_TIME.total_seconds()) # It should be possible to login again, make sure it is. self.test_valid_login() + @patch('axes.decorators.COOLOFF_TIME', TEST_COOLOFF_TIME) def test_cooling_off_for_trusted_user(self): """Test the cooling time for a trusted user """ From 8b4ca6e53811a62e22c02fac017f2f46e62f53f9 Mon Sep 17 00:00:00 2001 From: Camilo Nova Date: Tue, 9 May 2017 18:52:18 -0500 Subject: [PATCH 10/10] Added pytz as a requirement. Fixes #230 --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index a8a4668..c937536 100644 --- a/setup.py +++ b/setup.py @@ -20,6 +20,7 @@ setup( url='https://github.com/django-pci/django-axes', license='MIT', package_dir={'axes': 'axes'}, + install_requires=['pytz'], include_package_data=True, packages=find_packages(), classifiers=[