Skip to content

Commit

Permalink
Add test coverage for ComplianceConfig (opensearch-project#3952)
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Nied <petern@amazon.com>
  • Loading branch information
@peternied
peternied authored Jan 17, 2024
1 parent 6047d66 commit 09051f4
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 6 deletions.
37 changes: 31 additions & 6 deletions src/main/java/org/opensearch/security/compliance/ComplianceConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -177,6 +188,7 @@ public ComplianceConfig(
final boolean logDiffsForWrite,
final List<String> watchedWriteIndicesPatterns,
final Set<String> ignoredComplianceUsersForWrite,
final Supplier<DateTime> dateProvider,
Settings settings
) {
this(
Expand All @@ -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
);
}

Expand Down Expand Up @@ -253,6 +266,7 @@ public static ComplianceConfig from(Map<String, Object> properties, @JacksonInje
logDiffsForWrite,
watchedWriteIndicesPatterns,
ignoredComplianceUsersForWrite,
null,
settings
);
}
Expand All @@ -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
Expand Down Expand Up @@ -326,6 +350,7 @@ public static ComplianceConfig from(Settings settings) {
logDiffsForWrite,
watchedWriteIndices,
ignoredComplianceUsersForWrite,
dateProvider,
settings
);
}
Expand Down Expand Up @@ -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());
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
96 changes: 96 additions & 0 deletions src/test/java/org/opensearch/security/auditlog/compliance/ComplianceConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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));
}
}
1 change: 1 addition & 0 deletions src/test/java/org/opensearch/security/auditlog/config/AuditConfigSerializeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 09051f4

Please sign in to comment.