From 84b887902d81889f01df00f3f9d6b999864707bf Mon Sep 17 00:00:00 2001 From: Gilles Dartiguelongue Date: Fri, 20 Apr 2018 13:51:55 +0200 Subject: [PATCH 1/3] Simplify cooloff time type check --- axes/attempts.py | 4 ++-- axes/decorators.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/axes/attempts.py b/axes/attempts.py index 19d8c3c..d673e84 100644 --- a/axes/attempts.py +++ b/axes/attempts.py @@ -88,7 +88,7 @@ def get_cache_timeout(): cache_timeout = None cool_off = settings.AXES_COOLOFF_TIME if cool_off: - if (isinstance(cool_off, int) or isinstance(cool_off, float)): + if isinstance(cool_off, (int, float)): cache_timeout = timedelta(hours=cool_off).total_seconds() else: cache_timeout = cool_off.total_seconds() @@ -104,7 +104,7 @@ def get_user_attempts(request): cool_off = settings.AXES_COOLOFF_TIME if cool_off: - if (isinstance(cool_off, int) or isinstance(cool_off, float)): + if isinstance(cool_off, (int, float)): cool_off = timedelta(hours=cool_off) for attempt in attempts: diff --git a/axes/decorators.py b/axes/decorators.py index e689acc..f982be1 100644 --- a/axes/decorators.py +++ b/axes/decorators.py @@ -55,7 +55,7 @@ def lockout_response(request): cool_off = settings.AXES_COOLOFF_TIME if cool_off: - if (isinstance(cool_off, int) or isinstance(cool_off, float)): + if isinstance(cool_off, (int, float)): cool_off = timedelta(hours=cool_off) context.update({ From bbe693739553d91e0eaefa5ff9f20ab46c5dae56 Mon Sep 17 00:00:00 2001 From: Gilles Dartiguelongue Date: Fri, 20 Apr 2018 13:58:48 +0200 Subject: [PATCH 2/3] Do not use len to assert existance of attempts It forces unnecessary evaluation of the length of attemps. --- axes/signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axes/signals.py b/axes/signals.py index 7bfdb2c..2fa6072 100644 --- a/axes/signals.py +++ b/axes/signals.py @@ -62,7 +62,7 @@ def log_user_login_failed(sender, credentials, request, **kwargs): get_axes_cache().set(cache_hash_key, failures, cache_timeout) # has already attempted, update the info - if len(attempts): + if attempts: for attempt in attempts: attempt.get_data = '%s\n---------\n%s' % ( attempt.get_data, From 089447a2e7c40b6d460185bccb3d62d0fee9eca1 Mon Sep 17 00:00:00 2001 From: Gilles Dartiguelongue Date: Fri, 20 Apr 2018 14:08:34 +0200 Subject: [PATCH 3/3] Leave logging to formatting as needed Do not force it as it might be used programmatically down the line, see Sentry/raven [1] or fluentd [2] for example. [1] https://github.com/getsentry/raven-python [2] https://github.com/fluent/fluent-logger-python --- axes/decorators.py | 2 +- axes/signals.py | 28 ++++++++++++++-------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/axes/decorators.py b/axes/decorators.py index f982be1..69dced5 100644 --- a/axes/decorators.py +++ b/axes/decorators.py @@ -17,7 +17,7 @@ from axes.utils import iso8601, get_lockout_message log = logging.getLogger(settings.AXES_LOGGER) if settings.AXES_VERBOSE: log.info('AXES: BEGIN LOG') - log.info('AXES: Using django-axes ' + get_version()) + log.info('AXES: Using django-axes %s', get_version()) if settings.AXES_ONLY_USER_FAILURES: log.info('AXES: blocking by username only.') elif settings.AXES_LOCK_OUT_BY_COMBINATION_USER_AND_IP: diff --git a/axes/signals.py b/axes/signals.py index 2fa6072..52a2799 100644 --- a/axes/signals.py +++ b/axes/signals.py @@ -78,13 +78,12 @@ def log_user_login_failed(sender, credentials, request, **kwargs): attempt.attempt_time = timezone.now() attempt.save() - fail_msg = 'AXES: Repeated login failure by {0}.'.format( - get_client_str(username, ip_address, user_agent, path_info) + log.info( + 'AXES: Repeated login failure by %s. Count = %d of %d', + get_client_str(username, ip_address, user_agent, path_info), + failures, + settings.AXES_FAILURE_LIMIT ) - count_msg = 'Count = {0} of {1}'.format( - failures, settings.AXES_FAILURE_LIMIT - ) - log.info('{0} {1}'.format(fail_msg, count_msg)) else: # Record failed attempt. Whether or not the IP address or user agent is # used in counting failures is handled elsewhere, so we just record @@ -101,9 +100,8 @@ def log_user_login_failed(sender, credentials, request, **kwargs): ) log.info( - 'AXES: New login failure by {0}. Creating access record.'.format( - get_client_str(username, ip_address, user_agent, path_info) - ) + 'AXES: New login failure by %s. Creating access record.', + get_client_str(username, ip_address, user_agent, path_info) ) # no matter what, we want to lock them out if they're past the number of @@ -113,9 +111,10 @@ def log_user_login_failed(sender, credentials, request, **kwargs): settings.AXES_LOCK_OUT_AT_FAILURE and is_user_lockable(request) ): - log.warning('AXES: locked out {0} after repeated login attempts.'.format( + log.warning( + 'AXES: locked out %s after repeated login attempts.', get_client_str(username, ip_address, user_agent, path_info) - )) + ) # send signal when someone is locked out. user_locked_out.send( @@ -132,9 +131,10 @@ def log_user_logged_in(sender, request, user, **kwargs): user_agent = request.META.get('HTTP_USER_AGENT', '')[:255] path_info = request.META.get('PATH_INFO', '')[:255] http_accept = request.META.get('HTTP_ACCEPT', '')[:1025] - log.info('AXES: Successful login by {0}.'.format( + log.info( + 'AXES: Successful login by %s.', get_client_str(username, ip_address, user_agent, path_info) - )) + ) if not settings.AXES_DISABLE_SUCCESS_ACCESS_LOG: AccessLog.objects.create( @@ -151,7 +151,7 @@ def log_user_logged_in(sender, request, user, **kwargs): def log_user_logged_out(sender, request, user, **kwargs): """ When a user logs out, update the access log """ - log.info('AXES: Successful logout by {0}.'.format(user)) + log.info('AXES: Successful logout by %s.', user) if user and not settings.AXES_DISABLE_ACCESS_LOG: AccessLog.objects.filter(