From 289af19ce9aecfeebfaf61b4a41df71162328730 Mon Sep 17 00:00:00 2001 From: Yurii Parfinenko <77358903+irkonikovich@users.noreply.github.com> Date: Thu, 29 Jan 2026 12:53:21 -0500 Subject: [PATCH] Use redis cache in `get_approx_account_lockouts_from_login_attempts` (#250) * Use redis cache in `get_approx_account_lockouts_from_login_attempts` * use django_redis in ci * Add `django_redis` and `redis` to requirements.txt * Fix an issue detected by tests: clear redis cache upon block reset * Remove the unnecessary `if` --- defender/ci_settings.py | 2 +- defender/data.py | 26 +++++++++++++++++++++----- defender/tests.py | 15 ++++++++++++++- defender/utils.py | 18 +++++++++++++++++- requirements.txt | 3 ++- tox.ini | 3 ++- 6 files changed, 57 insertions(+), 10 deletions(-) diff --git a/defender/ci_settings.py b/defender/ci_settings.py index af001db..97bb79b 100644 --- a/defender/ci_settings.py +++ b/defender/ci_settings.py @@ -7,7 +7,7 @@ from celery import Celery DATABASES = {"default": {"ENGINE": "django.db.backends.sqlite3", "NAME": ":memory:",}} CACHES = { - "default": {"BACKEND": "redis_cache.RedisCache", "LOCATION": "localhost:6379",} + "default": {"BACKEND": "django_redis.cache.RedisCache", "LOCATION": "redis://localhost:6379",} } SITE_ID = 1 diff --git a/defender/data.py b/defender/data.py index a86037e..5110d75 100644 --- a/defender/data.py +++ b/defender/data.py @@ -1,6 +1,7 @@ from datetime import timedelta from defender import config +from defender.connection import get_redis_connection from .models import AccessAttempt from django.db.models import Q from django.utils import timezone @@ -19,6 +20,14 @@ def store_login_attempt( login_valid=login_valid, ) + +def get_approx_lockouts_cache_key(ip_address, username): + """get cache key for approximate number of account lockouts""" + return "{0}:approx_lockouts:ip:{1}:user:{2}".format( + config.CACHE_PREFIX, ip_address or "", username.lower() if username else "" + ) + + def get_approx_account_lockouts_from_login_attempts(ip_address=None, username=None): """Get the approximate number of account lockouts in a period of ACCESS_ATTEMPT_EXPIRATION hours. This is approximate because we do not consider the time between these failed @@ -31,10 +40,6 @@ def get_approx_account_lockouts_from_login_attempts(ip_address=None, username=No Returns: int: The minimum of the count of logged failure attempts and the length of the LOCKOUT_COOLOFF_TIMES - 1, or 0 dependant on either configuration or argument parameters (ie. both ip_address and username being None). """ - - # TODO: Possibly add logic to temporarily store this info in the cache - # to help mitigate any potential performance impact this could have. - if not config.STORE_ACCESS_ATTEMPTS or not (ip_address or username): # If we're not storing login attempts OR both ip_address and username are # None we should return 0. @@ -57,4 +62,15 @@ def get_approx_account_lockouts_from_login_attempts(ip_address=None, username=No # conditions, we're in an inappropriate context. raise Exception("Invalid state requested") - return AccessAttempt.objects.filter(q).count() // failure_limit + cache_key = get_approx_lockouts_cache_key(ip_address, username) + + redis_client = get_redis_connection() + cached_value = redis_client.get(cache_key) + if cached_value is not None: + return int(cached_value) + + lockouts = AccessAttempt.objects.filter(q).count() // failure_limit + + redis_client.set(cache_key, int(lockouts), 60) + + return lockouts diff --git a/defender/tests.py b/defender/tests.py index d2c1de9..729958a 100644 --- a/defender/tests.py +++ b/defender/tests.py @@ -13,7 +13,7 @@ from django.urls import reverse import redis -from defender.data import get_approx_account_lockouts_from_login_attempts +from defender.data import get_approx_account_lockouts_from_login_attempts, get_approx_lockouts_cache_key from . import utils from . import config @@ -964,6 +964,19 @@ class AccessAttemptTest(DefenderTestCase): def test_approx_account_lockout_count_default_case_invalid_args_pt2(self): with self.assertRaises(Exception): get_approx_account_lockouts_from_login_attempts(username=VALID_USERNAME) + + def test_approx_account_lockout_uses_redis_cache(self): + get_approx_account_lockouts_from_login_attempts( + ip_address="127.0.0.1", username=VALID_USERNAME + ) + + redis_client = get_redis_connection() + cached_value = redis_client.get( + get_approx_lockouts_cache_key( + ip_address="127.0.0.1", username=VALID_USERNAME + ) + ) + self.assertIsNotNone(cached_value) class SignalTest(DefenderTestCase): diff --git a/defender/utils.py b/defender/utils.py index 3ab6241..75ef8fa 100644 --- a/defender/utils.py +++ b/defender/utils.py @@ -12,7 +12,11 @@ from django.utils.module_loading import import_string from .connection import get_redis_connection from . import config -from .data import get_approx_account_lockouts_from_login_attempts, store_login_attempt +from .data import ( + get_approx_account_lockouts_from_login_attempts, + get_approx_lockouts_cache_key, + store_login_attempt, +) from .signals import ( send_username_block_signal, send_ip_block_signal, @@ -331,6 +335,10 @@ def unblock_ip(ip_address, pipe=None): pipe.execute() send_ip_unblock_signal(ip_address) + redis_cache_key = get_approx_lockouts_cache_key(ip_address, None) + redis_client = get_redis_connection() + redis_client.delete(redis_cache_key) + def unblock_username(username, pipe=None): """ unblock the given Username """ @@ -345,6 +353,10 @@ def unblock_username(username, pipe=None): pipe.execute() send_username_unblock_signal(username) + redis_cache_key = get_approx_lockouts_cache_key(None, username) + redis_client = get_redis_connection() + redis_client.delete(redis_cache_key) + def reset_failed_attempts(ip_address=None, username=None): """ reset the failed attempts for these ip's and usernames @@ -357,6 +369,10 @@ def reset_failed_attempts(ip_address=None, username=None): unblock_ip(ip_address, pipe=pipe) unblock_username(username, pipe=pipe) + redis_cache_key = get_approx_lockouts_cache_key(ip_address, username) + redis_client = get_redis_connection() + redis_client.delete(redis_cache_key) + pipe.execute() diff --git a/requirements.txt b/requirements.txt index 47dbe03..c83d2fb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,8 @@ -e . coverage mockredispy -django-redis-cache +django-redis>=5,<6 +redis>=5,<6 importlib-metadata<5.0 celery sphinx_rtd_theme==2.0.0 diff --git a/tox.ini b/tox.ini index b3b5de8..a1b4209 100644 --- a/tox.ini +++ b/tox.ini @@ -2,7 +2,8 @@ envlist = # list of supported Django/Python versions: py{37,38,39,py3}-dj{32} - py{38,39,310,311}-dj{40,41,42,main} + py{38,39,310,311}-dj{40,41,42} + py{311}-dj{main} py38-{lint,docs} [gh-actions]