From 5698a10c1f48007c00f3c62cc0a81127e897fa4d Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 19 Nov 2024 12:54:39 -0500 Subject: [PATCH] Honor log_request_body setting in compliance audit log (#4832) Signed-off-by: Craig Perkins --- .../auditlog/impl/AbstractAuditLog.java | 47 ++++++++------ .../compliance/ComplianceAuditlogTest.java | 64 +++++++++++++++++++ 2 files changed, 90 insertions(+), 21 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 index a5dd5290f6..302ac96442 100644 --- 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), @@ -628,7 +633,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index } } - if (!complianceConfig.shouldLogWriteMetadataOnly()) { + if (!complianceConfig.shouldLogWriteMetadataOnly() && !complianceConfig.shouldLogDiffsForWrite()) { if (securityIndex.equals(shardId.getIndexName())) { // current source, normally not null or empty try ( diff --git a/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceAuditlogTest.java index e86593a7b0..202b211526 100644 --- a/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceAuditlogTest.java @@ -46,6 +46,8 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.AnyOf.anyOf; import static org.hamcrest.core.IsEqual.equalTo; import static org.junit.Assert.assertThrows; @@ -443,4 +445,66 @@ public void testWriteHistory() throws Exception { }); Assert.assertTrue(TestAuditlogImpl.sb.toString().split(".*audit_compliance_diff_content.*replace.*").length == 1); } + + @Test + public void testWriteLogDiffsEnabledAndLogRequestBodyDisabled() throws Exception { + Settings additionalSettings = Settings.builder().put("plugins.security.audit.type", TestAuditlogImpl.class.getName()).build(); + + setup(additionalSettings); + + rh.sendAdminCertificate = true; + rh.keystore = "auditlog/kirk-keystore.jks"; + + // watch emp for write + AuditConfig auditConfig = new AuditConfig( + true, + AuditConfig.Filter.from(Settings.builder().put("plugins.security.audit.config.log_request_body", false).build()), + ComplianceConfig.from( + ImmutableMap.of( + "enabled", + true, + "write_watched_indices", + Collections.singletonList("emp"), + "write_log_diffs", + true, + "write_metadata_only", + false + ), + additionalSettings + ) + ); + updateAuditConfig(AuditTestUtils.createAuditPayload(auditConfig)); + + List messages = TestAuditlogImpl.doThenWaitForMessages(() -> { + try (Client tc = getClient()) { + rh.executePutRequest("emp/_doc/0?refresh", "{\"name\" : \"Criag\", \"title\" : \"Software Engineer\"}"); + } + }, 7); + + AuditMessage complianceDocWriteMessage = messages.stream() + .filter(m -> m.getCategory().equals(AuditCategory.COMPLIANCE_DOC_WRITE)) + .findFirst() + .orElse(null); + assertThat(complianceDocWriteMessage, 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()) { + rh.executePutRequest("emp/_doc/0?refresh", "{\"name\" : \"Craig\", \"title\" : \"Software Engineer\"}"); + } + }, 1); + + complianceDocWriteMessage = messages.get(0); + assertThat( + (String) complianceDocWriteMessage.getAsMap().get("audit_compliance_diff_content"), + containsString("[{\"op\":\"replace\",\"path\":\"/name\",\"value\":\"Craig\"}]") + ); + assertThat(complianceDocWriteMessage.getRequestBody(), nullValue()); + } }