From adaa86b6b1f05143ce567637ea2c88315f653052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20Kestila=CC=88?= Date: Wed, 10 Jan 2024 12:01:08 +0200 Subject: [PATCH 1/6] feat: audit log applicant attachment operations --- backend/benefit/applications/api/v1/views.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/backend/benefit/applications/api/v1/views.py b/backend/benefit/applications/api/v1/views.py index e23174d00a..4a64f2d48f 100755 --- a/backend/benefit/applications/api/v1/views.py +++ b/backend/benefit/applications/api/v1/views.py @@ -45,6 +45,8 @@ ) from common.permissions import BFIsApplicant, BFIsHandler, TermsOfServiceAccepted from messages.models import MessageType +from shared.audit_log import audit_logging +from shared.audit_log.enums import Operation from shared.audit_log.viewsets import AuditLoggingModelViewSet from users.utils import get_company_from_request @@ -241,7 +243,14 @@ def delete_attachment(self, request, attachment_pk, *args, **kwargs): status=status.HTTP_403_FORBIDDEN, ) if instance := self._get_attachment(attachment_pk): + audit_logging.log( + request.user, + "", + Operation.DELETE, + instance, + ) instance.delete() + return Response(status=status.HTTP_204_NO_CONTENT) else: return self._attachment_not_found() @@ -258,6 +267,12 @@ def download_attachment(self, request, attachment_pk, *args, **kwargs): if ( attachment := self._get_attachment(attachment_pk) ) and attachment.attachment_file: + audit_logging.log( + request.user, + "", + Operation.READ, + attachment, + ) return FileResponse(attachment.attachment_file) else: return self._attachment_not_found() From 69fe3e153628e135bb7eb51b497dafa5b8445f1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20Kestila=CC=88?= Date: Wed, 10 Jan 2024 13:44:17 +0200 Subject: [PATCH 2/6] feat: audit log employee in applicant form --- .../api/v1/serializers/application.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/backend/benefit/applications/api/v1/serializers/application.py b/backend/benefit/applications/api/v1/serializers/application.py index 5d23f1f643..76fbd83219 100755 --- a/backend/benefit/applications/api/v1/serializers/application.py +++ b/backend/benefit/applications/api/v1/serializers/application.py @@ -56,6 +56,8 @@ from companies.api.v1.serializers import CompanySerializer from companies.models import Company from messages.automatic_messages import send_application_reopened_message +from shared.audit_log import audit_logging +from shared.audit_log.enums import Operation from terms.api.v1.serializers import ( ApplicantTermsApprovalSerializer, ApproveTermsSerializer, @@ -1247,9 +1249,22 @@ def create(self, validated_data): return application def _update_or_create_employee(self, application, employee_data): - employee, _ = Employee.objects.update_or_create( + employee, was_created = Employee.objects.update_or_create( application=application, defaults=employee_data ) + user = self.get_logged_in_user() + + if was_created: + audit_log_operation = Operation.CREATE + else: + audit_log_operation = Operation.UPDATE + + audit_logging.log( + user, + "", + audit_log_operation, + employee, + ) return employee def get_company(self, validated_data): From 982c7adcf79d24df0bd5cc812a7b9950a7b76b98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20Kestila=CC=88?= Date: Wed, 10 Jan 2024 15:40:41 +0200 Subject: [PATCH 3/6] feat: audit log de_minimis_aid creation --- .../api/v1/serializers/application.py | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/backend/benefit/applications/api/v1/serializers/application.py b/backend/benefit/applications/api/v1/serializers/application.py index 76fbd83219..e78fe8f013 100755 --- a/backend/benefit/applications/api/v1/serializers/application.py +++ b/backend/benefit/applications/api/v1/serializers/application.py @@ -1284,6 +1284,8 @@ def assign_default_fields_from_company(self, application, company): def _update_de_minimis_aid(self, application, de_minimis_data): serializer = DeMinimisAidSerializer(data=de_minimis_data, many=True) + user = self.get_logged_in_user() + if not serializer.is_valid(): raise BenefitAPIException( format_lazy( @@ -1293,13 +1295,30 @@ def _update_de_minimis_aid(self, application, de_minimis_data): ) # Clear the previous DeMinimisAid objects from the database. # The request must always contain all the DeMinimisAid objects for this application. - application.de_minimis_aid_set.all().delete() + current_de_minimis_aid_set = application.de_minimis_aid_set.all() + for de_minimis in current_de_minimis_aid_set: + audit_logging.log( + user, + "", + Operation.DELETE, + de_minimis, + ) + current_de_minimis_aid_set.delete() + for idx, aid_item in enumerate(serializer.validated_data): aid_item["application_id"] = application.pk aid_item[ "ordering" ] = idx # use the ordering defined in the JSON sent by the client - serializer.save() + + de_minimis_list = serializer.save() + for de_minimis in de_minimis_list: + audit_logging.log( + user, + "", + Operation.CREATE, + de_minimis, + ) def get_logged_in_user(self): return get_request_user_from_context(self) From a57e24cf3660be49c7fc0b29d7d6490e4ed246dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20Kestila=CC=88?= Date: Thu, 11 Jan 2024 10:51:11 +0200 Subject: [PATCH 4/6] feat: log non-sensitive employee data to audit log --- .../migrations/0049_historicalemployee.py | 60 +++++++++++++++++++ backend/benefit/applications/models.py | 5 ++ .../shared/shared/audit_log/audit_logging.py | 14 +++++ 3 files changed, 79 insertions(+) create mode 100644 backend/benefit/applications/migrations/0049_historicalemployee.py diff --git a/backend/benefit/applications/migrations/0049_historicalemployee.py b/backend/benefit/applications/migrations/0049_historicalemployee.py new file mode 100644 index 0000000000..f9beabf1e5 --- /dev/null +++ b/backend/benefit/applications/migrations/0049_historicalemployee.py @@ -0,0 +1,60 @@ +# Generated by Django 3.2.23 on 2024-01-10 13:46 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import encrypted_fields.fields +import phonenumber_field.modelfields +import simple_history.models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('applications', '0048_alter_attachment_attachment_type'), + ] + + operations = [ + migrations.CreateModel( + name='HistoricalEmployee', + fields=[ + ('created_at', models.DateTimeField(blank=True, editable=False, verbose_name='time created')), + ('modified_at', models.DateTimeField(blank=True, editable=False, verbose_name='time modified')), + ('id', models.UUIDField(db_index=True, default=uuid.uuid4, editable=False)), + ('encrypted_first_name', encrypted_fields.fields.EncryptedCharField(blank=True, max_length=128, verbose_name='first name')), + ('encrypted_last_name', encrypted_fields.fields.EncryptedCharField(blank=True, max_length=128, verbose_name='last name')), + ('first_name', encrypted_fields.fields.SearchField(blank=True, db_index=True, encrypted_field_name='encrypted_first_name', hash_key='02c5b8605cd4f9c188eee422209069b7bd3a607f0ae0a166eab0da223d1b6735', max_length=66, null=True)), + ('last_name', encrypted_fields.fields.SearchField(blank=True, db_index=True, encrypted_field_name='encrypted_last_name', hash_key='af1b5a67d11197865a731c26bf9659716b9ded71c2802b4363856fe613b6b527', max_length=66, null=True)), + ('encrypted_social_security_number', encrypted_fields.fields.EncryptedCharField(blank=True, max_length=11, verbose_name='social security number')), + ('social_security_number', encrypted_fields.fields.SearchField(blank=True, db_index=True, encrypted_field_name='encrypted_social_security_number', hash_key='ee235e39ebc238035a6264c063dd829d4b6d2270604b57ee1f463e676ec44669', max_length=66, null=True)), + ('phone_number', phonenumber_field.modelfields.PhoneNumberField(blank=True, max_length=128, region=None, verbose_name='phone number')), + ('email', models.EmailField(blank=True, max_length=254, verbose_name='email')), + ('employee_language', models.CharField(blank=True, choices=[('fi', 'suomi'), ('sv', 'svenska'), ('en', 'english')], default='fi', max_length=2)), + ('job_title', models.CharField(blank=True, max_length=128, verbose_name='job title')), + ('monthly_pay', models.DecimalField(blank=True, decimal_places=2, max_digits=7, null=True, verbose_name='monthly pay')), + ('vacation_money', models.DecimalField(blank=True, decimal_places=2, max_digits=7, null=True, verbose_name='vacation money')), + ('other_expenses', models.DecimalField(blank=True, decimal_places=2, max_digits=7, null=True, verbose_name='other expenses')), + ('working_hours', models.DecimalField(blank=True, decimal_places=2, max_digits=5, null=True, verbose_name='working hour')), + ('collective_bargaining_agreement', models.CharField(blank=True, max_length=64, verbose_name='collective bargaining agreement')), + ('is_living_in_helsinki', models.BooleanField(default=False, verbose_name='is living in helsinki')), + ('commission_amount', models.DecimalField(blank=True, decimal_places=2, max_digits=7, null=True, verbose_name='amount of the commission (eur)')), + ('commission_description', models.CharField(blank=True, max_length=256, verbose_name='Description of the commission')), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField(db_index=True)), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('application', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='applications.application', verbose_name='application')), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'historical employee', + 'verbose_name_plural': 'historical employees', + 'db_table': 'bf_applications_employee_history', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': ('history_date', 'history_id'), + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + ] diff --git a/backend/benefit/applications/models.py b/backend/benefit/applications/models.py index 44295414dc..b23ec73ab7 100755 --- a/backend/benefit/applications/models.py +++ b/backend/benefit/applications/models.py @@ -710,6 +710,11 @@ class Employee(UUIDModel, TimeStampedModel): on_delete=models.CASCADE, ) + history = HistoricalRecords( + table_name="bf_applications_employee_history", + cascade_delete_history=True, + ) + encrypted_first_name = EncryptedCharField( max_length=128, verbose_name=_("first name"), blank=True ) diff --git a/backend/shared/shared/audit_log/audit_logging.py b/backend/shared/shared/audit_log/audit_logging.py index 39fcf1b3d8..cf84ed06c7 100644 --- a/backend/shared/shared/audit_log/audit_logging.py +++ b/backend/shared/shared/audit_log/audit_logging.py @@ -103,6 +103,8 @@ def _add_changes(target: Union[Model, ModelBase], message: dict) -> None: changes_list = [] for change in delta.changes: + if _is_sensitive_field(change.field): + continue changes_list.append( f"{change.field} changed from {change.old} to {change.new}" ) @@ -111,6 +113,18 @@ def _add_changes(target: Union[Model, ModelBase], message: dict) -> None: message["audit_event"]["target"]["changes"] = changes_list +def _is_sensitive_field(change_field: str) -> bool: + "Check if a given field is sensitive personal data." + return change_field in [ + "encrypted_social_security_number", + "encrypted_first_name", + "encrypted_last_name", + "first_name", + "last_name", + "social_security_number" + ] + + def _get_target_id(target: Union[Model, ModelBase]) -> Optional[str]: if isinstance(target, ModelBase): return "" From 074a59e269cfeb739e651ff052f67c70ded85f3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20Kestila=CC=88?= Date: Thu, 11 Jan 2024 14:06:07 +0200 Subject: [PATCH 5/6] fix: breaking tests --- .../applications/tests/test_applications_api.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/backend/benefit/applications/tests/test_applications_api.py b/backend/benefit/applications/tests/test_applications_api.py index f409469e28..bc8c20b5bc 100755 --- a/backend/benefit/applications/tests/test_applications_api.py +++ b/backend/benefit/applications/tests/test_applications_api.py @@ -427,9 +427,9 @@ def test_application_post_success(api_client, application): ) assert new_application.official_company_postcode == new_application.company.postcode assert new_application.official_company_city == new_application.company.city - audit_event = ( - audit_models.AuditLogEntry.objects.all().first().message["audit_event"] - ) + + audit_event = audit_models.AuditLogEntry.objects.last().message["audit_event"] + assert audit_event["status"] == "SUCCESS" assert audit_event["target"]["id"] == str(Application.objects.all().first().id) assert audit_event["operation"] == "CREATE" @@ -617,9 +617,9 @@ def test_application_put_edit_fields(api_client, application): ) # normalized format application.refresh_from_db() assert application.company_contact_person_phone_number == "0505658789" - audit_event = ( - audit_models.AuditLogEntry.objects.all().first().message["audit_event"] - ) + + audit_event = audit_models.AuditLogEntry.objects.last().message["audit_event"] + assert audit_event["status"] == "SUCCESS" assert audit_event["target"]["id"] == str(application.id) assert audit_event["operation"] == "UPDATE" From 09e3e077668d4d3b95b90529602d9aa644e2eb88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20Kestila=CC=88?= Date: Thu, 11 Jan 2024 14:19:17 +0200 Subject: [PATCH 6/6] fix: code style --- backend/shared/shared/audit_log/audit_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/shared/shared/audit_log/audit_logging.py b/backend/shared/shared/audit_log/audit_logging.py index cf84ed06c7..7cb25eec36 100644 --- a/backend/shared/shared/audit_log/audit_logging.py +++ b/backend/shared/shared/audit_log/audit_logging.py @@ -121,7 +121,7 @@ def _is_sensitive_field(change_field: str) -> bool: "encrypted_last_name", "first_name", "last_name", - "social_security_number" + "social_security_number", ]