From cdd9f40d848da4fd4e97dfab4ee41e4ebf1e4797 Mon Sep 17 00:00:00 2001 From: Saurabh Kumar Date: Wed, 18 May 2022 12:01:08 +0200 Subject: [PATCH 01/15] Disable `add` button in admin ui Co-authored-by: Hasan Ramezani --- CHANGELOG.md | 1 + auditlog/admin.py | 4 ++++ auditlog_tests/tests.py | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05c25c0..cd6db51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ #### Fixes - Fix inconsistent changes with JSONField ([#355](https://github.com/jazzband/django-auditlog/pull/355)) +- Disable `add` button in admin ui ([#378](https://github.com/jazzband/django-auditlog/pull/378)) ## 2.0.0 (2022-05-09) diff --git a/auditlog/admin.py b/auditlog/admin.py index 5583e07..d05a448 100644 --- a/auditlog/admin.py +++ b/auditlog/admin.py @@ -22,5 +22,9 @@ class LogEntryAdmin(admin.ModelAdmin, LogEntryAdminMixin): ("Changes", {"fields": ["action", "msg"]}), ] + def has_add_permission(self, request): + # As audit admin doesn't allow log creation from admin + return False + admin.site.register(LogEntry, LogEntryAdmin) diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index 2802b9c..6fd52da 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -933,7 +933,7 @@ class AdminPanelTest(TestCase): res = self.client.get("/admin/auditlog/logentry/") assert res.status_code == 200 res = self.client.get("/admin/auditlog/logentry/add/") - assert res.status_code == 200 + assert res.status_code == 403 res = self.client.get(f"/admin/auditlog/logentry/{log_pk}/", follow=True) assert res.status_code == 200 res = self.client.get(f"/admin/auditlog/logentry/{log_pk}/delete/") From d5d65cae2af8025b433665d3091c0b1745bc44d5 Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Fri, 20 May 2022 13:50:17 +0200 Subject: [PATCH 02/15] Update postgres image version to 12 in github action test workflow --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2731842..1eae44a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,7 +13,7 @@ jobs: services: postgres: - image: postgres:11 + image: postgres:12 env: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres From 8b47267a438593ec1c523fe65739851e18b460fc Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Tue, 17 May 2022 13:41:22 +0200 Subject: [PATCH 03/15] Revert "build: explicit the build system" This reverts commit 7bc39e6d9d51e7321bfca8793e21b8066d9859d3. --- pyproject.toml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 065ae19..7145f7d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,3 @@ -[build-system] -requires = ["setuptools>=45", "wheel", "setuptools_scm>=6.2"] -build-backend = "setuptools.build_meta" - [tool.black] target-version = ["py37"] From 32694b1324ab5ce04f4166a071af67d41abb159f Mon Sep 17 00:00:00 2001 From: yeongkwang Date: Mon, 23 May 2022 17:02:22 +0900 Subject: [PATCH 04/15] Add register model using Django settings (#368) Co-authored-by: Hasan Ramezani --- CHANGELOG.md | 4 ++ auditlog/apps.py | 5 ++ auditlog/conf.py | 17 +++++ auditlog/registry.py | 95 ++++++++++++++++++++++++- auditlog_tests/tests.py | 154 +++++++++++++++++++++++++++++++++++++++- docs/source/usage.rst | 54 ++++++++++++++ 6 files changed, 326 insertions(+), 3 deletions(-) create mode 100644 auditlog/conf.py diff --git a/CHANGELOG.md b/CHANGELOG.md index cd6db51..0880ba5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changes +#### Improvements + +- feat: Add register model from settings ([#368](https://github.com/jazzband/django-auditlog/pull/368)) + #### Fixes - Fix inconsistent changes with JSONField ([#355](https://github.com/jazzband/django-auditlog/pull/355)) diff --git a/auditlog/apps.py b/auditlog/apps.py index 097558f..0e9266e 100644 --- a/auditlog/apps.py +++ b/auditlog/apps.py @@ -5,3 +5,8 @@ class AuditlogConfig(AppConfig): name = "auditlog" verbose_name = "Audit log" default_auto_field = "django.db.models.AutoField" + + def ready(self): + from auditlog.registry import auditlog + + auditlog.register_from_settings() diff --git a/auditlog/conf.py b/auditlog/conf.py new file mode 100644 index 0000000..fdb685c --- /dev/null +++ b/auditlog/conf.py @@ -0,0 +1,17 @@ +from django.conf import settings + +# Register all models when set to True +settings.AUDITLOG_INCLUDE_ALL_MODELS = getattr( + settings, "AUDITLOG_INCLUDE_ALL_MODELS", False +) + +# Exclude models in registration process +# It will be considered when `AUDITLOG_INCLUDE_ALL_MODELS` is True +settings.AUDITLOG_EXCLUDE_TRACKING_MODELS = getattr( + settings, "AUDITLOG_EXCLUDE_TRACKING_MODELS", () +) + +# Register models and define their logging behaviour +settings.AUDITLOG_INCLUDE_TRACKING_MODELS = getattr( + settings, "AUDITLOG_INCLUDE_TRACKING_MODELS", () +) diff --git a/auditlog/registry.py b/auditlog/registry.py index 2b1bb3d..8093213 100644 --- a/auditlog/registry.py +++ b/auditlog/registry.py @@ -1,9 +1,13 @@ -from typing import Callable, Dict, List, Optional, Tuple +import copy +from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple, Union +from django.apps import apps from django.db.models import Model from django.db.models.base import ModelBase from django.db.models.signals import ModelSignal, post_delete, post_save, pre_save +from auditlog.conf import settings + DispatchUID = Tuple[int, str, int] @@ -12,6 +16,8 @@ class AuditlogModelRegistry: A registry that keeps track of the models that use Auditlog to track changes. """ + DEFAULT_EXCLUDE_MODELS = ("auditlog.LogEntry", "admin.LogEntry") + def __init__( self, create: bool = True, @@ -147,5 +153,92 @@ class AuditlogModelRegistry: """ return self.__hash__(), model.__qualname__, signal.__hash__() + def _get_model_classes(self, app_model: str) -> List[ModelBase]: + try: + try: + app_label, model_name = app_model.split(".") + return [apps.get_model(app_label, model_name)] + except ValueError: + return apps.get_app_config(app_model).get_models() + except LookupError: + return [] + + def _get_exclude_models( + self, exclude_tracking_models: Iterable[str] + ) -> List[ModelBase]: + exclude_models = [ + model + for app_model in exclude_tracking_models + self.DEFAULT_EXCLUDE_MODELS + for model in self._get_model_classes(app_model) + ] + return exclude_models + + def _register_models(self, models: Iterable[Union[str, Dict[str, Any]]]) -> None: + models = copy.deepcopy(models) + for model in models: + if isinstance(model, str): + for model_class in self._get_model_classes(model): + self.unregister(model_class) + self.register(model_class) + elif isinstance(model, dict): + model["model"] = self._get_model_classes(model["model"])[0] + self.unregister(model["model"]) + self.register(**model) + + def register_from_settings(self): + """ + Register models from settings variables + """ + if not isinstance(settings.AUDITLOG_INCLUDE_ALL_MODELS, bool): + raise TypeError("Setting 'AUDITLOG_INCLUDE_ALL_MODELS' must be a boolean") + + if not isinstance(settings.AUDITLOG_EXCLUDE_TRACKING_MODELS, (list, tuple)): + raise TypeError( + "Setting 'AUDITLOG_EXCLUDE_TRACKING_MODELS' must be a list or tuple" + ) + + if ( + not settings.AUDITLOG_INCLUDE_ALL_MODELS + and settings.AUDITLOG_EXCLUDE_TRACKING_MODELS + ): + raise ValueError( + "In order to use setting 'AUDITLOG_EXCLUDE_TRACKING_MODELS', " + "setting 'AUDITLOG_INCLUDE_ALL_MODELS' must set to 'True'" + ) + + if not isinstance(settings.AUDITLOG_INCLUDE_TRACKING_MODELS, (list, tuple)): + raise TypeError( + "Setting 'AUDITLOG_INCLUDE_TRACKING_MODELS' must be a list or tuple" + ) + + for item in settings.AUDITLOG_INCLUDE_TRACKING_MODELS: + if not isinstance(item, (str, dict)): + raise TypeError( + "Setting 'AUDITLOG_INCLUDE_TRACKING_MODELS' items must be str or dict" + ) + + if isinstance(item, dict): + if "model" not in item: + raise ValueError( + "Setting 'AUDITLOG_INCLUDE_TRACKING_MODELS' dict items must contain 'model' key" + ) + if "." not in item["model"]: + raise ValueError( + "Setting 'AUDITLOG_INCLUDE_TRACKING_MODELS' model must be in the format ." + ) + + if settings.AUDITLOG_INCLUDE_ALL_MODELS: + exclude_models = self._get_exclude_models( + settings.AUDITLOG_EXCLUDE_TRACKING_MODELS + ) + models = apps.get_models() + + for model in models: + if model in exclude_models: + continue + self.register(model) + + self._register_models(settings.AUDITLOG_INCLUDE_TRACKING_MODELS) + auditlog = AuditlogModelRegistry() diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index 6fd52da..5b18b5a 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -3,19 +3,20 @@ import json import django from dateutil.tz import gettz +from django.apps import apps from django.conf import settings from django.contrib import auth from django.contrib.auth.models import AnonymousUser, User from django.core.exceptions import ValidationError from django.db.models.signals import pre_save from django.http import HttpResponse -from django.test import RequestFactory, TestCase +from django.test import RequestFactory, TestCase, override_settings from django.utils import dateformat, formats, timezone from auditlog.diff import model_instance_diff from auditlog.middleware import AuditlogMiddleware from auditlog.models import LogEntry -from auditlog.registry import auditlog +from auditlog.registry import AuditlogModelRegistry, auditlog from auditlog_tests.models import ( AdditionalDataIncludedModel, AltPrimaryKeyModel, @@ -792,6 +793,155 @@ class UnregisterTest(TestCase): self.assertEqual(LogEntry.objects.count(), 0, msg="There are no log entries") +class RegisterModelSettingsTest(TestCase): + def setUp(self): + self.test_auditlog = AuditlogModelRegistry() + + def tearDown(self): + for model in self.test_auditlog.get_models(): + self.test_auditlog.unregister(model) + + def test_get_model_classes(self): + self.assertEqual( + len(list(self.test_auditlog._get_model_classes("auditlog"))), + len(list(apps.get_app_config("auditlog").get_models())), + ) + self.assertEqual([], self.test_auditlog._get_model_classes("fake_model")) + + def test_get_exclude_models(self): + # By default it returns DEFAULT_EXCLUDE_MODELS + self.assertEqual(len(self.test_auditlog._get_exclude_models(())), 2) + + # Exclude just one model + self.assertTrue( + SimpleExcludeModel + in self.test_auditlog._get_exclude_models( + ("auditlog_tests.SimpleExcludeModel",) + ) + ) + + # Exclude all model of an app + self.assertTrue( + SimpleExcludeModel + in self.test_auditlog._get_exclude_models(("auditlog_tests",)) + ) + + def test_register_models_no_models(self): + self.test_auditlog._register_models(()) + + self.assertEqual(self.test_auditlog._registry, {}) + + def test_register_models_register_single_model(self): + self.test_auditlog._register_models(("auditlog_tests.SimpleExcludeModel",)) + + self.assertTrue(self.test_auditlog.contains(SimpleExcludeModel)) + self.assertEqual(len(self.test_auditlog._registry), 1) + + def test_register_models_register_app(self): + self.test_auditlog._register_models(("auditlog_tests",)) + + self.assertTrue(self.test_auditlog.contains(SimpleExcludeModel)) + self.assertTrue(self.test_auditlog.contains(ChoicesFieldModel)) + self.assertEqual(len(self.test_auditlog.get_models()), 17) + + def test_register_models_register_model_with_attrs(self): + self.test_auditlog._register_models( + ( + { + "model": "auditlog_tests.SimpleExcludeModel", + "include_fields": ["label"], + "exclude_fields": [ + "text", + ], + }, + ) + ) + + self.assertTrue(self.test_auditlog.contains(SimpleExcludeModel)) + fields = self.test_auditlog.get_model_fields(SimpleExcludeModel) + self.assertEqual(fields["include_fields"], ["label"]) + self.assertEqual(fields["exclude_fields"], ["text"]) + + def test_register_from_settings_invalid_settings(self): + with override_settings(AUDITLOG_INCLUDE_ALL_MODELS="str"): + with self.assertRaisesMessage( + TypeError, "Setting 'AUDITLOG_INCLUDE_ALL_MODELS' must be a boolean" + ): + self.test_auditlog.register_from_settings() + + with override_settings(AUDITLOG_EXCLUDE_TRACKING_MODELS="str"): + with self.assertRaisesMessage( + TypeError, + "Setting 'AUDITLOG_EXCLUDE_TRACKING_MODELS' must be a list or tuple", + ): + self.test_auditlog.register_from_settings() + + with override_settings(AUDITLOG_EXCLUDE_TRACKING_MODELS=("app1.model1",)): + with self.assertRaisesMessage( + ValueError, + "In order to use setting 'AUDITLOG_EXCLUDE_TRACKING_MODELS', " + "setting 'AUDITLOG_INCLUDE_ALL_MODELS' must set to 'True'", + ): + self.test_auditlog.register_from_settings() + + with override_settings(AUDITLOG_INCLUDE_TRACKING_MODELS="str"): + with self.assertRaisesMessage( + TypeError, + "Setting 'AUDITLOG_INCLUDE_TRACKING_MODELS' must be a list or tuple", + ): + self.test_auditlog.register_from_settings() + + with override_settings(AUDITLOG_INCLUDE_TRACKING_MODELS=(1, 2)): + with self.assertRaisesMessage( + TypeError, + "Setting 'AUDITLOG_INCLUDE_TRACKING_MODELS' items must be str or dict", + ): + self.test_auditlog.register_from_settings() + + with override_settings(AUDITLOG_INCLUDE_TRACKING_MODELS=({"test": "test"},)): + with self.assertRaisesMessage( + ValueError, + "Setting 'AUDITLOG_INCLUDE_TRACKING_MODELS' dict items must contain 'model' key", + ): + self.test_auditlog.register_from_settings() + + with override_settings(AUDITLOG_INCLUDE_TRACKING_MODELS=({"model": "test"},)): + with self.assertRaisesMessage( + ValueError, + "Setting 'AUDITLOG_INCLUDE_TRACKING_MODELS' model must be in the format .", + ): + self.test_auditlog.register_from_settings() + + @override_settings( + AUDITLOG_INCLUDE_ALL_MODELS=True, + AUDITLOG_EXCLUDE_TRACKING_MODELS=("auditlog_tests.SimpleExcludeModel",), + ) + def test_register_from_settings_register_all_models_with_exclude_models(self): + self.test_auditlog.register_from_settings() + + self.assertFalse(self.test_auditlog.contains(SimpleExcludeModel)) + self.assertTrue(self.test_auditlog.contains(ChoicesFieldModel)) + + @override_settings( + AUDITLOG_INCLUDE_TRACKING_MODELS=( + { + "model": "auditlog_tests.SimpleExcludeModel", + "include_fields": ["label"], + "exclude_fields": [ + "text", + ], + }, + ) + ) + def test_register_from_settings_register_models(self): + self.test_auditlog.register_from_settings() + + self.assertTrue(self.test_auditlog.contains(SimpleExcludeModel)) + fields = self.test_auditlog.get_model_fields(SimpleExcludeModel) + self.assertEqual(fields["include_fields"], ["label"]) + self.assertEqual(fields["exclude_fields"], ["text"]) + + class ChoicesFieldModelTest(TestCase): def setUp(self): self.obj = ChoicesFieldModel.objects.create( diff --git a/docs/source/usage.rst b/docs/source/usage.rst index f5c3f99..f650c7c 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -91,6 +91,60 @@ For example, to mask the field ``address``, use:: Masking fields + +Settings +-------- + +**AUDITLOG_INCLUDE_ALL_MODELS** + +You can use this setting to register all your models: + +.. code-block:: python + + AUDITLOG_INCLUDE_ALL_MODELS=True + +.. versionadded:: 2.1.0 + +**AUDITLOG_EXCLUDE_TRACKING_MODELS** + +You can use this setting to exclude models in registration process. +It will be considered when ``AUDITLOG_INCLUDE_ALL_MODELS`` is `True`. + +.. code-block:: python + + AUDITLOG_EXCLUDE_TRACKING_MODELS = ( + "", + "." + ) + +.. versionadded:: 2.1.0 + +**AUDITLOG_INCLUDE_TRACKING_MODELS** + +You can use this setting to configure your models registration and other behaviours. +It must be a list or tuple. Each item in this setting can be a: + +* ``str``: To register a model. +* ``dict``: To register a model and define its logging behaviour. e.g. include_fields, exclude_fields. + +.. code-block:: python + + AUDITLOG_INCLUDE_TRACKING_MODELS = ( + ".", + { + "model": ".", + "include_fields": ["field1", "field2"], + "exclude_fields": ["field3", "field4"], + "mapping_fields": { + "field1": "FIELD", + }, + "mask_fields": ["field5", "field6"], + }, + ".", + ) + +.. versionadded:: 2.1.0 + Actors ------ From 340a01d3486251e28467b9546dbda31dd35d717b Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Mon, 23 May 2022 10:29:11 +0200 Subject: [PATCH 05/15] Replace importlib with pkg_resources with. Readthedoc uses Python3.7 Revert a30c8bbfc0a11e510005f80abc3c67cf62226510 --- docs/source/conf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/conf.py b/docs/source/conf.py index 2a4c7b4..dc3dc71 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -9,7 +9,7 @@ import os import sys from datetime import date -from importlib.metadata import version +from pkg_resources import get_distribution # -- Path setup -------------------------------------------------------------- @@ -32,7 +32,7 @@ project = "django-auditlog" author = "Jan-Jelle Kester and contributors" copyright = f"2013-{date.today().year}, {author}" -release = version("django-auditlog") +release = get_distribution('django-auditlog').version # for example take major/minor version = ".".join(release.split(".")[:2]) From 46abfbdc2d7823ad703550f807c51511dc3e9f1b Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Mon, 23 May 2022 10:29:24 +0200 Subject: [PATCH 06/15] Use Python3.7 for building the doc --- tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index cf086ee..78124df 100644 --- a/tox.ini +++ b/tox.ini @@ -2,7 +2,7 @@ envlist = {py37,py38,py39,py10}-django32 {py38,py39,py10}-django{40,main} - py38-docs + py37-docs py38-lint [testenv] @@ -30,7 +30,7 @@ basepython = py38: python3.8 py37: python3.7 -[testenv:py38-docs] +[testenv:py37-docs] changedir = docs/source deps = -rdocs/requirements.txt commands = sphinx-build -W -b html -d {envtmpdir}/doctrees . {envtmpdir}/html From ba19a8ca35894a9f4f36fe1efb6823ab0da910c6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 23 May 2022 08:31:09 +0000 Subject: [PATCH 07/15] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- docs/source/conf.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/source/conf.py b/docs/source/conf.py index dc3dc71..4dbcd08 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -9,6 +9,7 @@ import os import sys from datetime import date + from pkg_resources import get_distribution # -- Path setup -------------------------------------------------------------- @@ -32,7 +33,7 @@ project = "django-auditlog" author = "Jan-Jelle Kester and contributors" copyright = f"2013-{date.today().year}, {author}" -release = get_distribution('django-auditlog').version +release = get_distribution("django-auditlog").version # for example take major/minor version = ".".join(release.split(".")[:2]) From bcd0d43566f24a08839ac17e85864ad7b7ae8a90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alieh=20Ryma=C5=A1e=C5=ADski?= Date: Tue, 24 May 2022 10:33:54 +0300 Subject: [PATCH 08/15] Add set_actor context manager (#262) --- CHANGELOG.md | 1 + auditlog/context.py | 66 +++++++++++ auditlog/middleware.py | 98 +++------------- auditlog_tests/tests.py | 241 +++++++++++++++++++++++++++------------- docs/source/usage.rst | 19 ++++ 5 files changed, 265 insertions(+), 160 deletions(-) create mode 100644 auditlog/context.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0880ba5..12b151c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ #### Improvements - feat: Add register model from settings ([#368](https://github.com/jazzband/django-auditlog/pull/368)) +- Context manager set_actor() for use in Celery tasks ([#262](https://github.com/jazzband/django-auditlog/pull/262)) #### Fixes diff --git a/auditlog/context.py b/auditlog/context.py new file mode 100644 index 0000000..6e9513d --- /dev/null +++ b/auditlog/context.py @@ -0,0 +1,66 @@ +import contextlib +import threading +import time +from functools import partial + +from django.contrib.auth import get_user_model +from django.db.models.signals import pre_save + +from auditlog.models import LogEntry + +threadlocal = threading.local() + + +@contextlib.contextmanager +def set_actor(actor, remote_addr=None): + """Connect a signal receiver with current user attached.""" + # Initialize thread local storage + threadlocal.auditlog = { + "signal_duid": ("set_actor", time.time()), + "remote_addr": remote_addr, + } + + # Connect signal for automatic logging + set_actor = partial( + _set_actor, user=actor, signal_duid=threadlocal.auditlog["signal_duid"] + ) + pre_save.connect( + set_actor, + sender=LogEntry, + dispatch_uid=threadlocal.auditlog["signal_duid"], + weak=False, + ) + + try: + yield + finally: + try: + auditlog = threadlocal.auditlog + except AttributeError: + pass + else: + pre_save.disconnect(sender=LogEntry, dispatch_uid=auditlog["signal_duid"]) + del threadlocal.auditlog + + +def _set_actor(user, sender, instance, signal_duid, **kwargs): + """Signal receiver with extra 'user' and 'signal_duid' kwargs. + + This function becomes a valid signal receiver when it is curried with the actor and a dispatch id. + """ + try: + auditlog = threadlocal.auditlog + except AttributeError: + pass + else: + if signal_duid != auditlog["signal_duid"]: + return + auth_user_model = get_user_model() + if ( + sender == LogEntry + and isinstance(user, auth_user_model) + and instance.actor is None + ): + instance.actor = user + + instance.remote_addr = auditlog["remote_addr"] diff --git a/auditlog/middleware.py b/auditlog/middleware.py index ec1f563..202f114 100644 --- a/auditlog/middleware.py +++ b/auditlog/middleware.py @@ -1,97 +1,31 @@ -import threading -import time -from functools import partial +import contextlib -from django.apps import apps -from django.conf import settings -from django.db.models.signals import pre_save -from django.utils.deprecation import MiddlewareMixin - -from auditlog.models import LogEntry - -threadlocal = threading.local() +from auditlog.context import set_actor -class AuditlogMiddleware(MiddlewareMixin): +class AuditlogMiddleware: """ Middleware to couple the request's user to log items. This is accomplished by currying the signal receiver with the user from the request (or None if the user is not authenticated). """ - def process_request(self, request): - """ - Gets the current user from the request and prepares and connects a signal receiver with the user already - attached to it. - """ - # Initialize thread local storage - threadlocal.auditlog = { - "signal_duid": (self.__class__, time.time()), - "remote_addr": request.META.get("REMOTE_ADDR"), - } + def __init__(self, get_response=None): + self.get_response = get_response + + def __call__(self, request): - # In case of proxy, set 'original' address if request.META.get("HTTP_X_FORWARDED_FOR"): - threadlocal.auditlog["remote_addr"] = request.META.get( - "HTTP_X_FORWARDED_FOR" - ).split(",")[0] + # In case of proxy, set 'original' address + remote_addr = request.META.get("HTTP_X_FORWARDED_FOR").split(",")[0] + else: + remote_addr = request.META.get("REMOTE_ADDR") - # Connect signal for automatic logging if hasattr(request, "user") and getattr( request.user, "is_authenticated", False ): - set_actor = partial( - self.set_actor, - user=request.user, - signal_duid=threadlocal.auditlog["signal_duid"], - ) - pre_save.connect( - set_actor, - sender=LogEntry, - dispatch_uid=threadlocal.auditlog["signal_duid"], - weak=False, - ) + context = set_actor(actor=request.user, remote_addr=remote_addr) + else: + context = contextlib.nullcontext() - def process_response(self, request, response): - """ - Disconnects the signal receiver to prevent it from staying active. - """ - if hasattr(threadlocal, "auditlog"): - pre_save.disconnect( - sender=LogEntry, dispatch_uid=threadlocal.auditlog["signal_duid"] - ) - - return response - - def process_exception(self, request, exception): - """ - Disconnects the signal receiver to prevent it from staying active in case of an exception. - """ - if hasattr(threadlocal, "auditlog"): - pre_save.disconnect( - sender=LogEntry, dispatch_uid=threadlocal.auditlog["signal_duid"] - ) - - return None - - @staticmethod - def set_actor(user, sender, instance, signal_duid, **kwargs): - """ - Signal receiver with an extra, required 'user' kwarg. This method becomes a real (valid) signal receiver when - it is curried with the actor. - """ - if hasattr(threadlocal, "auditlog"): - if signal_duid != threadlocal.auditlog["signal_duid"]: - return - try: - app_label, model_name = settings.AUTH_USER_MODEL.split(".") - auth_user_model = apps.get_model(app_label, model_name) - except ValueError: - auth_user_model = apps.get_model("auth", "user") - if ( - sender == LogEntry - and isinstance(user, auth_user_model) - and instance.actor is None - ): - instance.actor = user - - instance.remote_addr = threadlocal.auditlog["remote_addr"] + with context: + return self.get_response(request) diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index 5b18b5a..2f4f28c 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -1,18 +1,20 @@ import datetime +import itertools import json +from unittest import mock -import django from dateutil.tz import gettz from django.apps import apps from django.conf import settings -from django.contrib import auth +from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser, User -from django.core.exceptions import ValidationError +from django.contrib.contenttypes.models import ContentType from django.db.models.signals import pre_save from django.http import HttpResponse from django.test import RequestFactory, TestCase, override_settings from django.utils import dateformat, formats, timezone +from auditlog.context import set_actor from auditlog.diff import model_instance_diff from auditlog.middleware import AuditlogMiddleware from auditlog.models import LogEntry @@ -40,7 +42,11 @@ from auditlog_tests.models import ( class SimpleModelTest(TestCase): def setUp(self): - self.obj = SimpleModel.objects.create(text="I am not difficult.") + self.obj = self.make_object() + super().setUp() + + def make_object(self): + return SimpleModel.objects.create(text="I am not difficult.") def test_create(self): """Creation is logged correctly.""" @@ -50,17 +56,14 @@ class SimpleModelTest(TestCase): # Check for log entries self.assertEqual(obj.history.count(), 1, msg="There is one log entry") - try: - history = obj.history.get() - except obj.history.DoesNotExist: - self.assertTrue(False, "Log entry exists") - else: - self.assertEqual( - history.action, LogEntry.Action.CREATE, msg="Action is 'CREATE'" - ) - self.assertEqual( - history.object_repr, str(obj), msg="Representation is equal" - ) + history = obj.history.get() + self.check_create_log_entry(obj, history) + + def check_create_log_entry(self, obj, history): + self.assertEqual( + history.action, LogEntry.Action.CREATE, msg="Action is 'CREATE'" + ) + self.assertEqual(history.object_repr, str(obj), msg="Representation is equal") def test_update(self): """Updates are logged correctly.""" @@ -68,8 +71,7 @@ class SimpleModelTest(TestCase): obj = self.obj # Change something - obj.boolean = True - obj.save() + self.update(obj) # Check for log entries self.assertEqual( @@ -79,7 +81,13 @@ class SimpleModelTest(TestCase): ) history = obj.history.get(action=LogEntry.Action.UPDATE) + self.check_update_log_entry(obj, history) + def update(self, obj): + obj.boolean = True + obj.save() + + def check_update_log_entry(self, obj, history): self.assertJSONEqual( history.changes, '{"boolean": ["False", "True"]}', @@ -134,25 +142,27 @@ class SimpleModelTest(TestCase): """Deletion is logged correctly.""" # Get the object to work with obj = self.obj - - history = obj.history.latest() + content_type = ContentType.objects.get_for_model(obj.__class__) + pk = obj.pk # Delete the object - obj.delete() + self.delete(obj) # Check for log entries - self.assertEqual( - LogEntry.objects.filter( - content_type=history.content_type, - object_pk=history.object_pk, - action=LogEntry.Action.DELETE, - ).count(), - 1, - msg="There is one log entry for 'DELETE'", - ) + qs = LogEntry.objects.filter(content_type=content_type, object_pk=pk) + self.assertEqual(qs.count(), 1, msg="There is one log entry for 'DELETE'") + + history = qs.get() + self.check_delete_log_entry(obj, history) + + def delete(self, obj): + obj.delete() + + def check_delete_log_entry(self, obj, history): + pass def test_recreate(self): - SimpleModel.objects.all().delete() + self.obj.delete() self.setUp() self.test_create() @@ -175,16 +185,79 @@ class SimpleModelTest(TestCase): ) # must be created in default database -class AltPrimaryKeyModelTest(SimpleModelTest): +class NoActorMixin: + def check_create_log_entry(self, obj, log_entry): + super().check_create_log_entry(obj, log_entry) + self.assertIsNone(log_entry.actor) + + def check_update_log_entry(self, obj, log_entry): + super().check_update_log_entry(obj, log_entry) + self.assertIsNone(log_entry.actor) + + def check_delete_log_entry(self, obj, log_entry): + super().check_delete_log_entry(obj, log_entry) + self.assertIsNone(log_entry.actor) + + +class WithActorMixin: + sequence = itertools.count() + def setUp(self): - self.obj = AltPrimaryKeyModel.objects.create( + username = "actor_{}".format(next(self.sequence)) + self.user = get_user_model().objects.create( + username=username, + email="{}@example.com".format(username), + password="secret", + ) + super().setUp() + + def tearDown(self): + self.user.delete() + super().tearDown() + + def make_object(self): + with set_actor(self.user): + return super().make_object() + + def check_create_log_entry(self, obj, log_entry): + super().check_create_log_entry(obj, log_entry) + self.assertEqual(log_entry.actor, self.user) + + def update(self, obj): + with set_actor(self.user): + return super().update(obj) + + def check_update_log_entry(self, obj, log_entry): + super().check_update_log_entry(obj, log_entry) + self.assertEqual(log_entry.actor, self.user) + + def delete(self, obj): + with set_actor(self.user): + return super().delete(obj) + + def check_delete_log_entry(self, obj, log_entry): + super().check_delete_log_entry(obj, log_entry) + self.assertEqual(log_entry.actor, self.user) + + +class AltPrimaryKeyModelBase(SimpleModelTest): + def make_object(self): + return AltPrimaryKeyModel.objects.create( key=str(datetime.datetime.now()), text="I am strange." ) -class UUIDPrimaryKeyModelModelTest(SimpleModelTest): - def setUp(self): - self.obj = UUIDPrimaryKeyModel.objects.create(text="I am strange.") +class AltPrimaryKeyModelTest(NoActorMixin, AltPrimaryKeyModelBase): + pass + + +class AltPrimaryKeyModelWithActorTest(WithActorMixin, AltPrimaryKeyModelBase): + pass + + +class UUIDPrimaryKeyModelModelBase(SimpleModelTest): + def make_object(self): + return UUIDPrimaryKeyModel.objects.create(text="I am strange.") def test_get_for_object(self): self.obj.boolean = True @@ -202,9 +275,27 @@ class UUIDPrimaryKeyModelModelTest(SimpleModelTest): ) -class ProxyModelTest(SimpleModelTest): - def setUp(self): - self.obj = ProxyModel.objects.create(text="I am not what you think.") +class UUIDPrimaryKeyModelModelTest(NoActorMixin, UUIDPrimaryKeyModelModelBase): + pass + + +class UUIDPrimaryKeyModelModelWithActorTest( + WithActorMixin, UUIDPrimaryKeyModelModelBase +): + pass + + +class ProxyModelBase(SimpleModelTest): + def make_object(self): + return ProxyModel.objects.create(text="I am not what you think.") + + +class ProxyModelTest(NoActorMixin, ProxyModelBase): + pass + + +class ProxyModelWithActorTest(WithActorMixin, ProxyModelBase): + pass class ManyRelatedModelTest(TestCase): @@ -237,72 +328,66 @@ class MiddlewareTest(TestCase): def get_response(request): return HttpResponse() - self.middleware = AuditlogMiddleware(get_response) + self.get_response_mock = mock.Mock() + self.response_mock = mock.Mock() + self.middleware = AuditlogMiddleware(get_response=self.get_response_mock) self.factory = RequestFactory() self.user = User.objects.create_user( username="test", email="test@example.com", password="top_secret" ) + def side_effect(self, assertion): + def inner(request): + assertion() + return self.response_mock + + return inner + + def assert_has_listeners(self): + self.assertTrue(pre_save.has_listeners(LogEntry)) + + def assert_no_listeners(self): + self.assertFalse(pre_save.has_listeners(LogEntry)) + def test_request_anonymous(self): """No actor will be logged when a user is not logged in.""" - # Create a request request = self.factory.get("/") request.user = AnonymousUser() - # Run middleware - self.middleware.process_request(request) + self.get_response_mock.side_effect = self.side_effect(self.assert_no_listeners) - # Validate result - self.assertFalse(pre_save.has_listeners(LogEntry)) + response = self.middleware(request) - # Finalize transaction - self.middleware.process_exception(request, None) + self.assertIs(response, self.response_mock) + self.get_response_mock.assert_called_once_with(request) + self.assert_no_listeners() def test_request(self): """The actor will be logged when a user is logged in.""" - # Create a request - request = self.factory.get("/") - request.user = self.user - # Run middleware - self.middleware.process_request(request) - - # Validate result - self.assertTrue(pre_save.has_listeners(LogEntry)) - - # Finalize transaction - self.middleware.process_exception(request, None) - - def test_response(self): - """The signal will be disconnected when the request is processed.""" - # Create a request request = self.factory.get("/") request.user = self.user - # Run middleware - self.middleware.process_request(request) - self.assertTrue( - pre_save.has_listeners(LogEntry) - ) # The signal should be present before trying to disconnect it. - self.middleware.process_response(request, HttpResponse()) + self.get_response_mock.side_effect = self.side_effect(self.assert_has_listeners) - # Validate result - self.assertFalse(pre_save.has_listeners(LogEntry)) + response = self.middleware(request) + + self.assertIs(response, self.response_mock) + self.get_response_mock.assert_called_once_with(request) + self.assert_no_listeners() def test_exception(self): """The signal will be disconnected when an exception is raised.""" - # Create a request request = self.factory.get("/") request.user = self.user - # Run middleware - self.middleware.process_request(request) - self.assertTrue( - pre_save.has_listeners(LogEntry) - ) # The signal should be present before trying to disconnect it. - self.middleware.process_exception(request, ValidationError("Test")) + SomeException = type("SomeException", (Exception,), {}) - # Validate result - self.assertFalse(pre_save.has_listeners(LogEntry)) + self.get_response_mock.side_effect = SomeException + + with self.assertRaises(SomeException): + self.middleware(request) + + self.assert_no_listeners() class SimpleIncludeModelTest(TestCase): diff --git a/docs/source/usage.rst b/docs/source/usage.rst index f650c7c..d0b3649 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -148,6 +148,9 @@ It must be a list or tuple. Each item in this setting can be a: Actors ------ +Middleware +********** + When using automatic logging, the actor is empty by default. However, auditlog can set the actor from the current request automatically. This does not need any custom code, adding a middleware class is enough. When an actor is logged the remote address of that actor will be logged as well. @@ -169,6 +172,22 @@ It is recommended to keep all middleware that alters the request loaded before A user as actor. To only have some object changes to be logged with the current request's user as actor manual logging is required. +Context manager +*************** + +.. versionadded:: 2.1.0 + +To enable the automatic logging of the actors outside of request context (e.g. in a Celery task), you can use a context +manager:: + + from auditlog.context import set_actor + + def do_stuff(actor_id: int): + actor = get_user(actor_id) + with set_actor(actor): + # if your code here leads to creation of LogEntry instances, these will have the actor set + ... + Object history -------------- From 1e7d320a93efda3f470641e9abefcb0e0733cd7a Mon Sep 17 00:00:00 2001 From: George Leslie-Waksman Date: Tue, 31 May 2022 00:01:13 -0700 Subject: [PATCH 09/15] Add an index to the timestamp column (#364) Many queries, including the default ordering, for the LogEntry model rely on the timestamp field. Adding an index will ensure reasonable scalability for large audit logs. --- CHANGELOG.md | 1 + .../0010_alter_logentry_timestamp.py | 20 +++++++++++++++++++ auditlog/models.py | 4 +++- 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 auditlog/migrations/0010_alter_logentry_timestamp.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b151c..5430345 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Improvements +- feat: Add db_index to the `LogEntry.timestamp` column ([#364](https://github.com/jazzband/django-auditlog/pull/364)) - feat: Add register model from settings ([#368](https://github.com/jazzband/django-auditlog/pull/368)) - Context manager set_actor() for use in Celery tasks ([#262](https://github.com/jazzband/django-auditlog/pull/262)) diff --git a/auditlog/migrations/0010_alter_logentry_timestamp.py b/auditlog/migrations/0010_alter_logentry_timestamp.py new file mode 100644 index 0000000..261828b --- /dev/null +++ b/auditlog/migrations/0010_alter_logentry_timestamp.py @@ -0,0 +1,20 @@ +# Generated by Django 4.0.3 on 2022-03-11 23:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("auditlog", "0009_alter_logentry_additional_data"), + ] + + operations = [ + migrations.AlterField( + model_name="logentry", + name="timestamp", + field=models.DateTimeField( + auto_now_add=True, db_index=True, verbose_name="timestamp" + ), + ), + ] diff --git a/auditlog/models.py b/auditlog/models.py index 76d4e7b..612c4dd 100644 --- a/auditlog/models.py +++ b/auditlog/models.py @@ -220,7 +220,9 @@ class LogEntry(models.Model): remote_addr = models.GenericIPAddressField( blank=True, null=True, verbose_name=_("remote address") ) - timestamp = models.DateTimeField(auto_now_add=True, verbose_name=_("timestamp")) + timestamp = models.DateTimeField( + db_index=True, auto_now_add=True, verbose_name=_("timestamp") + ) additional_data = models.JSONField( blank=True, null=True, verbose_name=_("additional data") ) From d9b0d76f3a7944c78e259652c8a09657c360629c Mon Sep 17 00:00:00 2001 From: Rich Rauenzahn Date: Tue, 31 May 2022 00:32:32 -0700 Subject: [PATCH 10/15] [tests] Check cases when reverse related field DoesNotExist. (#252) --- auditlog_tests/models.py | 13 +++++++++++-- auditlog_tests/tests.py | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/auditlog_tests/models.py b/auditlog_tests/models.py index f11e1d2..1d08aa1 100644 --- a/auditlog_tests/models.py +++ b/auditlog_tests/models.py @@ -60,12 +60,21 @@ class ProxyModel(SimpleModel): proxy = True -class RelatedModel(models.Model): +class RelatedModelParent(models.Model): + """ + Use multi table inheritance to make a OneToOneRel field + """ + + +class RelatedModel(RelatedModelParent): """ A model with a foreign key. """ - related = models.ForeignKey(to="self", on_delete=models.CASCADE) + related = models.ForeignKey(to="SimpleModel", on_delete=models.CASCADE) + one_to_one = models.OneToOneField( + to="SimpleModel", on_delete=models.CASCADE, related_name="reverse_one_to_one" + ) history = AuditlogHistoryField() diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index 2f4f28c..91e4b5e 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -927,7 +927,7 @@ class RegisterModelSettingsTest(TestCase): self.assertTrue(self.test_auditlog.contains(SimpleExcludeModel)) self.assertTrue(self.test_auditlog.contains(ChoicesFieldModel)) - self.assertEqual(len(self.test_auditlog.get_models()), 17) + self.assertEqual(len(self.test_auditlog.get_models()), 18) def test_register_models_register_model_with_attrs(self): self.test_auditlog._register_models( @@ -1276,7 +1276,39 @@ class JSONModelTest(TestCase): class ModelInstanceDiffTest(TestCase): - def test_when_field_doesnt_exit(self): + def test_diff_models_with_related_fields(self): + """No error is raised when comparing models with related fields.""" + + # This tests that track_field() does indeed ignore related fields. + + # a model without reverse relations + simple1 = SimpleModel() + simple1.save() + + # a model with reverse relations + simple2 = SimpleModel() + simple2.save() + related = RelatedModel(related=simple2, one_to_one=simple2) + related.save() + + # Demonstrate that simple1 can have DoesNotExist on reverse + # OneToOne relation. + with self.assertRaises( + SimpleModel.reverse_one_to_one.RelatedObjectDoesNotExist + ): + simple1.reverse_one_to_one # equals None + + # accessing relatedmodel_set won't trigger DoesNotExist. + self.assertEqual(simple1.relatedmodel_set.count(), 0) + + # simple2 DOES have these relations + self.assertEqual(simple2.reverse_one_to_one, related) + self.assertEqual(simple2.relatedmodel_set.count(), 1) + + model_instance_diff(simple2, simple1) + model_instance_diff(simple1, simple2) + + def test_when_field_doesnt_exist(self): """No error is raised and the default is returned.""" first = SimpleModel(boolean=True) second = SimpleModel() From 957680e239ba8f9380a7cd27345557ee5e48e07e Mon Sep 17 00:00:00 2001 From: Youngkwang Yang Date: Tue, 31 May 2022 20:05:43 +0900 Subject: [PATCH 11/15] Fix n+1 query problem (#381) --- CHANGELOG.md | 1 + auditlog/admin.py | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5430345..ea68d16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Fix inconsistent changes with JSONField ([#355](https://github.com/jazzband/django-auditlog/pull/355)) - Disable `add` button in admin ui ([#378](https://github.com/jazzband/django-auditlog/pull/378)) +- Fix n+1 query problem([#381](https://github.com/jazzband/django-auditlog/pull/381)) ## 2.0.0 (2022-05-09) diff --git a/auditlog/admin.py b/auditlog/admin.py index d05a448..b9f19f1 100644 --- a/auditlog/admin.py +++ b/auditlog/admin.py @@ -26,5 +26,9 @@ class LogEntryAdmin(admin.ModelAdmin, LogEntryAdminMixin): # As audit admin doesn't allow log creation from admin return False + def get_queryset(self, request): + queryset = super().get_queryset(request).select_related("content_type", "actor") + return queryset + admin.site.register(LogEntry, LogEntryAdmin) From dd89c3cefb2f5ab8c2c7c3e793774086266efeef Mon Sep 17 00:00:00 2001 From: Youngkwang Yang Date: Wed, 1 Jun 2022 00:50:34 +0900 Subject: [PATCH 12/15] Auditlog admin use list_select_related (#382) * Use list_select_related * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- auditlog/admin.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/auditlog/admin.py b/auditlog/admin.py index b9f19f1..2d79675 100644 --- a/auditlog/admin.py +++ b/auditlog/admin.py @@ -6,6 +6,7 @@ from auditlog.models import LogEntry class LogEntryAdmin(admin.ModelAdmin, LogEntryAdminMixin): + list_select_related = ["content_type", "actor"] list_display = ["created", "resource_url", "action", "msg_short", "user_url"] search_fields = [ "timestamp", @@ -26,9 +27,5 @@ class LogEntryAdmin(admin.ModelAdmin, LogEntryAdminMixin): # As audit admin doesn't allow log creation from admin return False - def get_queryset(self, request): - queryset = super().get_queryset(request).select_related("content_type", "actor") - return queryset - admin.site.register(LogEntry, LogEntryAdmin) From 128555fa29d71733cbba1201dd399f506e2ebf50 Mon Sep 17 00:00:00 2001 From: George Leslie-Waksman Date: Tue, 31 May 2022 10:05:50 -0700 Subject: [PATCH 13/15] Add date based auditlog filter for auditlogflush command (#365) --- CHANGELOG.md | 21 +--- auditlog/management/commands/auditlogflush.py | 24 +++- auditlog_tests/test_commands.py | 109 ++++++++++++++++++ docs/source/usage.rst | 15 ++- tox.ini | 1 + 5 files changed, 144 insertions(+), 26 deletions(-) create mode 100644 auditlog_tests/test_commands.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ea68d16..c9719c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Improvements +- feat: Add `--before-date` option to `auditlogflush` to support retention windows ([#365](https://github.com/jazzband/django-auditlog/pull/365)) - feat: Add db_index to the `LogEntry.timestamp` column ([#364](https://github.com/jazzband/django-auditlog/pull/364)) - feat: Add register model from settings ([#368](https://github.com/jazzband/django-auditlog/pull/368)) - Context manager set_actor() for use in Celery tasks ([#262](https://github.com/jazzband/django-auditlog/pull/262)) @@ -15,12 +16,14 @@ ## 2.0.0 (2022-05-09) #### Improvements + - feat: enable use of replica database (delegating the choice to `DATABASES_ROUTER`) ([#359](https://github.com/jazzband/django-auditlog/pull/359)) - Add `mask_fields` argument in `register` to mask sensitive information when logging ([#310](https://github.com/jazzband/django-auditlog/pull/310)) - Django: Drop 2.2 support. `django_jsonfield_backport` is not required anymore ([#370](https://github.com/jazzband/django-auditlog/pull/370)) - Remove `default_app_config` configuration ([#372](https://github.com/jazzband/django-auditlog/pull/372)) #### Important notes + - LogEntry no longer save to same database instance is using ## 1.0.0 (2022-01-24) @@ -54,7 +57,6 @@ - Support Django's save method `update_fields` kwarg ([#336](https://github.com/jazzband/django-auditlog/pull/336)) - Fix invalid escape sequence on Python 3.7 - ### Alpha 1 (1.0a1, 2020-09-07) #### Improvements @@ -69,14 +71,12 @@ - Fix field choices diff - Allow higher versions of python-dateutil than 2.6.0 - ## 0.4.8 (2019-11-12) ### Improvements - Add support for PostgreSQL 10 - ## 0.4.7 (2019-12-19) ### Improvements @@ -85,7 +85,6 @@ - Django: add 2.1 and 2.2 support, drop < 1.11 versions - Python: add 3.7 support - ## 0.4.6 (2018-09-18) ### Features @@ -102,14 +101,12 @@ - Fix the rendering of the `msg` field with Django 2.0 ([#166](https://github.com/jazzband/django-auditlog/pull/166)) - Mark `LogEntryAdminMixin` methods output as safe where required ([#167](https://github.com/jazzband/django-auditlog/pull/167)) - ## 0.4.5 (2018-01-12) ### Improvements Added support for Django 2.0, along with a number of bug fixes. - ## 0.4.4 (2017-11-17) ### Improvements @@ -126,14 +123,12 @@ Added support for Django 2.0, along with a number of bug fixes. - Add management commands package to setup.py ([#130](https://github.com/jazzband/django-auditlog/pull/130)) - Add `changes_display_dict` property to `LogEntry` model to display diff in a more human readable format ([#94](https://github.com/jazzband/django-auditlog/pull/94)) - ## 0.4.3 (2017-02-16) ### Fixes - Fixes cricital bug in admin mixin making the library only usable on Django 1.11 - ## 0.4.2 (2017-02-16) _As it turns out, haste is never good. Due to the focus on quickly releasing this version a nasty bug was not spotted, which makes this version only usable with Django 1.11 and above. Upgrading to 0.4.3 is not only encouraged but most likely necessary. Apologies for the inconvenience and lacking quality control._ @@ -147,7 +142,6 @@ _As it turns out, haste is never good. Due to the focus on quickly releasing thi - A lot, yes, [_really_ a lot](https://github.com/jjkester/django-auditlog/milestone/8?closed=1), of fixes for the admin integration - Flush command fixed for Django 1.10 - ## 0.4.1 (2016-12-27) ### Improvements @@ -158,7 +152,6 @@ _As it turns out, haste is never good. Due to the focus on quickly releasing thi - Fixed multithreading issue where the wrong user was written to the log - ## 0.4.0 (2016-08-17) ### Breaking changes @@ -179,7 +172,6 @@ _As it turns out, haste is never good. Due to the focus on quickly releasing thi - Solved migration error for MySQL users - ## 0.3.3 (2016-01-23) ### Fixes @@ -192,7 +184,6 @@ _As it turns out, haste is never good. Due to the focus on quickly releasing thi - The `object_pk` field is now limited to 255 chars - ## 0.3.2 (2015-10-19) ### New functionality @@ -203,14 +194,12 @@ _As it turns out, haste is never good. Due to the focus on quickly releasing thi - Enhanced performance for non-integer primary key lookups - ## 0.3.1 (2015-07-29) ### Fixes - Auditlog data is now correctly stored in the thread. - ## 0.3.0 (2015-07-22) ### Breaking changes @@ -231,14 +220,12 @@ _As it turns out, haste is never good. Due to the focus on quickly releasing thi - Better documentation - Compatibility with [django-polymorphic](https://pypi.org/project/django-polymorphic/) - ## 0.2.1 (2014-07-08) ### New functionality - South compatibility for `AuditlogHistoryField` - ## 0.2.0 (2014-03-08) Although this release contains mostly bugfixes, the improvements were significant enough to justify a higher version number. @@ -249,7 +236,6 @@ Although this release contains mostly bugfixes, the improvements were significan - Model diffs use unicode strings instead of regular strings - Tests on middleware - ## 0.1.1 (2013-12-12) ### New functionality @@ -261,7 +247,6 @@ Although this release contains mostly bugfixes, the improvements were significan - Only save a new log entry if there are actual changes - Better way of loading the user model in the middleware - ## 0.1.0 (2013-10-21) First beta release of Auditlog. diff --git a/auditlog/management/commands/auditlogflush.py b/auditlog/management/commands/auditlogflush.py index a2becd4..a5b1397 100644 --- a/auditlog/management/commands/auditlogflush.py +++ b/auditlog/management/commands/auditlogflush.py @@ -1,3 +1,5 @@ +import datetime + from django.core.management.base import BaseCommand from auditlog.models import LogEntry @@ -8,27 +10,43 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument( - "-y, --yes", + "-y", + "--yes", action="store_true", default=None, help="Continue without asking confirmation.", dest="yes", ) + parser.add_argument( + "-b", + "--before-date", + default=None, + help="Flush all entries with a timestamp before a given date (ISO 8601).", + dest="before_date", + type=datetime.date.fromisoformat, + ) def handle(self, *args, **options): answer = options["yes"] + before = options["before_date"] if answer is None: - self.stdout.write( + warning_message = ( "This action will clear all log entries from the database." ) + if before is not None: + warning_message = f"This action will clear all log entries before {before} from the database." + self.stdout.write(warning_message) response = ( input("Are you sure you want to continue? [y/N]: ").lower().strip() ) answer = response == "y" if answer: - count, _ = LogEntry.objects.all().delete() + entries = LogEntry.objects.all() + if before is not None: + entries = entries.filter(timestamp__lt=before) + count, _ = entries.delete() self.stdout.write("Deleted %d objects." % count) else: self.stdout.write("Aborted.") diff --git a/auditlog_tests/test_commands.py b/auditlog_tests/test_commands.py new file mode 100644 index 0000000..c7ec41b --- /dev/null +++ b/auditlog_tests/test_commands.py @@ -0,0 +1,109 @@ +"""Tests for auditlog.management.commands""" + +import datetime +from io import StringIO +from unittest import mock + +import freezegun +from django.core.management import call_command +from django.test import TestCase + +from auditlog_tests.models import SimpleModel + + +class AuditlogFlushTest(TestCase): + def setUp(self): + input_patcher = mock.patch("builtins.input") + self.mock_input = input_patcher.start() + self.addCleanup(input_patcher.stop) + + def make_object(self): + return SimpleModel.objects.create(text="I am a simple model.") + + def call_command(self, *args, **kwargs): + outbuf = StringIO() + errbuf = StringIO() + call_command("auditlogflush", *args, stdout=outbuf, stderr=errbuf, **kwargs) + return outbuf.getvalue().strip(), errbuf.getvalue().strip() + + def test_flush_yes(self): + obj = self.make_object() + self.assertEqual(obj.history.count(), 1, msg="There is one log entry.") + + out, err = self.call_command("--yes") + + self.assertEqual(obj.history.count(), 0, msg="There are no log entries.") + self.assertEqual( + out, "Deleted 1 objects.", msg="Output shows deleted 1 object." + ) + self.assertEqual(err, "", msg="No stderr") + + def test_flush_no(self): + obj = self.make_object() + self.assertEqual(obj.history.count(), 1, msg="There is one log entry.") + + self.mock_input.return_value = "N\n" + out, err = self.call_command() + + self.assertEqual(obj.history.count(), 1, msg="There is still one log entry.") + self.assertEqual( + out, + "This action will clear all log entries from the database.\nAborted.", + msg="Output shows warning and aborted.", + ) + self.assertEqual(err, "", msg="No stderr") + + def test_flush_input_yes(self): + obj = self.make_object() + self.assertEqual(obj.history.count(), 1, msg="There is one log entry.") + + self.mock_input.return_value = "Y\n" + out, err = self.call_command() + + self.assertEqual(obj.history.count(), 0, msg="There are no log entries.") + self.assertEqual( + out, + "This action will clear all log entries from the database.\nDeleted 1 objects.", + msg="Output shows warning and deleted 1 object.", + ) + self.assertEqual(err, "", msg="No stderr") + + def test_before_date_input(self): + self.mock_input.return_value = "N\n" + out, err = self.call_command("--before-date=2000-01-01") + self.assertEqual( + out, + "This action will clear all log entries before 2000-01-01 from the database.\nAborted.", + msg="Output shows warning with date and then aborted.", + ) + self.assertEqual(err, "", msg="No stderr") + + def test_before_date(self): + with freezegun.freeze_time("1999-12-31"): + obj = self.make_object() + + with freezegun.freeze_time("2000-01-02"): + obj.text = "I have new text" + obj.save() + + self.assertEqual( + {v["timestamp"] for v in obj.history.values("timestamp")}, + { + datetime.datetime(1999, 12, 31, tzinfo=datetime.timezone.utc), + datetime.datetime(2000, 1, 2, tzinfo=datetime.timezone.utc), + }, + msg="Entries exist for 1999-12-31 and 2000-01-02", + ) + + out, err = self.call_command("--yes", "--before-date=2000-01-01") + self.assertEqual( + {v["timestamp"] for v in obj.history.values("timestamp")}, + { + datetime.datetime(2000, 1, 2, tzinfo=datetime.timezone.utc), + }, + msg="An entry exists only for 2000-01-02", + ) + self.assertEqual( + out, "Deleted 1 objects.", msg="Output shows deleted 1 object." + ) + self.assertEqual(err, "", msg="No stderr") diff --git a/docs/source/usage.rst b/docs/source/usage.rst index d0b3649..5696a1d 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -56,15 +56,15 @@ If you have field names on your models that aren't intuitive or user friendly yo during the `register()` call. .. code-block:: python - + class MyModel(modelsModel): sku = models.CharField(max_length=20) version = models.CharField(max_length=5) product = models.CharField(max_length=50, verbose_name='Product Name') history = AuditlogHistoryField() - + auditlog.register(MyModel, mapping_fields={'sku': 'Product No.', 'version': 'Product Revision'}) - + .. code-block:: python log = MyModel.objects.first().history.latest() @@ -279,12 +279,17 @@ Management commands Auditlog provides the ``auditlogflush`` management command to clear all log entries from the database. -By default, the command asks for confirmation. It is possible to run the command with the `-y` or `--yes` flag to skip +By default, the command asks for confirmation. It is possible to run the command with the ``-y`` or ``--yes`` flag to skip confirmation and immediately delete all entries. +You may also specify a date using the ``-b`` or ``--before-date`` option in ISO 8601 format (YYYY-mm-dd) to delete all +log entries prior to a given date. This may be used to implement time based retention windows. + +.. versionadded:: 2.1.0 + .. warning:: - Using the ``auditlogflush`` command deletes all log entries permanently and irreversibly from the database. + Using the ``auditlogflush`` command deletes log entries permanently and irreversibly from the database. Django Admin integration ------------------------ diff --git a/tox.ini b/tox.ini index 78124df..bda1a65 100644 --- a/tox.ini +++ b/tox.ini @@ -16,6 +16,7 @@ deps = # Test requirements coverage codecov + freezegun psycopg2-binary==2.8.6 passenv= TEST_DB_HOST From 2e9466d1b49f97a5f89d6945aa4f78e1935bf760 Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Wed, 8 Jun 2022 16:47:13 +0200 Subject: [PATCH 14/15] Change auditlogflush management command before_date query filter to apply the filter only on date part of timestamp (#384) --- auditlog/management/commands/auditlogflush.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auditlog/management/commands/auditlogflush.py b/auditlog/management/commands/auditlogflush.py index a5b1397..e57ef2c 100644 --- a/auditlog/management/commands/auditlogflush.py +++ b/auditlog/management/commands/auditlogflush.py @@ -45,7 +45,7 @@ class Command(BaseCommand): if answer: entries = LogEntry.objects.all() if before is not None: - entries = entries.filter(timestamp__lt=before) + entries = entries.filter(timestamp__date__lt=before) count, _ = entries.delete() self.stdout.write("Deleted %d objects." % count) else: From 10c47181bb38ba49b0cb5927f5dab3fe4f8842e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alieh=20Ryma=C5=A1e=C5=ADski?= Date: Wed, 8 Jun 2022 18:09:27 +0300 Subject: [PATCH 15/15] Add logic to track changes to m2m fields (#309) --- CHANGELOG.md | 1 + auditlog/mixins.py | 67 +++++++++++++--- auditlog/models.py | 48 ++++++++++++ auditlog/receivers.py | 31 ++++++++ auditlog/registry.py | 70 ++++++++++++++--- auditlog_tests/models.py | 24 +++++- auditlog_tests/tests.py | 162 ++++++++++++++++++++++++++++++++++++--- docs/source/usage.rst | 21 ++++- 8 files changed, 382 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9719c0..0b62cd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - feat: Add db_index to the `LogEntry.timestamp` column ([#364](https://github.com/jazzband/django-auditlog/pull/364)) - feat: Add register model from settings ([#368](https://github.com/jazzband/django-auditlog/pull/368)) - Context manager set_actor() for use in Celery tasks ([#262](https://github.com/jazzband/django-auditlog/pull/262)) +- Tracking of changes in many-to-many fields ([#309](https://github.com/jazzband/django-auditlog/pull/309)) #### Fixes diff --git a/auditlog/mixins.py b/auditlog/mixins.py index 436f830..1105751 100644 --- a/auditlog/mixins.py +++ b/auditlog/mixins.py @@ -3,7 +3,7 @@ import json from django import urls as urlresolvers from django.conf import settings from django.urls.exceptions import NoReverseMatch -from django.utils.html import format_html +from django.utils.html import format_html, format_html_join from django.utils.safestring import mark_safe from auditlog.models import LogEntry @@ -63,16 +63,61 @@ class LogEntryAdminMixin: if obj.action == LogEntry.Action.DELETE: return "" # delete changes = json.loads(obj.changes) - msg = "" - for i, field in enumerate(sorted(changes), 1): - value = [i, field] + ( - ["***", "***"] if field == "password" else changes[field] - ) - msg += format_html( - "", *value - ) - msg += "
#FieldFromTo
{}{}{}{}
" - return mark_safe(msg) + atom_changes = {} + m2m_changes = {} + + for field, change in changes.items(): + if isinstance(change, dict): + assert ( + change["type"] == "m2m" + ), "Only m2m operations are expected to produce dict changes now" + m2m_changes[field] = change + else: + atom_changes[field] = change + + msg = [] + + if atom_changes: + msg.append("") + msg.append(self._format_header("#", "Field", "From", "To")) + for i, (field, change) in enumerate(sorted(atom_changes.items()), 1): + value = [i, field] + (["***", "***"] if field == "password" else change) + msg.append(self._format_line(*value)) + msg.append("
") + + if m2m_changes: + msg.append("") + msg.append(self._format_header("#", "Relationship", "Action", "Objects")) + for i, (field, change) in enumerate(sorted(m2m_changes.items()), 1): + change_html = format_html_join( + mark_safe("
"), + "{}", + [(value,) for value in change["objects"]], + ) + + msg.append( + format_html( + "", + i, + field, + change["operation"], + change_html, + ) + ) + + msg.append("
{}{}{}{}
") + + return mark_safe("".join(msg)) msg.short_description = "Changes" + + def _format_header(self, *labels): + return format_html( + "".join(["", "{}" * len(labels), ""]), *labels + ) + + def _format_line(self, *values): + return format_html( + "".join(["", "{}" * len(values), ""]), *values + ) diff --git a/auditlog/models.py b/auditlog/models.py index 612c4dd..b8ba813 100644 --- a/auditlog/models.py +++ b/auditlog/models.py @@ -69,6 +69,54 @@ class LogEntryManager(models.Manager): return self.create(**kwargs) return None + def log_m2m_changes( + self, changed_queryset, instance, operation, field_name, **kwargs + ): + """Create a new "changed" log entry from m2m record. + + :param changed_queryset: The added or removed related objects. + :type changed_queryset: QuerySet + :param instance: The model instance to log a change for. + :type instance: Model + :param operation: "add" or "delete". + :type action: str + :param field_name: The name of the changed m2m field. + :type field_name: str + :param kwargs: Field overrides for the :py:class:`LogEntry` object. + :return: The new log entry or `None` if there were no changes. + :rtype: LogEntry + """ + + pk = self._get_pk_value(instance) + if changed_queryset is not None: + kwargs.setdefault( + "content_type", ContentType.objects.get_for_model(instance) + ) + kwargs.setdefault("object_pk", pk) + kwargs.setdefault("object_repr", smart_str(instance)) + kwargs.setdefault("action", LogEntry.Action.UPDATE) + + if isinstance(pk, int): + kwargs.setdefault("object_id", pk) + + get_additional_data = getattr(instance, "get_additional_data", None) + if callable(get_additional_data): + kwargs.setdefault("additional_data", get_additional_data()) + + objects = [smart_str(instance) for instance in changed_queryset] + kwargs["changes"] = json.dumps( + { + field_name: { + "type": "m2m", + "operation": operation, + "objects": objects, + } + } + ) + return self.create(**kwargs) + + return None + def get_for_object(self, instance): """ Get log entries for the specified model instance. diff --git a/auditlog/receivers.py b/auditlog/receivers.py index 3debd0f..c2d3bb7 100644 --- a/auditlog/receivers.py +++ b/auditlog/receivers.py @@ -59,3 +59,34 @@ def log_delete(sender, instance, **kwargs): action=LogEntry.Action.DELETE, changes=json.dumps(changes), ) + + +def make_log_m2m_changes(field_name): + """Return a handler for m2m_changed with field_name enclosed.""" + + def log_m2m_changes(signal, action, **kwargs): + """Handle m2m_changed and call LogEntry.objects.log_m2m_changes as needed.""" + if action not in ["post_add", "post_clear", "post_remove"]: + return + + if action == "post_clear": + changed_queryset = kwargs["model"].objects.all() + else: + changed_queryset = kwargs["model"].objects.filter(pk__in=kwargs["pk_set"]) + + if action in ["post_add"]: + LogEntry.objects.log_m2m_changes( + changed_queryset, + kwargs["instance"], + "add", + field_name, + ) + elif action in ["post_remove", "post_clear"]: + LogEntry.objects.log_m2m_changes( + changed_queryset, + kwargs["instance"], + "delete", + field_name, + ) + + return log_m2m_changes diff --git a/auditlog/registry.py b/auditlog/registry.py index 8093213..1663384 100644 --- a/auditlog/registry.py +++ b/auditlog/registry.py @@ -1,14 +1,31 @@ import copy -from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple, Union +from collections import defaultdict +from typing import ( + Any, + Callable, + Collection, + Dict, + Iterable, + List, + Optional, + Tuple, + Union, +) from django.apps import apps from django.db.models import Model from django.db.models.base import ModelBase -from django.db.models.signals import ModelSignal, post_delete, post_save, pre_save +from django.db.models.signals import ( + ModelSignal, + m2m_changed, + post_delete, + post_save, + pre_save, +) from auditlog.conf import settings -DispatchUID = Tuple[int, str, int] +DispatchUID = Tuple[int, int, int] class AuditlogModelRegistry: @@ -23,12 +40,14 @@ class AuditlogModelRegistry: create: bool = True, update: bool = True, delete: bool = True, + m2m: bool = True, custom: Optional[Dict[ModelSignal, Callable]] = None, ): from auditlog.receivers import log_create, log_delete, log_update self._registry = {} self._signals = {} + self._m2m_signals = defaultdict(dict) if create: self._signals[post_save] = log_create @@ -36,6 +55,7 @@ class AuditlogModelRegistry: self._signals[pre_save] = log_update if delete: self._signals[post_delete] = log_delete + self._m2m = m2m if custom is not None: self._signals.update(custom) @@ -47,6 +67,7 @@ class AuditlogModelRegistry: exclude_fields: Optional[List[str]] = None, mapping_fields: Optional[Dict[str, str]] = None, mask_fields: Optional[List[str]] = None, + m2m_fields: Optional[Collection[str]] = None, ): """ Register a model with auditlog. Auditlog will then track mutations on this model's instances. @@ -56,6 +77,7 @@ class AuditlogModelRegistry: :param exclude_fields: The fields to exclude. Overrides the fields to include. :param mapping_fields: Mapping from field names to strings in diff. :param mask_fields: The fields to mask for sensitive info. + :param m2m_fields: The fields to handle as many to many. """ @@ -67,6 +89,8 @@ class AuditlogModelRegistry: mapping_fields = {} if mask_fields is None: mask_fields = [] + if m2m_fields is None: + m2m_fields = set() def registrar(cls): """Register models for a given class.""" @@ -78,6 +102,7 @@ class AuditlogModelRegistry: "exclude_fields": exclude_fields, "mapping_fields": mapping_fields, "mask_fields": mask_fields, + "m2m_fields": m2m_fields, } self._connect_signals(cls) @@ -132,11 +157,26 @@ class AuditlogModelRegistry: """ Connect signals for the model. """ - for signal in self._signals: - receiver = self._signals[signal] + from auditlog.receivers import make_log_m2m_changes + + for signal, receiver in self._signals.items(): signal.connect( - receiver, sender=model, dispatch_uid=self._dispatch_uid(signal, model) + receiver, + sender=model, + dispatch_uid=self._dispatch_uid(signal, receiver), ) + if self._m2m: + for field_name in self._registry[model]["m2m_fields"]: + receiver = make_log_m2m_changes(field_name) + self._m2m_signals[model][field_name] = receiver + field = getattr(model, field_name) + m2m_model = getattr(field, "through") + + m2m_changed.connect( + receiver, + sender=m2m_model, + dispatch_uid=self._dispatch_uid(m2m_changed, receiver), + ) def _disconnect_signals(self, model): """ @@ -144,14 +184,20 @@ class AuditlogModelRegistry: """ for signal, receiver in self._signals.items(): signal.disconnect( - sender=model, dispatch_uid=self._dispatch_uid(signal, model) + sender=model, dispatch_uid=self._dispatch_uid(signal, receiver) ) + for field_name, receiver in self._m2m_signals[model].items(): + field = getattr(model, field_name) + m2m_model = getattr(field, "through") + m2m_changed.disconnect( + sender=m2m_model, + dispatch_uid=self._dispatch_uid(m2m_changed, receiver), + ) + del self._m2m_signals[model] - def _dispatch_uid(self, signal, model) -> DispatchUID: - """ - Generate a dispatch_uid. - """ - return self.__hash__(), model.__qualname__, signal.__hash__() + def _dispatch_uid(self, signal, receiver) -> DispatchUID: + """Generate a dispatch_uid which is unique for a combination of self, signal, and receiver.""" + return id(self), id(signal), id(receiver) def _get_model_classes(self, app_model: str) -> List[ModelBase]: try: diff --git a/auditlog_tests/models.py b/auditlog_tests/models.py index 1d08aa1..68d04d5 100644 --- a/auditlog_tests/models.py +++ b/auditlog_tests/models.py @@ -4,7 +4,9 @@ from django.contrib.postgres.fields import ArrayField from django.db import models from auditlog.models import AuditlogHistoryField -from auditlog.registry import auditlog +from auditlog.registry import AuditlogModelRegistry, auditlog + +m2m_only_auditlog = AuditlogModelRegistry(create=False, update=False, delete=False) @auditlog.register() @@ -81,10 +83,23 @@ class RelatedModel(RelatedModelParent): class ManyRelatedModel(models.Model): """ - A model with a many to many relation. + A model with many-to-many relations. """ - related = models.ManyToManyField("self") + recursive = models.ManyToManyField("self") + related = models.ManyToManyField("ManyRelatedOtherModel", related_name="related") + + history = AuditlogHistoryField() + + def get_additional_data(self): + related = self.related.first() + return {"related_model_id": related.id if related else None} + + +class ManyRelatedOtherModel(models.Model): + """ + A model related to ManyRelatedModel as many-to-many. + """ history = AuditlogHistoryField() @@ -250,7 +265,8 @@ auditlog.register(UUIDPrimaryKeyModel) auditlog.register(ProxyModel) auditlog.register(RelatedModel) auditlog.register(ManyRelatedModel) -auditlog.register(ManyRelatedModel.related.through) +auditlog.register(ManyRelatedModel.recursive.through) +m2m_only_auditlog.register(ManyRelatedModel, m2m_fields={"related"}) auditlog.register(SimpleExcludeModel, exclude_fields=["text"]) auditlog.register(SimpleMappingModel, mapping_fields={"sku": "Product No."}) auditlog.register(AdditionalDataIncludedModel) diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index 91e4b5e..b79f1a6 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -6,14 +6,15 @@ from unittest import mock from dateutil.tz import gettz from django.apps import apps from django.conf import settings +from django.contrib.admin.sites import AdminSite from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser, User from django.contrib.contenttypes.models import ContentType from django.db.models.signals import pre_save -from django.http import HttpResponse from django.test import RequestFactory, TestCase, override_settings from django.utils import dateformat, formats, timezone +from auditlog.admin import LogEntryAdmin from auditlog.context import set_actor from auditlog.diff import model_instance_diff from auditlog.middleware import AuditlogMiddleware @@ -27,6 +28,7 @@ from auditlog_tests.models import ( DateTimeFieldModel, JSONModel, ManyRelatedModel, + ManyRelatedOtherModel, NoDeleteHistoryModel, PostgresArrayFieldModel, ProxyModel, @@ -300,22 +302,65 @@ class ProxyModelWithActorTest(WithActorMixin, ProxyModelBase): class ManyRelatedModelTest(TestCase): """ - Test the behaviour of a many-to-many relationship. + Test the behaviour of many-to-many relationships. """ def setUp(self): self.obj = ManyRelatedModel.objects.create() - self.rel_obj = ManyRelatedModel.objects.create() - self.obj.related.add(self.rel_obj) + self.recursive = ManyRelatedModel.objects.create() + self.related = ManyRelatedOtherModel.objects.create() + self.base_log_entry_count = ( + LogEntry.objects.count() + ) # created by the create() calls above - def test_related(self): + def test_recursive(self): + self.obj.recursive.add(self.recursive) self.assertEqual( - LogEntry.objects.get_for_objects(self.obj.related.all()).count(), - self.rel_obj.history.count(), + LogEntry.objects.get_for_objects(self.obj.recursive.all()).first(), + self.recursive.history.first(), ) + + def test_related_add_from_first_side(self): + self.obj.related.add(self.related) self.assertEqual( LogEntry.objects.get_for_objects(self.obj.related.all()).first(), - self.rel_obj.history.first(), + self.related.history.first(), + ) + self.assertEqual(LogEntry.objects.count(), self.base_log_entry_count + 1) + + def test_related_add_from_other_side(self): + self.related.related.add(self.obj) + self.assertEqual( + LogEntry.objects.get_for_objects(self.obj.related.all()).first(), + self.related.history.first(), + ) + self.assertEqual(LogEntry.objects.count(), self.base_log_entry_count + 1) + + def test_related_remove_from_first_side(self): + self.obj.related.add(self.related) + self.obj.related.remove(self.related) + self.assertEqual(LogEntry.objects.count(), self.base_log_entry_count + 2) + + def test_related_remove_from_other_side(self): + self.related.related.add(self.obj) + self.related.related.remove(self.obj) + self.assertEqual(LogEntry.objects.count(), self.base_log_entry_count + 2) + + def test_related_clear_from_first_side(self): + self.obj.related.add(self.related) + self.obj.related.clear() + self.assertEqual(LogEntry.objects.count(), self.base_log_entry_count + 2) + + def test_related_clear_from_other_side(self): + self.related.related.add(self.obj) + self.related.related.clear() + self.assertEqual(LogEntry.objects.count(), self.base_log_entry_count + 2) + + def test_additional_data(self): + self.obj.related.add(self.related) + log_entry = self.obj.history.first() + self.assertEqual( + log_entry.additional_data, {"related_model_id": self.related.id} ) @@ -325,9 +370,6 @@ class MiddlewareTest(TestCase): """ def setUp(self): - def get_response(request): - return HttpResponse() - self.get_response_mock = mock.Mock() self.response_mock = mock.Mock() self.middleware = AuditlogMiddleware(get_response=self.get_response_mock) @@ -927,7 +969,7 @@ class RegisterModelSettingsTest(TestCase): self.assertTrue(self.test_auditlog.contains(SimpleExcludeModel)) self.assertTrue(self.test_auditlog.contains(ChoicesFieldModel)) - self.assertEqual(len(self.test_auditlog.get_models()), 18) + self.assertEqual(len(self.test_auditlog.get_models()), 19) def test_register_models_register_model_with_attrs(self): self.test_auditlog._register_models( @@ -947,6 +989,21 @@ class RegisterModelSettingsTest(TestCase): self.assertEqual(fields["include_fields"], ["label"]) self.assertEqual(fields["exclude_fields"], ["text"]) + def test_register_models_register_model_with_m2m_fields(self): + self.test_auditlog._register_models( + ( + { + "model": "auditlog_tests.ManyRelatedModel", + "m2m_fields": {"related"}, + }, + ) + ) + + self.assertTrue(self.test_auditlog.contains(ManyRelatedModel)) + self.assertEqual( + self.test_auditlog._registry[ManyRelatedModel]["m2m_fields"], {"related"} + ) + def test_register_from_settings_invalid_settings(self): with override_settings(AUDITLOG_INCLUDE_ALL_MODELS="str"): with self.assertRaisesMessage( @@ -1177,6 +1234,87 @@ class AdminPanelTest(TestCase): assert res.status_code == 200 +class DiffMsgTest(TestCase): + def setUp(self): + super().setUp() + self.site = AdminSite() + self.admin = LogEntryAdmin(LogEntry, self.site) + + def _create_log_entry(self, action, changes): + return LogEntry.objects.log_create( + SimpleModel.objects.create(), # doesn't affect anything + action=action, + changes=json.dumps(changes), + ) + + def test_changes_msg__delete(self): + log_entry = self._create_log_entry(LogEntry.Action.DELETE, {}) + + self.assertEqual(self.admin.msg(log_entry), "") + + def test_changes_msg__create(self): + log_entry = self._create_log_entry( + LogEntry.Action.CREATE, + { + "field two": [None, 11], + "field one": [None, "a value"], + }, + ) + + self.assertEqual( + self.admin.msg(log_entry), + ( + "" + "" + "" + "" + "
#FieldFromTo
1field oneNonea value
2field twoNone11
" + ), + ) + + def test_changes_msg__update(self): + log_entry = self._create_log_entry( + LogEntry.Action.UPDATE, + { + "field two": [11, 42], + "field one": ["old value of field one", "new value of field one"], + }, + ) + + self.assertEqual( + self.admin.msg(log_entry), + ( + "" + "" + "" + "" + "
#FieldFromTo
1field oneold value of field onenew value of field one
2field two1142
" + ), + ) + + def test_changes_msg__m2m(self): + log_entry = self._create_log_entry( + LogEntry.Action.UPDATE, + { # mimicking the format used by log_m2m_changes + "some_m2m_field": { + "type": "m2m", + "operation": "add", + "objects": ["Example User (user 1)", "Illustration (user 42)"], + }, + }, + ) + + self.assertEqual( + self.admin.msg(log_entry), + ( + "" + "" + "" + "
#RelationshipActionObjects
1some_m2m_fieldaddExample User (user 1)
Illustration (user 42)
" + ), + ) + + class NoDeleteHistoryTest(TestCase): def test_delete_related(self): instance = SimpleModel.objects.create(integer=1) diff --git a/docs/source/usage.rst b/docs/source/usage.rst index 5696a1d..05154f6 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -11,6 +11,8 @@ even more convenience, :py:class:`LogEntryManager` provides a number of methods See :doc:`internals` for all details. +.. _Automatically logging changes: + Automatically logging changes ----------------------------- @@ -91,6 +93,19 @@ For example, to mask the field ``address``, use:: Masking fields +**Many-to-many fields** + +Changes to many-to-many fields are not tracked by default. If you want to enable tracking of a many-to-many field on a model, pass ``m2m_fields`` to the ``register`` method: + +.. code-block:: python + + auditlog.register(MyModel, m2m_fields={"tags", "contacts"}) + +This functionality is based on the ``m2m_changed`` signal sent by the ``through`` model of the relationship. + +Note that when the user changes multiple many-to-many fields on the same object through the admin, both adding and removing some objects from each, this code will generate multiple log entries: each log entry will represent a single operation (add or delete) of a single field, e.g. if you both add and delete values from 2 fields on the same form in the same request, you'll get 4 log entries. + +.. versionadded:: 2.1.0 Settings -------- @@ -139,6 +154,7 @@ It must be a list or tuple. Each item in this setting can be a: "field1": "FIELD", }, "mask_fields": ["field5", "field6"], + "m2m_fields": ["field7", "field8"], }, ".", ) @@ -250,10 +266,9 @@ Many-to-many relationships .. versionadded:: 0.3.0 -.. warning:: +.. note:: - To-many relations are not officially supported. However, this section shows a workaround which can be used for now. - In the future, this workaround may be used in an official API or a completly different strategy might be chosen. + This section shows a workaround which can be used to track many-to-many relationships on older versions of django-auditlog. For versions 2.1.0 and onwards, please see the many-to-many fields section of :ref:`Automatically logging changes`. **Do not rely on the workaround here to be stable across releases.** By default, many-to-many relationships are not tracked by Auditlog.