Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Add test coverage for ComplianceConfig #3957

Merged
merged 1 commit into from
Jan 17, 2024
Merged

[Backport 2.x] Add test coverage for ComplianceConfig #3957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions src/main/java/org/opensearch/security/compliance/ComplianceConfig.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading