Merge pull request #657 from ColonelThirtyTwo/oplog-sanitize-fix

Fix oplog sanitize, especially with large oplogs

Author: chrismaddalena

ghostwriter/oplog/tests/test_views.py

@@ -724,7 +724,7 @@ def test_view_with_invalid_fields(self):
             **{"HTTP_X_REQUESTED_WITH": "XMLHttpRequest"},
         )
         self.assertEqual(response.status_code, 200)
-        self.assertIn("One of the fields submitted for sanitization does not exist", force_str(response.content))
+        self.assertIn("No fields selected for sanitization", force_str(response.content))
 
     def test_view_with_empty_fields(self):
         data = {
@@ -769,6 +769,6 @@ def test_field_sanitization_with_extra_field(self):
         self.assertEqual(response.status_code, 200)
         self.assertJSONEqual(force_str(response.content), data)
         self.entry.refresh_from_db()
-        self.assertEqual(self.entry.user_context, None)
+        self.assertEqual(self.entry.user_context, "")
         self.assertEqual(self.entry.command, "some")
-        self.assertEqual(self.entry.extra_fields, {"test_field": None, "test_field_2": "test value"})
+        self.assertEqual(self.entry.extra_fields, {"test_field": "", "test_field_2": "test value"})

ghostwriter/oplog/views.py

@@ -9,7 +9,6 @@
 # Django Imports
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required
-from django.core.exceptions import FieldDoesNotExist
 from django.http import HttpResponse, HttpResponseRedirect, JsonResponse
 from django.shortcuts import get_object_or_404, redirect, render
 from django.urls import reverse
@@ -36,7 +35,6 @@
 # Using __name__ resolves to ghostwriter.oplog.views
 logger = logging.getLogger(__name__)
 
-
 def escape_message(message):
     """
     Escape single quotes, double quotes, newlines and other characters
@@ -109,30 +107,6 @@ def post(self, *args, **kwargs):
 
         return JsonResponse(data)
 
-
-def parse_fields(data: dict, entry_field_specs: dict) -> tuple:
-    """
-    Parse a dictionary of fields and return a list of fields and extra fields.
-
-    **Parameters**
-
-    ``data`` (dict)
-        Dictionary of fields
-    ``entry_field_specs`` (dict)
-        Dictionary of field specifications
-    """
-    fields = [field["name"] for field in data]
-    extra_fields = []
-    # Remove any extra fields from the list of fields
-    for field_spec in entry_field_specs:
-        if field_spec["internal_name"] in fields:
-            fields.pop(fields.index(field_spec["internal_name"]))
-            extra_fields.append(field_spec["internal_name"])
-    if extra_fields:
-        fields.append("extra_fields")
-    return fields, extra_fields
-
-
 class OplogSanitize(RoleBasedAccessControlMixin, SingleObjectMixin, View):
     """
     Sanitize all :model:`oplog.OplogEntry` objects associated with an individual :model:`oplog.Oplog`.
@@ -144,6 +118,20 @@ class OplogSanitize(RoleBasedAccessControlMixin, SingleObjectMixin, View):
 
     model = Oplog
 
+    clearable_fields = {
+        "identifier",
+        "start_date",
+        "end_date",
+        "source_ip",
+        "dest_ip",
+        "tool",
+        "user_context",
+        "description",
+        "output",
+        "comments",
+        "operator_name",
+    }
+
     def test_func(self):
         return verify_user_is_privileged(self.request.user)
 
@@ -155,17 +143,32 @@ def post(self, *args, **kwargs):
         obj = self.get_object()
         data = self.request.POST.get("fields", None)
         try:
-            json_data = json.loads(data)
+            fields_json = json.loads(data)
         except (json.JSONDecodeError, TypeError):
-            json_data = None
-
-        if json_data and len(json_data) > 0:
+            fields_json = []
+
+        entry_field_specs = {spec.internal_name: spec for spec in ExtraFieldSpec.for_model(OplogEntry)}
+        fields = [
+            field["name"]
+            for field in fields_json
+            if field["name"] == "command"
+                or field["name"] == "tags"
+                or field["name"] in self.clearable_fields
+                or field["name"] in entry_field_specs
+        ]
+
+        bulk_update_fields = [
+            field["name"]
+            for field in fields_json
+            if field["name"] == "command"
+                or field["name"] == "tags"
+                or field["name"] in self.clearable_fields
+        ]
+        if any(field["name"] in entry_field_specs for field in fields_json):
+            bulk_update_fields.append("extra_fields")
+
+        if fields:
             entries = obj.entries.all()
-            entry_field_specs = ExtraFieldsSpecSerializer(
-                ExtraFieldSpec.objects.filter(target_model=OplogEntry._meta.label), many=True
-            ).data
-            fields, _ = parse_fields(json_data, entry_field_specs)
-
             logger.info(
                 "Sanitizing log entries for %s %s by request of %s", obj.__class__.__name__, obj.id, self.request.user
             )
@@ -176,29 +179,22 @@ def post(self, *args, **kwargs):
             try:
                 for entry in entries:
                     extra_fields_data = entry.extra_fields
-                    for field in json_data:
-                        for field_spec in entry_field_specs:
-                            if field_spec["internal_name"] == field["name"]:
-                                extra_fields_data[field["name"]] = None
-                            else:
-                                if field["name"] == "command":
-                                    if entry.command:
-                                        setattr(entry, field["name"], entry.command.split(" ")[0])
-                                else:
-                                    setattr(entry, field["name"], None)
+                    for field in fields:
+                        if field == "command":
+                            if entry.command:
+                                setattr(entry, field, entry.command.split(" ")[0])
+                        elif field == "tags":
+                            entry.tags.clear()
+                        elif field in self.clearable_fields:
+                            setattr(entry, field, "")
+                        elif field in entry_field_specs:
+                            extra_fields_data[field] = entry_field_specs[field].empty_value()
                     entry.extra_fields = extra_fields_data
-                try:
-                    OplogEntry.objects.bulk_update(entries, fields)
-                except FieldDoesNotExist as exception:
-                    logger.error("One of the fields submitted for sanitization does not exist: %s", exception)
-                    data = {
-                        "result": "failed",
-                        "message": "One of the fields submitted for sanitization does not exist.",
-                    }
+                OplogEntry.objects.bulk_update(entries, bulk_update_fields, batch_size=100)
             except Exception as exception:  # pragma: no cover
                 template = "An exception of type {0} occurred. Arguments:\n{1!r}"
                 log_message = template.format(type(exception).__name__, exception.args)
-                logger.error(log_message)
+                logger.exception(log_message)
                 data = {
                     "result": "failed",
                     "message": "An error occurred while sanitizing log entries.",