Skip to content

Commit

Permalink
Merge pull request #1198 from sevdog/access-log-identify-session
Browse files Browse the repository at this point in the history
Add session hash to access log
  • Loading branch information
aleksihakli authored Jun 11, 2024
2 parents f70138e + 014483c commit 8724405
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 9 deletions.
9 changes: 8 additions & 1 deletion axes/handlers/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from axes.conf import settings
from axes.handlers.base import AxesBaseHandler, AbstractAxesHandler
from axes.helpers import (
get_client_session_hash,
get_client_str,
get_client_username,
get_credentials,
Expand Down Expand Up @@ -284,6 +285,9 @@ def user_logged_in(self, sender, request, user, **kwargs):
http_accept=request.axes_http_accept,
path_info=request.axes_path_info,
attempt_time=request.axes_attempt_time,
# evaluate session hash here to ensure having the correct
# value which is stored on the backend
session_hash=get_client_session_hash(request),
)

if settings.AXES_RESET_ON_SUCCESS:
Expand Down Expand Up @@ -317,7 +321,10 @@ def user_logged_out(self, sender, request, user, **kwargs):
if username and not settings.AXES_DISABLE_ACCESS_LOG:
# 2. database query: Update existing attempt logs with logout time
AccessLog.objects.filter(
username=username, logout_time__isnull=True
username=username,
logout_time__isnull=True,
# update only access log for given session
session_hash=get_client_session_hash(request),
).update(logout_time=request.axes_attempt_time)

def post_save_access_attempt(self, instance, **kwargs):
Expand Down
22 changes: 22 additions & 0 deletions axes/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.core.cache import BaseCache, caches
from django.http import HttpRequest, HttpResponse, JsonResponse, QueryDict
from django.shortcuts import redirect, render
from django.utils.encoding import force_bytes
from django.utils.module_loading import import_string

from axes.conf import settings
Expand Down Expand Up @@ -614,3 +615,24 @@ def inner(*args, **kwargs): # pylint: disable=inconsistent-return-statements
return func(*args, **kwargs)

return inner


def get_client_session_hash(request: HttpRequest) -> str:
"""
Get client session and returns the SHA256 hash of session key, forcing session creation if required.
If no session is available on request returns an empty string.
"""
try:
session = request.session
except AttributeError:
# when no session is available just return an empty string
return ""

# ensure that a session key exists at this point
# because session middleware usually creates the session key at the end
# of request cycle
if session.session_key is None:
session.create()

return sha256(force_bytes(session.session_key)).hexdigest()
22 changes: 22 additions & 0 deletions axes/migrations/0009_add_session_hash.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Generated by Django 4.2.2 on 2024-04-30 07:57

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("axes", "0008_accessfailurelog"),
]

operations = [
migrations.AddField(
model_name="accesslog",
name="session_hash",
field=models.CharField(
blank=True,
default="",
max_length=64,
verbose_name="Session key hash (sha256)",
),
),
]
1 change: 1 addition & 0 deletions axes/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class Meta:

class AccessLog(AccessBase):
logout_time = models.DateTimeField(_("Logout Time"), null=True, blank=True)
session_hash = models.CharField(_("Session key hash (sha256)"), default="", blank=True, max_length=64)

def __str__(self):
return f"Access Log for {self.username} @ {self.attempt_time}"
Expand Down
6 changes: 6 additions & 0 deletions docs/4_configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ with the ``AXES_HANDLER`` setting in project configuration:
logs attempts to database and creates AccessAttempt and AccessLog records
that persist until removed from the database manually or automatically
after their cool offs expire (checked on each login event).

.. note::
To keep track of concurrent sessions AccessLog stores an hash of ``session_key`` if the session engine is configured.
When no session engine is configured each access is stored with the same dummy value, then a logout will cause each *not-logged-out yet* logs to set a logout time.
Due to how ``django.contrib.auth`` works it is not possible to correctly track the logout of a session in which the user changed its password, since it will create a new session without firing any logout event.

- ``axes.handlers.cache.AxesCacheHandler``
only uses the cache for monitoring attempts and does not persist data
other than in the cache backend; this data can be purged automatically
Expand Down
20 changes: 12 additions & 8 deletions tests/test_logging.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from unittest.mock import patch

from django.test import override_settings
from django.urls import reverse

from axes import __version__
from axes.apps import AppConfig
Expand Down Expand Up @@ -59,16 +58,21 @@ def test_axes_config_log_user_or_ip(self, log):
class AccessLogTestCase(AxesTestCase):
def test_access_log_on_logout(self):
"""
Test a valid logout and make sure the logout_time is updated.
Test a valid logout and make sure the logout_time is updated only for that.
"""

self.login(is_valid_username=True, is_valid_password=True)
self.assertIsNone(AccessLog.objects.latest("id").logout_time)
latest_log = AccessLog.objects.latest("id")
self.assertIsNone(latest_log.logout_time)
other_log = self.create_log(session_hash='not-the-session')
self.assertIsNone(other_log.logout_time)

response = self.client.post(reverse("admin:logout"))
response = self.logout()
self.assertContains(response, "Logged out")

self.assertIsNotNone(AccessLog.objects.latest("id").logout_time)
other_log.refresh_from_db()
self.assertIsNone(other_log.logout_time)
latest_log.refresh_from_db()
self.assertIsNotNone(latest_log.logout_time)

@override_settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=1500)
def test_log_data_truncated(self):
Expand All @@ -86,7 +90,7 @@ def test_valid_logout_without_success_log(self):
AccessLog.objects.all().delete()

response = self.login(is_valid_username=True, is_valid_password=True)
response = self.client.post(reverse("admin:logout"))
response = self.logout()

self.assertEqual(AccessLog.objects.all().count(), 0)
self.assertContains(response, "Logged out", html=True)
Expand All @@ -109,7 +113,7 @@ def test_valid_logout_without_log(self):
AccessLog.objects.all().delete()

response = self.login(is_valid_username=True, is_valid_password=True)
response = self.client.post(reverse("admin:logout"))
response = self.logout()

self.assertEqual(AccessLog.objects.count(), 0)
self.assertContains(response, "Logged out", html=True)
Expand Down

0 comments on commit 8724405

Please sign in to comment.