If original source is empty, treat it as empty doc

Signed-off-by: Craig Perkins <cwperx@amazon.com>
opensearch-project · Oct 22, 2024 · 431ab35 · 431ab35
1 parent 3ed22ae
commit 431ab35
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 24 deletions.
diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java
@@ -566,30 +566,33 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
         msg.addComplianceDocVersion(result.getVersion());
         msg.addComplianceOperation(result.isCreated() ? Operation.CREATE : Operation.UPDATE);
 
-        if (complianceConfig.shouldLogDiffsForWrite()
-            && originalResult != null
-            && originalResult.isExists()
-            && originalResult.internalSourceRef() != null) {
+        if (complianceConfig.shouldLogDiffsForWrite()) {
             try {
                 String originalSource = null;
                 String currentSource = null;
+                if (!(originalResult != null && originalResult.isExists() && originalResult.internalSourceRef() != null)) {
+                    // originalSource is empty
+                    originalSource = "{}";
+                }
                 if (securityIndex.equals(shardId.getIndexName())) {
-                    try (
-                        XContentParser parser = XContentHelper.createParser(
-                            NamedXContentRegistry.EMPTY,
-                            THROW_UNSUPPORTED_OPERATION,
-                            originalResult.internalSourceRef(),
-                            XContentType.JSON
-                        )
-                    ) {
-                        Object base64 = parser.map().values().iterator().next();
-                        if (base64 instanceof String) {
-                            originalSource = (new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8));
-                        } else {
-                            originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
+                    if (originalSource == null) {
+                        try (
+                            XContentParser parser = XContentHelper.createParser(
+                                NamedXContentRegistry.EMPTY,
+                                THROW_UNSUPPORTED_OPERATION,
+                                originalResult.internalSourceRef(),
+                                XContentType.JSON
+                            )
+                        ) {
+                            Object base64 = parser.map().values().iterator().next();
+                            if (base64 instanceof String) {
+                                originalSource = (new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8));
+                            } else {
+                                originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
+                            }
+                        } catch (Exception e) {
+                            log.error(e.toString());
                         }
-                    } catch (Exception e) {
-                        log.error(e.toString());
                     }
 
                     try (
@@ -615,7 +618,9 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
                     );
                     msg.addSecurityConfigWriteDiffSource(diffnode.size() == 0 ? "" : diffnode.toString(), id);
                 } else {
-                    originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
+                    if (originalSource == null) {
+                        originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
+                    }
                     currentSource = XContentHelper.convertToJson(currentIndex.source(), false, XContentType.JSON);
                     final JsonNode diffnode = JsonDiff.asJson(
                         DefaultObjectMapper.objectMapper.readTree(originalSource),
@@ -639,7 +644,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
                         XContentType.JSON
                     )
                 ) {
-                    if (auditConfigFilter.shouldLogRequestBody() || originalResult == null) {
+                    if (auditConfigFilter.shouldLogRequestBody()) {
                         Object base64 = parser.map().values().iterator().next();
                         if (base64 instanceof String) {
                             msg.addSecurityConfigContentToRequestBody(
@@ -666,7 +671,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
                 // originalResult.internalSourceRef()));
 
                 // current source, normally not null or empty
-                if (auditConfigFilter.shouldLogRequestBody() || originalResult == null) {
+                if (auditConfigFilter.shouldLogRequestBody()) {
                     msg.addTupleToRequestBody(new Tuple<MediaType, BytesReference>(XContentType.JSON, currentIndex.source()));
                 }
             }

diff --git a/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceAuditlogTest.java
@@ -487,7 +487,13 @@ public void testWriteLogDiffsEnabledAndLogRequestBodyDisabled() throws Exception
             .findFirst()
             .orElse(null);
         assertThat(complianceDocWriteMessage, notNullValue());
-        assertThat(complianceDocWriteMessage.getRequestBody(), notNullValue());
+        assertThat(
+            (String) complianceDocWriteMessage.getAsMap().get("audit_compliance_diff_content"),
+            containsString(
+                "[{\"op\":\"add\",\"path\":\"/name\",\"value\":\"Criag\"},{\"op\":\"add\",\"path\":\"/title\",\"value\":\"Software Engineer\"}]"
+            )
+        );
+        assertThat(complianceDocWriteMessage.getRequestBody(), nullValue());
 
         messages = TestAuditlogImpl.doThenWaitForMessages(() -> {
             try (Client tc = getClient()) {
@@ -496,7 +502,10 @@ public void testWriteLogDiffsEnabledAndLogRequestBodyDisabled() throws Exception
         }, 1);
 
         complianceDocWriteMessage = messages.get(0);
-        assertThat(complianceDocWriteMessage, notNullValue());
+        assertThat(
+            (String) complianceDocWriteMessage.getAsMap().get("audit_compliance_diff_content"),
+            containsString("[{\"op\":\"replace\",\"path\":\"/name\",\"value\":\"Craig\"}]")
+        );
         assertThat(complianceDocWriteMessage.getRequestBody(), nullValue());
     }
 }