refactor: Add schema validation to PH log metadata

kobo/apps/audit_log/audit_log_metadata_schemas.py

@@ -0,0 +1,22 @@
+from kpi.constants import (
+    PROJECT_HISTORY_LOG_PERMISSION_SUBTYPE,
+    PROJECT_HISTORY_LOG_PROJECT_SUBTYPE,
+)
+
+PROJECT_HISTORY_LOG_METADATA_SCHEMA = {
+    'type': 'object',
+    'additionalProperties': True,
+    'properties': {
+        'ip_address': {'type': 'string'},
+        'source': {'type': 'string'},
+        'asset_uid': {'type': 'string'},
+        'log_subtype': {
+            'type': 'string',
+            'enum': [
+                PROJECT_HISTORY_LOG_PROJECT_SUBTYPE,
+                PROJECT_HISTORY_LOG_PERMISSION_SUBTYPE,
+            ],
+        },
+    },
+    'required': ['ip_address', 'source', 'asset_uid', 'log_subtype'],
+}

kobo/apps/audit_log/models.py

@@ -1,10 +1,14 @@
+import jsonschema
 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.audit_log.audit_log_metadata_schemas import (
+    PROJECT_HISTORY_LOG_METADATA_SCHEMA,
+)
 from kobo.apps.kobo_auth.shortcuts import User
 from kobo.apps.openrosa.libs.utils.viewer_tools import (
     get_client_ip,
@@ -288,9 +292,10 @@ def create(self, **kwargs):
             'user_uid': user.extra_details.uid,
         }
         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
+            # along with the ones derived from the user
             **new_kwargs,
         )
 
@@ -301,6 +306,22 @@ class ProjectHistoryLog(AuditLog):
     class Meta:
         proxy = True
 
+    def save(
+        self,
+        force_insert=False,
+        force_update=False,
+        using=None,
+        update_fields=None,
+    ):
+        # validate the metadata has the required fields
+        jsonschema.validate(self.metadata, PROJECT_HISTORY_LOG_METADATA_SCHEMA)
+        super().save(
+            force_insert=force_insert,
+            force_update=force_update,
+            using=using,
+            update_fields=update_fields,
+        )
+
     @classmethod
     def create_from_request(cls, request):
         if request.resolver_match.url_name == 'asset-deployment':

kobo/apps/audit_log/tests/test_models.py

@@ -2,10 +2,12 @@
 from datetime import timedelta
 from unittest.mock import patch
 
+from ddt import data, ddt, unpack
 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 jsonschema.exceptions import ValidationError
 
 from kobo.apps.audit_log.audit_actions import AuditAction
 from kobo.apps.audit_log.models import (
@@ -348,6 +350,7 @@ def test_with_submissions_grouped_groups_submissions(self):
         )
 
 
+@ddt
 class ProjectHistoryLogModelTestCase(BaseAuditLogTestCase):
 
     fixtures = ['test_data']
@@ -366,13 +369,26 @@ def test_create_project_history_log_sets_standard_fields(self):
         yesterday = timezone.now() - timedelta(days=1)
         log = ProjectHistoryLog.objects.create(
             user=user,
-            metadata={'foo': 'bar'},
+            metadata={
+                'ip_address': '1.2.3.4',
+                'source': 'source',
+                'asset_uid': asset.uid,
+                'log_subtype': 'project',
+            },
             date_created=yesterday,
             object_id=asset.id,
         )
         self._check_common_fields(log, user, asset)
-        self.assertEquals(log.date_created, yesterday)
-        self.assertDictEqual(log.metadata, {'foo': 'bar'})
+        self.assertEqual(log.date_created, yesterday)
+        self.assertDictEqual(
+            log.metadata,
+            {
+                'ip_address': '1.2.3.4',
+                'source': 'source',
+                'asset_uid': asset.uid,
+                'log_subtype': 'project',
+            },
+        )
 
     @patch('kobo.apps.audit_log.models.logging.warning')
     def test_create_project_history_log_ignores_attempt_to_override_standard_fields(
@@ -385,9 +401,45 @@ def test_create_project_history_log_ignores_attempt_to_override_standard_fields(
             model_name='foo',
             app_label='bar',
             object_id=asset.id,
+            metadata={
+                'ip_address': '1.2.3.4',
+                'source': 'source',
+                'asset_uid': asset.uid,
+                'log_subtype': 'project',
+            },
             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)
+
+    @data(
+        # source, asset_uid, ip_address, subtype
+        ('source', 'a1234', None, 'project'),  # missing ip
+        ('source', None, '1.2.3.4', 'project'),  # missing asset_uid
+        (None, 'a1234', '1.2.3.4', 'project'),  # missing source
+        ('source', 'a1234', '1.2.3.4', None),  # missing subtype
+        ('source', 'a1234', '1.2.3.4', 'bad_type'),  # bad subtype
+    )
+    @unpack
+    def test_create_project_history_log_requires_metadata_fields(
+        self, source, ip_address, asset_uid, subtype
+    ):
+        user = User.objects.get(username='someuser')
+        asset = Asset.objects.get(pk=1)
+        metadata = {
+            'source': source,
+            'ip_address': ip_address,
+            'asset_uid': asset_uid,
+            'log_subtype': subtype,
+        }
+        # remove whatever we set to None
+        # filtered = { k:v for k,v in metadata.items() if v is not None }
+
+        with self.assertRaises(ValidationError):
+            ProjectHistoryLog.objects.create(
+                object_id=asset.id,
+                metadata=metadata,
+                user=user,
+            )

kpi/constants.py

@@ -143,3 +143,4 @@
 ACCESS_LOG_AUTHORIZED_APP_TYPE = 'authorized-application'
 
 PROJECT_HISTORY_LOG_PROJECT_SUBTYPE = 'project'
+PROJECT_HISTORY_LOG_PERMISSION_SUBTYPE = 'permission'