Fix issue computing diffs in compliance audit log when writing to security index

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -703,7 +703,7 @@ public void onIndexModule(IndexModule indexModule) {
         if (!disabled && !client && !SSLConfig.isSslOnlyMode()) {
             log.debug("Handle auditLog {} for onIndexModule() of index {}", auditLog.getClass(), indexModule.getIndex().getName());
 
-            final ComplianceIndexingOperationListener ciol = new ComplianceIndexingOperationListenerImpl(auditLog);
+            final ComplianceIndexingOperationListener ciol = new ComplianceIndexingOperationListenerImpl(auditLog, threadPool);
             indexModule.addIndexOperationListener(ciol);
 
             indexModule.setReaderWrapper(

src/main/java/org/opensearch/security/compliance/ComplianceIndexingOperationListenerImpl.java

@@ -17,6 +17,7 @@
 import org.apache.logging.log4j.Logger;
 
 import org.opensearch.OpenSearchException;
+import org.opensearch.common.util.concurrent.ThreadContext;
 import org.opensearch.core.index.shard.ShardId;
 import org.opensearch.index.IndexService;
 import org.opensearch.index.engine.Engine.Delete;
@@ -26,16 +27,21 @@
 import org.opensearch.index.get.GetResult;
 import org.opensearch.index.shard.IndexShard;
 import org.opensearch.security.auditlog.AuditLog;
+import org.opensearch.threadpool.ThreadPool;
+
+import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER;
 
 public final class ComplianceIndexingOperationListenerImpl extends ComplianceIndexingOperationListener {
 
     private static final Logger log = LogManager.getLogger(ComplianceIndexingOperationListenerImpl.class);
     private final AuditLog auditlog;
+    private final ThreadPool threadPool;
     private volatile IndexService is;
 
-    public ComplianceIndexingOperationListenerImpl(final AuditLog auditlog) {
+    public ComplianceIndexingOperationListenerImpl(final AuditLog auditlog, final ThreadPool threadPool) {
         super();
         this.auditlog = auditlog;
+        this.threadPool = threadPool;
     }
 
     @Override
@@ -90,10 +96,9 @@ public Index preIndex(final ShardId shardId, final Index index) {
             }
 
             if (shard.isReadAllowed()) {
-                try {
-
+                try (ThreadContext.StoredContext ctx = threadPool.getThreadContext().stashContext()) {
+                    threadPool.getThreadContext().putHeader(OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true");
                     final GetResult getResult = shard.getService().getForUpdate(index.id(), index.getIfSeqNo(), index.getIfPrimaryTerm());
-
                     if (getResult.isExists()) {
                         threadContext.set(new Context(getResult));
                     } else {

src/main/java/org/opensearch/security/configuration/DlsFlsFilterLeafReader.java

@@ -126,7 +126,7 @@ class DlsFlsFilterLeafReader extends SequentialStoredFieldsLeafReader {
                 this.flsFieldInfos = delegate.getFieldInfos();
             }
 
-            dge = new DlsGetEvaluator(dlsQuery, in, applyDlsHere());
+            dge = new DlsGetEvaluator(dlsQuery, in, dlsQuery != null && applyDlsHere());
         } catch (IOException e) {
             throw ExceptionsHelper.convertToOpenSearchException(e);
         }

src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java

@@ -116,7 +116,7 @@ protected DirectoryReader dlsFlsWrap(final DirectoryReader reader, boolean isAdm
             log.trace("dlsFlsWrap(); index: {}; privilegeEvaluationContext: {}", index.getName(), privilegesEvaluationContext);
         }
 
-        if (isAdmin || privilegesEvaluationContext == null) {
+        if (isAdmin || privilegesEvaluationContext == null || this.dlsFlsBaseContext.isPrivilegedConfigRequest()) {
             return new DlsFlsFilterLeafReader.DlsFlsDirectoryReader(
                 reader,
                 FieldPrivileges.FlsRule.ALLOW_ALL,

src/test/java/org/opensearch/security/auditlog/compliance/ComplianceAuditlogTest.java

@@ -290,7 +290,7 @@ public void testInternalConfig() throws Exception {
 
             HttpResponse response = rh.executeGetRequest("_search?pretty", encodeBasicHeader("admin", "admin"));
             assertThat(response.getStatusCode(), equalTo(HttpStatus.SC_OK));
-        }, 14);
+        }, 21);
 
         final List<String> documentIds = messages.stream().map(AuditMessage::getDocId).distinct().collect(Collectors.toList());
         assertThat(documentIds, equalTo(expectedDocumentsTypes));
@@ -301,7 +301,13 @@ public void testInternalConfig() throws Exception {
             assertThat(
                 "Doc " + docId + " should have a read/write config message",
                 messagesByDocId.stream().map(AuditMessage::getCategory).collect(Collectors.toList()),
-                equalTo(List.of(AuditCategory.COMPLIANCE_INTERNAL_CONFIG_WRITE, AuditCategory.COMPLIANCE_INTERNAL_CONFIG_READ))
+                equalTo(
+                    List.of(
+                        AuditCategory.COMPLIANCE_INTERNAL_CONFIG_READ,
+                        AuditCategory.COMPLIANCE_INTERNAL_CONFIG_WRITE,
+                        AuditCategory.COMPLIANCE_INTERNAL_CONFIG_READ
+                    )
+                )
             );
         });
 

src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java

@@ -237,11 +237,13 @@ public void testBCryptHashRedaction() throws Exception {
         Assert.assertFalse(AuditMessage.HASH_REGEX_PATTERN.matcher(message2.toString()).matches());
 
         // create internal user and verify no BCrypt hash is present in audit logs
-        final AuditMessage message3 = TestAuditlogImpl.doThenWaitForMessage(() -> {
+        final List<AuditMessage> messages3 = TestAuditlogImpl.doThenWaitForMessages(() -> {
             rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"some new user password\"}");
-        });
+        }, 2);
 
-        Assert.assertFalse(AuditMessage.HASH_REGEX_PATTERN.matcher(message3.toString()).matches());
+        for (AuditMessage msg : messages3) {
+            Assert.assertFalse(AuditMessage.HASH_REGEX_PATTERN.matcher(msg.toString()).matches());
+        }
     }
 
     @Test
@@ -287,78 +289,91 @@ public void testPBKDF2HashRedaction() {
         Assert.assertTrue(message2.toString().contains("__HASH__"));
 
         // create internal user and verify no PBKDF2 hash is present in audit logs
-        final AuditMessage message3 = TestAuditlogImpl.doThenWaitForMessage(() -> {
+        final List<AuditMessage> messages3 = TestAuditlogImpl.doThenWaitForMessages(() -> {
             rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"some new user password\"}");
-        });
+        }, 2);
+
+        for (AuditMessage msg : messages3) {
+            Assert.assertFalse(
+                msg.toString()
+                    .contains(
+                        "$3$1331439861760512$wBFrJJIAokWuJxlO6BQPLashXgznvR4tRmXk3aEy9SpHWrb9kFjPPLByZZzMLBNQFjhepgbngYh7RfMh8TrPLw==$vqGlzGsxqGf9TgfxhORjdoqRFB3npvBd9B0GAtBMg9mD2zBbSTohRYlOxUL7UQLma66zZdD67c4RNE9BKelabw=="
+                    )
+            );
 
-        Assert.assertFalse(
-            message3.toString()
-                .contains(
-                    "$3$1331439861760512$wBFrJJIAokWuJxlO6BQPLashXgznvR4tRmXk3aEy9SpHWrb9kFjPPLByZZzMLBNQFjhepgbngYh7RfMh8TrPLw==$vqGlzGsxqGf9TgfxhORjdoqRFB3npvBd9B0GAtBMg9mD2zBbSTohRYlOxUL7UQLma66zZdD67c4RNE9BKelabw=="
-                )
-        );
-        Assert.assertTrue(message3.toString().contains("__HASH__"));
+            Assert.assertTrue(msg.toString().contains("__HASH__"));
+        }
 
         // test with various users and different PBKDF2 hash formats to make sure they all get redacted
-        final AuditMessage message4 = TestAuditlogImpl.doThenWaitForMessage(() -> {
+        final List<AuditMessage> messages4 = TestAuditlogImpl.doThenWaitForMessages(() -> {
             rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user1", "user1"));
-        });
-
-        Assert.assertFalse(
-            message4.toString()
-                .contains(
-                    "$1$4294967296128$VmnDMbQ4wLiFUq178RKvj+EYfdb3Q26qCiDcJDoCxpYnKpyuG0JhTC2wpUkMUveV5RmBFzldKQkdqZEfE0XAgg==$9u3aMWc6VP2oGkXei7UaXA=="
-                )
-        );
-        Assert.assertTrue(message4.toString().contains("__HASH__"));
+        }, 1);
+
+        for (AuditMessage msg : messages4) {
+            Assert.assertFalse(
+                msg.toString()
+                    .contains(
+                        "$1$4294967296128$VmnDMbQ4wLiFUq178RKvj+EYfdb3Q26qCiDcJDoCxpYnKpyuG0JhTC2wpUkMUveV5RmBFzldKQkdqZEfE0XAgg==$9u3aMWc6VP2oGkXei7UaXA=="
+                    )
+            );
+            Assert.assertTrue(msg.toString().contains("__HASH__"));
+        }
 
-        final AuditMessage message5 = TestAuditlogImpl.doThenWaitForMessage(() -> {
+        final List<AuditMessage> messages5 = TestAuditlogImpl.doThenWaitForMessages(() -> {
             rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user2", "user2"));
-        });
-
-        Assert.assertFalse(
-            message5.toString()
-                .contains(
-                    "$2$214748364800224$eQgqv2RI6yo95yeVnM5sfwUCwxHo6re0w+wpx6ZqZtHQV+dzlyP6YFitjNG2mlaTkg0pR56xArQaAgapdVcBQQ==$tGHWhoc83cd5nZ7QIZKPORjW/N5jklhYhRgXpw=="
-                )
-        );
-        Assert.assertTrue(message5.toString().contains("__HASH__"));
+        }, 1);
+
+        for (AuditMessage msg : messages5) {
+            Assert.assertFalse(
+                msg.toString()
+                    .contains(
+                        "$2$214748364800224$eQgqv2RI6yo95yeVnM5sfwUCwxHo6re0w+wpx6ZqZtHQV+dzlyP6YFitjNG2mlaTkg0pR56xArQaAgapdVcBQQ==$tGHWhoc83cd5nZ7QIZKPORjW/N5jklhYhRgXpw=="
+                    )
+            );
+            Assert.assertTrue(msg.toString().contains("__HASH__"));
+        }
 
-        final AuditMessage message6 = TestAuditlogImpl.doThenWaitForMessage(() -> {
+        final List<AuditMessage> messages6 = TestAuditlogImpl.doThenWaitForMessages(() -> {
             rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user3", "user3"));
-        });
-
-        Assert.assertFalse(
-            message6.toString()
-                .contains(
-                    "$3$322122547200256$5b3wEAsMc05EZFxfncCUfZRERgvbwlBhYXd5vVR14kNJtmhXSpYMzydRZxO9096IPTkc47doH4hIdKX6LTguLg==$oQQvAtUyOC6cwdAYi5WeIM7rGUN9l3IdJ9y2RNxZCWo="
-                )
-        );
-        Assert.assertTrue(message6.toString().contains("__HASH__"));
+        }, 1);
+
+        for (AuditMessage msg : messages6) {
+            Assert.assertFalse(
+                msg.toString()
+                    .contains(
+                        "$3$322122547200256$5b3wEAsMc05EZFxfncCUfZRERgvbwlBhYXd5vVR14kNJtmhXSpYMzydRZxO9096IPTkc47doH4hIdKX6LTguLg==$oQQvAtUyOC6cwdAYi5WeIM7rGUN9l3IdJ9y2RNxZCWo="
+                    )
+            );
+            Assert.assertTrue(msg.toString().contains("__HASH__"));
+        }
 
-        final AuditMessage message7 = TestAuditlogImpl.doThenWaitForMessage(() -> {
+        final List<AuditMessage> messages7 = TestAuditlogImpl.doThenWaitForMessages(() -> {
             rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user4", "user4"));
-        });
-
-        Assert.assertFalse(
-            message7.toString()
-                .contains(
-                    "$4$429496729600384$+SNSgbZD67a1bd92iuEiHCq5pvvrCx3HrNIf5hbGIJdxgXegpWilpB6vUGvYigegAUzZqE9iIsL4pSJztUNJYw==$lTxZ7tax6dBQ0r4qPJpc8d7YuoTBiUujY9HJeAZvARXMjIgvnJwa6FeYugttOKc0"
-                )
-        );
-        Assert.assertTrue(message7.toString().contains("__HASH__"));
+        }, 1);
+
+        for (AuditMessage msg : messages7) {
+            Assert.assertFalse(
+                msg.toString()
+                    .contains(
+                        "$4$429496729600384$+SNSgbZD67a1bd92iuEiHCq5pvvrCx3HrNIf5hbGIJdxgXegpWilpB6vUGvYigegAUzZqE9iIsL4pSJztUNJYw==$lTxZ7tax6dBQ0r4qPJpc8d7YuoTBiUujY9HJeAZvARXMjIgvnJwa6FeYugttOKc0"
+                    )
+            );
+            Assert.assertTrue(msg.toString().contains("__HASH__"));
+        }
 
-        final AuditMessage message8 = TestAuditlogImpl.doThenWaitForMessage(() -> {
+        final List<AuditMessage> messages8 = TestAuditlogImpl.doThenWaitForMessages(() -> {
             rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user5", "user5"));
-        });
-
-        Assert.assertFalse(
-            message8.toString()
-                .contains(
-                    "$5$644245094400512$HQe/MOv/NAlgodNhqTmjqj5jGxBwG5xuRaxKwn7r4nlUba1kj/CYnpdFaXGvVeRxt2NLm8fbekS6NYonv358Ew==$1sDx+0tMbtGzU6jlQg/Dyt30Yxuy5RdNmP9B1EzMTxYWi8k1xg2gXLy7w1XbetEC8UD/lpyXJPlaoxXpsaADyA=="
-                )
-        );
-        Assert.assertTrue(message8.toString().contains("__HASH__"));
+        }, 1);
+
+        for (AuditMessage msg : messages8) {
+            Assert.assertFalse(
+                msg.toString()
+                    .contains(
+                        "$5$644245094400512$HQe/MOv/NAlgodNhqTmjqj5jGxBwG5xuRaxKwn7r4nlUba1kj/CYnpdFaXGvVeRxt2NLm8fbekS6NYonv358Ew==$1sDx+0tMbtGzU6jlQg/Dyt30Yxuy5RdNmP9B1EzMTxYWi8k1xg2gXLy7w1XbetEC8UD/lpyXJPlaoxXpsaADyA=="
+                    )
+            );
+            Assert.assertTrue(msg.toString().contains("__HASH__"));
+        }
 
     }
 

src/test/java/org/opensearch/security/dlic/rest/api/AllowlistApiTest.java

@@ -215,7 +215,7 @@ public void testAllowlistAuditComplianceLogging() throws Exception {
         // TESTS THAT 1 READ AND 1 WRITE HAPPENS IN testGetAndPut()
         final Map<AuditCategory, Long> expectedCategoryCounts = ImmutableMap.of(
             AuditCategory.COMPLIANCE_INTERNAL_CONFIG_READ,
-            1L,
+            2L,
             AuditCategory.COMPLIANCE_INTERNAL_CONFIG_WRITE,
             1L
         );

src/test/java/org/opensearch/security/dlic/rest/api/NodesDnApiTest.java

@@ -305,7 +305,7 @@ public void testNodesDnApiAuditComplianceLogging() throws Exception {
 
         final Map<AuditCategory, Long> expectedCategoryCounts = ImmutableMap.of(
             AuditCategory.COMPLIANCE_INTERNAL_CONFIG_READ,
-            4L,
+            8L,
             AuditCategory.COMPLIANCE_INTERNAL_CONFIG_WRITE,
             4L
         );