Skip to content

Commit

Permalink
jazzband#1093: Privacy by Design for Logging
Browse files Browse the repository at this point in the history
  • Loading branch information
Ronny Vedrilla committed Jul 26, 2023
1 parent 5e204be commit 3fb510f
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 6 deletions.
2 changes: 1 addition & 1 deletion axes/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@

settings.AXES_COOLOFF_TIME = getattr(settings, "AXES_COOLOFF_TIME", None)

settings.AXES_VERBOSE = getattr(settings, "AXES_VERBOSE", settings.AXES_ENABLED)
settings.AXES_VERBOSE = getattr(settings, "AXES_VERBOSE", False)

# whitelist and blacklist
settings.AXES_NEVER_LOCKOUT_WHITELIST = getattr(
Expand Down
23 changes: 23 additions & 0 deletions axes/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from urllib.parse import urlencode

import ipware.ip
from django.contrib.auth import get_user_model
from django.core.cache import caches, BaseCache
from django.http import HttpRequest, HttpResponse, JsonResponse, QueryDict
from django.shortcuts import render, redirect
Expand Down Expand Up @@ -210,6 +211,19 @@ def get_client_parameters(username: str, ip_address: str, user_agent: str) -> li
return filter_query


def get_client_id(username: str) -> Optional[int]:
"""
Tries to fetch the user database ID via the "username" used for logging in.
:param username: str Username from request
:return: int|None
"""
User = get_user_model()
filter_params = {getattr(User, 'USERNAME_FIELD', 'username'): username}
possible_user = User.objects.filter(**filter_params).first()
return possible_user.id if possible_user else None


def make_cache_key_list(filter_kwargs_list):
cache_keys = []
for filter_kwargs in filter_kwargs_list:
Expand Down Expand Up @@ -290,6 +304,15 @@ def get_client_str(
for client in client_list:
client_dict.update(client)

# We don't want the IP address if we are not logging verbosely
client_dict.pop('ip_address', None)

# If the user is in our database, the "username" is sensitive, so we log only the user id instead
user_id = get_client_id(username=username)
if user_id:
client_dict['user_id'] = user_id
client_dict.pop('username', None)

# Path info is always included as last component in the client string for traceability purposes
if path_info and isinstance(path_info, (tuple, list)):
path_info = path_info[0]
Expand Down
2 changes: 1 addition & 1 deletion docs/4_configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ The following ``settings.py`` options are available for customizing Axes behavio
+------------------------------------------------------+----------------------------------------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| AXES_LOCKOUT_URL | None | If set, specifies a URL to redirect to on lockout. If both ``AXES_LOCKOUT_TEMPLATE`` and ``AXES_LOCKOUT_URL`` are set, the template will be used. |
+------------------------------------------------------+----------------------------------------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| AXES_VERBOSE | True | If ``True``, you'll see slightly more logging for Axes. |
| AXES_VERBOSE | False | If ``True``, you'll see slightly more logging for Axes including potentially sensitive data like IP address and username. |
+------------------------------------------------------+----------------------------------------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| AXES_USERNAME_FORM_FIELD | 'username' | The name of the form field that contains your users usernames. |
+------------------------------------------------------+----------------------------------------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
Expand Down
25 changes: 21 additions & 4 deletions tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from unittest.mock import patch

from django.contrib.auth import get_user_model
from django.contrib.auth.models import User
from django.http import JsonResponse, HttpResponseRedirect, HttpResponse, HttpRequest
from django.test import override_settings, RequestFactory

Expand Down Expand Up @@ -136,13 +137,29 @@ def test_verbose_ip_only_client_details_tuple(self):
self.assertEqual(expected, actual)

@override_settings(AXES_VERBOSE=False)
def test_non_verbose_ip_only_client_details(self):
def test_non_verbose_ip_not_in_client_details(self):
username = "test@example.com"
ip_address = "127.0.0.1"
user_agent = "Googlebot/2.1 (+http://www.googlebot.com/bot.html)"
path_info = "/admin/"

expected = '{ip_address: "127.0.0.1", path_info: "/admin/"}'
expected = '{path_info: "/admin/"}'
actual = get_client_str(
username, ip_address, user_agent, path_info, self.request
)

self.assertEqual(expected, actual)

@override_settings(AXES_VERBOSE=False)
def test_non_verbose_user_in_database_id_logged_instead(self):
username = "test@example.com"
ip_address = "127.0.0.1"
user_agent = "Googlebot/2.1 (+http://www.googlebot.com/bot.html)"
path_info = "/admin/"

user = User.objects.create(username=username)

expected = '{user_id: "%s", path_info: "/admin/"}' % user.id
actual = get_client_str(
username, ip_address, user_agent, path_info, self.request
)
Expand Down Expand Up @@ -206,7 +223,7 @@ def test_non_verbose_user_ip_combo_client_details(self):
user_agent = "Googlebot/2.1 (+http://www.googlebot.com/bot.html)"
path_info = "/admin/"

expected = '{username: "test@example.com", ip_address: "127.0.0.1", path_info: "/admin/"}'
expected = '{username: "test@example.com", path_info: "/admin/"}'
actual = get_client_str(
username, ip_address, user_agent, path_info, self.request
)
Expand Down Expand Up @@ -238,7 +255,7 @@ def test_non_verbose_user_agent_client_details(self):
user_agent = "Googlebot/2.1 (+http://www.googlebot.com/bot.html)"
path_info = "/admin/"

expected = '{ip_address: "127.0.0.1", user_agent: "Googlebot/2.1 (+http://www.googlebot.com/bot.html)", path_info: "/admin/"}'
expected = '{user_agent: "Googlebot/2.1 (+http://www.googlebot.com/bot.html)", path_info: "/admin/"}'
actual = get_client_str(
username, ip_address, user_agent, path_info, self.request
)
Expand Down

0 comments on commit 3fb510f

Please sign in to comment.