From e6ea1f1ead910a1b25e93e83909c30a4c333f384 Mon Sep 17 00:00:00 2001 From: vasileios Date: Fri, 13 May 2022 08:56:09 +0200 Subject: [PATCH 1/6] [#559] Updated logging --- src/open_inwoner/accounts/signals.py | 19 +- .../accounts/tests/test_logging.py | 238 ++++++++++++------ src/open_inwoner/accounts/views/__init__.py | 5 + src/open_inwoner/accounts/views/auth.py | 35 +++ src/open_inwoner/conf/base.py | 2 +- src/open_inwoner/haalcentraal/signals.py | 5 +- .../haalcentraal/tests/test_signal.py | 80 ++++++ src/open_inwoner/pdc/admin/category.py | 14 +- src/open_inwoner/pdc/admin/product.py | 14 +- src/open_inwoner/pdc/tests/test_logging.py | 163 ++++++++++++ src/open_inwoner/search/tests/test_logging.py | 46 ++++ src/open_inwoner/search/views.py | 8 +- src/open_inwoner/urls.py | 23 +- src/open_inwoner/utils/admin.py | 8 +- src/open_inwoner/utils/logentry.py | 10 +- src/open_inwoner/utils/mixins.py | 2 +- src/open_inwoner/utils/signals.py | 25 +- src/open_inwoner/utils/views.py | 2 +- 18 files changed, 593 insertions(+), 106 deletions(-) create mode 100644 src/open_inwoner/accounts/views/auth.py create mode 100644 src/open_inwoner/pdc/tests/test_logging.py create mode 100644 src/open_inwoner/search/tests/test_logging.py diff --git a/src/open_inwoner/accounts/signals.py b/src/open_inwoner/accounts/signals.py index b1c6b0cdd1..ddc6fafb19 100644 --- a/src/open_inwoner/accounts/signals.py +++ b/src/open_inwoner/accounts/signals.py @@ -5,16 +5,27 @@ from open_inwoner.utils.logentry import user_action +MESSAGE_TYPE = { + "admin": _("user was logged in via admin page"), + "frontend_email": _("user was logged in via frontend using email"), + "frontend_digid": _("user was logged in via frontend using digid"), + "logout": _("user was logged out"), +} + @receiver(user_logged_in) def log_user_login(sender, user, request, *args, **kwargs): - if request.path.startswith(reverse("admin:login")): - user_action(request, user, _("user was logged in via admin page")) + current_path = request.path + + if current_path == reverse("admin:login"): + user_action(request, user, MESSAGE_TYPE["admin"]) + elif current_path == reverse("digid:acs"): + user_action(request, user, MESSAGE_TYPE["frontend_digid"]) else: - user_action(request, user, _("user was logged in via frontend")) + user_action(request, user, MESSAGE_TYPE["frontend_email"]) @receiver(user_logged_out) def log_user_logout(sender, user, request, *args, **kwargs): if user: - user_action(request, user, _("user was logged out")) + user_action(request, user, MESSAGE_TYPE["logout"]) diff --git a/src/open_inwoner/accounts/tests/test_logging.py b/src/open_inwoner/accounts/tests/test_logging.py index 7904840bb7..5120672a3a 100644 --- a/src/open_inwoner/accounts/tests/test_logging.py +++ b/src/open_inwoner/accounts/tests/test_logging.py @@ -2,7 +2,7 @@ from django.contrib.admin.models import ADDITION, CHANGE, DELETION from django.core.files.uploadedfile import SimpleUploadedFile -from django.urls import reverse, reverse_lazy +from django.urls import reverse from django.utils import timezone from django.utils.translation import ugettext_lazy as _ @@ -15,6 +15,7 @@ from open_inwoner.pdc.tests.factories import CategoryFactory from open_inwoner.utils.logentry import LOG_ACTIONS +from ..choices import LoginTypeChoices from ..models import Action, Contact, Document, Message, User from .factories import ( ActionFactory, @@ -46,11 +47,11 @@ def test_registration_is_logged(self): form.submit() log_entry = TimelineLog.objects.first() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, User.objects.all()[1].id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, User.objects.all()[1].id) + self.assertEqual( log_entry.extra_data, { "message": _("user was created"), @@ -67,11 +68,11 @@ def test_users_modification_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { "message": _("profile was modified"), @@ -91,11 +92,11 @@ def test_categories_modification_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { "message": _("categories were modified"), @@ -108,11 +109,11 @@ def test_login_via_admin_is_logged(self): self.app.post(reverse("admin:login"), user=self.user) log_entry = TimelineLog.objects.first() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { "message": _("user was logged in via admin page"), @@ -121,32 +122,50 @@ def test_login_via_admin_is_logged(self): }, ) - def test_login_via_frontend_is_logged(self): + def test_login_via_frontend_using_email_is_logged(self): self.app.post(reverse("login"), user=self.user) log_entry = TimelineLog.objects.first() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { - "message": _("user was logged in via frontend"), + "message": _("user was logged in via frontend using email"), "action_flag": list(LOG_ACTIONS[4]), "content_object_repr": self.user.email, }, ) + def test_login_via_frontend_using_digid_is_logged(self): + user = UserFactory(login_type=LoginTypeChoices.digid) + self.app.get(reverse("digid:acs"), {"bsn": "123123222"}, user=user) + log_entry = TimelineLog.objects.first() + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual(log_entry.content_object.id, user.id) + self.assertEqual( + log_entry.extra_data, + { + "message": _("user was logged in via frontend using digid"), + "action_flag": list(LOG_ACTIONS[4]), + "content_object_repr": user.email, + }, + ) + def test_logout_is_logged(self): self.app.get(reverse("logout"), user=self.user) log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { "message": _("user was logged out"), @@ -162,11 +181,11 @@ def test_users_deactivation_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { "message": _("user was deactivated via frontend"), @@ -175,6 +194,77 @@ def test_users_deactivation_is_logged(self): }, ) + def test_password_change_is_logged(self): + user = UserFactory(password="test") + form = self.app.get(reverse("password_change"), user=user).forms[0] + form["old_password"] = "test" + form["new_password1"] = "newPassw0rd" + form["new_password2"] = "newPassw0rd" + form.submit() + log_entry = TimelineLog.objects.last() + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual(log_entry.content_object.id, user.id) + self.assertEqual( + log_entry.extra_data, + { + "message": _("password was changed"), + "action_flag": list(LOG_ACTIONS[4]), + "content_object_repr": user.email, + }, + ) + + def test_password_reset_access_is_logged(self): + form = self.app.get(reverse("password_reset")).forms["password-reset-form"] + form["email"] = self.user.email + form.submit() + log_entry = TimelineLog.objects.last() + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual( + log_entry.extra_data, + { + "message": _("password reset was accessed"), + "log_level": None, + "action_flag": list(LOG_ACTIONS[5]), + "content_object_repr": "", + }, + ) + + def test_password_reset_confirm_is_logged(self): + form = self.app.get(reverse("password_reset")).forms["password-reset-form"] + form["email"] = self.user.email + response = form.submit() + + token = response.context[0]["token"] + uid = response.context[0]["uid"] + confirm_response = self.app.get( + reverse("password_reset_confirm", kwargs={"token": token, "uidb64": uid}) + ).follow() + form = confirm_response.forms["password-reset-confirm"] + form["new_password1"] = "passW0rd@" + form["new_password2"] = "passW0rd@" + form.submit() + + log_entry = TimelineLog.objects.last() + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual( + log_entry.extra_data, + { + "message": _("password reset was completed"), + "log_level": None, + "action_flag": list(LOG_ACTIONS[5]), + "content_object_repr": self.user.email, + }, + ) + @freeze_time("2021-10-18 13:00:00") class TestInvites(WebTest): @@ -189,16 +279,16 @@ def test_accepted_invite_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.invitee.id, self.invitee.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.invitee.id, self.invitee.id) + self.assertEqual( log_entry.extra_data, { "message": _("invitation accepted"), "log_level": None, - "action_flag": list(LOG_ACTIONS[4]), + "action_flag": list(LOG_ACTIONS[5]), "content_object_repr": _("For: {invitee} (2021-10-18)").format( invitee=self.invitee.email ), @@ -215,16 +305,16 @@ def test_expired_invite_is_logged(self): log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.invitee.id, self.invitee.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.invitee.id, self.invitee.id) + self.assertEqual( log_entry.extra_data, { "message": _("invitation expired"), "log_level": None, - "action_flag": list(LOG_ACTIONS[4]), + "action_flag": list(LOG_ACTIONS[5]), "content_object_repr": _("For: {invitee} (2021-09-18)").format( invitee=self.invitee.email ), @@ -251,13 +341,13 @@ def test_contact_addition_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals( + self.assertEqual( log_entry.content_object.id, Contact.objects.get(email=contact.email).id ) - self.assertEquals( + self.assertEqual( log_entry.extra_data, { "message": _("contact was created"), @@ -275,11 +365,11 @@ def test_contact_update_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.contact.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.contact.id) + self.assertEqual( log_entry.extra_data, { "message": _("contact was modified"), @@ -295,11 +385,11 @@ def test_contact_deletion_is_logged(self): ) log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.object_id, str(self.contact.id)) - self.assertEquals( + self.assertEqual(log_entry.object_id, str(self.contact.id)) + self.assertEqual( log_entry.extra_data, { "action_flag": list(LOG_ACTIONS[DELETION]), @@ -324,11 +414,11 @@ def test_document_download_is_logged(self): ) log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.document.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.document.id) + self.assertEqual( log_entry.extra_data, { "message": _("file was downloaded"), @@ -348,11 +438,11 @@ def test_document_upload_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, Document.objects.all()[1].id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, Document.objects.all()[1].id) + self.assertEqual( log_entry.extra_data, { "message": _("file was uploaded"), @@ -369,11 +459,11 @@ def test_document_deletion_is_logged(self): ) log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.object_id, str(self.document.id)) - self.assertEquals( + self.assertEqual(log_entry.object_id, str(self.document.id)) + self.assertEqual( log_entry.extra_data, { "message": _("file was deleted"), @@ -398,11 +488,11 @@ def test_action_addition_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, Action.objects.all()[1].id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, Action.objects.all()[1].id) + self.assertEqual( log_entry.extra_data, { "message": _("action was created"), @@ -420,11 +510,11 @@ def test_action_update_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.action.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.action.id) + self.assertEqual( log_entry.extra_data, { "message": _("action was modified"), @@ -454,11 +544,11 @@ def test_created_message_action_from_contacts_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, Message.objects.first().id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, Message.objects.first().id) + self.assertEqual( log_entry.extra_data, { "message": _("message was created"), @@ -479,11 +569,11 @@ def test_created_message_action_from_start_is_logged(self): form["content"] = "some content" form.submit() log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, Message.objects.first().id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, Message.objects.first().id) + self.assertEqual( log_entry.extra_data, { "message": _("message was created"), @@ -503,11 +593,11 @@ def test_download_file_from_messages_is_logged(self): self.app.get(download_url, user=message.receiver) log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, message.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, message.id) + self.assertEqual( log_entry.extra_data, { "message": _("file was downloaded"), @@ -530,11 +620,11 @@ def test_profile_export_is_logged(self): self.app.get(reverse("accounts:profile_export"), user=self.user) log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { "message": _("file profile.pdf was exported"), @@ -550,11 +640,11 @@ def test_action_export_is_logged(self): ) log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { "message": _("file action_{action_uuid}.pdf was exported").format( @@ -572,11 +662,11 @@ def test_action_list_export_is_logged(self): ) log_entry = TimelineLog.objects.last() - self.assertEquals( + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) - self.assertEquals(log_entry.content_object.id, self.user.id) - self.assertEquals( + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( log_entry.extra_data, { "message": _("file actions.pdf was exported"), diff --git a/src/open_inwoner/accounts/views/__init__.py b/src/open_inwoner/accounts/views/__init__.py index 69a7a6532b..9f01b069b4 100644 --- a/src/open_inwoner/accounts/views/__init__.py +++ b/src/open_inwoner/accounts/views/__init__.py @@ -6,6 +6,11 @@ ActionPrivateMediaView, ActionUpdateView, ) +from .auth import ( + LogPasswordChangeView, + LogPasswordResetConfirmView, + LogPasswordResetView, +) from .cases import CasesListView, CasesStatusView from .contacts import ( ContactCreateView, diff --git a/src/open_inwoner/accounts/views/auth.py b/src/open_inwoner/accounts/views/auth.py new file mode 100644 index 0000000000..1a840c19e2 --- /dev/null +++ b/src/open_inwoner/accounts/views/auth.py @@ -0,0 +1,35 @@ +from django.contrib.auth import update_session_auth_hash +from django.contrib.auth.views import ( + PasswordChangeView, + PasswordResetConfirmView, + PasswordResetView, +) +from django.http import HttpResponseRedirect +from django.utils.translation import gettext as _ + +from open_inwoner.utils.views import LogMixin + + +class LogPasswordChangeView(LogMixin, PasswordChangeView): + def form_valid(self, form): + object = form.save() + self.log_user_action(object, _("password was changed")) + # Updating the password logs out all other sessions for the user + # except the current one. + update_session_auth_hash(self.request, form.user) + return super().form_valid(form) + + +class LogPasswordResetView(LogMixin, PasswordResetView): + def form_valid(self, form): + self.log_system_action(_("password reset was accessed")) + return super().form_valid(form) + + +class LogPasswordResetConfirmView(LogMixin, PasswordResetConfirmView): + def form_valid(self, form): + super().form_valid(form) + object = self.get_user(self.kwargs["uidb64"]) + + self.log_system_action(_("password reset was completed"), object) + return HttpResponseRedirect(self.get_success_url()) diff --git a/src/open_inwoner/conf/base.py b/src/open_inwoner/conf/base.py index 68ad42a471..6d411f18f1 100644 --- a/src/open_inwoner/conf/base.py +++ b/src/open_inwoner/conf/base.py @@ -778,7 +778,7 @@ # file upload limits MIN_UPLOAD_SIZE = 1 # in bytes -MAX_UPLOAD_SIZE = 1024 ** 2 * 100 # 100MB +MAX_UPLOAD_SIZE = 1024**2 * 100 # 100MB UPLOAD_FILE_TYPES = "application/vnd.openxmlformats-officedocument.wordprocessingml.document,application/msword,application/vnd.openxmlformats-officedocument.spreadsheetml.sheet,application/vnd.ms-excel,text/plain,application/vnd.oasis.opendocument.text,application/vnd.oasis.opendocument.formula,application/vnd.oasis.opendocument.spreadsheet,application/pdf,image/jpeg,image/png" # diff --git a/src/open_inwoner/haalcentraal/signals.py b/src/open_inwoner/haalcentraal/signals.py index 024253943a..e68a727265 100644 --- a/src/open_inwoner/haalcentraal/signals.py +++ b/src/open_inwoner/haalcentraal/signals.py @@ -1,9 +1,9 @@ import logging -from sys import exc_info from urllib.parse import urljoin from django.db.models.signals import pre_save from django.dispatch import receiver +from django.utils.translation import gettext as _ from glom import PathAccessError, glom from requests import RequestException @@ -12,6 +12,7 @@ from open_inwoner.accounts.choices import LoginTypeChoices from open_inwoner.accounts.models import User from open_inwoner.haalcentraal.models import HaalCentraalConfig +from open_inwoner.utils.logentry import system_action logger = logging.getLogger(__name__) @@ -62,3 +63,5 @@ def on_bsn_change(instance, **kwargs): logger.exception( "exception while trying to access fetched data", exc_info=e ) + else: + system_action(_("data was retrieved from haal centraal"), instance) diff --git a/src/open_inwoner/haalcentraal/tests/test_signal.py b/src/open_inwoner/haalcentraal/tests/test_signal.py index 52bcd53fc3..70bf5f5f90 100644 --- a/src/open_inwoner/haalcentraal/tests/test_signal.py +++ b/src/open_inwoner/haalcentraal/tests/test_signal.py @@ -3,12 +3,16 @@ from datetime import date from django.test import TestCase +from django.utils.translation import gettext as _ import requests_mock +from freezegun import freeze_time +from timeline_logger.models import TimelineLog from open_inwoner.accounts.choices import LoginTypeChoices from open_inwoner.accounts.models import User from open_inwoner.accounts.tests.factories import UserFactory +from open_inwoner.utils.logentry import LOG_ACTIONS from ..models import HaalCentraalConfig from .factories import ServiceFactory @@ -184,3 +188,79 @@ def test_user_is_not_updated_when_http_500(self, m): self.assertEqual(updated_user[0].last_name, "") self.assertEqual(updated_user[0].birthday, None) self.assertFalse(updated_user[0].is_prepopulated) + + +class TestLogging(TestCase): + @freeze_time("2021-10-18 13:00:00") + @requests_mock.Mocker() + def test_signal_updates_logging(self, m): + m.get( + "https://personen/api/schema/openapi.yaml?v=3", + status_code=200, + content=load_binary_mock("personen.yaml"), + ) + m.get( + "https://personen/api/ingeschrevenpersonen/999993847", + status_code=200, + json=load_json_mock("ingeschrevenpersonen.999993847.json"), + ) + + config = HaalCentraalConfig.get_solo() + service = ServiceFactory( + api_root="https://personen/api/", + oas="https://personen/api/schema/openapi.yaml", + ) + config.service = service + config.save() + + user = UserFactory( + first_name="", last_name="", login_type=LoginTypeChoices.digid + ) + user.bsn = "999993847" + user.save() + + log_entry = TimelineLog.objects.first() + + self.assertEquals( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEquals(log_entry.content_object.id, user.id) + self.assertEquals( + log_entry.extra_data, + { + "message": _("data was retrieved from haal centraal"), + "log_level": None, + "action_flag": list(LOG_ACTIONS[5]), + "content_object_repr": user.email, + }, + ) + + @requests_mock.Mocker() + def test_nothing_is_logged_when_there_is_an_error(self, m): + m.get( + "https://personen/api/schema/openapi.yaml?v=3", + status_code=200, + content=load_binary_mock("personen.yaml"), + ) + m.get( + "https://personen/api/ingeschrevenpersonen/999993847", + status_code=500, + ) + + config = HaalCentraalConfig.get_solo() + service = ServiceFactory( + api_root="https://personen/api/", + oas="https://personen/api/schema/openapi.yaml", + ) + config.service = service + config.save() + + user = UserFactory( + first_name="", last_name="", login_type=LoginTypeChoices.digid + ) + user.bsn = "999993847" + user.save() + + log_entries = TimelineLog.objects.count() + + self.assertEqual(log_entries, 0) diff --git a/src/open_inwoner/pdc/admin/category.py b/src/open_inwoner/pdc/admin/category.py index a917dd9be4..d5e4e8f9ad 100644 --- a/src/open_inwoner/pdc/admin/category.py +++ b/src/open_inwoner/pdc/admin/category.py @@ -1,17 +1,20 @@ from django.contrib import admin +from django.utils.translation import gettext as _ from import_export.admin import ImportExportMixin from import_export.formats import base_formats from treebeard.admin import TreeAdmin from treebeard.forms import movenodeform_factory +from open_inwoner.utils.views import LogMixin + from ..models import Category from ..resources import CategoryExportResource, CategoryImportResource from .faq import QuestionInline @admin.register(Category) -class CategoryAdmin(ImportExportMixin, TreeAdmin): +class CategoryAdmin(ImportExportMixin, LogMixin, TreeAdmin): change_list_template = "admin/category_change_list.html" form = movenodeform_factory(Category, fields="__all__") inlines = (QuestionInline,) @@ -26,3 +29,12 @@ class CategoryAdmin(ImportExportMixin, TreeAdmin): def get_export_resource_class(self): return CategoryExportResource + + def export_action(self, request, *args, **kwargs): + response = super().export_action(request, *args, **kwargs) + + if request.method == "POST": + user = request.user + self.log_system_action(_("categories were exported"), user) + + return response diff --git a/src/open_inwoner/pdc/admin/product.py b/src/open_inwoner/pdc/admin/product.py index 140001719f..ae3d887f67 100644 --- a/src/open_inwoner/pdc/admin/product.py +++ b/src/open_inwoner/pdc/admin/product.py @@ -1,12 +1,13 @@ from django import forms from django.contrib import admin -from django.utils.translation import ugettext_lazy as _ +from django.utils.translation import gettext as _ from import_export.admin import ImportExportMixin from import_export.formats import base_formats from ordered_model.admin import OrderedModelAdmin from open_inwoner.ckeditor5.widgets import CKEditorWidget +from open_inwoner.utils.views import LogMixin from ..models import ( Product, @@ -38,7 +39,7 @@ class Meta: @admin.register(Product) -class ProductAdmin(ImportExportMixin, admin.ModelAdmin): +class ProductAdmin(ImportExportMixin, LogMixin, admin.ModelAdmin): list_display = ("name", "created_on", "display_categories") list_filter = ("categories", "tags") date_hierarchy = "created_on" @@ -69,6 +70,15 @@ class ProductAdmin(ImportExportMixin, admin.ModelAdmin): def get_export_resource_class(self): return ProductExportResource + def export_action(self, request, *args, **kwargs): + response = super().export_action(request, *args, **kwargs) + + if request.method == "POST": + user = request.user + self.log_system_action(_("products were exported"), user) + + return response + def get_queryset(self, request): qs = super().get_queryset(request) return qs.prefetch_related("links", "locations", "contacts") diff --git a/src/open_inwoner/pdc/tests/test_logging.py b/src/open_inwoner/pdc/tests/test_logging.py new file mode 100644 index 0000000000..9fdb51e4e5 --- /dev/null +++ b/src/open_inwoner/pdc/tests/test_logging.py @@ -0,0 +1,163 @@ +from urllib import response + +from django.urls import reverse +from django.utils.translation import gettext as _ + +import tablib +from django_webtest import WebTest +from freezegun import freeze_time +from timeline_logger.models import TimelineLog +from webtest import Upload + +from open_inwoner.accounts.tests.factories import UserFactory +from open_inwoner.utils.logentry import LOG_ACTIONS + +from ..models.category import Category +from ..models.product import Product +from .factories import CategoryFactory, ProductFactory + + +@freeze_time("2021-10-18 13:00:00") +class TestProductLogging(WebTest): + def setUp(self): + self.category = CategoryFactory() + self.product = ProductFactory.build(categories=(self.category,)) + self.user = UserFactory(is_superuser=True, is_staff=True) + + def test_import_is_logged(self): + dataset = tablib.Dataset( + [ + self.product.name, + self.product.summary, + self.product.content, + self.category.slug, + "", + "", + "", + "", + "", + "", + ], + headers=[ + "name", + "summary", + "content", + "categories", + "slug", + "link", + "related_products", + "tags", + "costs", + "organizations", + ], + ) + byte_data = str.encode(dataset.export("csv")) + response = self.app.get(reverse("admin:pdc_product_import"), user=self.user) + form = response.forms[0] + form["import_file"] = Upload("products.csv", byte_data, "text/csv") + form["input_format"] = 1 + response_form = form.submit().forms[0] + response_form.submit() + log_entry = TimelineLog.objects.last() + product = Product.objects.first() + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual(log_entry.content_object.id, product.id) + self.assertEqual( + log_entry.extra_data, + { + "message": _("new through import_export"), + "action_flag": list(LOG_ACTIONS[1]), + "content_object_repr": self.product.name, + }, + ) + + def test_export_is_logged(self): + ProductFactory(categories=(self.category,)) + response = self.app.get(reverse("admin:pdc_product_export"), user=self.user) + form = response.forms[0] + form["file_format"] = 1 + form.submit() + log_entry = TimelineLog.objects.last() + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( + log_entry.extra_data, + { + "message": _("products were exported"), + "log_level": None, + "action_flag": list(LOG_ACTIONS[5]), + "content_object_repr": self.user.email, + }, + ) + + +@freeze_time("2021-10-18 13:00:00") +class TestCategoryLogging(WebTest): + def setUp(self): + self.category = CategoryFactory.build() + self.user = UserFactory(is_superuser=True, is_staff=True) + + def test_import_is_logged(self): + category = CategoryFactory.build() + dataset = tablib.Dataset( + [ + self.category.name, + self.category.description, + "", + ], + headers=[ + "name", + "description", + "slug", + ], + ) + byte_data = str.encode(dataset.export("csv")) + response = self.app.get(reverse("admin:pdc_category_import"), user=self.user) + form = response.forms[0] + form["import_file"] = Upload("categories.csv", byte_data, "text/csv") + form["input_format"] = 1 + response_form = form.submit().forms[0] + response_form.submit() + log_entry = TimelineLog.objects.last() + category = Category.objects.first() + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual(log_entry.content_object.id, category.id) + self.assertEqual( + log_entry.extra_data, + { + "message": _("new through import_export"), + "action_flag": list(LOG_ACTIONS[1]), + "content_object_repr": self.category.name, + }, + ) + + def test_export_is_logged(self): + CategoryFactory() + response = self.app.get(reverse("admin:pdc_category_export"), user=self.user) + form = response.forms[0] + form["file_format"] = 1 + form.submit() + log_entry = TimelineLog.objects.last() + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual(log_entry.content_object.id, self.user.id) + self.assertEqual( + log_entry.extra_data, + { + "message": _("categories were exported"), + "log_level": None, + "action_flag": list(LOG_ACTIONS[5]), + "content_object_repr": self.user.email, + }, + ) diff --git a/src/open_inwoner/search/tests/test_logging.py b/src/open_inwoner/search/tests/test_logging.py new file mode 100644 index 0000000000..f8efbf71c0 --- /dev/null +++ b/src/open_inwoner/search/tests/test_logging.py @@ -0,0 +1,46 @@ +from django.urls import reverse +from django.utils.translation import gettext as _ + +from django_webtest import WebTest +from freezegun import freeze_time +from timeline_logger.models import TimelineLog + +from open_inwoner.accounts.tests.factories import UserFactory +from open_inwoner.utils.logentry import LOG_ACTIONS + +from .utils import ESMixin + + +@freeze_time("2021-10-18 13:00:00") +class TestLogging(ESMixin, WebTest): + def test_search_query_of_logged_in_user_is_logged(self): + user = UserFactory() + form = self.app.get(reverse("search:search"), user=user).forms["search-form"] + form["query"] = "search for something" + form.submit() + + log_entry = TimelineLog.objects.all()[1] + + self.assertEqual( + log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" + ) + self.assertEqual(log_entry.content_object.id, user.id) + self.assertEqual( + log_entry.extra_data, + { + "message": (_("search query: {query}")).format( + query="search for something" + ), + "action_flag": list(LOG_ACTIONS[4]), + "content_object_repr": user.email, + }, + ) + + def test_search_query_of_anonymous_user_is_not_logged(self): + form = self.app.get(reverse("search:search")).forms["search-form"] + form["query"] = "search for something" + form.submit() + + log_entries = TimelineLog.objects.count() + + self.assertEqual(log_entries, 0) diff --git a/src/open_inwoner/search/views.py b/src/open_inwoner/search/views.py index 9c9e5bdcb6..d9df268c63 100644 --- a/src/open_inwoner/search/views.py +++ b/src/open_inwoner/search/views.py @@ -6,12 +6,13 @@ from django.views.generic import FormView from open_inwoner.utils.mixins import PaginationMixin +from open_inwoner.utils.views import LogMixin from .forms import FeedbackForm, SearchForm from .searches import search_products -class SearchView(PaginationMixin, FormView): +class SearchView(LogMixin, PaginationMixin, FormView): form_class = SearchForm template_name = "pages/search.html" paginate_by = 20 @@ -40,10 +41,15 @@ def search(self, form): data = form.cleaned_data.copy() query = data.pop("query") context = self.get_context_data(form=form) + user = self.request.user if not query: return self.render_to_response(context) + # log search query of authenticated users + if user.is_authenticated: + self.log_user_action(user, (_("search query: {query}")).format(query=query)) + # perform search results = search_products(query, filters=data) diff --git a/src/open_inwoner/urls.py b/src/open_inwoner/urls.py index 539720e029..9aed9701c7 100644 --- a/src/open_inwoner/urls.py +++ b/src/open_inwoner/urls.py @@ -7,7 +7,13 @@ from django.urls import include, path from open_inwoner.accounts.forms import CustomRegistrationForm -from open_inwoner.accounts.views import CustomRegistrationView, PasswordResetView +from open_inwoner.accounts.views import ( + CustomRegistrationView, + LogPasswordChangeView, + LogPasswordResetConfirmView, + LogPasswordResetView, + PasswordResetView, +) from open_inwoner.pdc.views import FAQView, HomeView handler500 = "open_inwoner.utils.views.server_error" @@ -46,6 +52,21 @@ CustomRegistrationView.as_view(form_class=CustomRegistrationForm), name="django_registration_register", ), + path( + "accounts/password_change/", + LogPasswordChangeView.as_view(), + name="password_change", + ), + path( + "accounts/password_reset/", + LogPasswordResetView.as_view(), + name="password_reset", + ), + path( + "accounts/reset///", + LogPasswordResetConfirmView.as_view(), + name="password_reset_confirm", + ), path("accounts/", include("django_registration.backends.one_step.urls")), path("accounts/", include("django.contrib.auth.urls")), path("api/", include("open_inwoner.api.urls", namespace="api")), diff --git a/src/open_inwoner/utils/admin.py b/src/open_inwoner/utils/admin.py index 3ce4adc2be..48c4cbf189 100644 --- a/src/open_inwoner/utils/admin.py +++ b/src/open_inwoner/utils/admin.py @@ -1,6 +1,6 @@ from django.contrib import admin from django.contrib.admin.models import ADDITION, CHANGE, DELETION -from django.utils.translation import ugettext_lazy as _ +from django.utils.translation import gettext as _ from import_export.admin import ExportMixin from import_export.formats import base_formats @@ -22,7 +22,9 @@ class CustomTimelineLogAdmin(ExportMixin, TimelineLogAdmin): ] def get_object_title(self, obj): - return f"{obj.content_type.name} - {obj.object_id}" + if obj.content_type: + return f"{obj.content_type.name} - {obj.object_id}" + return _("System - anonymous user") get_object_title.short_description = _("Logboekvermelding") @@ -36,6 +38,8 @@ def get_action_flag(self, obj): return _("Verwijderd") if obj.extra_data.get("action_flag")[0] == 4: return _("User action") + if obj.extra_data.get("action_flag")[0] == 5: + return _("System action") return "" get_action_flag.short_description = _("Actie") diff --git a/src/open_inwoner/utils/logentry.py b/src/open_inwoner/utils/logentry.py index ae75a23d45..a2be702bbb 100644 --- a/src/open_inwoner/utils/logentry.py +++ b/src/open_inwoner/utils/logentry.py @@ -12,6 +12,7 @@ models.CHANGE: (models.CHANGE, "Change"), models.DELETION: (models.DELETION, "Deletion"), 4: (4, "User action"), + 5: (5, "System action"), } logger = logging.getLogger(__name__) @@ -105,12 +106,13 @@ def user_action(request, object, message): def system_action(message, object=None, user=None, log_level=None): """ - Log a generic action done by business logic. ``User`` instance in the log - will be a randomly selected superuser if not specified + Log a generic action done by business logic. """ + object_text = object if object else "Anonymous user" + logger.info( ("System action: {object}, {message}. \n").format( - object=object, message=message + object=object_text, message=message ) ) TimelineLog.objects.create( @@ -118,7 +120,7 @@ def system_action(message, object=None, user=None, log_level=None): user=user, extra_data={ "content_object_repr": force_str(object) if object else "", - "action_flag": LOG_ACTIONS[4], + "action_flag": LOG_ACTIONS[5], "message": message, "log_level": log_level, }, diff --git a/src/open_inwoner/utils/mixins.py b/src/open_inwoner/utils/mixins.py index 2e6f11ee8f..7ed47837d4 100644 --- a/src/open_inwoner/utils/mixins.py +++ b/src/open_inwoner/utils/mixins.py @@ -20,7 +20,7 @@ class ThrottleMixin: # n visits per period (in seconds) throttle_visits = 100 - throttle_period = 60 ** 2 # in seconds + throttle_period = 60**2 # in seconds throttle_403 = True throttle_name = "default" diff --git a/src/open_inwoner/utils/signals.py b/src/open_inwoner/utils/signals.py index 5a0b65b9fa..28ccf647e9 100644 --- a/src/open_inwoner/utils/signals.py +++ b/src/open_inwoner/utils/signals.py @@ -1,7 +1,7 @@ import logging from django.contrib.admin import models -from django.db.models.signals import pre_save +from django.db.models.signals import post_save from django.dispatch import receiver from timeline_logger.models import TimelineLog @@ -11,22 +11,21 @@ logger = logging.getLogger(__name__) -@receiver(pre_save, sender=models.LogEntry) -def copy_log_entry_to_timeline_logger(sender, **kwargs): - log_entry = kwargs.get("instance") +@receiver(post_save, sender=models.LogEntry) +def copy_log_entry_to_timeline_logger(sender, instance, **kwargs): TimelineLog.objects.create( - timestamp=log_entry.action_time, - object_id=log_entry.object_id, - content_type=log_entry.content_type, - user=log_entry.user, + timestamp=instance.action_time, + object_id=instance.object_id, + content_type=instance.content_type, + user=instance.user, extra_data={ - "content_object_repr": log_entry.object_repr or "", - "action_flag": LOG_ACTIONS[log_entry.action_flag], - "message": log_entry.get_change_message(), + "content_object_repr": instance.object_repr or "", + "action_flag": LOG_ACTIONS[instance.action_flag], + "message": str(instance.get_change_message()), }, ) logger.info( "Modified: %s, %s", - log_entry.content_type, - log_entry.get_change_message(), + instance.content_type, + instance.get_change_message(), ) diff --git a/src/open_inwoner/utils/views.py b/src/open_inwoner/utils/views.py index 572e0cf149..80769d535a 100644 --- a/src/open_inwoner/utils/views.py +++ b/src/open_inwoner/utils/views.py @@ -82,7 +82,7 @@ def log_user_action(self, instance, message): """ user_action(self.request, instance, message) - def log_system_action(self, message, instance): + def log_system_action(self, message, instance=None): """ Log system events not related to a specific user. """ From ee6c831b54d653248b63b18fb7f51be6eb05eccf Mon Sep 17 00:00:00 2001 From: vasileios Date: Fri, 13 May 2022 09:07:08 +0200 Subject: [PATCH 2/6] [#559] Fixed formatting --- src/open_inwoner/conf/base.py | 2 +- src/open_inwoner/utils/mixins.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/open_inwoner/conf/base.py b/src/open_inwoner/conf/base.py index 6d411f18f1..68ad42a471 100644 --- a/src/open_inwoner/conf/base.py +++ b/src/open_inwoner/conf/base.py @@ -778,7 +778,7 @@ # file upload limits MIN_UPLOAD_SIZE = 1 # in bytes -MAX_UPLOAD_SIZE = 1024**2 * 100 # 100MB +MAX_UPLOAD_SIZE = 1024 ** 2 * 100 # 100MB UPLOAD_FILE_TYPES = "application/vnd.openxmlformats-officedocument.wordprocessingml.document,application/msword,application/vnd.openxmlformats-officedocument.spreadsheetml.sheet,application/vnd.ms-excel,text/plain,application/vnd.oasis.opendocument.text,application/vnd.oasis.opendocument.formula,application/vnd.oasis.opendocument.spreadsheet,application/pdf,image/jpeg,image/png" # diff --git a/src/open_inwoner/utils/mixins.py b/src/open_inwoner/utils/mixins.py index 7ed47837d4..2e6f11ee8f 100644 --- a/src/open_inwoner/utils/mixins.py +++ b/src/open_inwoner/utils/mixins.py @@ -20,7 +20,7 @@ class ThrottleMixin: # n visits per period (in seconds) throttle_visits = 100 - throttle_period = 60**2 # in seconds + throttle_period = 60 ** 2 # in seconds throttle_403 = True throttle_name = "default" From 26952276484eb667dc50e17518194f55c30b4a21 Mon Sep 17 00:00:00 2001 From: vasileios Date: Mon, 16 May 2022 10:24:57 +0200 Subject: [PATCH 3/6] [#559] Improvements according to PR review --- src/open_inwoner/accounts/views/auth.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/open_inwoner/accounts/views/auth.py b/src/open_inwoner/accounts/views/auth.py index 1a840c19e2..5014fefb18 100644 --- a/src/open_inwoner/accounts/views/auth.py +++ b/src/open_inwoner/accounts/views/auth.py @@ -12,12 +12,11 @@ class LogPasswordChangeView(LogMixin, PasswordChangeView): def form_valid(self, form): - object = form.save() + response = super().form_valid(form) + + object = self.request.user self.log_user_action(object, _("password was changed")) - # Updating the password logs out all other sessions for the user - # except the current one. - update_session_auth_hash(self.request, form.user) - return super().form_valid(form) + return response class LogPasswordResetView(LogMixin, PasswordResetView): @@ -28,8 +27,8 @@ def form_valid(self, form): class LogPasswordResetConfirmView(LogMixin, PasswordResetConfirmView): def form_valid(self, form): - super().form_valid(form) - object = self.get_user(self.kwargs["uidb64"]) + response = super().form_valid(form) + object = self.get_user(self.kwargs["uidb64"]) self.log_system_action(_("password reset was completed"), object) - return HttpResponseRedirect(self.get_success_url()) + return response From 12ac9f4f536a2defd760db11fbbc192dded478ee Mon Sep 17 00:00:00 2001 From: vasileios Date: Mon, 16 May 2022 10:50:29 +0200 Subject: [PATCH 4/6] [#559] Fixed tests after frontend change of password-change-form --- src/open_inwoner/accounts/tests/test_logging.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/open_inwoner/accounts/tests/test_logging.py b/src/open_inwoner/accounts/tests/test_logging.py index 5120672a3a..2bff47f99f 100644 --- a/src/open_inwoner/accounts/tests/test_logging.py +++ b/src/open_inwoner/accounts/tests/test_logging.py @@ -196,7 +196,9 @@ def test_users_deactivation_is_logged(self): def test_password_change_is_logged(self): user = UserFactory(password="test") - form = self.app.get(reverse("password_change"), user=user).forms[0] + form = self.app.get(reverse("password_change"), user=user).forms[ + "password-change-form" + ] form["old_password"] = "test" form["new_password1"] = "newPassw0rd" form["new_password2"] = "newPassw0rd" From 1e9fcf1df3384fa5d94a2c45934bb4c71103c08b Mon Sep 17 00:00:00 2001 From: vasileios Date: Wed, 18 May 2022 16:35:40 +0200 Subject: [PATCH 5/6] [#621] Ensure logs are read-only and searchable --- src/open_inwoner/utils/admin.py | 61 ++++++++++++++++--- .../utils/tests/test_timeline_logger.py | 34 ----------- 2 files changed, 51 insertions(+), 44 deletions(-) diff --git a/src/open_inwoner/utils/admin.py b/src/open_inwoner/utils/admin.py index 48c4cbf189..c872e44275 100644 --- a/src/open_inwoner/utils/admin.py +++ b/src/open_inwoner/utils/admin.py @@ -1,5 +1,7 @@ from django.contrib import admin from django.contrib.admin.models import ADDITION, CHANGE, DELETION +from django.urls import NoReverseMatch, reverse +from django.utils.html import escape, format_html from django.utils.translation import gettext as _ from import_export.admin import ExportMixin @@ -10,9 +12,24 @@ class CustomTimelineLogAdmin(ExportMixin, TimelineLogAdmin): - list_display = ["get_object_title", "timestamp", "get_action_flag", "user"] + show_full_result_count = False + fields = ["content_type", "timestamp", "extra_data", "user"] + list_display = [ + "timestamp", + "user", + "content_type", + "object_link", + "object_id", + "get_action_flag", + "message", + ] list_filter = ["timestamp", "content_type"] list_select_related = ["content_type"] + search_fields = [ + "user__email", + "extra_data", + ] + date_hierarchy = "timestamp" # export resource_class = TimelineLogResource @@ -26,8 +43,6 @@ def get_object_title(self, obj): return f"{obj.content_type.name} - {obj.object_id}" return _("System - anonymous user") - get_object_title.short_description = _("Logboekvermelding") - def get_action_flag(self, obj): if obj.extra_data: if obj.extra_data.get("action_flag")[0] == ADDITION: @@ -42,19 +57,45 @@ def get_action_flag(self, obj): return _("System action") return "" + def message(self, obj): + return obj.extra_data.get("message") if obj.extra_data else "" + + def object_link(self, obj): + if not obj.extra_data: + return "" + + if obj.extra_data.get("action_flag") == DELETION: + link = escape(obj.extra_data.get("content_object_repr")) or "" + else: + ct = obj.content_type + try: + url = reverse( + ("admin:{app_label}_{model}_change").format( + app_label=ct.app_label, model=ct.model + ), + args=[obj.object_id], + ) + link = format_html( + '{}', + url, + escape(obj.extra_data.get("content_object_repr")), + ) + except NoReverseMatch: + link = escape(obj.extra_data.get("content_object_repr")) + except Exception: + link = "" + + return link + get_action_flag.short_description = _("Actie") + message.short_description = _("Bericht") + object_link.short_description = _("Object") def has_add_permission(self, request): return False def has_change_permission(self, request, obj=None): - """ - Show the audit log both to superusers and the users with 'change' - permissions. - """ - return ( - request.user.is_superuser or request.user.has_perm("admin.change_logentry") - ) and request.method != "POST" + return False def has_delete_permission(self, request, obj=None): return False diff --git a/src/open_inwoner/utils/tests/test_timeline_logger.py b/src/open_inwoner/utils/tests/test_timeline_logger.py index 8c2f0964cf..be88008385 100644 --- a/src/open_inwoner/utils/tests/test_timeline_logger.py +++ b/src/open_inwoner/utils/tests/test_timeline_logger.py @@ -97,40 +97,6 @@ def test_user_does_not_have_add_permission(self): response = self.app.get(url, user=self.user, expect_errors=True) self.assertEquals(response.status_code, 403) - def test_user_does_not_have_change_permission(self): - add_url = reverse("admin:accounts_contact_add") - add_form = self.app.get(add_url, user=self.user).forms.get("contact_form") - add_form["first_name"] = self.contact.first_name - add_form["last_name"] = self.contact.last_name - add_form["email"] = self.contact.email - add_form["created_by"] = self.user.id - add_form.submit() - log_entry = TimelineLog.objects.first() - log_url = reverse( - "admin:timeline_logger_timelinelog_change", - kwargs={"object_id": log_entry.id}, - ) - log_form = self.app.get(log_url, user=self.user).forms["timelinelog_form"] - log_form["object_id"] = 29 - response = log_form.submit(expect_errors=True) - self.assertEquals(response.status_code, 403) - - def test_user_does_not_have_delete_permission(self): - add_url = reverse("admin:accounts_contact_add") - add_form = self.app.get(add_url, user=self.user).forms.get("contact_form") - add_form["first_name"] = self.contact.first_name - add_form["last_name"] = self.contact.last_name - add_form["email"] = self.contact.email - add_form["created_by"] = self.user.id - add_form.submit() - log_entry = TimelineLog.objects.first() - log_url = reverse( - "admin:timeline_logger_timelinelog_delete", - kwargs={"object_id": log_entry.id}, - ) - response = self.app.post(log_url, user=self.user, expect_errors=True) - self.assertEquals(response.status_code, 403) - def test_get_action_returns_addition_when_object_is_added(self): url = reverse("admin:accounts_contact_add") form = self.app.get(url, user=self.user).forms.get("contact_form") From ce7c91189bce9affddea247ef98222cf575698ed Mon Sep 17 00:00:00 2001 From: vasileios Date: Tue, 24 May 2022 10:30:11 +0200 Subject: [PATCH 6/6] [#621] Fixed tests according to PR review --- .../utils/tests/test_timeline_logger.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/open_inwoner/utils/tests/test_timeline_logger.py b/src/open_inwoner/utils/tests/test_timeline_logger.py index be88008385..52ee96f0e7 100644 --- a/src/open_inwoner/utils/tests/test_timeline_logger.py +++ b/src/open_inwoner/utils/tests/test_timeline_logger.py @@ -97,6 +97,43 @@ def test_user_does_not_have_add_permission(self): response = self.app.get(url, user=self.user, expect_errors=True) self.assertEquals(response.status_code, 403) + def test_superuser_does_not_have_add_permission(self): + log_url = reverse("admin:timeline_logger_timelinelog_add") + response = self.app.get(log_url, user=self.user, expect_errors=True) + self.assertEquals(response.status_code, 403) + + def test_superuser_does_not_have_change_permission(self): + add_url = reverse("admin:accounts_contact_add") + add_form = self.app.get(add_url, user=self.user).forms.get("contact_form") + add_form["first_name"] = self.contact.first_name + add_form["last_name"] = self.contact.last_name + add_form["email"] = self.contact.email + add_form["created_by"] = self.user.id + add_form.submit() + log_entry = TimelineLog.objects.first() + log_url = reverse( + "admin:timeline_logger_timelinelog_change", + kwargs={"object_id": log_entry.id}, + ) + response = self.app.post(log_url, user=self.user, expect_errors=True) + self.assertEquals(response.status_code, 403) + + def test_superuser_does_not_have_delete_permission(self): + add_url = reverse("admin:accounts_contact_add") + add_form = self.app.get(add_url, user=self.user).forms.get("contact_form") + add_form["first_name"] = self.contact.first_name + add_form["last_name"] = self.contact.last_name + add_form["email"] = self.contact.email + add_form["created_by"] = self.user.id + add_form.submit() + log_entry = TimelineLog.objects.first() + log_url = reverse( + "admin:timeline_logger_timelinelog_delete", + kwargs={"object_id": log_entry.id}, + ) + response = self.app.post(log_url, user=self.user, expect_errors=True) + self.assertEquals(response.status_code, 403) + def test_get_action_returns_addition_when_object_is_added(self): url = reverse("admin:accounts_contact_add") form = self.app.get(url, user=self.user).forms.get("contact_form") @@ -147,6 +184,16 @@ def test_get_action_returns_empty_string_when_no_extra_data_exists(self): ) self.assertEquals(action, "") + def test_object_link_returns_right_link(self): + self.add_instance() + url = reverse("admin:timeline_logger_timelinelog_changelist") + response = self.app.get(url, user=self.user) + contact = Contact.objects.first() + obj_link = reverse( + "admin:accounts_contact_change", kwargs={"object_id": contact.id} + ) + self.assertContains(response, obj_link) + class TestTimelineLogExport(WebTest): @freeze_time("2021-10-18 13:00:00")