From d1fe30e5819834a122163d3cc89ff41b54ef6619 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 16 Oct 2024 11:26:10 -0400 Subject: [PATCH 01/10] feat: add new base viewset for audit logging --- kobo/apps/audit_log/base_views.py | 100 +++++++++++++++++++ kobo/apps/audit_log/tests/test_base_views.py | 77 ++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 kobo/apps/audit_log/base_views.py create mode 100644 kobo/apps/audit_log/tests/test_base_views.py diff --git a/kobo/apps/audit_log/base_views.py b/kobo/apps/audit_log/base_views.py new file mode 100644 index 0000000000..b66bc0b873 --- /dev/null +++ b/kobo/apps/audit_log/base_views.py @@ -0,0 +1,100 @@ +# coding: utf-8 +from rest_framework import viewsets +from rest_framework import mixins + + +def get_nested_field(obj, field: str): + """ + Retrieve a period-separated nested field from an object or dict + + Raises an exception if the field is not found + """ + split = field.split('.') + attribute = getattr(obj, split[0]) + if len(split) > 1: + for inner_field in split[1:]: + if isinstance(attribute, dict): + attribute = attribute.get(inner_field) + else: + attribute = getattr(attribute, inner_field) + return attribute + + +class AuditLoggedViewSet(viewsets.GenericViewSet): + """ + A ViewSet for adding arbitrary object data to a request before and after request + + Useful for storing information for audit logs on create/update. Allows inheriting + ViewSets to implement additional logic for get_object, perform_update, + perform_create, and perform_destroy via the get_object_override, + perform_update_override, perform_create_override, and perform_destroy_override + methods. + + Sets the values on the inner HttpRequest object rather than the DRF Request + so middleware can access them. + """ + logged_fields = [] + + def get_object(self): + # actually fetch the object + obj = self.get_object_override() + if self.request.method in ['GET','HEAD']: + # since this is for audit logs, don't worry about read-only requests + return obj + audit_log_data = {} + for field in self.logged_fields: + value = get_nested_field(obj, field) + audit_log_data[field] = value + self.request._request.initial_data = audit_log_data + return obj + + def perform_update(self, serializer): + self.perform_update_override(serializer) + audit_log_data = {} + for field in self.logged_fields: + value = get_nested_field(serializer.instance, field) + audit_log_data[field] = value + self.request._request.updated_data = audit_log_data + + def perform_create(self, serializer): + self.perform_create_override(serializer) + audit_log_data = {} + for field in self.logged_fields: + value = get_nested_field(serializer.instance, field) + audit_log_data[field] = value + self.request._request.updated_data = audit_log_data + + def perform_destroy(self, instance): + audit_log_data = {} + for field in self.logged_fields: + value = get_nested_field(instance, field) + audit_log_data[field] = value + self.request._request.initial_data = audit_log_data + self.perform_destroy_override(instance) + + def perform_destroy_override(self, instance): + super().perform_destroy(instance) + + def perform_create_override(self, serializer): + super().perform_create(serializer) + + def perform_update_override(self, serializer): + super().perform_update(serializer) + + def get_object_override(self): + return super().get_object() + + +class AuditLoggedModelViewSet(AuditLoggedViewSet, mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin): + pass + +class AuditLoggedNoUpdateModelViewSet(AuditLoggedModelViewSet, + mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin): + pass diff --git a/kobo/apps/audit_log/tests/test_base_views.py b/kobo/apps/audit_log/tests/test_base_views.py new file mode 100644 index 0000000000..60390cf74b --- /dev/null +++ b/kobo/apps/audit_log/tests/test_base_views.py @@ -0,0 +1,77 @@ +from unittest import TestCase +from django.urls import include, path, reverse +from django.test import override_settings +from django.db import models + + +from kobo.apps.accounts.serializers import EmailAddressSerializer +from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet +from kobo.apps.kobo_auth.shortcuts import User + + +from kobo.apps.audit_log.views import AuditLogViewSet +from kobo.settings.base import ROOT_URLCONF +from kpi.models import Asset +from kpi.serializers.v2.asset import AssetSerializer +from kpi.serializers.v2.user import UserSerializer +from kpi.tests.kpi_test_case import KpiTestCase +from rest_framework.routers import DefaultRouter +from rest_framework import permissions +from allauth.account.models import EmailAddress +from django.test.utils import isolate_apps +from rest_framework import serializers, renderers + +class DummyEmailSerializer(serializers.ModelSerializer): + """ + Basic model serializer for EmailAddresses + """ + class Meta: + model = EmailAddress + fields = '__all__' + + +class DummyViewSet(AuditLoggedModelViewSet): + """ + DummyViewSet for testing the functionality of the AuditLoggedModelViewSet + + Uses the email address model because it's simple + """ + permission_classes = (permissions.AllowAny,) + queryset = EmailAddress.objects.all() + serializer_class = DummyEmailSerializer + logged_fields = ['email', 'verified'] + + +class TestUrls: + """ + Register our DummyViewSet at a test-only url + """ + router = DefaultRouter() + router.register(f'test', DummyViewSet, basename='test-vs') + urlpatterns = router.urls + +@override_settings(ROOT_URLCONF=TestUrls) +class TestAuditLoggedViewSet(KpiTestCase): + fixtures = ['test_data'] + + def test_creating_model_records_fields(self): + response = self.client.post(reverse('test-vs-list'), data={'user': 1, 'email': 'new_email@example.com'}) + request = response.wsgi_request + self.assertDictEqual(request.updated_data, {'email': 'new_email@example.com', 'verified': False}) + + def test_updating_model_records_fields(self): + user = User.objects.get(pk=1) + email_address, _ = EmailAddress.objects.get_or_create(user=user, email='initial_email@example.com') + email_address.save() + response = self.client.patch(reverse('test-vs-detail', args=[email_address.pk]), data={'email': 'newer_email@example.com'}) + request = response.wsgi_request + self.assertEqual(request.initial_data, {'email': 'initial_email@example.com', 'verified': False}) + self.assertEqual(request.updated_data, {'email': 'newer_email@example.com', 'verified': False}) + + def test_destroying_model_records_fields(self): + user = User.objects.get(pk=1) + email_address, _ = EmailAddress.objects.get_or_create(user=user, email='initial_email@example.com') + email_address.save() + response = self.client.delete(reverse('test-vs-detail', args=[email_address.pk])) + request = response.wsgi_request + self.assertEqual(request.initial_data, {'email': 'initial_email@example.com', 'verified': False}) From de9f9fd48b98e886550983d4cd32c606b7d41496 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 16 Oct 2024 11:37:47 -0400 Subject: [PATCH 02/10] fixup!: format --- kobo/apps/audit_log/base_views.py | 32 ++++++---- kobo/apps/audit_log/tests/test_base_views.py | 66 ++++++++++++-------- 2 files changed, 59 insertions(+), 39 deletions(-) diff --git a/kobo/apps/audit_log/base_views.py b/kobo/apps/audit_log/base_views.py index b66bc0b873..52093bb38b 100644 --- a/kobo/apps/audit_log/base_views.py +++ b/kobo/apps/audit_log/base_views.py @@ -1,6 +1,5 @@ # coding: utf-8 -from rest_framework import viewsets -from rest_framework import mixins +from rest_framework import mixins, viewsets def get_nested_field(obj, field: str): @@ -33,12 +32,13 @@ class AuditLoggedViewSet(viewsets.GenericViewSet): Sets the values on the inner HttpRequest object rather than the DRF Request so middleware can access them. """ + logged_fields = [] def get_object(self): # actually fetch the object obj = self.get_object_override() - if self.request.method in ['GET','HEAD']: + if self.request.method in ['GET', 'HEAD']: # since this is for audit logs, don't worry about read-only requests return obj audit_log_data = {} @@ -85,16 +85,22 @@ def get_object_override(self): return super().get_object() -class AuditLoggedModelViewSet(AuditLoggedViewSet, mixins.CreateModelMixin, - mixins.RetrieveModelMixin, - mixins.UpdateModelMixin, - mixins.DestroyModelMixin, - mixins.ListModelMixin): +class AuditLoggedModelViewSet( + AuditLoggedViewSet, + mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin, +): pass -class AuditLoggedNoUpdateModelViewSet(AuditLoggedModelViewSet, - mixins.CreateModelMixin, - mixins.RetrieveModelMixin, - mixins.DestroyModelMixin, - mixins.ListModelMixin): + +class AuditLoggedNoUpdateModelViewSet( + AuditLoggedModelViewSet, + mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin, +): pass diff --git a/kobo/apps/audit_log/tests/test_base_views.py b/kobo/apps/audit_log/tests/test_base_views.py index 60390cf74b..aba2876696 100644 --- a/kobo/apps/audit_log/tests/test_base_views.py +++ b/kobo/apps/audit_log/tests/test_base_views.py @@ -1,30 +1,19 @@ -from unittest import TestCase -from django.urls import include, path, reverse +from allauth.account.models import EmailAddress from django.test import override_settings -from django.db import models - +from django.urls import reverse +from rest_framework import permissions, serializers +from rest_framework.routers import DefaultRouter -from kobo.apps.accounts.serializers import EmailAddressSerializer from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet from kobo.apps.kobo_auth.shortcuts import User - - -from kobo.apps.audit_log.views import AuditLogViewSet -from kobo.settings.base import ROOT_URLCONF -from kpi.models import Asset -from kpi.serializers.v2.asset import AssetSerializer -from kpi.serializers.v2.user import UserSerializer from kpi.tests.kpi_test_case import KpiTestCase -from rest_framework.routers import DefaultRouter -from rest_framework import permissions -from allauth.account.models import EmailAddress -from django.test.utils import isolate_apps -from rest_framework import serializers, renderers + class DummyEmailSerializer(serializers.ModelSerializer): """ Basic model serializer for EmailAddresses """ + class Meta: model = EmailAddress fields = '__all__' @@ -36,6 +25,7 @@ class DummyViewSet(AuditLoggedModelViewSet): Uses the email address model because it's simple """ + permission_classes = (permissions.AllowAny,) queryset = EmailAddress.objects.all() serializer_class = DummyEmailSerializer @@ -46,32 +36,56 @@ class TestUrls: """ Register our DummyViewSet at a test-only url """ + router = DefaultRouter() router.register(f'test', DummyViewSet, basename='test-vs') urlpatterns = router.urls + @override_settings(ROOT_URLCONF=TestUrls) class TestAuditLoggedViewSet(KpiTestCase): fixtures = ['test_data'] def test_creating_model_records_fields(self): - response = self.client.post(reverse('test-vs-list'), data={'user': 1, 'email': 'new_email@example.com'}) + response = self.client.post( + reverse('test-vs-list'), data={'user': 1, 'email': 'new_email@example.com'} + ) request = response.wsgi_request - self.assertDictEqual(request.updated_data, {'email': 'new_email@example.com', 'verified': False}) + self.assertDictEqual( + request.updated_data, {'email': 'new_email@example.com', 'verified': False} + ) def test_updating_model_records_fields(self): user = User.objects.get(pk=1) - email_address, _ = EmailAddress.objects.get_or_create(user=user, email='initial_email@example.com') + email_address, _ = EmailAddress.objects.get_or_create( + user=user, email='initial_email@example.com' + ) email_address.save() - response = self.client.patch(reverse('test-vs-detail', args=[email_address.pk]), data={'email': 'newer_email@example.com'}) + response = self.client.patch( + reverse('test-vs-detail', args=[email_address.pk]), + data={'email': 'newer_email@example.com'}, + ) request = response.wsgi_request - self.assertEqual(request.initial_data, {'email': 'initial_email@example.com', 'verified': False}) - self.assertEqual(request.updated_data, {'email': 'newer_email@example.com', 'verified': False}) + self.assertEqual( + request.initial_data, + {'email': 'initial_email@example.com', 'verified': False}, + ) + self.assertEqual( + request.updated_data, + {'email': 'newer_email@example.com', 'verified': False}, + ) def test_destroying_model_records_fields(self): user = User.objects.get(pk=1) - email_address, _ = EmailAddress.objects.get_or_create(user=user, email='initial_email@example.com') + email_address, _ = EmailAddress.objects.get_or_create( + user=user, email='initial_email@example.com' + ) email_address.save() - response = self.client.delete(reverse('test-vs-detail', args=[email_address.pk])) + response = self.client.delete( + reverse('test-vs-detail', args=[email_address.pk]) + ) request = response.wsgi_request - self.assertEqual(request.initial_data, {'email': 'initial_email@example.com', 'verified': False}) + self.assertEqual( + request.initial_data, + {'email': 'initial_email@example.com', 'verified': False}, + ) From 0d951dc53059da98f9f60c0446ac8a7c0f8f2f51 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 16 Oct 2024 11:42:23 -0400 Subject: [PATCH 03/10] fixup!: s/f/r/ --- kobo/apps/audit_log/tests/test_base_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/audit_log/tests/test_base_views.py b/kobo/apps/audit_log/tests/test_base_views.py index aba2876696..56077a297a 100644 --- a/kobo/apps/audit_log/tests/test_base_views.py +++ b/kobo/apps/audit_log/tests/test_base_views.py @@ -38,7 +38,7 @@ class TestUrls: """ router = DefaultRouter() - router.register(f'test', DummyViewSet, basename='test-vs') + router.register(r'test', DummyViewSet, basename='test-vs') urlpatterns = router.urls From 94793fe80993a640e1b5e3f3c1c4ea23a5225885 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 16 Oct 2024 14:41:59 -0400 Subject: [PATCH 04/10] feat: record basic project changes --- kobo/apps/audit_log/base_views.py | 8 ++++ kobo/apps/audit_log/middleware.py | 12 +++++ kobo/apps/audit_log/models.py | 79 +++++++++++++++++++++++++++---- kobo/settings/base.py | 1 + kpi/views/v2/asset.py | 22 ++++++--- 5 files changed, 106 insertions(+), 16 deletions(-) create mode 100644 kobo/apps/audit_log/middleware.py diff --git a/kobo/apps/audit_log/base_views.py b/kobo/apps/audit_log/base_views.py index 52093bb38b..91c43a4712 100644 --- a/kobo/apps/audit_log/base_views.py +++ b/kobo/apps/audit_log/base_views.py @@ -1,6 +1,8 @@ # coding: utf-8 from rest_framework import mixins, viewsets +from kobo.apps.audit_log.models import AuditType + def get_nested_field(obj, field: str): """ @@ -34,6 +36,8 @@ class AuditLoggedViewSet(viewsets.GenericViewSet): """ logged_fields = [] + log_type = None + model_name = None def get_object(self): # actually fetch the object @@ -46,6 +50,8 @@ def get_object(self): value = get_nested_field(obj, field) audit_log_data[field] = value self.request._request.initial_data = audit_log_data + self.request._request.log_type = self.log_type + self.request._request.model_name = self.model_name return obj def perform_update(self, serializer): @@ -63,6 +69,8 @@ def perform_create(self, serializer): value = get_nested_field(serializer.instance, field) audit_log_data[field] = value self.request._request.updated_data = audit_log_data + self.request._request.log_type = self.log_type + self.request._request.model_name = self.model_name def perform_destroy(self, instance): audit_log_data = {} diff --git a/kobo/apps/audit_log/middleware.py b/kobo/apps/audit_log/middleware.py new file mode 100644 index 0000000000..8dc176bdda --- /dev/null +++ b/kobo/apps/audit_log/middleware.py @@ -0,0 +1,12 @@ +from kobo.apps.audit_log.models import ProjectHistoryLog, AuditType +from kpi.utils.log import logging + +def create_project_history_log_middleware(get_response): + def do_thing(request): + response = get_response(request) + if request.method in ['GET', 'HEAD']: + return response + if request.log_type == AuditType.PROJECT_HISTORY: + ProjectHistoryLog.create_from_request(request) + return response + return do_thing diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index b16f7e6f7e..ca9b9e85cf 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -1,3 +1,5 @@ +import json + from django.conf import settings from django.db import models from django.db.models import Case, Count, F, Min, Value, When @@ -289,21 +291,25 @@ def create(self, **kwargs): # always be the same on a project history log self.remove_common_fields_and_warn('project history', kwargs) user = kwargs.pop('user') - asset = kwargs.pop('asset') - if not asset.asset_type or asset.asset_type != ASSET_TYPE_SURVEY: + asset = kwargs.pop('asset', None) + if asset is not None and asset.asset_type != ASSET_TYPE_SURVEY: raise BadAssetTypeException( 'Cannot create project history log for non-survey asset' ) + new_kwargs = { + 'app_label': Asset._meta.app_label, + 'model_name': Asset._meta.model_name, + 'log_type': AuditType.PROJECT_HISTORY, + 'user': user, + 'user_uid': user.extra_details.uid + } + if asset is not None: + new_kwargs['object_id']=asset.id + new_kwargs.update(**kwargs) return super().create( # set the fields that are always the same for all project history logs, # along with the ones derived from the user and asset - app_label=Asset._meta.app_label, - model_name=Asset._meta.model_name, - log_type=AuditType.PROJECT_HISTORY, - user=user, - user_uid=user.extra_details.uid, - object_id=asset.id, - **kwargs, + **new_kwargs, ) @@ -343,3 +349,58 @@ def create_from_deployment_request( metadata=metadata, asset=asset, ) + + @staticmethod + def create_from_request(request): + view_name = request.resolver_match.url_name + if view_name == 'asset-detail': + # we don't record project creation, so we only need the detail view + ProjectHistoryLog.create_from_asset_view_set_request(request) + + @staticmethod + def create_from_asset_view_set_request(request): + initial_data = request.get('initial_data', None) + updated_data = request.get('updated_data', None) + asset_uid = request.resolver_match.kwargs['uid'] + object_id = initial_data.get('id', None) + common_metadata ={ + 'asset_uid': asset_uid, + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + 'ip_address': get_client_ip(request), + 'source': get_human_readable_client_user_agent(request), + } + + if updated_data is None: + # if updated_data is empty, the object was deleted + ProjectHistoryLog.objects.create( + user=request.user, + object_id=object_id, + action=AuditAction.DELETE, + metadata=common_metadata, + ) + return + + if initial_data['name'] != updated_data['name']: + # create PH log for name change + metadata = { + 'old_name': initial_data['name'], + 'new_name': updated_data['name'], + **common_metadata, + } + ProjectHistoryLog.objects.create( + object_id=object_id, + user=request.user, + action=AuditAction.UPDATE, + metadata=metadata + ) + + if json.dumps(initial_data['content']) != json.dumps(updated_data['content']): + metadata = { + 'new_version_uid': updated_data['latest_version.uid'] + } + ProjectHistoryLog.objects.create( + object_id=object_id, + user=request.user, + action=AuditAction.UPDATE, + metadata=metadata, + ) diff --git a/kobo/settings/base.py b/kobo/settings/base.py index 8a85af50c5..0e697029b2 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -155,6 +155,7 @@ 'allauth.account.middleware.AccountMiddleware', 'allauth.usersessions.middleware.UserSessionsMiddleware', 'django.middleware.common.CommonMiddleware', + 'kobo.apps.audit_log.middleware.create_project_history_log_middleware', # Still needed really? 'kobo.apps.openrosa.libs.utils.middleware.LocaleMiddlewareWithTweaks', 'django.middleware.csrf.CsrfViewMiddleware', diff --git a/kpi/views/v2/asset.py b/kpi/views/v2/asset.py index 350057a2b5..c7ebc29a94 100644 --- a/kpi/views/v2/asset.py +++ b/kpi/views/v2/asset.py @@ -12,7 +12,8 @@ from rest_framework.response import Response from rest_framework_extensions.mixins import NestedViewSetMixin -from kobo.apps.audit_log.models import ProjectHistoryLog +from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet +from kobo.apps.audit_log.models import ProjectHistoryLog, AuditType from kpi.constants import ( ASSET_TYPE_ARG_NAME, ASSET_TYPE_SURVEY, @@ -45,12 +46,13 @@ from kpi.utils.bugfix import repair_file_column_content_and_save from kpi.utils.hash import calculate_hash from kpi.utils.kobo_to_xlsform import to_xlsform_structure +from kpi.utils.log import logging from kpi.utils.object_permission import get_database_user, get_objects_for_user from kpi.utils.ss_structure_to_mdtable import ss_structure_to_mdtable class AssetViewSet( - ObjectPermissionViewSetMixin, NestedViewSetMixin, viewsets.ModelViewSet + ObjectPermissionViewSetMixin, NestedViewSetMixin, AuditLoggedModelViewSet ): """ * Assign a asset to a collection partially implemented @@ -379,7 +381,12 @@ class AssetViewSet( 'uid__icontains', ] - def get_object(self): + logged_fields = ['content', 'settings', 'data_sharing', 'latest_version.uid', + 'latest_deployed_version_uid', 'name', 'advanced_features'] + log_type = AuditType.PROJECT_HISTORY + model_name = 'Asset' + + def get_object_override(self): if self.request.method in ['PATCH', 'GET']: try: asset = Asset.objects.get(uid=self.kwargs['uid']) @@ -399,7 +406,7 @@ def get_object(self): return asset - return super().get_object() + return super().get_object_override() @action( detail=False, @@ -779,10 +786,11 @@ def partial_update(self, request, *args, **kwargs): partial=True) serializer.is_valid(raise_exception=True) - self.perform_update(serializer) + logging.info('I am updating the instance') + super().perform_update(serializer) return Response(serializer.data) - def perform_create(self, serializer): + def perform_create_override(self, serializer): user = get_database_user(self.request.user) serializer.save( owner=user, @@ -790,7 +798,7 @@ def perform_create(self, serializer): last_modified_by=user.username ) - def perform_destroy(self, instance): + def perform_destroy_override(self, instance): self._bulk_asset_actions( {'payload': {'asset_uids': [instance.uid], 'action': 'delete'}} ) From ed302dd1e1c9247b41cdfaa4c00ffee59e34da8e Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 21 Oct 2024 16:04:19 -0400 Subject: [PATCH 05/10] fixup!: tests passing --- kobo/apps/accounts/tests/test_backend.py | 3 +- kobo/apps/audit_log/audit_actions.py | 22 ++ kobo/apps/audit_log/base_views.py | 2 + kobo/apps/audit_log/middleware.py | 5 +- kobo/apps/audit_log/models.py | 214 ++++++++------ .../tests/api/v2/test_api_audit_log.py | 2 +- kobo/apps/audit_log/tests/test_models.py | 68 +---- .../audit_log/tests/test_one_time_auth.py | 3 +- .../tests/test_project_history_logs.py | 266 ++++++++++++++++++ kobo/apps/audit_log/tests/test_signals.py | 3 +- kobo/apps/trash_bin/tasks.py | 3 +- kobo/apps/trash_bin/tests/test_utils.py | 3 +- kobo/apps/trash_bin/utils.py | 3 +- kpi/fixtures/test_data.json | 41 +++ kpi/models/asset.py | 2 +- kpi/tests/api/v2/test_api_assets.py | 11 +- kpi/views/v2/asset.py | 35 +-- kpi/views/v2/data.py | 3 +- 18 files changed, 499 insertions(+), 190 deletions(-) create mode 100644 kobo/apps/audit_log/audit_actions.py create mode 100644 kobo/apps/audit_log/tests/test_project_history_logs.py diff --git a/kobo/apps/accounts/tests/test_backend.py b/kobo/apps/accounts/tests/test_backend.py index c7ef6dc0eb..306faa9c30 100644 --- a/kobo/apps/accounts/tests/test_backend.py +++ b/kobo/apps/accounts/tests/test_backend.py @@ -9,7 +9,8 @@ from mock import patch from rest_framework import status -from kobo.apps.audit_log.models import AuditAction, AuditLog +from kobo.apps.audit_log.models import AuditLog +from ...audit_log.audit_actions import AuditAction from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.apps.main.models import UserProfile from .constants import SOCIALACCOUNT_PROVIDERS diff --git a/kobo/apps/audit_log/audit_actions.py b/kobo/apps/audit_log/audit_actions.py new file mode 100644 index 0000000000..4b6aad683e --- /dev/null +++ b/kobo/apps/audit_log/audit_actions.py @@ -0,0 +1,22 @@ +from django.db import models + + +class AuditAction(models.TextChoices): + CREATE = 'create' + DELETE = 'delete' + IN_TRASH = 'in-trash' + PUT_BACK = 'put-back' + REMOVE = 'remove' + UPDATE = 'update' + AUTH = 'auth' + DEPLOY = 'deploy' + ARCHIVE = 'archive' + UNARCHIVE = 'unarchive' + REDEPLOY = 'redeploy' + UPDATE_NAME = 'update-name' + UPDATE_FORM = 'update-form' + UPDATE_SETTINGS = 'update-settings' + UPDATE_QA = 'update-qa' + DISABLE_SHARING = 'disable-sharing' + ENABLE_SHARING = 'enable-sharing' + MODIFY_SHARING = 'modify-sharing' diff --git a/kobo/apps/audit_log/base_views.py b/kobo/apps/audit_log/base_views.py index 91c43a4712..b966881970 100644 --- a/kobo/apps/audit_log/base_views.py +++ b/kobo/apps/audit_log/base_views.py @@ -58,6 +58,8 @@ def perform_update(self, serializer): self.perform_update_override(serializer) audit_log_data = {} for field in self.logged_fields: + if field == 'data_sharing': + pass value = get_nested_field(serializer.instance, field) audit_log_data[field] = value self.request._request.updated_data = audit_log_data diff --git a/kobo/apps/audit_log/middleware.py b/kobo/apps/audit_log/middleware.py index 8dc176bdda..2001f7d033 100644 --- a/kobo/apps/audit_log/middleware.py +++ b/kobo/apps/audit_log/middleware.py @@ -1,12 +1,15 @@ + from kobo.apps.audit_log.models import ProjectHistoryLog, AuditType from kpi.utils.log import logging +from rest_framework import status def create_project_history_log_middleware(get_response): def do_thing(request): response = get_response(request) if request.method in ['GET', 'HEAD']: return response - if request.log_type == AuditType.PROJECT_HISTORY: + log_type = getattr(request, 'log_type', None) + if log_type == AuditType.PROJECT_HISTORY: ProjectHistoryLog.create_from_request(request) return response return do_thing diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index ca9b9e85cf..3e0fe66194 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -1,43 +1,28 @@ -import json - from django.conf import settings from django.db import models from django.db.models import Case, Count, F, Min, Value, When from django.db.models.functions import Cast, Concat, Trunc from django.utils import timezone +from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.libs.utils.viewer_tools import ( get_client_ip, get_human_readable_client_user_agent, ) +from kobo.static_lists import PROJECT_METADATA_DEFAULT_LABELS from kpi.constants import ( ACCESS_LOG_LOGINAS_AUTH_TYPE, ACCESS_LOG_SUBMISSION_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, ACCESS_LOG_UNKNOWN_AUTH_TYPE, - ASSET_TYPE_SURVEY, PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) -from kpi.exceptions import BadAssetTypeException from kpi.fields.kpi_uid import UUID_LENGTH from kpi.models import Asset from kpi.utils.log import logging -class AuditAction(models.TextChoices): - CREATE = 'create' - DELETE = 'delete' - IN_TRASH = 'in-trash' - PUT_BACK = 'put-back' - REMOVE = 'remove' - UPDATE = 'update' - AUTH = 'auth' - DEPLOY = 'deploy' - ARCHIVE = 'archive' - UNARCHIVE = 'unarchive' - REDEPLOY = 'redeploy' - - class AuditType(models.TextChoices): ACCESS = 'access' PROJECT_HISTORY = 'project-history' @@ -61,7 +46,7 @@ class AuditLog(models.Model): date_created = models.DateTimeField(default=timezone.now, db_index=True) metadata = models.JSONField(default=dict) action = models.CharField( - max_length=10, + max_length=30, choices=AuditAction.choices, default=AuditAction.DELETE, db_index=True, @@ -291,11 +276,6 @@ def create(self, **kwargs): # always be the same on a project history log self.remove_common_fields_and_warn('project history', kwargs) user = kwargs.pop('user') - asset = kwargs.pop('asset', None) - if asset is not None and asset.asset_type != ASSET_TYPE_SURVEY: - raise BadAssetTypeException( - 'Cannot create project history log for non-survey asset' - ) new_kwargs = { 'app_label': Asset._meta.app_label, 'model_name': Asset._meta.model_name, @@ -303,8 +283,6 @@ def create(self, **kwargs): 'user': user, 'user_uid': user.extra_details.uid } - if asset is not None: - new_kwargs['object_id']=asset.id new_kwargs.update(**kwargs) return super().create( # set the fields that are always the same for all project history logs, @@ -319,88 +297,140 @@ class ProjectHistoryLog(AuditLog): class Meta: proxy = True + @classmethod + def create_from_request(cls, request): + if request.resolver_match.url_name == 'asset-detail': + cls.create_from_asset_view_set(request) + elif request.resolver_match.url_name == 'asset-deployment': + cls.create_from_deployment_request(request) + + @staticmethod - def create_from_deployment_request( - request, asset, first_deployment=False, only_active_changed=False - ): - ip = get_client_ip(request) - source = get_human_readable_client_user_agent(request) - if only_active_changed: - action = ( - AuditAction.UNARCHIVE - if asset.deployment.active - else AuditAction.ARCHIVE - ) - else: - action = AuditAction.DEPLOY if first_deployment else AuditAction.REDEPLOY + def create_from_deployment_request(request): + if not hasattr(request, 'additional_audit_log_info'): + # if we didn't set this information, the request failed so don't try to create a log + return + only_active_changed = request.additional_audit_log_info.get('only_active_changed',False) + initial_data = request.initial_data + asset_uid = request.resolver_match.kwargs['uid'] + object_id = request.initial_data['id'] metadata = { - 'ip_address': ip, - 'source': source, - 'asset_uid': asset.uid, + 'asset_uid': asset_uid, 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + 'ip_address': get_client_ip(request), + 'source': get_human_readable_client_user_agent(request), } - if action in [AuditAction.REDEPLOY, AuditAction.DEPLOY]: - version = asset.latest_deployed_version - metadata['version_uid'] = version.uid - user = request.user - return ProjectHistoryLog.objects.create( - user=user, + + if only_active_changed: + action = AuditAction.ARCHIVE if request.additional_audit_log_info['active'] is False else AuditAction.UNARCHIVE + else: + action = AuditAction.REDEPLOY if initial_data['has_deployment'] is True else AuditAction.DEPLOY + metadata.update({ + 'latest_deployed_version_uid': request.additional_audit_log_info['latest_deployed_version_uid'] + }) + + ProjectHistoryLog.objects.create( + user=request.user, + object_id=object_id, action=action, metadata=metadata, - asset=asset, ) - @staticmethod - def create_from_request(request): - view_name = request.resolver_match.url_name - if view_name == 'asset-detail': - # we don't record project creation, so we only need the detail view - ProjectHistoryLog.create_from_asset_view_set_request(request) - - @staticmethod - def create_from_asset_view_set_request(request): - initial_data = request.get('initial_data', None) - updated_data = request.get('updated_data', None) + @classmethod + def create_from_asset_view_set(cls, request): + initial_data = getattr(request, 'initial_data', None) + updated_data = getattr(request, 'updated_data', None) asset_uid = request.resolver_match.kwargs['uid'] - object_id = initial_data.get('id', None) - common_metadata ={ + object_id = initial_data['id'] + + common_metadata = { 'asset_uid': asset_uid, 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, 'ip_address': get_client_ip(request), 'source': get_human_readable_client_user_agent(request), } - if updated_data is None: - # if updated_data is empty, the object was deleted - ProjectHistoryLog.objects.create( - user=request.user, - object_id=object_id, - action=AuditAction.DELETE, - metadata=common_metadata, - ) + if initial_data is None or updated_data is None: + # Something went wrong with the request, don't try to create a log return - if initial_data['name'] != updated_data['name']: - # create PH log for name change - metadata = { - 'old_name': initial_data['name'], - 'new_name': updated_data['name'], - **common_metadata, - } - ProjectHistoryLog.objects.create( - object_id=object_id, - user=request.user, - action=AuditAction.UPDATE, - metadata=metadata - ) + common_metadata.update({'latest_version_uid': updated_data['latest_version.uid']}) + + changed_field_to_action_map = { + 'name': cls.name_change, + 'content': cls.content_change, + 'advanced_features.qual': cls.qa_change, + 'data_sharing': cls.sharing_change, + 'settings': cls.settings_change + } - if json.dumps(initial_data['content']) != json.dumps(updated_data['content']): - metadata = { - 'new_version_uid': updated_data['latest_version.uid'] + for field, method in changed_field_to_action_map.items(): + old_field = initial_data[field] + new_field = updated_data[field] + if old_field != new_field: + action, additional_metadata = method(old_field, new_field) + full_metadata = {**common_metadata, **additional_metadata} + ProjectHistoryLog.objects.create( + user=request.user, + object_id=object_id, + action=action, + metadata=full_metadata, + ) + + @staticmethod + def name_change(old_field, new_field): + metadata = { + 'name': { + 'old': old_field, + 'new': new_field } - ProjectHistoryLog.objects.create( - object_id=object_id, - user=request.user, - action=AuditAction.UPDATE, - metadata=metadata, - ) + } + return AuditAction.UPDATE_NAME, metadata + + @staticmethod + def settings_change(old_field, new_field): + settings = {} + for setting_name in PROJECT_METADATA_DEFAULT_LABELS.keys(): + if old_field[setting_name] != new_field[setting_name]: + old = old_field[setting_name] + new = new_field[setting_name] + metadata_field_subdict = {} + if isinstance(old, list) and isinstance(new, list): + removed_values = [val for val in old if val not in new] + added_values = [val for val in new if val not in old] + metadata_field_subdict['added'] = added_values + metadata_field_subdict['removed'] = removed_values + else: + metadata_field_subdict['old'] = old + metadata_field_subdict['new'] = new + settings[setting_name] = metadata_field_subdict + + return AuditAction.UPDATE_SETTINGS, {'settings': settings} + + @staticmethod + def content_change(*_): + return AuditAction.UPDATE_FORM, {} + + @staticmethod + def qa_change(_, new_field): + return AuditAction.UPDATE_QA, {'qa_questions': {'new': new_field}} + + @staticmethod + def sharing_change(old_fields, new_fields): + old_enabled = old_fields['enabled'] + old_shared_fields = old_fields['fields'] + new_enabled = new_fields['enabled'] + new_shared_fields = new_fields['fields'] + shared_fields_dict = {} + if old_enabled is True and new_enabled is False: + action = AuditAction.DISABLE_SHARING + elif old_enabled is False and new_enabled is True: + action = AuditAction.ENABLE_SHARING + shared_fields_dict['added'] = new_shared_fields + else: + removed_fields = [field for field in old_shared_fields if field not in new_shared_fields] + added_fields = [field for field in new_shared_fields if field not in old_shared_fields] + action = AuditAction.MODIFY_SHARING + shared_fields_dict['added'] = added_fields + shared_fields_dict['removed'] = removed_fields + return action, {'shared_fields': shared_fields_dict} diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index ee8f5fd940..62bcb9f882 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -7,10 +7,10 @@ from kobo.apps.audit_log.models import ( AccessLog, - AuditAction, AuditLog, AuditType, ) +from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.audit_log.tests.test_signals import skip_login_access_log from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ( diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index 40379277db..24e59dff75 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -13,11 +13,11 @@ ACCESS_LOG_LOGINAS_AUTH_TYPE, ACCESS_LOG_UNKNOWN_AUTH_TYPE, AccessLog, - AuditAction, AuditLog, AuditType, ProjectHistoryLog, ) +from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ( ASSET_TYPE_QUESTION, @@ -366,6 +366,14 @@ def _check_common_fields(self, log: ProjectHistoryLog, user, asset): self.assertEqual(log.user_uid, user.extra_details.uid) self.assertEqual(log.log_type, AuditType.PROJECT_HISTORY) + def _create_request(self, url: str, user, data): + factory = RequestFactory() + request = factory.post(url) + request.user = user + request.resolver_match = resolve(url) + request.data = data + return request + def test_create_project_history_log_sets_standard_fields(self): user = User.objects.get(username='someuser') asset = Asset.objects.get(pk=1) @@ -374,7 +382,7 @@ def test_create_project_history_log_sets_standard_fields(self): user=user, metadata={'foo': 'bar'}, date_created=yesterday, - asset=asset, + object_id=asset.id, ) self._check_common_fields(log, user, asset) self.assertEquals(log.date_created, yesterday) @@ -390,64 +398,10 @@ def test_create_project_history_log_ignores_attempt_to_override_standard_fields( log_type=AuditType.DATA_EDITING, model_name='foo', app_label='bar', - asset=asset, + object_id=asset.id, user=user, ) # the standard fields should be set the same as any other project history logs self._check_common_fields(log, user, asset) # we logged a warning for each attempt to override a field self.assertEquals(patched_warning.call_count, 3) - - def test_cannot_create_project_history_log_from_non_survey_asset(self): - block_asset = baker.make(Asset) - block_asset.asset_type = ASSET_TYPE_QUESTION - with self.assertRaises(BadAssetTypeException): - ProjectHistoryLog.objects.create( - user=User.objects.get(username='someuser'), asset=block_asset - ) - - @data( - # first_deployment, only_active_changed, is_active, - # expected_action, expect_extra_metadata - (True, False, True, AuditAction.DEPLOY, True), - (False, False, True, AuditAction.REDEPLOY, True), - (False, True, False, AuditAction.ARCHIVE, False), - (False, True, True, AuditAction.UNARCHIVE, False), - ) - @unpack - def test_create_log_for_deployment_changes( - self, - first_deployment, - only_active_changed, - is_active, - expected_action, - expect_extra_metadata, - ): - user = User.objects.get(username='someuser') - factory = RequestFactory() - request = factory.post('') - request.user = user - asset = Asset.objects.get(pk=1) - asset.save() - asset.deploy(backend='mock', active=is_active) - log = ProjectHistoryLog.create_from_deployment_request( - request, - asset, - first_deployment=first_deployment, - only_active_changed=only_active_changed, - ) - self._check_common_fields(log, user, asset) - expected_metadata = { - 'ip_address': '127.0.0.1', - 'source': 'source', - 'asset_uid': asset.uid, - 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, - } - if expect_extra_metadata: - expected_metadata.update( - { - 'version_uid': asset.latest_deployed_version_uid, - } - ) - self.assertDictEqual(log.metadata, expected_metadata) - self.assertEqual(log.action, expected_action) diff --git a/kobo/apps/audit_log/tests/test_one_time_auth.py b/kobo/apps/audit_log/tests/test_one_time_auth.py index 13fc8caa8c..6d74bd2356 100644 --- a/kobo/apps/audit_log/tests/test_one_time_auth.py +++ b/kobo/apps/audit_log/tests/test_one_time_auth.py @@ -7,7 +7,8 @@ from rest_framework.authtoken.models import Token from trench.utils import get_mfa_model -from kobo.apps.audit_log.models import AuditAction, AuditLog +from kobo.apps.audit_log.models import AuditLog +from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.apps.main.models import UserProfile from kpi.models import AuthorizedApplication diff --git a/kobo/apps/audit_log/tests/test_project_history_logs.py b/kobo/apps/audit_log/tests/test_project_history_logs.py new file mode 100644 index 0000000000..1cbac74b9a --- /dev/null +++ b/kobo/apps/audit_log/tests/test_project_history_logs.py @@ -0,0 +1,266 @@ +import copy +import unittest + +from kobo.apps.audit_log.models import ProjectHistoryLog +from kobo.apps.audit_log.audit_actions import AuditAction +from kobo.apps.audit_log.tests.test_models import BaseAuditLogTestCase +from kpi.deployment_backends.mock_backend import MockDeploymentBackend +from kpi.models import Asset +from kpi.tests.kpi_test_case import KpiTestCase +from kobo.apps.kobo_auth.shortcuts import User +from django.urls import reverse +from django.test import override_settings + + +@override_settings(DEFAULT_DEPLOYMENT_BACKEND='mock') +class TestProjectHistoryLogs(BaseAuditLogTestCase): + + fixtures = ['test_data'] + + def setUp(self): + super().setUp() + # log in as admin + user = User.objects.get(username='admin') + self.client.force_login(user=user) + # use the same asset + asset = Asset.objects.get(pk=3) + # save to create a version + asset.save() + self.asset = asset + + def _check_common_metadata(self, metadata_dict, asset): + self.assertEqual(metadata_dict['asset_uid'], asset.uid) + self.assertEqual(metadata_dict['ip_address'], '127.0.0.1') + self.assertEqual(metadata_dict['source'], 'source') + self.assertEqual(metadata_dict['latest_version_uid'], self.asset.latest_version.uid) + + def test_change_project_name_creates_log(self): + old_name = self.asset.name + self.client.patch( + reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), + data={'name': 'new_name'} + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self.assertEqual(log.metadata['name']['new'], 'new_name') + self.assertEqual(log.metadata['name']['old'], old_name) + self.assertEqual(log.action, AuditAction.UPDATE_NAME) + self.assertEqual(log.object_id, self.asset.id) + + def test_change_project_settings_creates_log(self): + old_settings = copy.deepcopy(self.asset.settings) + user = User.objects.get(username='admin') + self.client.force_login(user=user) + patch_data = {'settings': { + 'sector': {'label': 'Health', 'value': 'Health'}, + 'country': [{'label': 'Albania', 'value': 'ALB'}], + 'operational_purpose': 'New operational purpose', + 'collects_pii': True, + 'description': 'New description', + }} + self.client.patch( + reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), + data = patch_data, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self.assertEqual(log.object_id, self.asset.id) + log_settings_metadata = log.metadata['settings'] + # check non-list settings just store old and new information + self.assertDictEqual(log_settings_metadata['sector']['old'], old_settings['sector']) + self.assertDictEqual(log_settings_metadata['sector']['new'], {'label': 'Health', 'value': 'Health'}) + self.assertEqual(log_settings_metadata['operational_purpose']['old'], old_settings['operational_purpose']) + self.assertEqual(log_settings_metadata['operational_purpose']['new'], 'New operational purpose') + self.assertEqual(log_settings_metadata['collects_pii']['old'], old_settings['collects_pii']) + self.assertEqual(log_settings_metadata['collects_pii']['new'], True) + self.assertEqual(log_settings_metadata['description']['old'], old_settings['description']) + self.assertEqual(log_settings_metadata['description']['new'], 'New description') + # check list settings store added and removed fields + self.assertListEqual(log_settings_metadata['country']['added'], [{'label': 'Albania', 'value': 'ALB'}]) + self.assertListEqual(log_settings_metadata['country']['removed'], [{'label': 'United States', 'value': 'USA'}]) + + def test_unchanged_settings_not_recorded_on_log(self): + patch_data = {'settings': { + 'sector': self.asset.settings['sector'], + 'country': self.asset.settings['country'], + 'operational_purpose': self.asset.settings['operational_purpose'], + 'collects_pii': self.asset.settings['collects_pii'], + 'description': 'New description', + }} + self.client.patch( + reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), + data = patch_data, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + log_settings_metadata = log.metadata['settings'] + self.assertNotIn('sector', log_settings_metadata.keys()) + self.assertNotIn('country', log_settings_metadata.keys()) + self.assertNotIn('operational_purpose', log_settings_metadata.keys()) + self.assertNotIn('collects_pii', log_settings_metadata.keys()) + + def test_first_time_deployment_creates_log(self): + post_data = { + 'active': True, + 'backend': 'mock', + } + self.client.post( + reverse('api_v2:asset-deployment', kwargs={'uid': self.asset.uid}), + data = post_data, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self.assertEqual(log.object_id, self.asset.id) + self.assertEqual(log.metadata['latest_deployed_version_uid'], self.asset.latest_version.uid) + self.assertEqual(log.action, AuditAction.DEPLOY) + + def test_redeployment_creates_log(self): + self.asset.deploy(backend='mock', active=True) + patch_data = { + 'active': True, + 'version_id': self.asset.version_id + } + self.client.patch( + reverse('api_v2:asset-deployment', kwargs={'uid': self.asset.uid}), + data = patch_data, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self.assertEqual(log.object_id, self.asset.id) + self.assertEqual(log.metadata['latest_deployed_version_uid'], self.asset.latest_version.uid) + self.assertEqual(log.action, AuditAction.REDEPLOY) + + def test_change_project_content_creates_log(self): + new_content = { + 'survey': [ + {'type': 'text', 'label': 'new question', 'name': 'new_question', 'kuid': 'hijk'} + ] + } + self.client.patch( + reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), + data={'content': new_content}, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self._check_common_metadata(log.metadata, self.asset) + self.assertEqual(log.action, AuditAction.UPDATE_FORM) + + def test_enable_sharing_creates_log(self): + patch_data = { + 'data_sharing': { + 'enabled': True, + 'fields': ['q1'] + } + } + self.client.patch( + reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), + data=patch_data, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self._check_common_metadata(log.metadata, self.asset) + self.assertEqual(log.action, AuditAction.ENABLE_SHARING) + self.assertListEqual(log.metadata['shared_fields']['added'], ['q1']) + + def test_disable_sharing_creates_log(self): + # turn sharing on + self.asset.data_sharing = { + 'enabled': True, + 'fields': ['q1', 'q2'] + } + self.asset.save() + patch = { + 'data_sharing': { + 'enabled': False, + 'fields': [] + } + } + self.client.patch( + reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), + data=patch, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self._check_common_metadata(log.metadata, self.asset) + self.assertEqual(log.action, AuditAction.DISABLE_SHARING) + + def test_modify_shared_fields_creates_log(self): + self.asset.content = { 'survey':[ + {"type": "text", "label": "fixture q1", "name": "q1", "kuid": "abc"}, + {"type": "text", "label": "fixture q2", "name": "q2", "kuid": "def"}, + {"type": "text", "label": "fixture q3", "name": "q3", "kuid": "def"} + ]} + # turn sharing on + self.asset.data_sharing = { + 'enabled': True, + 'fields': ['q1', 'q2'] + } + self.asset.save() + post_data = { + 'data_sharing': { + 'enabled': True, + 'fields': ['q2', 'q3'] + } + } + self.client.patch( + reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), + data=post_data, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self._check_common_metadata(log.metadata, self.asset) + self.assertEqual(log.action, AuditAction.MODIFY_SHARING) + self.assertListEqual(log.metadata['shared_fields']['added'], ['q3']) + self.assertListEqual(log.metadata['shared_fields']['removed'], ['q1']) + + def test_archive_creates_log(self): + # can only archive deployed asset + self.asset.deploy(backend='mock', active=True) + post_data = { + 'active': False, + } + self.client.patch( + reverse('api_v2:asset-deployment', kwargs={'uid': self.asset.uid}), + data = post_data, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self.assertEqual(log.object_id, self.asset.id) + self.assertEqual(log.action, AuditAction.ARCHIVE) + + def test_unarchive_creates_log(self): + # can only unarchive deployed asset + self.asset.deploy(backend='mock', active=False) + post_data = { + 'active': True, + } + self.client.patch( + reverse('api_v2:asset-deployment', kwargs={'uid': self.asset.uid}), + data = post_data, + format='json', + ) + logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(logs.count(), 1) + log = logs.first() + self.assertEqual(log.object_id, self.asset.id) + self.assertEqual(log.action, AuditAction.UNARCHIVE) + diff --git a/kobo/apps/audit_log/tests/test_signals.py b/kobo/apps/audit_log/tests/test_signals.py index d183ae37b8..605706a0da 100644 --- a/kobo/apps/audit_log/tests/test_signals.py +++ b/kobo/apps/audit_log/tests/test_signals.py @@ -7,7 +7,8 @@ from django.urls import reverse from trench.utils import get_mfa_model -from kobo.apps.audit_log.models import AuditAction, AuditLog +from kobo.apps.audit_log.models import AuditLog +from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.audit_log.signals import create_access_log from kobo.apps.kobo_auth.shortcuts import User from kpi.tests.base_test_case import BaseTestCase diff --git a/kobo/apps/trash_bin/tasks.py b/kobo/apps/trash_bin/tasks.py index 46d6ed878b..1af128679c 100644 --- a/kobo/apps/trash_bin/tasks.py +++ b/kobo/apps/trash_bin/tasks.py @@ -14,7 +14,8 @@ from requests.exceptions import HTTPError from kobo.apps.trackers.models import NLPUsageCounter -from kobo.apps.audit_log.models import AuditLog, AuditAction, AuditType +from kobo.apps.audit_log.models import AuditLog, AuditType +from ..audit_log.audit_actions import AuditAction from kobo.celery import celery_app from kpi.deployment_backends.kc_access.utils import delete_kc_user from kpi.exceptions import KobocatCommunicationError diff --git a/kobo/apps/trash_bin/tests/test_utils.py b/kobo/apps/trash_bin/tests/test_utils.py index 3c6da52239..3c28a54dc2 100644 --- a/kobo/apps/trash_bin/tests/test_utils.py +++ b/kobo/apps/trash_bin/tests/test_utils.py @@ -7,7 +7,8 @@ from django_celery_beat.models import PeriodicTask from mock import patch -from kobo.apps.audit_log.models import AuditAction, AuditLog, AuditType +from kobo.apps.audit_log.models import AuditLog, AuditType +from ...audit_log.audit_actions import AuditAction from kpi.models import Asset from ..constants import DELETE_PROJECT_STR_PREFIX, DELETE_USER_STR_PREFIX from ..models.account import AccountTrash diff --git a/kobo/apps/trash_bin/utils.py b/kobo/apps/trash_bin/utils.py index f7344c3932..53324ac9c2 100644 --- a/kobo/apps/trash_bin/utils.py +++ b/kobo/apps/trash_bin/utils.py @@ -12,7 +12,8 @@ from django.utils.timezone import now from django_celery_beat.models import ClockedSchedule, PeriodicTask, PeriodicTasks -from kobo.apps.audit_log.models import AuditAction, AuditLog, AuditType +from kobo.apps.audit_log.models import AuditLog, AuditType +from ..audit_log.audit_actions import AuditAction from kpi.exceptions import InvalidXFormException, MissingXFormException from kpi.models import Asset, ExportTask, ImportTask from kpi.utils.mongo_helper import MongoHelper diff --git a/kpi/fixtures/test_data.json b/kpi/fixtures/test_data.json index 2c6fee0074..30486a02df 100644 --- a/kpi/fixtures/test_data.json +++ b/kpi/fixtures/test_data.json @@ -145,6 +145,47 @@ "uid": "aPEANgNgTH3xzhJGcctURu" } }, + { + "model": "kpi.asset", + "pk": 3, + "parent": 3, + "fields": { + "name": "Asset with settings and qa", + "content": {"survey": [ + {"type": "text", "label": "fixture q1", "name": "q1", "kuid": "abc"}, + {"type": "text", "label": "fixture q2", "name": "q2", "kuid": "def"} + ]}, + "settings": { + "description": "Description", + "country": [{"label": "United States", "value": "USA"}], + "sector": {"label": "Education", "value": "Education"}, + "operational_purpose": "Purpose", + "collects_pii": false + }, + "data_sharing": { + "fields": [], + "enabled": false + }, + "advanced_features": { + "qual": { + "qual_survey": [ + { + "type": "qual_note", + "uuid": "12345", + "scope": "by_question#survey", + "xpath": "q1", + "labels": {"_default": "QA Question"} + } + ] + } + + }, + "date_created": "2022-04-05T21:00:22.402Z", + "date_modified": "2022-04-05T21:00:33.946Z", + "owner": 1, + "uid": "aVq7yMt4zKpUtTeZUnoeJF" + } + }, { "model": "hub.sitewidemessage", "pk": 1, diff --git a/kpi/models/asset.py b/kpi/models/asset.py index fff9eafeeb..c7211cb844 100644 --- a/kpi/models/asset.py +++ b/kpi/models/asset.py @@ -186,7 +186,7 @@ class Asset( # JSON with subset of fields to share # { - # 'enable': True, + # 'enabled': True, # 'fields': [] # shares all when empty # } data_sharing = LazyDefaultJSONBField(default=dict) diff --git a/kpi/tests/api/v2/test_api_assets.py b/kpi/tests/api/v2/test_api_assets.py index 5c40c9eb77..c1cdb8e615 100644 --- a/kpi/tests/api/v2/test_api_assets.py +++ b/kpi/tests/api/v2/test_api_assets.py @@ -1727,8 +1727,7 @@ def test_upload_form_media_with_slashes(self): class AssetDeploymentTest(BaseAssetDetailTestCase): - @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') - def test_asset_deployment(self, patched_create_log): + def test_asset_deployment(self): deployment_url = reverse(self._get_endpoint('asset-deployment'), kwargs={'uid': self.asset_uid}) @@ -1739,11 +1738,6 @@ def test_asset_deployment(self, patched_create_log): self.assertEqual(response1.data['asset']['deployment__active'], True) self.assertEqual(response1.data['asset']['has_deployment'], True) - request = response1.renderer_context['request'] - patched_create_log.assert_called_once_with( - request, self.asset, first_deployment=True - ) - patched_create_log.reset_mock() response2 = self.client.get(self.asset_url, format='json') @@ -1753,8 +1747,7 @@ def test_asset_deployment(self, patched_create_log): response2.data['deployment_status'] == AssetDeploymentStatus.DEPLOYED.value ) - # nothing should be logged for a GET request - patched_create_log.assert_not_called() + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') def test_asset_redeployment(self, patched_create_log): diff --git a/kpi/views/v2/asset.py b/kpi/views/v2/asset.py index c7ebc29a94..7e648cef02 100644 --- a/kpi/views/v2/asset.py +++ b/kpi/views/v2/asset.py @@ -12,7 +12,7 @@ from rest_framework.response import Response from rest_framework_extensions.mixins import NestedViewSetMixin -from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet +from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet, get_nested_field from kobo.apps.audit_log.models import ProjectHistoryLog, AuditType from kpi.constants import ( ASSET_TYPE_ARG_NAME, @@ -382,9 +382,8 @@ class AssetViewSet( ] logged_fields = ['content', 'settings', 'data_sharing', 'latest_version.uid', - 'latest_deployed_version_uid', 'name', 'advanced_features'] + 'has_deployment', 'name', 'advanced_features.qual', 'id', 'latest_deployed_version_uid'] log_type = AuditType.PROJECT_HISTORY - model_name = 'Asset' def get_object_override(self): if self.request.method in ['PATCH', 'GET']: @@ -484,9 +483,11 @@ def deployment(self, request, uid): ) serializer.is_valid(raise_exception=True) serializer.save() - ProjectHistoryLog.create_from_deployment_request( - request, asset, first_deployment=True - ) + request._request.additional_audit_log_info = { + 'request_data': request._data, + 'id': asset.id, + 'latest_deployed_version_uid': asset.latest_deployed_version_uid + } # TODO: Understand why this 404s when `serializer.data` is not # coerced to a dict return Response(dict(serializer.data)) @@ -511,21 +512,12 @@ def deployment(self, request, uid): ) serializer.is_valid(raise_exception=True) serializer.save() - - # create a project history log - # note a log will be created even if nothing changed, since - # we care about request intent more than effect - - # copy the request data so we don't change the original object - request_data_copy = request.data.copy() - # remove the 'backend' key, we don't care about it for this part - request_data_copy.pop('backend', None) - - updated_fields = request_data_copy.keys() - only_active_changed = list(updated_fields) == ['active'] - ProjectHistoryLog.create_from_deployment_request( - request, asset, only_active_changed=only_active_changed - ) + only_active_changed = set(serializer.validated_data.keys()) == set(('active',)) + request._request.additional_audit_log_info = { + 'only_active_changed': only_active_changed, + 'active': serializer.data['active'], + 'latest_deployed_version_uid': asset.latest_deployed_version_uid + } # TODO: Understand why this 404s when `serializer.data` is not # coerced to a dict return Response(dict(serializer.data)) @@ -786,7 +778,6 @@ def partial_update(self, request, *args, **kwargs): partial=True) serializer.is_valid(raise_exception=True) - logging.info('I am updating the instance') super().perform_update(serializer) return Response(serializer.data) diff --git a/kpi/views/v2/data.py b/kpi/views/v2/data.py index b1808cf36e..28342e2408 100644 --- a/kpi/views/v2/data.py +++ b/kpi/views/v2/data.py @@ -16,7 +16,8 @@ from rest_framework.reverse import reverse from rest_framework_extensions.mixins import NestedViewSetMixin -from kobo.apps.audit_log.models import AuditAction, AuditLog, AuditType +from kobo.apps.audit_log.models import AuditLog, AuditType +from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.openrosa.libs.utils.logger_tools import http_open_rosa_error_handler from kpi.authentication import EnketoSessionAuthentication from kpi.constants import ( From 16e8faa03e7880ad68c12c2f256b6ddd58d7c37b Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 21 Oct 2024 17:07:03 -0400 Subject: [PATCH 06/10] fixup!: refactor tests --- kobo/apps/accounts/tests/test_backend.py | 4 +- kobo/apps/audit_log/base_views.py | 2 - kobo/apps/audit_log/middleware.py | 5 +- kobo/apps/audit_log/models.py | 91 ++-- .../tests/api/v2/test_api_audit_log.py | 2 +- kobo/apps/audit_log/tests/test_models.py | 8 +- .../audit_log/tests/test_one_time_auth.py | 2 +- .../tests/test_project_history_logs.py | 395 +++++++++--------- kobo/apps/audit_log/tests/test_signals.py | 2 +- kobo/apps/trash_bin/tasks.py | 10 +- kobo/apps/trash_bin/tests/test_utils.py | 4 +- kobo/apps/trash_bin/utils.py | 2 +- kpi/models/asset.py | 6 +- kpi/tests/api/v2/test_api_assets.py | 4 +- kpi/views/v2/asset.py | 30 +- kpi/views/v2/data.py | 2 +- 16 files changed, 313 insertions(+), 256 deletions(-) diff --git a/kobo/apps/accounts/tests/test_backend.py b/kobo/apps/accounts/tests/test_backend.py index 306faa9c30..8417cfaab4 100644 --- a/kobo/apps/accounts/tests/test_backend.py +++ b/kobo/apps/accounts/tests/test_backend.py @@ -1,4 +1,5 @@ import json +from unittest.mock import patch import responses from allauth.socialaccount.models import SocialAccount, SocialApp @@ -6,13 +7,12 @@ from django.test import TestCase from django.test.utils import override_settings from django.urls import reverse -from mock import patch from rest_framework import status from kobo.apps.audit_log.models import AuditLog -from ...audit_log.audit_actions import AuditAction from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.apps.main.models import UserProfile +from ...audit_log.audit_actions import AuditAction from .constants import SOCIALACCOUNT_PROVIDERS diff --git a/kobo/apps/audit_log/base_views.py b/kobo/apps/audit_log/base_views.py index b966881970..e7d8c71f70 100644 --- a/kobo/apps/audit_log/base_views.py +++ b/kobo/apps/audit_log/base_views.py @@ -1,8 +1,6 @@ # coding: utf-8 from rest_framework import mixins, viewsets -from kobo.apps.audit_log.models import AuditType - def get_nested_field(obj, field: str): """ diff --git a/kobo/apps/audit_log/middleware.py b/kobo/apps/audit_log/middleware.py index 2001f7d033..b8d29db480 100644 --- a/kobo/apps/audit_log/middleware.py +++ b/kobo/apps/audit_log/middleware.py @@ -1,7 +1,5 @@ +from kobo.apps.audit_log.models import AuditType, ProjectHistoryLog -from kobo.apps.audit_log.models import ProjectHistoryLog, AuditType -from kpi.utils.log import logging -from rest_framework import status def create_project_history_log_middleware(get_response): def do_thing(request): @@ -12,4 +10,5 @@ def do_thing(request): if log_type == AuditType.PROJECT_HISTORY: ProjectHistoryLog.create_from_request(request) return response + return do_thing diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index 3e0fe66194..f8416d8293 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -281,7 +281,7 @@ def create(self, **kwargs): 'model_name': Asset._meta.model_name, 'log_type': AuditType.PROJECT_HISTORY, 'user': user, - 'user_uid': user.extra_details.uid + 'user_uid': user.extra_details.uid, } new_kwargs.update(**kwargs) return super().create( @@ -304,13 +304,14 @@ def create_from_request(cls, request): elif request.resolver_match.url_name == 'asset-deployment': cls.create_from_deployment_request(request) - @staticmethod def create_from_deployment_request(request): - if not hasattr(request, 'additional_audit_log_info'): - # if we didn't set this information, the request failed so don't try to create a log + audit_log_info = getattr(request, 'additional_audit_log_info', None) + if audit_log_info is None: + # if we didn't set this information, the request failed + # so don't try to create a log return - only_active_changed = request.additional_audit_log_info.get('only_active_changed',False) + initial_data = request.initial_data asset_uid = request.resolver_match.kwargs['uid'] object_id = request.initial_data['id'] @@ -319,15 +320,36 @@ def create_from_deployment_request(request): 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, 'ip_address': get_client_ip(request), 'source': get_human_readable_client_user_agent(request), + 'latest_version_uid': audit_log_info['latest_version_uid'], } + # requests to archive/unarchive will only have the `active` param in the request + # we record this on the request at the view level + only_active_changed = request.additional_audit_log_info.get( + 'only_active_changed', False + ) + if only_active_changed: - action = AuditAction.ARCHIVE if request.additional_audit_log_info['active'] is False else AuditAction.UNARCHIVE + # if active is set to False, the request was to archive the project + # otherwise, request was to unarchive + action = ( + AuditAction.ARCHIVE + if request.additional_audit_log_info['active'] is False + else AuditAction.UNARCHIVE + ) else: - action = AuditAction.REDEPLOY if initial_data['has_deployment'] is True else AuditAction.DEPLOY - metadata.update({ - 'latest_deployed_version_uid': request.additional_audit_log_info['latest_deployed_version_uid'] - }) + # if the asset was already deployed, label this as a redeploy + action = ( + AuditAction.REDEPLOY + if initial_data['has_deployment'] is True + else AuditAction.DEPLOY + ) + latest_deployed_version_uid = audit_log_info['latest_deployed_version_uid'] + metadata.update( + { + 'latest_deployed_version_uid': latest_deployed_version_uid, + } + ) ProjectHistoryLog.objects.create( user=request.user, @@ -340,6 +362,11 @@ def create_from_deployment_request(request): def create_from_asset_view_set(cls, request): initial_data = getattr(request, 'initial_data', None) updated_data = getattr(request, 'updated_data', None) + + if initial_data is None or updated_data is None: + # Something went wrong with the request, don't try to create a log + return + asset_uid = request.resolver_match.kwargs['uid'] object_id = initial_data['id'] @@ -350,18 +377,17 @@ def create_from_asset_view_set(cls, request): 'source': get_human_readable_client_user_agent(request), } - if initial_data is None or updated_data is None: - # Something went wrong with the request, don't try to create a log - return - - common_metadata.update({'latest_version_uid': updated_data['latest_version.uid']}) + # always store the latest version uid + common_metadata.update( + {'latest_version_uid': updated_data['latest_version.uid']} + ) changed_field_to_action_map = { 'name': cls.name_change, 'content': cls.content_change, 'advanced_features.qual': cls.qa_change, 'data_sharing': cls.sharing_change, - 'settings': cls.settings_change + 'settings': cls.settings_change, } for field, method in changed_field_to_action_map.items(): @@ -371,20 +397,19 @@ def create_from_asset_view_set(cls, request): action, additional_metadata = method(old_field, new_field) full_metadata = {**common_metadata, **additional_metadata} ProjectHistoryLog.objects.create( - user=request.user, - object_id=object_id, - action=action, - metadata=full_metadata, + user=request.user, + object_id=object_id, + action=action, + metadata=full_metadata, ) + # additional metadata should generally follow the pattern + # 'field': {'old': old_value, 'new': new_value } or + # 'field': {'added': [], 'removed'} + @staticmethod def name_change(old_field, new_field): - metadata = { - 'name': { - 'old': old_field, - 'new': new_field - } - } + metadata = {'name': {'old': old_field, 'new': new_field}} return AuditAction.UPDATE_NAME, metadata @staticmethod @@ -409,10 +434,13 @@ def settings_change(old_field, new_field): @staticmethod def content_change(*_): + # content is too long/complicated for meaningful comparison, + # so don't store values return AuditAction.UPDATE_FORM, {} @staticmethod def qa_change(_, new_field): + # old version isn't needed on ph logs return AuditAction.UPDATE_QA, {'qa_questions': {'new': new_field}} @staticmethod @@ -423,13 +451,20 @@ def sharing_change(old_fields, new_fields): new_shared_fields = new_fields['fields'] shared_fields_dict = {} if old_enabled is True and new_enabled is False: + # sharing went from enabled to disabled action = AuditAction.DISABLE_SHARING elif old_enabled is False and new_enabled is True: + # sharing went from disabled to enabled action = AuditAction.ENABLE_SHARING shared_fields_dict['added'] = new_shared_fields else: - removed_fields = [field for field in old_shared_fields if field not in new_shared_fields] - added_fields = [field for field in new_shared_fields if field not in old_shared_fields] + # the specific fields shared changed + removed_fields = [ + field for field in old_shared_fields if field not in new_shared_fields + ] + added_fields = [ + field for field in new_shared_fields if field not in old_shared_fields + ] action = AuditAction.MODIFY_SHARING shared_fields_dict['added'] = added_fields shared_fields_dict['removed'] = removed_fields diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index 62bcb9f882..aa6d3f3076 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -5,12 +5,12 @@ from rest_framework import status from rest_framework.reverse import reverse +from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.audit_log.models import ( AccessLog, AuditLog, AuditType, ) -from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.audit_log.tests.test_signals import skip_login_access_log from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ( diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index 24e59dff75..adec73c78a 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -2,13 +2,13 @@ from datetime import timedelta from unittest.mock import patch -from ddt import data, ddt, unpack +from ddt import ddt from django.contrib.auth.models import AnonymousUser from django.test.client import RequestFactory from django.urls import resolve, reverse from django.utils import timezone -from model_bakery import baker +from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.audit_log.models import ( ACCESS_LOG_LOGINAS_AUTH_TYPE, ACCESS_LOG_UNKNOWN_AUTH_TYPE, @@ -17,15 +17,11 @@ AuditType, ProjectHistoryLog, ) -from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ( - ASSET_TYPE_QUESTION, ACCESS_LOG_SUBMISSION_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, - PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) -from kpi.exceptions import BadAssetTypeException from kpi.models import Asset from kpi.tests.base_test_case import BaseTestCase diff --git a/kobo/apps/audit_log/tests/test_one_time_auth.py b/kobo/apps/audit_log/tests/test_one_time_auth.py index 6d74bd2356..c030f49d07 100644 --- a/kobo/apps/audit_log/tests/test_one_time_auth.py +++ b/kobo/apps/audit_log/tests/test_one_time_auth.py @@ -7,8 +7,8 @@ from rest_framework.authtoken.models import Token from trench.utils import get_mfa_model -from kobo.apps.audit_log.models import AuditLog from kobo.apps.audit_log.audit_actions import AuditAction +from kobo.apps.audit_log.models import AuditLog from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.apps.main.models import UserProfile from kpi.models import AuthorizedApplication diff --git a/kobo/apps/audit_log/tests/test_project_history_logs.py b/kobo/apps/audit_log/tests/test_project_history_logs.py index 1cbac74b9a..afffaf2c32 100644 --- a/kobo/apps/audit_log/tests/test_project_history_logs.py +++ b/kobo/apps/audit_log/tests/test_project_history_logs.py @@ -1,15 +1,13 @@ import copy -import unittest -from kobo.apps.audit_log.models import ProjectHistoryLog +from django.test import override_settings +from django.urls import reverse + from kobo.apps.audit_log.audit_actions import AuditAction +from kobo.apps.audit_log.models import ProjectHistoryLog from kobo.apps.audit_log.tests.test_models import BaseAuditLogTestCase -from kpi.deployment_backends.mock_backend import MockDeploymentBackend -from kpi.models import Asset -from kpi.tests.kpi_test_case import KpiTestCase from kobo.apps.kobo_auth.shortcuts import User -from django.urls import reverse -from django.test import override_settings +from kpi.models import Asset @override_settings(DEFAULT_DEPLOYMENT_BACKEND='mock') @@ -27,240 +25,265 @@ def setUp(self): # save to create a version asset.save() self.asset = asset + self.detail_url = 'api_v2:asset-detail' + self.deployment_url = 'api_v2:asset-deployment' - def _check_common_metadata(self, metadata_dict, asset): - self.assertEqual(metadata_dict['asset_uid'], asset.uid) + def _check_common_metadata(self, metadata_dict): + self.assertEqual(metadata_dict['asset_uid'], self.asset.uid) self.assertEqual(metadata_dict['ip_address'], '127.0.0.1') self.assertEqual(metadata_dict['source'], 'source') - self.assertEqual(metadata_dict['latest_version_uid'], self.asset.latest_version.uid) + self.assertEqual( + metadata_dict['latest_version_uid'], self.asset.latest_version.uid + ) - def test_change_project_name_creates_log(self): - old_name = self.asset.name - self.client.patch( - reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), - data={'name': 'new_name'} + def _base_endpoint_test( + self, patch, url_name, request_data, expected_action, verify_additional_metadata + ): + request_method = self.client.patch if patch else self.client.post + response = request_method( + reverse(url_name, kwargs={'uid': self.asset.uid}), + data=request_data, + format='json', ) + print(response.status_code) logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) self.assertEqual(logs.count(), 1) log = logs.first() - self.assertEqual(log.metadata['name']['new'], 'new_name') - self.assertEqual(log.metadata['name']['old'], old_name) - self.assertEqual(log.action, AuditAction.UPDATE_NAME) + self._check_common_metadata(log.metadata) self.assertEqual(log.object_id, self.asset.id) + self.assertEqual(log.action, expected_action) + verify_additional_metadata(log.metadata) + + def test_change_project_name_creates_log(self): + old_name = self.asset.name + + def verify_metadata(log_metadata): + self.assertEqual(log_metadata['name']['new'], 'new_name') + self.assertEqual(log_metadata['name']['old'], old_name) + + self._base_endpoint_test( + patch=True, + url_name=self.detail_url, + request_data={'name': 'new_name'}, + verify_additional_metadata=verify_metadata, + expected_action=AuditAction.UPDATE_NAME, + ) def test_change_project_settings_creates_log(self): old_settings = copy.deepcopy(self.asset.settings) - user = User.objects.get(username='admin') - self.client.force_login(user=user) - patch_data = {'settings': { - 'sector': {'label': 'Health', 'value': 'Health'}, - 'country': [{'label': 'Albania', 'value': 'ALB'}], - 'operational_purpose': 'New operational purpose', - 'collects_pii': True, - 'description': 'New description', - }} - self.client.patch( - reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), - data = patch_data, - format='json', + patch_data = { + 'settings': { + 'sector': {'label': 'Health', 'value': 'Health'}, + 'country': [{'label': 'Albania', 'value': 'ALB'}], + 'operational_purpose': 'New operational purpose', + 'collects_pii': True, + 'description': 'New description', + } + } + + def verify_metadata(log_metadata): + # check non-list settings just store old and new information + settings_dict = log_metadata['settings'] + self.assertDictEqual(settings_dict['sector']['old'], old_settings['sector']) + self.assertDictEqual( + settings_dict['sector']['new'], + {'label': 'Health', 'value': 'Health'}, + ) + self.assertEqual( + settings_dict['operational_purpose']['old'], + old_settings['operational_purpose'], + ) + self.assertEqual( + settings_dict['operational_purpose']['new'], + 'New operational purpose', + ) + self.assertEqual( + settings_dict['collects_pii']['old'], old_settings['collects_pii'] + ) + self.assertEqual(settings_dict['collects_pii']['new'], True) + self.assertEqual( + settings_dict['description']['old'], old_settings['description'] + ) + self.assertEqual(settings_dict['description']['new'], 'New description') + # check list settings store added and removed fields + self.assertListEqual( + settings_dict['country']['added'], + [{'label': 'Albania', 'value': 'ALB'}], + ) + self.assertListEqual( + settings_dict['country']['removed'], + [{'label': 'United States', 'value': 'USA'}], + ) + + self._base_endpoint_test( + patch=True, + url_name=self.detail_url, + request_data=patch_data, + verify_additional_metadata=verify_metadata, + expected_action=AuditAction.UPDATE_SETTINGS, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self.assertEqual(log.object_id, self.asset.id) - log_settings_metadata = log.metadata['settings'] - # check non-list settings just store old and new information - self.assertDictEqual(log_settings_metadata['sector']['old'], old_settings['sector']) - self.assertDictEqual(log_settings_metadata['sector']['new'], {'label': 'Health', 'value': 'Health'}) - self.assertEqual(log_settings_metadata['operational_purpose']['old'], old_settings['operational_purpose']) - self.assertEqual(log_settings_metadata['operational_purpose']['new'], 'New operational purpose') - self.assertEqual(log_settings_metadata['collects_pii']['old'], old_settings['collects_pii']) - self.assertEqual(log_settings_metadata['collects_pii']['new'], True) - self.assertEqual(log_settings_metadata['description']['old'], old_settings['description']) - self.assertEqual(log_settings_metadata['description']['new'], 'New description') - # check list settings store added and removed fields - self.assertListEqual(log_settings_metadata['country']['added'], [{'label': 'Albania', 'value': 'ALB'}]) - self.assertListEqual(log_settings_metadata['country']['removed'], [{'label': 'United States', 'value': 'USA'}]) def test_unchanged_settings_not_recorded_on_log(self): - patch_data = {'settings': { - 'sector': self.asset.settings['sector'], - 'country': self.asset.settings['country'], - 'operational_purpose': self.asset.settings['operational_purpose'], - 'collects_pii': self.asset.settings['collects_pii'], - 'description': 'New description', - }} - self.client.patch( - reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), - data = patch_data, - format='json', + patch_data = { + 'settings': { + 'sector': self.asset.settings['sector'], + 'country': self.asset.settings['country'], + 'operational_purpose': self.asset.settings['operational_purpose'], + 'collects_pii': self.asset.settings['collects_pii'], + 'description': 'New description', + } + } + + def verify_metadata(log_metadata): + self.assertNotIn('sector', log_metadata.keys()) + self.assertNotIn('country', log_metadata.keys()) + self.assertNotIn('operational_purpose', log_metadata.keys()) + self.assertNotIn('collects_pii', log_metadata.keys()) + + self._base_endpoint_test( + patch=True, + url_name=self.detail_url, + request_data=patch_data, + verify_additional_metadata=verify_metadata, + expected_action=AuditAction.UPDATE_SETTINGS, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - log_settings_metadata = log.metadata['settings'] - self.assertNotIn('sector', log_settings_metadata.keys()) - self.assertNotIn('country', log_settings_metadata.keys()) - self.assertNotIn('operational_purpose', log_settings_metadata.keys()) - self.assertNotIn('collects_pii', log_settings_metadata.keys()) def test_first_time_deployment_creates_log(self): post_data = { 'active': True, 'backend': 'mock', } - self.client.post( - reverse('api_v2:asset-deployment', kwargs={'uid': self.asset.uid}), - data = post_data, - format='json', + + def verify_metadata(log_metadata): + self.assertEqual( + log_metadata['latest_deployed_version_uid'], + self.asset.latest_version.uid, + ) + + self._base_endpoint_test( + patch=False, + url_name=self.deployment_url, + request_data=post_data, + expected_action=AuditAction.DEPLOY, + verify_additional_metadata=verify_metadata, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self.assertEqual(log.object_id, self.asset.id) - self.assertEqual(log.metadata['latest_deployed_version_uid'], self.asset.latest_version.uid) - self.assertEqual(log.action, AuditAction.DEPLOY) def test_redeployment_creates_log(self): + # create an initial deployment self.asset.deploy(backend='mock', active=True) - patch_data = { - 'active': True, - 'version_id': self.asset.version_id - } - self.client.patch( - reverse('api_v2:asset-deployment', kwargs={'uid': self.asset.uid}), - data = patch_data, - format='json', + patch_data = {'active': True, 'version_id': self.asset.version_id} + + def verify_metadata(log_metadata): + self.assertEqual( + log_metadata['latest_deployed_version_uid'], + self.asset.latest_version.uid, + ) + + self._base_endpoint_test( + patch=True, + url_name=self.deployment_url, + request_data=patch_data, + expected_action=AuditAction.REDEPLOY, + verify_additional_metadata=verify_metadata, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self.assertEqual(log.object_id, self.asset.id) - self.assertEqual(log.metadata['latest_deployed_version_uid'], self.asset.latest_version.uid) - self.assertEqual(log.action, AuditAction.REDEPLOY) def test_change_project_content_creates_log(self): - new_content = { - 'survey': [ - {'type': 'text', 'label': 'new question', 'name': 'new_question', 'kuid': 'hijk'} - ] + request_data = { + 'content': { + 'survey': [ + { + 'type': 'text', + 'label': 'new question', + 'name': 'new_question', + 'kuid': 'hijk', + } + ] + } } - self.client.patch( - reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), - data={'content': new_content}, - format='json', + self._base_endpoint_test( + patch=True, + url_name=self.detail_url, + request_data=request_data, + expected_action=AuditAction.UPDATE_FORM, + verify_additional_metadata=lambda x: None, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self._check_common_metadata(log.metadata, self.asset) - self.assertEqual(log.action, AuditAction.UPDATE_FORM) def test_enable_sharing_creates_log(self): - patch_data = { - 'data_sharing': { - 'enabled': True, - 'fields': ['q1'] - } - } - self.client.patch( - reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), - data=patch_data, - format='json', + request_data = {'data_sharing': {'enabled': True, 'fields': ['q1']}} + + def verify_metadata(log_metadata): + self.assertListEqual(log_metadata['shared_fields']['added'], ['q1']) + + self._base_endpoint_test( + patch=True, + url_name=self.detail_url, + request_data=request_data, + expected_action=AuditAction.ENABLE_SHARING, + verify_additional_metadata=verify_metadata, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self._check_common_metadata(log.metadata, self.asset) - self.assertEqual(log.action, AuditAction.ENABLE_SHARING) - self.assertListEqual(log.metadata['shared_fields']['added'], ['q1']) def test_disable_sharing_creates_log(self): # turn sharing on - self.asset.data_sharing = { - 'enabled': True, - 'fields': ['q1', 'q2'] - } + self.asset.data_sharing = {'enabled': True, 'fields': ['q1', 'q2']} self.asset.save() - patch = { - 'data_sharing': { - 'enabled': False, - 'fields': [] - } - } - self.client.patch( - reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), - data=patch, - format='json', + request_data = {'data_sharing': {'enabled': False, 'fields': []}} + self._base_endpoint_test( + patch=True, + url_name=self.detail_url, + request_data=request_data, + expected_action=AuditAction.DISABLE_SHARING, + verify_additional_metadata=lambda x: None, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self._check_common_metadata(log.metadata, self.asset) - self.assertEqual(log.action, AuditAction.DISABLE_SHARING) def test_modify_shared_fields_creates_log(self): - self.asset.content = { 'survey':[ - {"type": "text", "label": "fixture q1", "name": "q1", "kuid": "abc"}, - {"type": "text", "label": "fixture q2", "name": "q2", "kuid": "def"}, - {"type": "text", "label": "fixture q3", "name": "q3", "kuid": "def"} - ]} - # turn sharing on - self.asset.data_sharing = { - 'enabled': True, - 'fields': ['q1', 'q2'] + self.asset.content = { + 'survey': [ + {'type': 'text', 'label': 'fixture q1', 'name': 'q1', 'kuid': 'abc'}, + {'type': 'text', 'label': 'fixture q2', 'name': 'q2', 'kuid': 'def'}, + {'type': 'text', 'label': 'fixture q3', 'name': 'q3', 'kuid': 'def'}, + ] } + # turn sharing on + self.asset.data_sharing = {'enabled': True, 'fields': ['q1', 'q2']} self.asset.save() - post_data = { - 'data_sharing': { - 'enabled': True, - 'fields': ['q2', 'q3'] - } - } - self.client.patch( - reverse('api_v2:asset-detail', kwargs={'uid': self.asset.uid}), - data=post_data, - format='json', + request_data = {'data_sharing': {'enabled': True, 'fields': ['q2', 'q3']}} + + def verify_metadata(log_metadata): + self.assertListEqual(log_metadata['shared_fields']['added'], ['q3']) + self.assertListEqual(log_metadata['shared_fields']['removed'], ['q1']) + + self._base_endpoint_test( + patch=True, + url_name=self.detail_url, + request_data=request_data, + expected_action=AuditAction.MODIFY_SHARING, + verify_additional_metadata=verify_metadata, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self._check_common_metadata(log.metadata, self.asset) - self.assertEqual(log.action, AuditAction.MODIFY_SHARING) - self.assertListEqual(log.metadata['shared_fields']['added'], ['q3']) - self.assertListEqual(log.metadata['shared_fields']['removed'], ['q1']) def test_archive_creates_log(self): # can only archive deployed asset self.asset.deploy(backend='mock', active=True) - post_data = { + request_data = { 'active': False, } - self.client.patch( - reverse('api_v2:asset-deployment', kwargs={'uid': self.asset.uid}), - data = post_data, - format='json', + self._base_endpoint_test( + patch=True, + url_name=self.deployment_url, + request_data=request_data, + expected_action=AuditAction.ARCHIVE, + verify_additional_metadata=lambda x: None, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self.assertEqual(log.object_id, self.asset.id) - self.assertEqual(log.action, AuditAction.ARCHIVE) def test_unarchive_creates_log(self): # can only unarchive deployed asset self.asset.deploy(backend='mock', active=False) - post_data = { + request_data = { 'active': True, } - self.client.patch( - reverse('api_v2:asset-deployment', kwargs={'uid': self.asset.uid}), - data = post_data, - format='json', + self._base_endpoint_test( + patch=True, + url_name=self.deployment_url, + request_data=request_data, + expected_action=AuditAction.UNARCHIVE, + verify_additional_metadata=lambda x: None, ) - logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) - self.assertEqual(logs.count(), 1) - log = logs.first() - self.assertEqual(log.object_id, self.asset.id) - self.assertEqual(log.action, AuditAction.UNARCHIVE) - diff --git a/kobo/apps/audit_log/tests/test_signals.py b/kobo/apps/audit_log/tests/test_signals.py index 605706a0da..ed99d46080 100644 --- a/kobo/apps/audit_log/tests/test_signals.py +++ b/kobo/apps/audit_log/tests/test_signals.py @@ -7,8 +7,8 @@ from django.urls import reverse from trench.utils import get_mfa_model -from kobo.apps.audit_log.models import AuditLog from kobo.apps.audit_log.audit_actions import AuditAction +from kobo.apps.audit_log.models import AuditLog from kobo.apps.audit_log.signals import create_access_log from kobo.apps.kobo_auth.shortcuts import User from kpi.tests.base_test_case import BaseTestCase diff --git a/kobo/apps/trash_bin/tasks.py b/kobo/apps/trash_bin/tasks.py index 1af128679c..ee78df6327 100644 --- a/kobo/apps/trash_bin/tasks.py +++ b/kobo/apps/trash_bin/tasks.py @@ -6,21 +6,17 @@ from django.db import transaction from django.db.models.signals import post_delete from django.utils.timezone import now -from django_celery_beat.models import ( - ClockedSchedule, - PeriodicTask, - PeriodicTasks, -) +from django_celery_beat.models import ClockedSchedule, PeriodicTask, PeriodicTasks from requests.exceptions import HTTPError -from kobo.apps.trackers.models import NLPUsageCounter from kobo.apps.audit_log.models import AuditLog, AuditType -from ..audit_log.audit_actions import AuditAction +from kobo.apps.trackers.models import NLPUsageCounter from kobo.celery import celery_app from kpi.deployment_backends.kc_access.utils import delete_kc_user from kpi.exceptions import KobocatCommunicationError from kpi.models.asset import Asset from kpi.utils.storage import rmdir +from ..audit_log.audit_actions import AuditAction from .constants import DELETE_PROJECT_STR_PREFIX, DELETE_USER_STR_PREFIX from .exceptions import TrashTaskInProgressError from .models import TrashStatus diff --git a/kobo/apps/trash_bin/tests/test_utils.py b/kobo/apps/trash_bin/tests/test_utils.py index 3c28a54dc2..2d921ba052 100644 --- a/kobo/apps/trash_bin/tests/test_utils.py +++ b/kobo/apps/trash_bin/tests/test_utils.py @@ -1,15 +1,15 @@ # coding: utf-8 from datetime import timedelta +from unittest.mock import patch from django.contrib.auth import get_user_model from django.test import TestCase from django.utils.timezone import now from django_celery_beat.models import PeriodicTask -from mock import patch from kobo.apps.audit_log.models import AuditLog, AuditType -from ...audit_log.audit_actions import AuditAction from kpi.models import Asset +from ...audit_log.audit_actions import AuditAction from ..constants import DELETE_PROJECT_STR_PREFIX, DELETE_USER_STR_PREFIX from ..models.account import AccountTrash from ..models.project import ProjectTrash diff --git a/kobo/apps/trash_bin/utils.py b/kobo/apps/trash_bin/utils.py index 53324ac9c2..ff5a9f97f7 100644 --- a/kobo/apps/trash_bin/utils.py +++ b/kobo/apps/trash_bin/utils.py @@ -13,11 +13,11 @@ from django_celery_beat.models import ClockedSchedule, PeriodicTask, PeriodicTasks from kobo.apps.audit_log.models import AuditLog, AuditType -from ..audit_log.audit_actions import AuditAction from kpi.exceptions import InvalidXFormException, MissingXFormException from kpi.models import Asset, ExportTask, ImportTask from kpi.utils.mongo_helper import MongoHelper from kpi.utils.storage import rmdir +from ..audit_log.audit_actions import AuditAction from .constants import DELETE_PROJECT_STR_PREFIX, DELETE_USER_STR_PREFIX from .exceptions import ( TrashIntegrityError, diff --git a/kpi/models/asset.py b/kpi/models/asset.py index c7211cb844..082f072ea6 100644 --- a/kpi/models/asset.py +++ b/kpi/models/asset.py @@ -12,12 +12,12 @@ from django.db.models import F, Prefetch, Q from django.utils.translation import gettext_lazy as t from django_request_cache import cache_for_request -from taggit.managers import TaggableManager, _TaggableManager -from taggit.utils import require_instance_manager - from formpack.utils.flatten_content import flatten_content from formpack.utils.json_hash import json_hash from formpack.utils.kobo_locking import strip_kobo_locking_profile +from taggit.managers import TaggableManager, _TaggableManager +from taggit.utils import require_instance_manager + from kobo.apps.reports.constants import DEFAULT_REPORTS_KEY, SPECIFIC_REPORTS_KEY from kobo.apps.subsequences.advanced_features_params_schema import ( ADVANCED_FEATURES_PARAMS_SCHEMA, diff --git a/kpi/tests/api/v2/test_api_assets.py b/kpi/tests/api/v2/test_api_assets.py index c1cdb8e615..e7d0f65a65 100644 --- a/kpi/tests/api/v2/test_api_assets.py +++ b/kpi/tests/api/v2/test_api_assets.py @@ -4,13 +4,12 @@ import json import os from io import StringIO +from unittest.mock import patch import dateutil.parser from django.urls import reverse from django.utils import timezone from rest_framework import status -from unittest.mock import patch - from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.project_views.models.project_view import ProjectView @@ -1748,7 +1747,6 @@ def test_asset_deployment(self): == AssetDeploymentStatus.DEPLOYED.value ) - @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') def test_asset_redeployment(self, patched_create_log): self.test_asset_deployment() diff --git a/kpi/views/v2/asset.py b/kpi/views/v2/asset.py index 7e648cef02..c98c357a01 100644 --- a/kpi/views/v2/asset.py +++ b/kpi/views/v2/asset.py @@ -7,13 +7,13 @@ from django.db.models import Count from django.http import Http404 from django.shortcuts import get_object_or_404 -from rest_framework import exceptions, renderers, status, viewsets +from rest_framework import exceptions, renderers, status from rest_framework.decorators import action from rest_framework.response import Response from rest_framework_extensions.mixins import NestedViewSetMixin -from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet, get_nested_field -from kobo.apps.audit_log.models import ProjectHistoryLog, AuditType +from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet +from kobo.apps.audit_log.models import AuditType from kpi.constants import ( ASSET_TYPE_ARG_NAME, ASSET_TYPE_SURVEY, @@ -46,7 +46,6 @@ from kpi.utils.bugfix import repair_file_column_content_and_save from kpi.utils.hash import calculate_hash from kpi.utils.kobo_to_xlsform import to_xlsform_structure -from kpi.utils.log import logging from kpi.utils.object_permission import get_database_user, get_objects_for_user from kpi.utils.ss_structure_to_mdtable import ss_structure_to_mdtable @@ -381,8 +380,17 @@ class AssetViewSet( 'uid__icontains', ] - logged_fields = ['content', 'settings', 'data_sharing', 'latest_version.uid', - 'has_deployment', 'name', 'advanced_features.qual', 'id', 'latest_deployed_version_uid'] + logged_fields = [ + 'content', + 'settings', + 'data_sharing', + 'latest_version.uid', + 'has_deployment', + 'name', + 'advanced_features.qual', + 'id', + 'latest_deployed_version_uid', + ] log_type = AuditType.PROJECT_HISTORY def get_object_override(self): @@ -486,7 +494,8 @@ def deployment(self, request, uid): request._request.additional_audit_log_info = { 'request_data': request._data, 'id': asset.id, - 'latest_deployed_version_uid': asset.latest_deployed_version_uid + 'latest_deployed_version_uid': asset.latest_deployed_version_uid, + 'latest_version_uid': asset.latest_version.uid, } # TODO: Understand why this 404s when `serializer.data` is not # coerced to a dict @@ -512,11 +521,14 @@ def deployment(self, request, uid): ) serializer.is_valid(raise_exception=True) serializer.save() - only_active_changed = set(serializer.validated_data.keys()) == set(('active',)) + only_active_changed = set(serializer.validated_data.keys()) == { + 'active' + } request._request.additional_audit_log_info = { 'only_active_changed': only_active_changed, 'active': serializer.data['active'], - 'latest_deployed_version_uid': asset.latest_deployed_version_uid + 'latest_deployed_version_uid': asset.latest_deployed_version_uid, + 'latest_version_uid': asset.latest_version.uid, } # TODO: Understand why this 404s when `serializer.data` is not # coerced to a dict diff --git a/kpi/views/v2/data.py b/kpi/views/v2/data.py index 28342e2408..19d60b35b2 100644 --- a/kpi/views/v2/data.py +++ b/kpi/views/v2/data.py @@ -16,8 +16,8 @@ from rest_framework.reverse import reverse from rest_framework_extensions.mixins import NestedViewSetMixin -from kobo.apps.audit_log.models import AuditLog, AuditType from kobo.apps.audit_log.audit_actions import AuditAction +from kobo.apps.audit_log.models import AuditLog, AuditType from kobo.apps.openrosa.libs.utils.logger_tools import http_open_rosa_error_handler from kpi.authentication import EnketoSessionAuthentication from kpi.constants import ( From a306ad24237d945c71df0e567a6e62e68839e13b Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 21 Oct 2024 17:09:00 -0400 Subject: [PATCH 07/10] fixup!: comments --- kobo/apps/audit_log/tests/test_project_history_logs.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/kobo/apps/audit_log/tests/test_project_history_logs.py b/kobo/apps/audit_log/tests/test_project_history_logs.py index afffaf2c32..76eb53aea2 100644 --- a/kobo/apps/audit_log/tests/test_project_history_logs.py +++ b/kobo/apps/audit_log/tests/test_project_history_logs.py @@ -39,16 +39,19 @@ def _check_common_metadata(self, metadata_dict): def _base_endpoint_test( self, patch, url_name, request_data, expected_action, verify_additional_metadata ): + # requests are either patches or posts request_method = self.client.patch if patch else self.client.post - response = request_method( + # hit the endpoint with the correct data + request_method( reverse(url_name, kwargs={'uid': self.asset.uid}), data=request_data, format='json', ) - print(response.status_code) + # make sure a log was created logs = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) self.assertEqual(logs.count(), 1) log = logs.first() + # check the log has the expected fields and metadata self._check_common_metadata(log.metadata) self.assertEqual(log.object_id, self.asset.id) self.assertEqual(log.action, expected_action) From 1b5e805ef6c21f9e86f7971a32f4f0a04e09944b Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 22 Oct 2024 08:28:56 -0400 Subject: [PATCH 08/10] fixup!: new uid --- kpi/fixtures/test_data.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kpi/fixtures/test_data.json b/kpi/fixtures/test_data.json index 30486a02df..839d0ec209 100644 --- a/kpi/fixtures/test_data.json +++ b/kpi/fixtures/test_data.json @@ -183,7 +183,7 @@ "date_created": "2022-04-05T21:00:22.402Z", "date_modified": "2022-04-05T21:00:33.946Z", "owner": 1, - "uid": "aVq7yMt4zKpUtTeZUnoeJF" + "uid": "aNp9yMt4zKpUtTeZUnozYG" } }, { From 8465fce99032acd91853ad3d67fa4d3b94411ddc Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 22 Oct 2024 12:45:52 -0400 Subject: [PATCH 09/10] fixup!: fix some tests --- kobo/apps/audit_log/base_views.py | 24 ++++--- kobo/apps/audit_log/models.py | 10 +-- .../tests/test_project_history_logs.py | 2 +- kpi/fixtures/asset_with_settings_and_qa.json | 66 +++++++++++++++++++ kpi/fixtures/test_data.json | 41 ------------ kpi/tests/api/v2/test_api_assets.py | 43 ++---------- 6 files changed, 90 insertions(+), 96 deletions(-) create mode 100644 kpi/fixtures/asset_with_settings_and_qa.json diff --git a/kobo/apps/audit_log/base_views.py b/kobo/apps/audit_log/base_views.py index a39088ff6f..f0772bb5bc 100644 --- a/kobo/apps/audit_log/base_views.py +++ b/kobo/apps/audit_log/base_views.py @@ -1,21 +1,25 @@ from rest_framework import mixins, viewsets - +from kpi.utils.log import logging def get_nested_field(obj, field: str): """ Retrieve a period-separated nested field from an object or dict - Raises an exception if the field is not found + Logs a warning and returns None if the field is not found """ split = field.split('.') - attribute = getattr(obj, split[0]) - if len(split) > 1: - for inner_field in split[1:]: - if isinstance(attribute, dict): - attribute = attribute.get(inner_field) - else: - attribute = getattr(attribute, inner_field) - return attribute + try: + attribute = getattr(obj, split[0]) + if len(split) > 1: + for inner_field in split[1:]: + if isinstance(attribute, dict): + attribute = attribute.get(inner_field) + else: + attribute = getattr(attribute, inner_field) + return attribute + except (AttributeError, KeyError): + logging.warning(f'Attribute not found: {field} on object {obj}') + return None class AuditLoggedViewSet(viewsets.GenericViewSet): diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index f8416d8293..c148f0123a 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -416,9 +416,9 @@ def name_change(old_field, new_field): def settings_change(old_field, new_field): settings = {} for setting_name in PROJECT_METADATA_DEFAULT_LABELS.keys(): - if old_field[setting_name] != new_field[setting_name]: - old = old_field[setting_name] - new = new_field[setting_name] + old = old_field.get(setting_name, None) + new = new_field.get(setting_name, None) + if old != new: metadata_field_subdict = {} if isinstance(old, list) and isinstance(new, list): removed_values = [val for val in old if val not in new] @@ -445,8 +445,8 @@ def qa_change(_, new_field): @staticmethod def sharing_change(old_fields, new_fields): - old_enabled = old_fields['enabled'] - old_shared_fields = old_fields['fields'] + old_enabled = old_fields.get('enabled', False) + old_shared_fields = old_fields.get('fields', []) new_enabled = new_fields['enabled'] new_shared_fields = new_fields['fields'] shared_fields_dict = {} diff --git a/kobo/apps/audit_log/tests/test_project_history_logs.py b/kobo/apps/audit_log/tests/test_project_history_logs.py index 76eb53aea2..4c246a4bed 100644 --- a/kobo/apps/audit_log/tests/test_project_history_logs.py +++ b/kobo/apps/audit_log/tests/test_project_history_logs.py @@ -13,7 +13,7 @@ @override_settings(DEFAULT_DEPLOYMENT_BACKEND='mock') class TestProjectHistoryLogs(BaseAuditLogTestCase): - fixtures = ['test_data'] + fixtures = ['test_data', 'asset_with_settings_and_qa'] def setUp(self): super().setUp() diff --git a/kpi/fixtures/asset_with_settings_and_qa.json b/kpi/fixtures/asset_with_settings_and_qa.json new file mode 100644 index 0000000000..7e2ce3e7e0 --- /dev/null +++ b/kpi/fixtures/asset_with_settings_and_qa.json @@ -0,0 +1,66 @@ +[ + { + "model":"kpi.asset", + "pk":3, + "parent":3, + "fields":{ + "name":"fixture asset with settings and qa", + "content":{ + "survey":[ + { + "type":"text", + "label":"fixture q1", + "name":"q1", + "kuid":"abc" + }, + { + "type":"text", + "label":"fixture q2", + "name":"q2", + "kuid":"def" + } + ] + }, + "settings":{ + "description":"Description", + "country":[ + { + "label":"United States", + "value":"USA" + } + ], + "sector":{ + "label":"Education", + "value":"Education" + }, + "operational_purpose":"Purpose", + "collects_pii":false + }, + "data_sharing":{ + "fields":[ + + ], + "enabled":false + }, + "advanced_features":{ + "qual":{ + "qual_survey":[ + { + "type":"qual_note", + "uuid":"12345", + "scope":"by_question#survey", + "xpath":"q1", + "labels":{ + "_default":"QA Question" + } + } + ] + } + }, + "date_created":"2022-04-05T21:00:22.402Z", + "date_modified":"2022-04-05T21:00:33.946Z", + "owner":1, + "uid":"aNp9yMt4zKpUtTeZUnozYG" + } + } +] diff --git a/kpi/fixtures/test_data.json b/kpi/fixtures/test_data.json index 839d0ec209..2c6fee0074 100644 --- a/kpi/fixtures/test_data.json +++ b/kpi/fixtures/test_data.json @@ -145,47 +145,6 @@ "uid": "aPEANgNgTH3xzhJGcctURu" } }, - { - "model": "kpi.asset", - "pk": 3, - "parent": 3, - "fields": { - "name": "Asset with settings and qa", - "content": {"survey": [ - {"type": "text", "label": "fixture q1", "name": "q1", "kuid": "abc"}, - {"type": "text", "label": "fixture q2", "name": "q2", "kuid": "def"} - ]}, - "settings": { - "description": "Description", - "country": [{"label": "United States", "value": "USA"}], - "sector": {"label": "Education", "value": "Education"}, - "operational_purpose": "Purpose", - "collects_pii": false - }, - "data_sharing": { - "fields": [], - "enabled": false - }, - "advanced_features": { - "qual": { - "qual_survey": [ - { - "type": "qual_note", - "uuid": "12345", - "scope": "by_question#survey", - "xpath": "q1", - "labels": {"_default": "QA Question"} - } - ] - } - - }, - "date_created": "2022-04-05T21:00:22.402Z", - "date_modified": "2022-04-05T21:00:33.946Z", - "owner": 1, - "uid": "aNp9yMt4zKpUtTeZUnozYG" - } - }, { "model": "hub.sitewidemessage", "pk": 1, diff --git a/kpi/tests/api/v2/test_api_assets.py b/kpi/tests/api/v2/test_api_assets.py index e7d0f65a65..8be8f3796a 100644 --- a/kpi/tests/api/v2/test_api_assets.py +++ b/kpi/tests/api/v2/test_api_assets.py @@ -759,11 +759,10 @@ def setUp(self): ) def test_asset_version(self): - self.assertEqual(Asset.objects.count(), 2) - self.assertEqual(AssetVersion.objects.count(), 1) + version_count = AssetVersion.objects.count() resp = self.client.get(self.version_list_url, format='json') self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertEqual(resp.data['count'], 1) + self.assertEqual(resp.data['count'], version_count) _version_detail_url = resp.data['results'][0].get('url') resp2 = self.client.get(_version_detail_url, format='json') self.assertTrue('survey' in resp2.data['content']) @@ -1747,8 +1746,7 @@ def test_asset_deployment(self): == AssetDeploymentStatus.DEPLOYED.value ) - @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') - def test_asset_redeployment(self, patched_create_log): + def test_asset_redeployment(self): self.test_asset_deployment() # Update asset to redeploy it @@ -1774,8 +1772,6 @@ def test_asset_redeployment(self, patched_create_log): }) self.assertEqual(redeploy_response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) - # no log should be created for failed request - patched_create_log.assert_not_called() # ... but we can with `PATCH` redeploy_response = self.client.patch(deployment_url, { @@ -1788,9 +1784,6 @@ def test_asset_redeployment(self, patched_create_log): # check project history log request = redeploy_response.renderer_context['request'] - patched_create_log.assert_called_once_with( - request, self.asset, only_active_changed=False - ) # Validate version id self.asset.refresh_from_db() @@ -1877,8 +1870,7 @@ def test_asset_deployment_dates(self): ' modification date of that version' ) - @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') - def test_archive_asset(self, patched_create_log): + def test_archive_asset(self): self.test_asset_deployment() deployment_url = reverse(self._get_endpoint('asset-deployment'), @@ -1895,9 +1887,6 @@ def test_archive_asset(self, patched_create_log): == AssetDeploymentStatus.ARCHIVED.value ) request = response1.renderer_context['request'] - patched_create_log.assert_called_once_with( - request, self.asset, only_active_changed=True - ) response2 = self.client.get(self.asset_url, format='json') self.assertEqual(response2.data['deployment__active'], False) @@ -1933,30 +1922,6 @@ def test_archive_asset_does_not_modify_date_deployed(self): self.asset.refresh_from_db() assert self.asset.date_deployed == original_date_deployed - @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') - def test_unarchive_asset_creates_log(self, patched_create_log): - # manually deploy as archived - self.asset.deploy(backend='mock', active=False) - - # unarchive - deployment_url = reverse( - self._get_endpoint('asset-deployment'), kwargs={'uid': self.asset_uid} - ) - - response = self.client.patch( - deployment_url, - { - 'backend': 'mock', - 'active': True, - }, - ) - assert response.status_code == status.HTTP_200_OK - self.asset.refresh_from_db() - request = response.renderer_context['request'] - patched_create_log.assert_called_once_with( - request, self.asset, only_active_changed=True - ) - class TestCreatedByAndLastModifiedByAsset(BaseAssetTestCase): fixtures = ['test_data'] From 4a490cc6c7cd79cc4306f19d4e2c25d53e994087 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 23 Oct 2024 09:45:03 -0400 Subject: [PATCH 10/10] fixup!: migration --- .../migrations/0012_alter_auditlog_action.py | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 kobo/apps/audit_log/migrations/0012_alter_auditlog_action.py diff --git a/kobo/apps/audit_log/migrations/0012_alter_auditlog_action.py b/kobo/apps/audit_log/migrations/0012_alter_auditlog_action.py new file mode 100644 index 0000000000..67f9bd4ad0 --- /dev/null +++ b/kobo/apps/audit_log/migrations/0012_alter_auditlog_action.py @@ -0,0 +1,73 @@ +# Generated by Django 4.2.15 on 2024-10-23 13:32 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + replaces = [ + ('audit_log', '0012_alter_auditlog_action'), + ('audit_log', '0013_alter_auditlog_action'), + ] + + dependencies = [ + ('audit_log', '0011_projecthistorylog'), + ] + + operations = [ + migrations.AlterField( + model_name='auditlog', + name='action', + field=models.CharField( + choices=[ + ('create', 'Create'), + ('delete', 'Delete'), + ('in-trash', 'In Trash'), + ('put-back', 'Put Back'), + ('remove', 'Remove'), + ('update', 'Update'), + ('auth', 'Auth'), + ('deploy', 'Deploy'), + ('archive', 'Archive'), + ('unarchive', 'Unarchive'), + ('redeploy', 'Redeploy'), + ('qa-update', 'Qa Update'), + ('sh-disabled', 'Sharing Disabled'), + ('sh-enabled', 'Sharing Enabled'), + ('sh-modified', 'Sharing Modified'), + ], + db_index=True, + default='delete', + max_length=30, + ), + ), + migrations.AlterField( + model_name='auditlog', + name='action', + field=models.CharField( + choices=[ + ('create', 'Create'), + ('delete', 'Delete'), + ('in-trash', 'In Trash'), + ('put-back', 'Put Back'), + ('remove', 'Remove'), + ('update', 'Update'), + ('auth', 'Auth'), + ('deploy', 'Deploy'), + ('archive', 'Archive'), + ('unarchive', 'Unarchive'), + ('redeploy', 'Redeploy'), + ('update-name', 'Update Name'), + ('update-form', 'Update Form'), + ('update-settings', 'Update Settings'), + ('update-qa', 'Update Qa'), + ('disable-sharing', 'Disable Sharing'), + ('enable-sharing', 'Enable Sharing'), + ('modify-sharing', 'Modify Sharing'), + ], + db_index=True, + default='delete', + max_length=30, + ), + ), + ]