From 14950ee83a846ebaafc537c6d9a31ecb564387dd Mon Sep 17 00:00:00 2001 From: Jorge Galvis Date: Wed, 2 Nov 2016 00:25:32 -0500 Subject: [PATCH 1/6] WP: Cache failures in cache --- axes/decorators.py | 69 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/axes/decorators.py b/axes/decorators.py index 3e69716..b334afe 100644 --- a/axes/decorators.py +++ b/axes/decorators.py @@ -1,6 +1,7 @@ import json import logging from socket import inet_pton, AF_INET6 +from hashlib import md5 from django.contrib.auth import get_user_model from django.contrib.auth import logout @@ -9,6 +10,7 @@ from django.http import HttpResponseRedirect from django.shortcuts import render from django.utils import six from django.utils import timezone as datetime +from django.core.cache import cache from axes.models import AccessAttempt from axes.models import AccessLog @@ -148,6 +150,8 @@ def _get_user_attempts(request): def get_user_attempts(request): objects_deleted = False attempts = _get_user_attempts(request) + cache_hash_key = get_cache_key(request) + cache_timeout = get_cache_timeout() if COOLOFF_TIME: for attempt in attempts: @@ -155,9 +159,15 @@ def get_user_attempts(request): if attempt.trusted: attempt.failures_since_start = 0 attempt.save() + cache.set(cache_hash_key, 0, cache_timeout) else: attempt.delete() objects_deleted = True + failures_cached = cache.get(cache_hash_key) + if failures_cached is not None: + cache.set(cache_hash_key, + failures_cached - 1, + cache_timeout) # If objects were deleted, we need to update the queryset to reflect this, # so force a reload. @@ -290,9 +300,15 @@ def is_already_locked(request): if not is_user_lockable(request): return False - for attempt in get_user_attempts(request): - if attempt.failures_since_start >= FAILURE_LIMIT and LOCK_OUT_AT_FAILURE: - return True + cache_hash_key = get_cache_key(request) + failures_cached = cache.get(cache_hash_key) + if failures_cached is not None: + return failures_cached >= FAILURE_LIMIT and LOCK_OUT_AT_FAILURE + else: + for attempt in get_user_attempts(request): + if attempt.failures_since_start >= FAILURE_LIMIT and \ + LOCK_OUT_AT_FAILURE: + return True return False @@ -302,13 +318,20 @@ def check_request(request, login_unsuccessful): username = request.POST.get(USERNAME_FORM_FIELD, None) failures = 0 attempts = get_user_attempts(request) + cache_hash_key = get_cache_key(request) + cache_timeout = get_cache_timeout() - for attempt in attempts: - failures = max(failures, attempt.failures_since_start) + failures_cached = cache.get(cache_hash_key) + if failures_cached is not None: + failures = failures_cached + else: + for attempt in attempts: + failures = max(failures, attempt.failures_since_start) if login_unsuccessful: # add a failed attempt for this user failures += 1 + cache.set(cache_hash_key, failures, cache_timeout) # Create an AccessAttempt record if the login wasn't successful # has already attempted, update the info @@ -342,10 +365,16 @@ def check_request(request, login_unsuccessful): for attempt in attempts: if not attempt.trusted: attempt.delete() + failures_cached = cache.get(cache_hash_key) + if failures_cached is not None: + cache.set(cache_hash_key, + failures_cached - 1, + cache_timeout) else: trusted_record_exists = True attempt.failures_since_start = 0 attempt.save() + cache.set(cache_hash_key, 0, cache_timeout) if trusted_record_exists is False: create_new_trusted_record(request) @@ -418,3 +447,33 @@ def create_new_trusted_record(request): failures_since_start=0, trusted=True ) + + +def get_cache_key(request_or_object): + """ + Build cache key name from request or AccessAttempt object. + :param request_or_object: Request or AccessAttempt object + :return cache-key: String, key to be used in cache system + """ + ua = None + if isinstance(request_or_object, AccessAttempt): + ip = request_or_object.ip_address + ua = request_or_object.user_agent + else: + ip = get_ip(request_or_object) + ua = request_or_object.META.get('HTTP_USER_AGENT', '')[:255] + + if ua: + cache_hash_key = 'axes-{}'.format(md5(ip+ua).hexdigest()) + else: + cache_hash_key = 'axes-{}'.format(md5(ip).hexdigest()) + + return cache_hash_key + + +def get_cache_timeout(): + "Returns timeout according to COOLOFF_TIME." + cache_timeout = None + if COOLOFF_TIME: + cache_timeout = COOLOFF_TIME.total_seconds() + return cache_timeout From 187195664b1cc6bf3bdfd6802b46d6d1798f9e6b Mon Sep 17 00:00:00 2001 From: Jorge Galvis Date: Tue, 6 Dec 2016 16:46:16 -0500 Subject: [PATCH 2/6] Fix tests after apply cache workflow --- axes/tests.py | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/axes/tests.py b/axes/tests.py index 6fabb34..9adc8aa 100644 --- a/axes/tests.py +++ b/axes/tests.py @@ -79,7 +79,9 @@ class AccessAttemptTest(TestCase): password=self.VALID_PASSWORD, ) - def test_failure_limit_once(self): + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_failure_limit_once(self, cache_get_mock, cache_set_mock): """Tests the login lock trying to login one more time than failure limit """ @@ -109,13 +111,17 @@ class AccessAttemptTest(TestCase): response = self._login() self.assertContains(response, self.LOCKED_MESSAGE, status_code=403) - def test_valid_login(self): + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_valid_login(self, cache_set_mock, cache_get_mock): """Tests a valid login for a real username """ response = self._login(is_valid_username=True, is_valid_password=True) self.assertNotContains(response, self.LOGIN_FORM_KEY, status_code=302) - def test_valid_logout(self): + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_valid_logout(self, cache_set_mock, cache_get_mock): """Tests a valid logout and make sure the logout_time is updated """ response = self._login(is_valid_username=True, is_valid_password=True) @@ -145,7 +151,9 @@ class AccessAttemptTest(TestCase): # Try the cooling off time self.test_cooling_off() - def test_long_user_agent_valid(self): + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_long_user_agent_valid(self, cache_set_mock, cache_get_mock): """Tests if can handle a long user agent """ long_user_agent = 'ie6' * 1024 @@ -214,7 +222,10 @@ class AccessAttemptTest(TestCase): self.assertEquals(scope.signal_received, 2) @patch('axes.decorators.LOCK_OUT_BY_COMBINATION_USER_AND_IP', True) - def test_lockout_by_combination_user_and_ip(self): + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_lockout_by_combination_user_and_ip(self, cache_set_mock, + cache_get_mock): """Tests the login lock with a valid username and invalid password when AXES_LOCK_OUT_BY_COMBINATION_USER_AND_IP is True """ @@ -233,7 +244,9 @@ class AccessAttemptTest(TestCase): self.assertContains(response, self.LOCKED_MESSAGE, status_code=403) @override_settings(AXES_ONLY_USER_FAILURES=True) - def test_lockout_by_user_only(self): + @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): """Tests the login lock with a valid username and invalid password when AXES_ONLY_USER_FAILURES is True """ @@ -270,7 +283,10 @@ class AccessAttemptTest(TestCase): response = self._login(is_valid_username=True, is_valid_password=True) self.assertNotContains(response, self.LOGIN_FORM_KEY, status_code=302) - def test_log_data_truncated(self): + + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_log_data_truncated(self, cache_set_mock, cache_get_mock): """Tests that query2str properly truncates data to the max_length (default 1024) """ @@ -300,7 +316,9 @@ class AccessAttemptTest(TestCase): self.assertContains(response, 'Logged out') @patch('axes.decorators.DISABLE_ACCESS_LOG', True) - def test_non_valid_login_without_log(self): + @patch('axes.decorators.cache.set', return_value=None) + @patch('axes.decorators.cache.get', return_value=None) + def test_non_valid_login_without_log(self, cache_set_mock, cache_get_mock): AccessLog.objects.all().delete() response = self._login(is_valid_username=True, is_valid_password=False) From 2357a4616bd7e200f2d262d144f29dddddae7da5 Mon Sep 17 00:00:00 2001 From: Jorge Galvis Date: Tue, 6 Dec 2016 18:08:13 -0500 Subject: [PATCH 3/6] Make it Python3 compatible --- axes/decorators.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/axes/decorators.py b/axes/decorators.py index dacb324..de9c3af 100644 --- a/axes/decorators.py +++ b/axes/decorators.py @@ -476,6 +476,8 @@ def get_cache_key(request_or_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 ua = request_or_object.user_agent @@ -483,7 +485,10 @@ def get_cache_key(request_or_object): ip = get_ip(request_or_object) ua = request_or_object.META.get('HTTP_USER_AGENT', '')[:255] + ip = ip.encode('utf-8') + if ua: + ua = ua.encode('utf-8') cache_hash_key = 'axes-{}'.format(md5(ip+ua).hexdigest()) else: cache_hash_key = 'axes-{}'.format(md5(ip).hexdigest()) From 5b791f65f40d6a08c2304f4d1f6dc3f3eee9d896 Mon Sep 17 00:00:00 2001 From: Jorge Galvis Date: Tue, 6 Dec 2016 19:41:04 -0500 Subject: [PATCH 4/6] Add signals for setting/deleting cache keys --- axes/signals.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/axes/signals.py b/axes/signals.py index f48d23d..0339010 100644 --- a/axes/signals.py +++ b/axes/signals.py @@ -2,8 +2,10 @@ from django.dispatch import receiver from django.dispatch import Signal from django.utils.timezone import now from django.contrib.auth.signals import user_logged_out +from django.db.models.signals import post_save, post_delete +from django.core.cache import cache -from axes.models import AccessLog +from axes.models import AccessLog, AccessAttempt from axes.settings import DISABLE_ACCESS_LOG @@ -26,3 +28,19 @@ if not DISABLE_ACCESS_LOG: access_log = access_logs[0] access_log.logout_time = now() access_log.save() + + +@receiver(post_save, sender=AccessAttempt) +def update_cache_after_save(instance, **kwargs): + from axes.decorators import get_cache_timeout, get_cache_key + cache_hash_key = get_cache_key(instance) + if not cache.get(cache_hash_key): + cache_timeout = get_cache_timeout() + cache.set(cache_hash_key, instance.failures_since_start, cache_timeout) + + +@receiver(post_delete, sender=AccessAttempt) +def delete_cache_after_delete(instance, **kwargs): + from axes.decorators import get_cache_key + cache_hash_key = get_cache_key(instance) + cache.delete(cache_hash_key) From f277007e46b7c6d8c978011d7356b7527ba91133 Mon Sep 17 00:00:00 2001 From: Jorge Galvis Date: Tue, 6 Dec 2016 20:01:44 -0500 Subject: [PATCH 5/6] Delete cache key in reset command line --- axes/utils.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/axes/utils.py b/axes/utils.py index 8a050ed..9f03531 100644 --- a/axes/utils.py +++ b/axes/utils.py @@ -1,3 +1,5 @@ +from django.core.cache import cache + from axes.models import AccessAttempt @@ -15,8 +17,13 @@ def reset(ip=None, username=None): if attempts: count = attempts.count() - attempts.delete() + from axes.decorators import get_cache_key + for attempt in attempts: + cache_hash_key = get_cache_key(attempt) + if cache.get(cache_hash_key): + cache.delete(cache_hash_key) + attempts.delete() return count From de9fe09f5c37e484e0a782282a4658e108350a50 Mon Sep 17 00:00:00 2001 From: Jorge Galvis Date: Tue, 6 Dec 2016 21:36:49 -0500 Subject: [PATCH 6/6] Add test for get_cache_key function --- axes/tests.py | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/axes/tests.py b/axes/tests.py index bd6c877..1c32c6d 100644 --- a/axes/tests.py +++ b/axes/tests.py @@ -4,6 +4,7 @@ import time import json import datetime +from hashlib import md5 from mock import patch from django.conf import settings @@ -13,8 +14,9 @@ from django.contrib.auth.models import User from django.core.urlresolvers import NoReverseMatch from django.core.urlresolvers import reverse from django.utils import six +from django.test.client import RequestFactory -from axes.decorators import get_ip +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 @@ -205,6 +207,39 @@ class AccessAttemptTest(TestCase): # Make a login attempt again self.test_valid_login() + @patch('axes.decorators.get_ip', return_value='127.0.0.1') + def test_get_cache_key(self, get_ip_mock): + """ Test the cache key format""" + # Getting cache key from request + ip = '127.0.0.1'.encode('utf-8') + ua = ''.encode('utf-8') + + cache_hash_key_checker = 'axes-{}'.format(md5((ip+ua)).hexdigest()) + + request_factory = RequestFactory() + request = request_factory.post('/admin/login/', + data={ + 'username': self.VALID_USERNAME, + 'password': 'test' + }) + + cache_hash_key = get_cache_key(request) + self.assertEqual(cache_hash_key_checker, cache_hash_key) + + # Getting cache key from AccessAttempt Object + attempt = AccessAttempt( + user_agent='', + ip_address='127.0.0.1', + username=self.VALID_USERNAME, + get_data='', + post_data='', + http_accept=request.META.get('HTTP_ACCEPT', ''), + path_info=request.META.get('PATH_INFO', ''), + failures_since_start=0, + ) + cache_hash_key = get_cache_key(attempt) + self.assertEqual(cache_hash_key_checker, cache_hash_key) + def test_send_lockout_signal(self): """Test if the lockout signal is emitted """