Add test coverage for ComplianceConfig (opensearch-project#3952)

Signed-off-by: Peter Nied <[email protected]>

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

@@ -30,8 +30,10 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
+import java.util.function.Supplier;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.cache.CacheBuilder;
@@ -73,11 +75,11 @@
 @JsonInclude(JsonInclude.Include.NON_NULL)
 public class ComplianceConfig {
 
+ public static Set<String> FIELDS = DefaultObjectMapper.getFields(ComplianceConfig.class);
  private static final Logger log = LogManager.getLogger(ComplianceConfig.class);
  public static final ComplianceConfig DEFAULT = ComplianceConfig.from(Settings.EMPTY);
  private static final int CACHE_SIZE = 1000;
  private static final String INTERNAL_OPENSEARCH = "internal_opensearch";
- public static Set<String> FIELDS = DefaultObjectMapper.getFields(ComplianceConfig.class);
 
  private final boolean logExternalConfig;
  private final boolean logInternalConfig;
@@ -104,6 +106,7 @@ public class ComplianceConfig {
  private final DateTimeFormatter auditLogPattern;
  private final String auditLogIndex;
  private final boolean enabled;
+ private final Supplier<DateTime> dateProvider;
 
  private ComplianceConfig(
  final boolean enabled,
@@ -118,7 +121,8 @@ private ComplianceConfig(
  final Set<String> ignoredComplianceUsersForWrite,
  final String securityIndex,
  final String destinationType,
- final String destinationIndex
+ final String destinationIndex,
+ final Supplier<DateTime> dateProvider
  ) {
  this.enabled = enabled;
  this.logExternalConfig = logExternalConfig;
@@ -148,6 +152,11 @@ private ComplianceConfig(
  try {
  auditLogPattern = DateTimeFormat.forPattern(destinationIndex); // throws IllegalArgumentException if no pattern
  } catch (IllegalArgumentException e) {
+ log.warn(
+ "Unable to translate {} as a DateTimeFormat, will instead treat this as a static audit log index name. Error: {}",
+ destinationIndex,
+ e.getMessage()
+ );
  // no pattern
  auditLogIndex = destinationIndex;
  } catch (Exception e) {
@@ -163,6 +172,8 @@ public WildcardMatcher load(String index) throws Exception {
  return WildcardMatcher.from(getFieldsForIndex(index));
  }
  });
+
+ this.dateProvider = Optional.ofNullable(dateProvider).orElse(() -> DateTime.now(DateTimeZone.UTC));
  }
 
  @VisibleForTesting
@@ -177,6 +188,7 @@ public ComplianceConfig(
  final boolean logDiffsForWrite,
  final List<String> watchedWriteIndicesPatterns,
  final Set<String> ignoredComplianceUsersForWrite,
+ final Supplier<DateTime> dateProvider,
  Settings settings
  ) {
  this(
@@ -195,7 +207,8 @@ public ComplianceConfig(
  settings.get(
  ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX,
  "'security-auditlog-'YYYY.MM.dd"
- )
+ ),
+ dateProvider
  );
  }
 
@@ -253,6 +266,7 @@ public static ComplianceConfig from(Map<String, Object> properties, @JacksonInje
  logDiffsForWrite,
  watchedWriteIndicesPatterns,
  ignoredComplianceUsersForWrite,
+ null,
  settings
  );
  }
@@ -263,6 +277,16 @@ public static ComplianceConfig from(Map<String, Object> properties, @JacksonInje
  * @return compliance configuration
  */
  public static ComplianceConfig from(Settings settings) {
+ return ComplianceConfig.from(settings, null);
+ }
+
+ /**
+ * Create compliance configuration from Settings defined in opensearch.yml
+ * @param settings settings
+ * @param dateProvider how the current date/time is evalated for audit logs that rollover
+ * @return compliance configuration
+ */
+ public static ComplianceConfig from(Settings settings, Supplier<DateTime> dateProvider) {
  final boolean logExternalConfig = settings.getAsBoolean(
  ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_EXTERNAL_CONFIG_ENABLED,
  false
@@ -326,6 +350,7 @@ public static ComplianceConfig from(Settings settings) {
  logDiffsForWrite,
  watchedWriteIndices,
  ignoredComplianceUsersForWrite,
+ dateProvider,
  settings
  );
  }
@@ -469,7 +494,7 @@ private String getExpandedIndexName(DateTimeFormatter indexPattern, String index
  if (indexPattern == null) {
  return index;
  }
- return indexPattern.print(DateTime.now(DateTimeZone.UTC));
+ return indexPattern.print(dateProvider.get());
  }
 
  /**
@@ -507,7 +532,7 @@ public boolean writeHistoryEnabledForIndex(String index) {
  * @return true/false
  */
  public boolean readHistoryEnabledForIndex(String index) {
- if (!this.isEnabled()) {
+ if (index == null || !this.isEnabled()) {
  return false;
  }
  // if security index (internal index) check if internal config logging is enabled
@@ -529,7 +554,7 @@ public boolean readHistoryEnabledForIndex(String index) {
  * @return true/false
  */
  public boolean readHistoryEnabledForField(String index, String field) {
- if (!this.isEnabled()) {
+ if (index == null || !this.isEnabled()) {
  return false;
  }
  // if security index (internal index) check if internal config logging is enabled

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

@@ -12,19 +12,35 @@
 package org.opensearch.security.auditlog.compliance;
 
 import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
 
 import com.google.common.collect.ImmutableSet;
+import org.apache.logging.log4j.Logger;
 import org.junit.Test;
 
 import org.opensearch.common.settings.Settings;
 import org.opensearch.security.compliance.ComplianceConfig;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.support.WildcardMatcher;
 
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.mockito.Mockito;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 public class ComplianceConfigTest {
 
@@ -136,4 +152,84 @@ public void testEmpty() {
  assertSame(WildcardMatcher.NONE, complianceConfig.getIgnoredComplianceUsersForReadMatcher());
  assertSame(WildcardMatcher.NONE, complianceConfig.getIgnoredComplianceUsersForWriteMatcher());
  }
+
+ @Test
+ public void testLogState() {
+ // arrange
+ final var logger = Mockito.mock(Logger.class);
+ final ComplianceConfig complianceConfig = ComplianceConfig.from(Settings.EMPTY);
+ // act
+ complianceConfig.log(logger);
+ // assert: don't validate content, but ensure message's logged is generally consistant
+ verify(logger, times(6)).info(anyString(), anyString());
+ verify(logger, times(1)).info(anyString(), isNull(String.class));
+ verify(logger, times(1)).info(anyString(), any(Map.class));
+ verify(logger, times(3)).info(anyString(), any(WildcardMatcher.class));
+ verifyNoMoreInteractions(logger);
+ }
+
+ @Test
+ public void testReadWriteHistoryEnabledForIndex_rollingIndex() {
+ // arrange
+ final var date = new AtomicReference<DateTime>();
+ final Consumer<Integer> setYear = (year) -> { date.set(new DateTime(year, 1, 1, 1, 2, DateTimeZone.UTC)); };
+ final ComplianceConfig complianceConfig = ComplianceConfig.from(
+ Settings.builder()
+ .put(
+ ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX,
+ "'audit-log-index'-YYYY-MM-dd"
+ )
+ .put(ConfigConstants.SECURITY_AUDIT_TYPE_DEFAULT, "internal_opensearch")
+ .putList(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_READ_WATCHED_FIELDS, "*")
+ .putList(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_WATCHED_INDICES, "*")
+ .build(),
+ date::get
+ );
+
+ // act: Don't log for null indices
+ assertThat(complianceConfig.readHistoryEnabledForIndex(null), equalTo(false));
+ assertThat(complianceConfig.writeHistoryEnabledForIndex(null), equalTo(false));
+ // act: Don't log for the security indices
+ assertThat(complianceConfig.readHistoryEnabledForIndex(complianceConfig.getSecurityIndex()), equalTo(false));
+ assertThat(complianceConfig.writeHistoryEnabledForIndex(complianceConfig.getSecurityIndex()), equalTo(false));
+
+ // act: Don't log for the current audit log
+ setYear.accept(1337);
+ assertThat(complianceConfig.readHistoryEnabledForIndex("audit-log-index-1337-01-01"), equalTo(false));
+ assertThat(complianceConfig.writeHistoryEnabledForIndex("audit-log-index-1337-01-01"), equalTo(false));
+
+ // act: Log for current audit log when it does not match the date
+ setYear.accept(2048);
+ // See https://github.com/opensearch-project/security/issues/3950
+ // assertThat(complianceConfig.readHistoryEnabledForIndex("audit-log-index-1337-01-01"), equalTo(true));
+ assertThat(complianceConfig.writeHistoryEnabledForIndex("audit-log-index-1337-01-01"), equalTo(true));
+
+ // act: Log for any matching index
+ assertThat(complianceConfig.readHistoryEnabledForIndex("my-data"), equalTo(true));
+ assertThat(complianceConfig.writeHistoryEnabledForIndex("my-data"), equalTo(true));
+ }
+
+ @Test
+ public void testReadWriteHistoryEnabledForIndex_staticIndex() {
+ // arrange
+ final ComplianceConfig complianceConfig = ComplianceConfig.from(
+ Settings.builder()
+ .put(
+ ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX,
+ "audit-log-index"
+ )
+ .put(ConfigConstants.SECURITY_AUDIT_TYPE_DEFAULT, "internal_opensearch")
+ .putList(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_READ_WATCHED_FIELDS, "*")
+ .putList(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_WATCHED_INDICES, "*")
+ .build()
+ );
+
+ // act: Don't log for the static audit log
+ assertThat(complianceConfig.readHistoryEnabledForIndex(complianceConfig.getAuditLogIndex()), equalTo(false));
+ assertThat(complianceConfig.writeHistoryEnabledForIndex(complianceConfig.getAuditLogIndex()), equalTo(false));
+
+ // act: Log for any matching index
+ assertThat(complianceConfig.readHistoryEnabledForIndex("my-data"), equalTo(true));
+ assertThat(complianceConfig.writeHistoryEnabledForIndex("my-data"), equalTo(true));
+ }
 }

src/test/java/org/opensearch/security/auditlog/config/AuditConfigSerializeTest.java

@@ -214,6 +214,7 @@ public void testSerialize() throws IOException {
  false,
  Collections.singletonList("test-write-watch-index"),
  Collections.singleton("test-user-2"),
+ null,
  Settings.EMPTY
  );
  final AuditConfig auditConfig = new AuditConfig(true, audit, compliance);