From 447f27a21272d34c3e8d3402ca09c89663dcebe8 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 29 Apr 2022 19:36:56 +0000 Subject: [PATCH 01/17] Add missing settings to plugin allowed list Created FilterEntries enum, as the filter keys/namespaced-keys are used in more than once spot if we add a new filter key, we need a way to know if the entries were updated in the otherside of the codebase where the defaults are used. In the settings configuration, if a new enum value is added and not included a runtime exception will fire on startup, ensuring new settings are always included. This could be better where the enum defines the default values and how the settings are extracted from json, but that would be a considerable larger scope with the inclusion of generics. Signed-off-by: Peter Nied --- .../security/OpenSearchSecurityPlugin.java | 28 ++++++++++++ .../security/auditlog/config/AuditConfig.java | 45 ++++++++++++++----- .../integration/BasicAuditlogTest.java | 1 + 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index c22a950738..bb981147a9 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -40,6 +40,7 @@ import java.security.PrivilegedAction; import java.security.Security; import java.util.*; +import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -130,6 +131,7 @@ import org.opensearch.security.action.whoami.WhoAmIAction; import org.opensearch.security.auditlog.AuditLog; import org.opensearch.security.auditlog.AuditLog.Origin; +import org.opensearch.security.auditlog.config.AuditConfig.Filter.FilterEntries; import org.opensearch.security.auditlog.AuditLogSslExceptionHandler; import org.opensearch.security.auth.BackendRegistry; import org.opensearch.security.compliance.ComplianceIndexingOperationListener; @@ -933,6 +935,7 @@ public List> getSettings() { settings.add(Setting.boolSetting(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_INDICES, true, Property.NodeScope, Property.Filtered)); settings.add(Setting.boolSetting(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_REST, true, Property.NodeScope, Property.Filtered)); settings.add(Setting.boolSetting(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true, Property.NodeScope, Property.Filtered)); + settings.add(Setting.boolSetting("plugins.security.audit.config.enable_transport", true, Property.NodeScope, Property.Filtered)); final List disabledCategories = new ArrayList(2); disabledCategories.add("AUTHENTICATED"); disabledCategories.add("GRANTED_PRIVILEGES"); @@ -945,6 +948,31 @@ public List> getSettings() { settings.add(Setting.boolSetting(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, false, Property.NodeScope, Property.Filtered)); settings.add(Setting.boolSetting(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_EXCLUDE_SENSITIVE_HEADERS, true, Property.NodeScope, Property.Filtered)); + final BiFunction> boolSettingNodeScopeFiltered = (String keyWithNamespace, Boolean value) -> Setting.boolSetting(keyWithNamespace, value, Property.NodeScope, Property.Filtered); + + Arrays.stream(FilterEntries.values()).map(filterEntry -> { + switch(filterEntry) { + case DISABLE_REST_CATEGORIES: + case DISABLE_TRANSPORT_CATEGORIES: + return Setting.listSetting(filterEntry.getKeyWithNamepace(), disabledCategories, Function.identity(), Property.NodeScope); + case IGNORE_REQUESTS: + return Setting.listSetting(filterEntry.getKeyWithNamepace(), Collections.emptyList(), Function.identity(), Property.NodeScope); + case IGNORE_USERS: + return Setting.listSetting(filterEntry.getKeyWithNamepace(), ignoredUsers, Function.identity(), Property.NodeScope); + // All boolean settings with default of true + case ENABLE_REST: + case ENABLE_TRANSPORT: + case EXCLUDE_SENSITIVE_HEADERS: + case LOG_REQUEST_BODY: + case RESOLVE_INDICES: + return boolSettingNodeScopeFiltered.apply(filterEntry.getKeyWithNamepace(), true); + case RESOLVE_BULK_REQUESTS: + return boolSettingNodeScopeFiltered.apply(filterEntry.getKeyWithNamepace(), false); + default: + throw new RuntimeException("Please add support for new FilterEntries value '" + filterEntry.name() + "'"); + } + }).forEach(settings::add); + // Security - Audit - Sink settings.add(Setting.simpleString(ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, Property.NodeScope, Property.Filtered)); diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index 2f4aa7ece2..f6ad1bfb2d 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -41,6 +41,7 @@ import java.util.stream.Collectors; import static org.opensearch.security.DefaultObjectMapper.getOrDefault; +import static org.opensearch.security.support.ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT; /** * Class represents configuration for audit logging. @@ -174,6 +175,30 @@ public static class Filter { this.disabledTransportCategories = disabledTransportCategories; } + public enum FilterEntries { + ENABLE_REST("enable_rest"), + ENABLE_TRANSPORT("enable_transport"), + RESOLVE_BULK_REQUESTS("resolve_bulk_requests"), + LOG_REQUEST_BODY("log_request_body"), + RESOLVE_INDICES("resolve_indices"), + EXCLUDE_SENSITIVE_HEADERS("exclude_sensitive_headers"), + DISABLE_REST_CATEGORIES("disabled_rest_categories"), + DISABLE_TRANSPORT_CATEGORIES("disabled_transport_categories"), + IGNORE_USERS("ignore_users"), + IGNORE_REQUESTS("ignore_requests"); + + private final String key; + private FilterEntries(final String entryKey) { + this.key = entryKey; + } + public String getKey() { + return this.key; + } + public String getKeyWithNamepace() { + return SECURITY_AUDIT_CONFIG_DEFAULT + this.key; + } + } + @JsonCreator @VisibleForTesting public static Filter from(Map properties) throws JsonProcessingException { @@ -181,16 +206,16 @@ public static Filter from(Map properties) throws JsonProcessingE throw new UnrecognizedPropertyException(null, "Unrecognized field(s) present in the input data for audit filter config", null, Filter.class, null, null); } - final boolean isRestApiAuditEnabled = getOrDefault(properties,"enable_rest", true); - final boolean isTransportAuditEnabled = getOrDefault(properties,"enable_transport", true); - final boolean resolveBulkRequests = getOrDefault(properties, "resolve_bulk_requests", false); - final boolean logRequestBody = getOrDefault(properties, "log_request_body", true); - final boolean resolveIndices = getOrDefault(properties, "resolve_indices", true); - final boolean excludeSensitiveHeaders = getOrDefault(properties, "exclude_sensitive_headers", true); - final Set disabledRestCategories = AuditCategory.parse(getOrDefault(properties,"disabled_rest_categories", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); - final Set disabledTransportCategories = AuditCategory.parse(getOrDefault(properties, "disabled_transport_categories", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); - final Set ignoredAuditUsers = ImmutableSet.copyOf(getOrDefault(properties, "ignore_users", DEFAULT_IGNORED_USERS)); - final Set ignoreAuditRequests = ImmutableSet.copyOf(getOrDefault(properties, "ignore_requests", Collections.emptyList())); + final boolean isRestApiAuditEnabled = getOrDefault(properties, FilterEntries.ENABLE_REST.getKey(), true); + final boolean isTransportAuditEnabled = getOrDefault(properties, FilterEntries.ENABLE_TRANSPORT.getKey(), true); + final boolean resolveBulkRequests = getOrDefault(properties, FilterEntries.RESOLVE_BULK_REQUESTS.getKey(), false); + final boolean logRequestBody = getOrDefault(properties, FilterEntries.LOG_REQUEST_BODY.getKey(), true); + final boolean resolveIndices = getOrDefault(properties, FilterEntries.RESOLVE_INDICES.getKey(), true); + final boolean excludeSensitiveHeaders = getOrDefault(properties, FilterEntries.EXCLUDE_SENSITIVE_HEADERS.getKey(), true); + final Set disabledRestCategories = AuditCategory.parse(getOrDefault(properties, FilterEntries.DISABLE_REST_CATEGORIES.getKey(), ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); + final Set disabledTransportCategories = AuditCategory.parse(getOrDefault(properties, FilterEntries.DISABLE_TRANSPORT_CATEGORIES.getKey(), ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); + final Set ignoredAuditUsers = ImmutableSet.copyOf(getOrDefault(properties, FilterEntries.IGNORE_USERS.getKey(), DEFAULT_IGNORED_USERS)); + final Set ignoreAuditRequests = ImmutableSet.copyOf(getOrDefault(properties, FilterEntries.IGNORE_REQUESTS.getKey(), Collections.emptyList())); return new Filter( isRestApiAuditEnabled, diff --git a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java index 3317fada98..566af2a77b 100644 --- a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java @@ -64,6 +64,7 @@ public class BasicAuditlogTest extends AbstractAuditlogiUnitTest { public void testAuditLogEnable() throws Exception { Settings additionalSettings = Settings.builder() .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) + .put("plugins.security.audit.config.enable_transport", true) .build(); setup(additionalSettings); From acbbaf3773ad58064e3f5e3d44d87750b5f880e8 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Wed, 11 May 2022 21:19:28 +0000 Subject: [PATCH 02/17] Attempt to fix test cases Signed-off-by: Peter Nied --- build.gradle | 9 +- .../security/OpenSearchSecurityPlugin.java | 11 ++- .../security/auditlog/config/AuditConfig.java | 90 ++++++++++++------- .../security/auditlog/impl/AuditCategory.java | 4 - .../security/auditlog/impl/AuditMessage.java | 8 ++ .../auditlog/AbstractAuditlogiUnitTest.java | 26 ++++-- .../integration/BasicAuditlogTest.java | 64 ++++++++----- 7 files changed, 140 insertions(+), 72 deletions(-) diff --git a/build.gradle b/build.gradle index 90ca3e49ce..a5e228313a 100644 --- a/build.gradle +++ b/build.gradle @@ -217,6 +217,11 @@ testsJar { libsDirName = '.' } +tasks.withType(JavaCompile) { + configure(options) { + options.compilerArgs << '-Xlint:none' + } +} test { maxParallelForks = 3 @@ -226,8 +231,8 @@ test { } retry { failOnPassedAfterRetry = false - maxFailures = 30 - maxRetries = 5 + // maxFailures = 30 + // maxRetries = 5 } jacoco { excludes = [ diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index bb981147a9..8eaf608113 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -935,7 +935,6 @@ public List> getSettings() { settings.add(Setting.boolSetting(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_INDICES, true, Property.NodeScope, Property.Filtered)); settings.add(Setting.boolSetting(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_REST, true, Property.NodeScope, Property.Filtered)); settings.add(Setting.boolSetting(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true, Property.NodeScope, Property.Filtered)); - settings.add(Setting.boolSetting("plugins.security.audit.config.enable_transport", true, Property.NodeScope, Property.Filtered)); final List disabledCategories = new ArrayList(2); disabledCategories.add("AUTHENTICATED"); disabledCategories.add("GRANTED_PRIVILEGES"); @@ -954,20 +953,20 @@ public List> getSettings() { switch(filterEntry) { case DISABLE_REST_CATEGORIES: case DISABLE_TRANSPORT_CATEGORIES: - return Setting.listSetting(filterEntry.getKeyWithNamepace(), disabledCategories, Function.identity(), Property.NodeScope); + return Setting.listSetting(filterEntry.getKeyWithNamespace(), disabledCategories, Function.identity(), Property.NodeScope); case IGNORE_REQUESTS: - return Setting.listSetting(filterEntry.getKeyWithNamepace(), Collections.emptyList(), Function.identity(), Property.NodeScope); + return Setting.listSetting(filterEntry.getKeyWithNamespace(), Collections.emptyList(), Function.identity(), Property.NodeScope); case IGNORE_USERS: - return Setting.listSetting(filterEntry.getKeyWithNamepace(), ignoredUsers, Function.identity(), Property.NodeScope); + return Setting.listSetting(filterEntry.getKeyWithNamespace(), ignoredUsers, Function.identity(), Property.NodeScope); // All boolean settings with default of true case ENABLE_REST: case ENABLE_TRANSPORT: case EXCLUDE_SENSITIVE_HEADERS: case LOG_REQUEST_BODY: case RESOLVE_INDICES: - return boolSettingNodeScopeFiltered.apply(filterEntry.getKeyWithNamepace(), true); + return boolSettingNodeScopeFiltered.apply(filterEntry.getKeyWithNamespace(), true); case RESOLVE_BULK_REQUESTS: - return boolSettingNodeScopeFiltered.apply(filterEntry.getKeyWithNamepace(), false); + return boolSettingNodeScopeFiltered.apply(filterEntry.getKeyWithNamespace(), false); default: throw new RuntimeException("Please add support for new FilterEntries value '" + filterEntry.name() + "'"); } diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index f6ad1bfb2d..233c91726c 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -176,26 +176,31 @@ public static class Filter { } public enum FilterEntries { - ENABLE_REST("enable_rest"), - ENABLE_TRANSPORT("enable_transport"), - RESOLVE_BULK_REQUESTS("resolve_bulk_requests"), - LOG_REQUEST_BODY("log_request_body"), - RESOLVE_INDICES("resolve_indices"), - EXCLUDE_SENSITIVE_HEADERS("exclude_sensitive_headers"), - DISABLE_REST_CATEGORIES("disabled_rest_categories"), - DISABLE_TRANSPORT_CATEGORIES("disabled_transport_categories"), - IGNORE_USERS("ignore_users"), - IGNORE_REQUESTS("ignore_requests"); + ENABLE_REST("enable_rest", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_REST), + ENABLE_TRANSPORT("enable_transport", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT), + RESOLVE_BULK_REQUESTS("resolve_bulk_requests", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS), + LOG_REQUEST_BODY("log_request_body", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_LOG_REQUEST_BODY), + RESOLVE_INDICES("resolve_indices", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_INDICES), + EXCLUDE_SENSITIVE_HEADERS("exclude_sensitive_headers", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_EXCLUDE_SENSITIVE_HEADERS), + DISABLE_REST_CATEGORIES("disabled_rest_categories", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES), + DISABLE_TRANSPORT_CATEGORIES("disabled_transport_categories", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES), + IGNORE_USERS("ignore_users", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_USERS), + IGNORE_REQUESTS("ignore_requests", ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_REQUESTS); private final String key; - private FilterEntries(final String entryKey) { + private final String legacyKeyWithNamespace; + private FilterEntries(final String entryKey, final String legacyKeyWithNamespace) { this.key = entryKey; + this.legacyKeyWithNamespace = legacyKeyWithNamespace; } public String getKey() { return this.key; } - public String getKeyWithNamepace() { - return SECURITY_AUDIT_CONFIG_DEFAULT + this.key; + public String getKeyWithNamespace() { + return SECURITY_AUDIT_CONFIG_DEFAULT + "."+ this.key; + } + public String getLegacyKeyWithNamespace() { + return this.legacyKeyWithNamespace; } } @@ -231,30 +236,55 @@ public static Filter from(Map properties) throws JsonProcessingE } + private static boolean getFromSettingBoolean(final Settings settings, FilterEntries filterEntry, final boolean defaultValue) { + return settings.getAsBoolean(filterEntry.getKeyWithNamespace(), settings.getAsBoolean(filterEntry.getLegacyKeyWithNamespace(), defaultValue)); + } + + private static Set getFromSettingStringSet(final Settings settings, FilterEntries filterEntry, final List defaultValue) { + final List defaultDetector = ImmutableList.of("__DEFAULT_DETECTION__"); + final Set stringSetOfKey = ConfigConstants.getSettingAsSet( + settings, + filterEntry.getKeyWithNamespace(), + defaultDetector, + false); + if (!defaultDetector.containsAll(stringSetOfKey)) { + return stringSetOfKey; + } + return ConfigConstants.getSettingAsSet( + settings, + filterEntry.getLegacyKeyWithNamespace(), + defaultValue, + false); + } + /** * Generate audit logging configuration from settings defined in opensearch.yml * @param settings settings * @return audit configuration filter */ public static Filter from(Settings settings) { - final boolean isRestApiAuditEnabled = settings.getAsBoolean(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_REST, true); - final boolean isTransportAuditEnabled = settings.getAsBoolean(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true); - final boolean resolveBulkRequests = settings.getAsBoolean(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, false); - final boolean logRequestBody = settings.getAsBoolean(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_LOG_REQUEST_BODY, true); - final boolean resolveIndices = settings.getAsBoolean(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_INDICES, true); - final boolean excludeSensitiveHeaders = settings.getAsBoolean(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_EXCLUDE_SENSITIVE_HEADERS, true); - final Set disabledRestCategories = AuditCategory.from(settings, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES); - final Set disabledTransportCategories = AuditCategory.from(settings, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES); - - final Set ignoredAuditUsers = ConfigConstants.getSettingAsSet( - settings, - ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_USERS, - DEFAULT_IGNORED_USERS, - false); + final boolean isRestApiAuditEnabled = getFromSettingBoolean(settings, FilterEntries.ENABLE_REST, true); + final boolean isTransportAuditEnabled = getFromSettingBoolean(settings, FilterEntries.ENABLE_TRANSPORT, true); + final boolean resolveBulkRequests = getFromSettingBoolean(settings, FilterEntries.RESOLVE_BULK_REQUESTS, false); + final boolean logRequestBody = getFromSettingBoolean(settings, FilterEntries.LOG_REQUEST_BODY, true); + final boolean resolveIndices = getFromSettingBoolean(settings, FilterEntries.RESOLVE_INDICES, true); + final boolean excludeSensitiveHeaders = getFromSettingBoolean(settings, FilterEntries.EXCLUDE_SENSITIVE_HEADERS, true); + final Set disabledRestCategories = AuditCategory.parse(getFromSettingStringSet(settings, FilterEntries.DISABLE_REST_CATEGORIES, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); + final Set disabledTransportCategories = AuditCategory.parse(getFromSettingStringSet(settings, FilterEntries.DISABLE_TRANSPORT_CATEGORIES, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); + final Set ignoredAuditUsers = getFromSettingStringSet(settings, FilterEntries.IGNORE_USERS, DEFAULT_IGNORED_USERS); + final Set ignoreAuditRequests = getFromSettingStringSet(settings, FilterEntries.IGNORE_REQUESTS, Collections.emptyList()); + + + + System.err.println("FROM Filter.from(Settings)\n\n"); + new RuntimeException().printStackTrace(); + System.err.println("settings? " + settings); + System.err.println("raw rest audit enabled: " + settings.get(FilterEntries.ENABLE_REST.getKeyWithNamespace())); + System.err.println("raw disabled rest: " + settings.get(FilterEntries.DISABLE_REST_CATEGORIES.getKeyWithNamespace())); + System.err.println("disabledRestCategories: " + disabledRestCategories); + System.err.println("disabledTransportCategories: " + disabledTransportCategories); + System.err.println("ignoredAuditUsers: " + ignoredAuditUsers); - final Set ignoreAuditRequests = ImmutableSet.copyOf(settings.getAsList( - ConfigConstants.OPENDISTRO_SECURITY_AUDIT_IGNORE_REQUESTS, - Collections.emptyList())); return new Filter(isRestApiAuditEnabled, isTransportAuditEnabled, diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java index 2bac7960bc..06179bb0a6 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java @@ -49,8 +49,4 @@ public static Set parse(final Collection categories) { .map(AuditCategory::valueOf) .collect(ImmutableSet.toImmutableSet()); } - - public static Set from(final Settings settings, final String key) { - return parse(ConfigConstants.getSettingAsSet(settings, key, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT, true)); - } } diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java index 8c3a19586e..81c9eed49c 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java @@ -441,6 +441,14 @@ public AuditCategory getCategory() { return msgCategory; } + public Origin getOrigin() { + return (Origin) this.auditInfo.get(ORIGIN); + } + + public String getPrivilege() { + return (String) this.auditInfo.get(PRIVILEGE); + } + public String getExceptionStackTrace() { return (String) this.auditInfo.get(EXCEPTION); } diff --git a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java index 0d1490997b..ccdcb36dc3 100644 --- a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java +++ b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java @@ -40,20 +40,28 @@ protected String getResourceFolder() { return "auditlog"; } - protected final void setup(Settings additionalSettings) throws Exception { - final Settings.Builder auditSettingsBuilder = Settings.builder(); - final Settings.Builder additionalSettingsBuilder = Settings.builder().put(additionalSettings); - AuditConfig.DEPRECATED_KEYS.forEach(key -> { - if (additionalSettingsBuilder.get(key) != null) { - auditSettingsBuilder.put(key, additionalSettings.get(key)); - additionalSettingsBuilder.remove(key); + protected final void setup(Settings settings) throws Exception { + + + final Settings.Builder auditConfigSettings = Settings.builder(); + final Settings.Builder defaultNodeSettings = Settings.builder(); + settings.keySet().stream().forEach(key -> { + final boolean isAnAuditConfigSetting = key.contains("plugins.security.audit") + || key.contains("opendistro_security.audit"); + if (isAnAuditConfigSetting) { + auditConfigSettings.put(key, settings.get(key)); + } else { + defaultNodeSettings.put(key, settings.get(key)); } }); - final Settings nodeSettings = defaultNodeSettings(additionalSettingsBuilder.build()); + System.err.println("AUDIT CONFIG:" + auditConfigSettings.build()); + System.err.println("DEFAULT CONFIG:" + defaultNodeSettings.build()); + + final Settings nodeSettings = defaultNodeSettings(defaultNodeSettings.build()); setup(Settings.EMPTY, new DynamicSecurityConfig(), nodeSettings, init); rh = restHelper(); - updateAuditConfig(auditSettingsBuilder.build()); + updateAuditConfig(auditConfigSettings.build()); } protected Settings defaultNodeSettings(Settings additionalSettings) { diff --git a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java index 566af2a77b..25da105359 100644 --- a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java @@ -19,7 +19,9 @@ import org.opensearch.client.RequestOptions; import org.opensearch.client.RestHighLevelClient; import org.opensearch.security.auditlog.AuditTestUtils; +import org.opensearch.security.auditlog.AuditLog.Origin; import org.opensearch.security.auditlog.config.AuditConfig; +import org.opensearch.security.auditlog.config.AuditConfig.Filter.FilterEntries; import org.opensearch.security.auditlog.impl.AuditCategory; import org.opensearch.security.compliance.ComplianceConfig; import com.google.common.collect.ImmutableMap; @@ -52,6 +54,8 @@ import java.util.List; import java.util.Objects; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; import static org.opensearch.rest.RestRequest.Method.GET; import static org.opensearch.rest.RestRequest.Method.DELETE; import static org.opensearch.rest.RestRequest.Method.PATCH; @@ -65,6 +69,7 @@ public void testAuditLogEnable() throws Exception { Settings additionalSettings = Settings.builder() .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) .put("plugins.security.audit.config.enable_transport", true) + .put("plugins.security.audit.config.enable_transport", true) .build(); setup(additionalSettings); @@ -89,31 +94,48 @@ public void testAuditLogEnable() throws Exception { } @Test - public void testSimpleAuthenticated() throws Exception { + public void testSimpleAuthenticatedSetting() throws Exception { + final Settings settings = Settings.builder() + .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true) + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, true) + .put(FilterEntries.DISABLE_TRANSPORT_CATEGORIES.getKeyWithNamespace(), "AUTHENTICATED") + .put(FilterEntries.DISABLE_REST_CATEGORIES.getKeyWithNamespace(), "AUTHENTICATED") + .build(); + verifyAuthenticated(settings); + } - Settings additionalSettings = Settings.builder() - .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true) - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, true) - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES, "authenticated") - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, "authenticated") - .build(); + @Test + public void testSimpleAuthenticatedLegacySetting() throws Exception { + final Settings settings = Settings.builder() + .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true) + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, true) + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES, "AUTHENTICATED") + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, "AUTHENTICATED") + .build(); + verifyAuthenticated(settings); + } - setup(additionalSettings); + private void verifyAuthenticated(final Settings settings) throws Exception { + setup(settings); setupStarfleetIndex(); - TestAuditlogImpl.clear(); - System.out.println("#### testSimpleAuthenticated"); - HttpResponse response = rh.executeGetRequest("_search", encodeBasicHeader("admin", "admin")); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - Thread.sleep(1500); - Assert.assertEquals(1, TestAuditlogImpl.messages.size()); - System.out.println(TestAuditlogImpl.sb.toString()); - Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("GRANTED_PRIVILEGES")); - Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("indices:data/read/search")); - Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("REST")); - Assert.assertFalse(TestAuditlogImpl.sb.toString().toLowerCase().contains("authorization")); - Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages)); + final List messages = TestAuditlogImpl.doThenWaitForMessages( + () -> { + final HttpResponse response = rh.executeGetRequest("_search", encodeBasicHeader("admin", "admin")); + assertThat(response.getStatusCode(), equalTo(HttpStatus.SC_OK)); + System.out.println(">>>RESPONSE\n\n" + response.getBody() + "\n\n>>>>>RESPONSE"); + }, + /* expectedCount */ 1); + + assertThat(messages.size(), equalTo(1)); + + System.out.println(messages.get(0).toJson()); + System.out.println("\n\n?????" + TestAuditlogImpl.sb.toString().toLowerCase() + "????\n\n"); + assertThat(messages.get(0).getCategory(), equalTo(AuditCategory.GRANTED_PRIVILEGES)); + assertThat(messages.get(0).getOrigin(), equalTo(Origin.REST)); + assertThat(messages.get(0).getPrivilege(), equalTo("indices:data/read/search")); } @Test From 23ed50c1c1e1f53b3e7c029026672b5bbabd03d1 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 26 May 2022 22:46:02 +0000 Subject: [PATCH 03/17] Fix test cases Signed-off-by: Peter Nied --- .../security/auditlog/config/AuditConfig.java | 58 ++++++++----------- .../auditlog/AbstractAuditlogiUnitTest.java | 6 +- .../integration/BasicAuditlogTest.java | 20 ++----- 3 files changed, 34 insertions(+), 50 deletions(-) diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index 233c91726c..2c5b7fe8a6 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -236,27 +236,6 @@ public static Filter from(Map properties) throws JsonProcessingE } - private static boolean getFromSettingBoolean(final Settings settings, FilterEntries filterEntry, final boolean defaultValue) { - return settings.getAsBoolean(filterEntry.getKeyWithNamespace(), settings.getAsBoolean(filterEntry.getLegacyKeyWithNamespace(), defaultValue)); - } - - private static Set getFromSettingStringSet(final Settings settings, FilterEntries filterEntry, final List defaultValue) { - final List defaultDetector = ImmutableList.of("__DEFAULT_DETECTION__"); - final Set stringSetOfKey = ConfigConstants.getSettingAsSet( - settings, - filterEntry.getKeyWithNamespace(), - defaultDetector, - false); - if (!defaultDetector.containsAll(stringSetOfKey)) { - return stringSetOfKey; - } - return ConfigConstants.getSettingAsSet( - settings, - filterEntry.getLegacyKeyWithNamespace(), - defaultValue, - false); - } - /** * Generate audit logging configuration from settings defined in opensearch.yml * @param settings settings @@ -274,18 +253,6 @@ public static Filter from(Settings settings) { final Set ignoredAuditUsers = getFromSettingStringSet(settings, FilterEntries.IGNORE_USERS, DEFAULT_IGNORED_USERS); final Set ignoreAuditRequests = getFromSettingStringSet(settings, FilterEntries.IGNORE_REQUESTS, Collections.emptyList()); - - - System.err.println("FROM Filter.from(Settings)\n\n"); - new RuntimeException().printStackTrace(); - System.err.println("settings? " + settings); - System.err.println("raw rest audit enabled: " + settings.get(FilterEntries.ENABLE_REST.getKeyWithNamespace())); - System.err.println("raw disabled rest: " + settings.get(FilterEntries.DISABLE_REST_CATEGORIES.getKeyWithNamespace())); - System.err.println("disabledRestCategories: " + disabledRestCategories); - System.err.println("disabledTransportCategories: " + disabledTransportCategories); - System.err.println("ignoredAuditUsers: " + ignoredAuditUsers); - - return new Filter(isRestApiAuditEnabled, isTransportAuditEnabled, resolveBulkRequests, @@ -297,6 +264,31 @@ public static Filter from(Settings settings) { disabledRestCategories, disabledTransportCategories); } + + private static boolean getFromSettingBoolean(final Settings settings, FilterEntries filterEntry, final boolean defaultValue) { + return settings.getAsBoolean(filterEntry.getKeyWithNamespace(), settings.getAsBoolean(filterEntry.getLegacyKeyWithNamespace(), defaultValue)); + } + + private static Set getFromSettingStringSet(final Settings settings, FilterEntries filterEntry, final List defaultValue) { + final List defaultDetector = ImmutableList.of("__DEFAULT_DETECTION__"); + final Set stringSetOfKey = ConfigConstants.getSettingAsSet( + settings, + filterEntry.getKeyWithNamespace(), + defaultDetector, + false); + + final boolean settingWasResolved = stringSetOfKey.containsAll(defaultDetector); + if (settingWasResolved) { + return stringSetOfKey; + } + + // Fallback to the legacy keyname + return ConfigConstants.getSettingAsSet( + settings, + filterEntry.getLegacyKeyWithNamespace(), + defaultValue, + false); + } /** * Checks if auditing for REST API is enabled or disabled diff --git a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java index ccdcb36dc3..7e39c7c138 100644 --- a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java +++ b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java @@ -46,8 +46,10 @@ protected final void setup(Settings settings) throws Exception { final Settings.Builder auditConfigSettings = Settings.builder(); final Settings.Builder defaultNodeSettings = Settings.builder(); settings.keySet().stream().forEach(key -> { - final boolean isAnAuditConfigSetting = key.contains("plugins.security.audit") - || key.contains("opendistro_security.audit"); + final boolean isAnAuditConfigSetting = + (key.contains("plugins.security.audit") + || key.contains("opendistro_security.audit")) && + ! "plugins.security.audit.type".equals(key); if (isAnAuditConfigSetting) { auditConfigSettings.put(key, settings.get(key)); } else { diff --git a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java index 25da105359..179f19dec2 100644 --- a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java @@ -68,8 +68,6 @@ public class BasicAuditlogTest extends AbstractAuditlogiUnitTest { public void testAuditLogEnable() throws Exception { Settings additionalSettings = Settings.builder() .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) - .put("plugins.security.audit.config.enable_transport", true) - .put("plugins.security.audit.config.enable_transport", true) .build(); setup(additionalSettings); @@ -97,10 +95,7 @@ public void testAuditLogEnable() throws Exception { public void testSimpleAuthenticatedSetting() throws Exception { final Settings settings = Settings.builder() .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true) - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, true) - .put(FilterEntries.DISABLE_TRANSPORT_CATEGORIES.getKeyWithNamespace(), "AUTHENTICATED") - .put(FilterEntries.DISABLE_REST_CATEGORIES.getKeyWithNamespace(), "AUTHENTICATED") + .put(FilterEntries.DISABLE_TRANSPORT_CATEGORIES.getKeyWithNamespace(), "NONE") .build(); verifyAuthenticated(settings); } @@ -109,30 +104,24 @@ public void testSimpleAuthenticatedSetting() throws Exception { public void testSimpleAuthenticatedLegacySetting() throws Exception { final Settings settings = Settings.builder() .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true) - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, true) - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES, "AUTHENTICATED") - .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, "AUTHENTICATED") + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES, "NONE") .build(); verifyAuthenticated(settings); } private void verifyAuthenticated(final Settings settings) throws Exception { setup(settings); - setupStarfleetIndex(); + final List messages = TestAuditlogImpl.doThenWaitForMessages( () -> { final HttpResponse response = rh.executeGetRequest("_search", encodeBasicHeader("admin", "admin")); assertThat(response.getStatusCode(), equalTo(HttpStatus.SC_OK)); - System.out.println(">>>RESPONSE\n\n" + response.getBody() + "\n\n>>>>>RESPONSE"); }, - /* expectedCount */ 1); + /* expectedCount*/ 1); assertThat(messages.size(), equalTo(1)); - System.out.println(messages.get(0).toJson()); - System.out.println("\n\n?????" + TestAuditlogImpl.sb.toString().toLowerCase() + "????\n\n"); assertThat(messages.get(0).getCategory(), equalTo(AuditCategory.GRANTED_PRIVILEGES)); assertThat(messages.get(0).getOrigin(), equalTo(Origin.REST)); assertThat(messages.get(0).getPrivilege(), equalTo("indices:data/read/search")); @@ -765,6 +754,7 @@ public void testIndexRequests() throws Exception { TestAuditlogImpl.clear(); rh.executePutRequest("/twitter", "{\"settings\":{\"index\":{\"number_of_shards\":3,\"number_of_replicas\":2}}}", encodeBasicHeader("admin", "admin")); String auditlogs = TestAuditlogImpl.sb.toString(); + System.out.println("&&&& auditlogs \n\n" + auditlogs + "\n\n&&&&&"); Assert.assertTrue(auditlogs.contains("\"audit_category\" : \"INDEX_EVENT\"")); Assert.assertTrue(auditlogs.contains("\"audit_transport_request_type\" : \"CreateIndexRequest\",")); Assert.assertTrue(auditlogs.contains("\"audit_request_body\" : \"{\\\"index\\\":{\\\"number_of_shards\\\":\\\"3\\\",\\\"number_of_replicas\\\":\\\"2\\\"}}\"")); From 33a98c6881b9637b6f1e25bcf641e6b9870c10fd Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 26 May 2022 22:47:22 +0000 Subject: [PATCH 04/17] Revert build.gradle debugging Signed-off-by: Peter Nied --- build.gradle | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/build.gradle b/build.gradle index a5e228313a..90ca3e49ce 100644 --- a/build.gradle +++ b/build.gradle @@ -217,11 +217,6 @@ testsJar { libsDirName = '.' } -tasks.withType(JavaCompile) { - configure(options) { - options.compilerArgs << '-Xlint:none' - } -} test { maxParallelForks = 3 @@ -231,8 +226,8 @@ test { } retry { failOnPassedAfterRetry = false - // maxFailures = 30 - // maxRetries = 5 + maxFailures = 30 + maxRetries = 5 } jacoco { excludes = [ From ab39340bbc2775ee1ef0417e80ebc5f54eb6a296 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 26 May 2022 22:51:36 +0000 Subject: [PATCH 05/17] Clarify how the setup works for tests Signed-off-by: Peter Nied --- .../auditlog/AbstractAuditlogiUnitTest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java index 7e39c7c138..6fdd750e03 100644 --- a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java +++ b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java @@ -41,15 +41,18 @@ protected String getResourceFolder() { } protected final void setup(Settings settings) throws Exception { - - final Settings.Builder auditConfigSettings = Settings.builder(); final Settings.Builder defaultNodeSettings = Settings.builder(); + // Seperate the cluster defaults from audit settings that will be applied after the cluster is up settings.keySet().stream().forEach(key -> { - final boolean isAnAuditConfigSetting = - (key.contains("plugins.security.audit") - || key.contains("opendistro_security.audit")) && - ! "plugins.security.audit.type".equals(key); + final boolean isAuditLoaderConfigurationKey = "plugins.security.audit.type".equals(key); + if (isAuditLoaderConfigurationKey) { + defaultNodeSettings.put(key, settings.get(key)); + return; + } + + final boolean isAnAuditConfigSetting = key.contains("plugins.security.audit") + || key.contains("opendistro_security.audit"); if (isAnAuditConfigSetting) { auditConfigSettings.put(key, settings.get(key)); } else { @@ -57,9 +60,6 @@ protected final void setup(Settings settings) throws Exception { } }); - System.err.println("AUDIT CONFIG:" + auditConfigSettings.build()); - System.err.println("DEFAULT CONFIG:" + defaultNodeSettings.build()); - final Settings nodeSettings = defaultNodeSettings(defaultNodeSettings.build()); setup(Settings.EMPTY, new DynamicSecurityConfig(), nodeSettings, init); rh = restHelper(); From 750aa2277cc81053240ff265ac3c3daff7e2ee45 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 26 May 2022 22:59:37 +0000 Subject: [PATCH 06/17] Remove debug statement Signed-off-by: Peter Nied --- .../security/auditlog/integration/BasicAuditlogTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java index b2b34c13c2..9f743fc03d 100644 --- a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java @@ -744,7 +744,6 @@ public void testIndexRequests() throws Exception { TestAuditlogImpl.clear(); rh.executePutRequest("/twitter", "{\"settings\":{\"index\":{\"number_of_shards\":3,\"number_of_replicas\":2}}}", encodeBasicHeader("admin", "admin")); String auditlogs = TestAuditlogImpl.sb.toString(); - System.out.println("&&&& auditlogs \n\n" + auditlogs + "\n\n&&&&&"); Assert.assertTrue(auditlogs.contains("\"audit_category\" : \"INDEX_EVENT\"")); Assert.assertTrue(auditlogs.contains("\"audit_transport_request_type\" : \"CreateIndexRequest\",")); Assert.assertTrue(auditlogs.contains("\"audit_request_body\" : \"{\\\"index\\\":{\\\"number_of_shards\\\":\\\"3\\\",\\\"number_of_replicas\\\":\\\"2\\\"}}\"")); From 3d2e4d860cb37d1247745f159ffb7061087688c1 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 26 May 2022 23:01:58 +0000 Subject: [PATCH 07/17] Fix spotless issues Signed-off-by: Peter Nied --- .../java/org/opensearch/security/OpenSearchSecurityPlugin.java | 2 +- .../security/auditlog/integration/BasicAuditlogTest.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index ce9bd67c6f..283cd21a03 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -121,9 +121,9 @@ import org.opensearch.security.action.whoami.WhoAmIAction; import org.opensearch.security.auditlog.AuditLog; import org.opensearch.security.auditlog.AuditLog.Origin; -import org.opensearch.security.auditlog.config.AuditConfig.Filter.FilterEntries; import org.opensearch.security.auditlog.AuditLogSslExceptionHandler; import org.opensearch.security.auditlog.NullAuditLog; +import org.opensearch.security.auditlog.config.AuditConfig.Filter.FilterEntries; import org.opensearch.security.auditlog.impl.AuditLogImpl; import org.opensearch.security.auth.BackendRegistry; import org.opensearch.security.compliance.ComplianceIndexingOperationListener; diff --git a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java index 9f743fc03d..bf37c7e8c4 100644 --- a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java @@ -43,9 +43,8 @@ import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; -import static org.opensearch.rest.RestRequest.Method.GET; +import static org.hamcrest.Matchers.equalTo; import static org.opensearch.rest.RestRequest.Method.DELETE; import static org.opensearch.rest.RestRequest.Method.GET; import static org.opensearch.rest.RestRequest.Method.PATCH; From be0e707c46362db643a5c313abcea9c36c98865d Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 26 May 2022 23:10:53 +0000 Subject: [PATCH 08/17] Fix broken import Signed-off-by: Peter Nied --- .../java/org/opensearch/security/OpenSearchSecurityPlugin.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 283cd21a03..c31f6e6aaa 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -40,6 +40,7 @@ import java.security.PrivilegedAction; import java.security.Security; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; From 1545e7d004cbc87d7e16bb73d05d81ab106d17e1 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 26 May 2022 23:30:24 +0000 Subject: [PATCH 09/17] Small fix Signed-off-by: Peter Nied --- .../org/opensearch/security/auditlog/impl/AuditCategory.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java index 2ca88e812e..21b2b7cbe6 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java @@ -21,9 +21,6 @@ import com.google.common.collect.ImmutableSet; -import org.opensearch.common.settings.Settings; -import org.opensearch.security.support.ConfigConstants; - public enum AuditCategory { BAD_HEADERS, FAILED_LOGIN, From 780ad32853a98f1a3cb5579522ffe754c21215ef Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 27 May 2022 16:12:07 +0000 Subject: [PATCH 10/17] Fix unused/missing imports Signed-off-by: Peter Nied --- .../opensearch/security/auditlog/AbstractAuditlogiUnitTest.java | 1 - .../security/auditlog/integration/BasicAuditlogTest.java | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java index 608b9b5f65..4c8be2c8a4 100644 --- a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java +++ b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java @@ -22,7 +22,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.security.DefaultObjectMapper; -import org.opensearch.security.auditlog.config.AuditConfig; import org.opensearch.security.auditlog.impl.AuditMessage; import org.opensearch.security.auditlog.routing.AuditMessageRouter; import org.opensearch.security.test.DynamicSecurityConfig; diff --git a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java index bf37c7e8c4..2f666a8b40 100644 --- a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java @@ -34,8 +34,10 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentType; import org.opensearch.security.auditlog.AbstractAuditlogiUnitTest; +import org.opensearch.security.auditlog.AuditLog.Origin; import org.opensearch.security.auditlog.AuditTestUtils; import org.opensearch.security.auditlog.config.AuditConfig; +import org.opensearch.security.auditlog.config.AuditConfig.Filter.FilterEntries; import org.opensearch.security.auditlog.impl.AuditCategory; import org.opensearch.security.auditlog.impl.AuditMessage; import org.opensearch.security.compliance.ComplianceConfig; From f4f004800b542b21ae70869db5d298ccc94daf03 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 27 May 2022 16:32:15 +0000 Subject: [PATCH 11/17] Clarify default detection logic Signed-off-by: Peter Nied --- .../opensearch/security/auditlog/config/AuditConfig.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index 4c3576bfb6..58f9a356d3 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -270,15 +270,15 @@ private static boolean getFromSettingBoolean(final Settings settings, FilterEntr } private static Set getFromSettingStringSet(final Settings settings, FilterEntries filterEntry, final List defaultValue) { - final List defaultDetector = ImmutableList.of("__DEFAULT_DETECTION__"); + final String defaultDetectorValue = "__DEFAULT_DETECTION__"; final Set stringSetOfKey = ConfigConstants.getSettingAsSet( settings, filterEntry.getKeyWithNamespace(), - defaultDetector, + ImmutableList.of(defaultDetectorValue), false); - final boolean settingWasResolved = stringSetOfKey.containsAll(defaultDetector); - if (settingWasResolved) { + final boolean foundDefault = stringSetOfKey.stream().anyMatch(s -> defaultDetectorValue.equals(s)); + if (!foundDefault) { return stringSetOfKey; } From f6cc61940b6896c0600b7f7546c5ab7af586da29 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 27 May 2022 17:21:00 +0000 Subject: [PATCH 12/17] Ignore case of 'none' value Signed-off-by: Peter Nied --- .../org/opensearch/security/auditlog/config/AuditConfig.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index 58f9a356d3..68d0fcc610 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -275,8 +275,9 @@ private static Set getFromSettingStringSet(final Settings settings, Filt settings, filterEntry.getKeyWithNamespace(), ImmutableList.of(defaultDetectorValue), - false); + true); + final String final boolean foundDefault = stringSetOfKey.stream().anyMatch(s -> defaultDetectorValue.equals(s)); if (!foundDefault) { return stringSetOfKey; @@ -287,7 +288,7 @@ private static Set getFromSettingStringSet(final Settings settings, Filt settings, filterEntry.getLegacyKeyWithNamespace(), defaultValue, - false); + true); } /** From 63c932ca329390132acff4e4496585d0f5e1288c Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 27 May 2022 18:11:34 +0000 Subject: [PATCH 13/17] Remove invalid java Signed-off-by: Peter Nied --- .../org/opensearch/security/auditlog/config/AuditConfig.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index 68d0fcc610..6b92be4a9a 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -277,7 +277,6 @@ private static Set getFromSettingStringSet(final Settings settings, Filt ImmutableList.of(defaultDetectorValue), true); - final String final boolean foundDefault = stringSetOfKey.stream().anyMatch(s -> defaultDetectorValue.equals(s)); if (!foundDefault) { return stringSetOfKey; From d952577162ff271799c02f09fff20fd6eb0fdfd1 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 27 May 2022 18:49:19 +0000 Subject: [PATCH 14/17] Starting unit tests Signed-off-by: Peter Nied --- .../config/AuditConfigFilterTest.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java index aa8518bded..1b927bad60 100644 --- a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java +++ b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java @@ -21,10 +21,14 @@ import org.junit.Test; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.Settings.Builder; +import org.opensearch.security.auditlog.config.AuditConfig.Filter.FilterEntries; import org.opensearch.security.auditlog.impl.AuditCategory; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.WildcardMatcher; +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; @@ -126,4 +130,37 @@ public void testEmpty() { assertTrue(auditConfigFilter.getDisabledRestCategories().isEmpty()); assertTrue(auditConfigFilter.getDisabledTransportCategories().isEmpty()); } + + + @Test + public void testFilterEntries() { + assertThat(FilterEntries.ENABLE_REST.getKey(), equalTo("enable_rest")); + assertThat(FilterEntries.ENABLE_REST.getKeyWithNamespace(), equalTo("plugins.security.audit.config.enable_rest")); + assertThat(FilterEntries.ENABLE_REST.getLegacyKeyWithNamespace(), equalTo("opendistro_security.audit.enable_rest")); + } + + @Test + public void getFromSettingBoolean() { + final FilterEntries entry = FilterEntries.ENABLE_REST; + + // Use primary key + final Settings settings1 = Settings.builder() + .put(entry.getKeyWithNamespace(), false) + .put(entry.getLegacyKeyWithNamespace(), true) + .build(); + // assertThat(AuditConfig.Filter.fromSettingBoolean(settings1, entry, true), equalTo(false)); + + // Use fallback key + final Settings settings2 = Settings.builder() + .put(entry.getLegacyKeyWithNamespace(), false) + .build(); + // assertThat(AuditConfig.Filter.fromSettingBoolean(settings2, entry, true), equalTo(false)); + + // Use default + // assertThat(AuditConfig.Filter.fromSettingBoolean(Settings.builder().build(), entry, true), equalTo(true)); + } + + @Test + public void getFromSettingStringSet() {} + } From b134d4df6a7e9faff59e039d8f6243abfa30807f Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 27 May 2022 18:49:40 +0000 Subject: [PATCH 15/17] renaming? Signed-off-by: Peter Nied --- .../security/auditlog/config/AuditConfig.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index 6b92be4a9a..7e998c4ab0 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -242,16 +242,16 @@ public static Filter from(Map properties) throws JsonProcessingE * @return audit configuration filter */ public static Filter from(Settings settings) { - final boolean isRestApiAuditEnabled = getFromSettingBoolean(settings, FilterEntries.ENABLE_REST, true); - final boolean isTransportAuditEnabled = getFromSettingBoolean(settings, FilterEntries.ENABLE_TRANSPORT, true); - final boolean resolveBulkRequests = getFromSettingBoolean(settings, FilterEntries.RESOLVE_BULK_REQUESTS, false); - final boolean logRequestBody = getFromSettingBoolean(settings, FilterEntries.LOG_REQUEST_BODY, true); - final boolean resolveIndices = getFromSettingBoolean(settings, FilterEntries.RESOLVE_INDICES, true); - final boolean excludeSensitiveHeaders = getFromSettingBoolean(settings, FilterEntries.EXCLUDE_SENSITIVE_HEADERS, true); - final Set disabledRestCategories = AuditCategory.parse(getFromSettingStringSet(settings, FilterEntries.DISABLE_REST_CATEGORIES, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); - final Set disabledTransportCategories = AuditCategory.parse(getFromSettingStringSet(settings, FilterEntries.DISABLE_TRANSPORT_CATEGORIES, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); - final Set ignoredAuditUsers = getFromSettingStringSet(settings, FilterEntries.IGNORE_USERS, DEFAULT_IGNORED_USERS); - final Set ignoreAuditRequests = getFromSettingStringSet(settings, FilterEntries.IGNORE_REQUESTS, Collections.emptyList()); + final boolean isRestApiAuditEnabled = fromSettingBoolean(settings, FilterEntries.ENABLE_REST, true); + final boolean isTransportAuditEnabled = fromSettingBoolean(settings, FilterEntries.ENABLE_TRANSPORT, true); + final boolean resolveBulkRequests = fromSettingBoolean(settings, FilterEntries.RESOLVE_BULK_REQUESTS, false); + final boolean logRequestBody = fromSettingBoolean(settings, FilterEntries.LOG_REQUEST_BODY, true); + final boolean resolveIndices = fromSettingBoolean(settings, FilterEntries.RESOLVE_INDICES, true); + final boolean excludeSensitiveHeaders = fromSettingBoolean(settings, FilterEntries.EXCLUDE_SENSITIVE_HEADERS, true); + final Set disabledRestCategories = AuditCategory.parse(fromSettingStringSet(settings, FilterEntries.DISABLE_REST_CATEGORIES, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); + final Set disabledTransportCategories = AuditCategory.parse(fromSettingStringSet(settings, FilterEntries.DISABLE_TRANSPORT_CATEGORIES, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); + final Set ignoredAuditUsers = fromSettingStringSet(settings, FilterEntries.IGNORE_USERS, DEFAULT_IGNORED_USERS); + final Set ignoreAuditRequests = fromSettingStringSet(settings, FilterEntries.IGNORE_REQUESTS, Collections.emptyList()); return new Filter(isRestApiAuditEnabled, isTransportAuditEnabled, @@ -264,12 +264,12 @@ public static Filter from(Settings settings) { disabledRestCategories, disabledTransportCategories); } - - private static boolean getFromSettingBoolean(final Settings settings, FilterEntries filterEntry, final boolean defaultValue) { + + private static boolean fromSettingBoolean(final Settings settings, FilterEntries filterEntry, final boolean defaultValue) { return settings.getAsBoolean(filterEntry.getKeyWithNamespace(), settings.getAsBoolean(filterEntry.getLegacyKeyWithNamespace(), defaultValue)); } - private static Set getFromSettingStringSet(final Settings settings, FilterEntries filterEntry, final List defaultValue) { + private static Set fromSettingStringSet(final Settings settings, FilterEntries filterEntry, final List defaultValue) { final String defaultDetectorValue = "__DEFAULT_DETECTION__"; final Set stringSetOfKey = ConfigConstants.getSettingAsSet( settings, From 2fc5860d9cd1bb2063f4d7b56dc6cbb599963fdb Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 27 May 2022 22:21:35 +0000 Subject: [PATCH 16/17] Add unit tests Signed-off-by: Peter Nied --- .../security/auditlog/config/AuditConfig.java | 10 +-- .../config/AuditConfigFilterTest.java | 75 +++++++++++++++++-- 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java index 7e998c4ab0..b428e4ec31 100644 --- a/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java +++ b/src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java @@ -131,9 +131,9 @@ public static AuditConfig from(final Settings settings) { */ @JsonInclude(JsonInclude.Include.NON_NULL) public static class Filter { + private static Set FIELDS = DefaultObjectMapper.getFields(Filter.class); @VisibleForTesting public static final Filter DEFAULT = Filter.from(Settings.EMPTY); - private static Set FIELDS = DefaultObjectMapper.getFields(Filter.class); private final boolean isRestApiAuditEnabled; private final boolean isTransportApiAuditEnabled; @@ -189,7 +189,7 @@ public enum FilterEntries { private final String key; private final String legacyKeyWithNamespace; - private FilterEntries(final String entryKey, final String legacyKeyWithNamespace) { + FilterEntries(final String entryKey, final String legacyKeyWithNamespace) { this.key = entryKey; this.legacyKeyWithNamespace = legacyKeyWithNamespace; } @@ -265,11 +265,11 @@ public static Filter from(Settings settings) { disabledTransportCategories); } - private static boolean fromSettingBoolean(final Settings settings, FilterEntries filterEntry, final boolean defaultValue) { + static boolean fromSettingBoolean(final Settings settings, FilterEntries filterEntry, final boolean defaultValue) { return settings.getAsBoolean(filterEntry.getKeyWithNamespace(), settings.getAsBoolean(filterEntry.getLegacyKeyWithNamespace(), defaultValue)); } - private static Set fromSettingStringSet(final Settings settings, FilterEntries filterEntry, final List defaultValue) { + static Set fromSettingStringSet(final Settings settings, FilterEntries filterEntry, final List defaultValue) { final String defaultDetectorValue = "__DEFAULT_DETECTION__"; final Set stringSetOfKey = ConfigConstants.getSettingAsSet( settings, @@ -277,7 +277,7 @@ private static Set fromSettingStringSet(final Settings settings, FilterE ImmutableList.of(defaultDetectorValue), true); - final boolean foundDefault = stringSetOfKey.stream().anyMatch(s -> defaultDetectorValue.equals(s)); + final boolean foundDefault = stringSetOfKey.stream().anyMatch(defaultDetectorValue::equals); if (!foundDefault) { return stringSetOfKey; } diff --git a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java index 1b927bad60..3864a4e053 100644 --- a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java +++ b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java @@ -17,11 +17,14 @@ import java.util.Collections; import java.util.EnumSet; +import java.util.List; +import java.util.Set; +import java.util.function.Function; +import com.google.common.collect.ImmutableSet; import org.junit.Test; import org.opensearch.common.settings.Settings; -import org.opensearch.common.settings.Settings.Builder; import org.opensearch.security.auditlog.config.AuditConfig.Filter.FilterEntries; import org.opensearch.security.auditlog.impl.AuditCategory; import org.opensearch.security.support.ConfigConstants; @@ -140,7 +143,7 @@ public void testFilterEntries() { } @Test - public void getFromSettingBoolean() { + public void fromSettingBoolean() { final FilterEntries entry = FilterEntries.ENABLE_REST; // Use primary key @@ -148,19 +151,79 @@ public void getFromSettingBoolean() { .put(entry.getKeyWithNamespace(), false) .put(entry.getLegacyKeyWithNamespace(), true) .build(); - // assertThat(AuditConfig.Filter.fromSettingBoolean(settings1, entry, true), equalTo(false)); + assertThat(AuditConfig.Filter.fromSettingBoolean(settings1, entry, true), equalTo(false)); // Use fallback key final Settings settings2 = Settings.builder() .put(entry.getLegacyKeyWithNamespace(), false) .build(); - // assertThat(AuditConfig.Filter.fromSettingBoolean(settings2, entry, true), equalTo(false)); + assertThat(AuditConfig.Filter.fromSettingBoolean(settings2, entry, true), equalTo(false)); // Use default - // assertThat(AuditConfig.Filter.fromSettingBoolean(Settings.builder().build(), entry, true), equalTo(true)); + assertThat(AuditConfig.Filter.fromSettingBoolean(Settings.builder().build(), entry, true), equalTo(true)); } @Test - public void getFromSettingStringSet() {} + public void fromSettingStringSet() { + final FilterEntries entry = FilterEntries.IGNORE_USERS; + // Use primary key + final Settings settings1 = Settings.builder() + .putList(entry.getKeyWithNamespace(), "abc") + .putList(entry.getLegacyKeyWithNamespace(), "def") + .build(); + assertThat(AuditConfig.Filter.fromSettingStringSet(settings1, entry, List.of("xyz")), equalTo(ImmutableSet.of("abc"))); + + // Use fallback key + final Settings settings2 = Settings.builder() + .putList(entry.getLegacyKeyWithNamespace(), "def") + .build(); + assertThat(AuditConfig.Filter.fromSettingStringSet(settings2, entry, List.of("xyz")), equalTo(ImmutableSet.of("def"))); + + // Use default + assertThat(AuditConfig.Filter.fromSettingStringSet(Settings.builder().build(), entry, List.of("xyz")), equalTo(ImmutableSet.of("xyz"))); + } + + @Test + public void fromSettingParseAuditCategory() { + final FilterEntries entry = FilterEntries.DISABLE_REST_CATEGORIES; + final Function> parse = (settings) -> + AuditCategory.parse(AuditConfig.Filter.fromSettingStringSet(settings, entry, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_CATEGORIES_DEFAULT)); + + final Settings noValues = Settings.builder().build(); + assertThat(parse.apply(noValues), equalTo(ImmutableSet.of(AUTHENTICATED, GRANTED_PRIVILEGES))); + + final Settings legacySettingNone = Settings.builder() + .put(entry.getLegacyKeyWithNamespace(), "NONE") + .build(); + assertThat(parse.apply(legacySettingNone), equalTo(ImmutableSet.of())); + + final Settings legacySettingValue = Settings.builder() + .put(entry.getLegacyKeyWithNamespace(), AUTHENTICATED.name()) + .build(); + assertThat(parse.apply(legacySettingValue), equalTo(ImmutableSet.of(AUTHENTICATED))); + + final Settings legacySettingMultipleValues = Settings.builder() + .putList(entry.getLegacyKeyWithNamespace(), AUTHENTICATED.name(), BAD_HEADERS.name()) + .build(); + assertThat(parse.apply(legacySettingMultipleValues), equalTo(ImmutableSet.of(AUTHENTICATED, BAD_HEADERS))); + + final Settings settingNone = Settings.builder() + .put(entry.getKeyWithNamespace(), "NONE") + .put(entry.getLegacyKeyWithNamespace(), FAILED_LOGIN.name()) + .build(); + assertThat(parse.apply(settingNone), equalTo(ImmutableSet.of())); + + final Settings settingValue = Settings.builder() + .put(entry.getKeyWithNamespace(), AUTHENTICATED.name()) + .put(entry.getLegacyKeyWithNamespace(), FAILED_LOGIN.name()) + .build(); + assertThat(parse.apply(settingValue), equalTo(ImmutableSet.of(AUTHENTICATED))); + + final Settings settingMultipleValues = Settings.builder() + .putList(entry.getKeyWithNamespace(), AUTHENTICATED.name(), BAD_HEADERS.name()) + .put(entry.getLegacyKeyWithNamespace(), FAILED_LOGIN.name()) + .build(); + assertThat(parse.apply(settingMultipleValues), equalTo(ImmutableSet.of(AUTHENTICATED, BAD_HEADERS))); + } } From a905e8e923e7b5a59780ea8119e9ffef33b28bbb Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 27 May 2022 23:21:48 +0000 Subject: [PATCH 17/17] Also handle deprecated settings when moving values to the updated audit config Signed-off-by: Peter Nied --- .../auditlog/AbstractAuditlogiUnitTest.java | 21 +++++++++---------- .../config/AuditConfigFilterTest.java | 6 ++++++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java index 4c8be2c8a4..61bc702df0 100644 --- a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java +++ b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java @@ -15,6 +15,7 @@ package org.opensearch.security.auditlog; +import java.util.Arrays; import java.util.Collection; import com.fasterxml.jackson.databind.JsonNode; @@ -22,6 +23,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.security.auditlog.config.AuditConfig; import org.opensearch.security.auditlog.impl.AuditMessage; import org.opensearch.security.auditlog.routing.AuditMessageRouter; import org.opensearch.security.test.DynamicSecurityConfig; @@ -29,6 +31,8 @@ import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.security.test.helper.rest.RestHelper; +import static org.opensearch.security.auditlog.config.AuditConfig.DEPRECATED_KEYS; + public abstract class AbstractAuditlogiUnitTest extends SingleClusterTest { protected RestHelper rh = null; @@ -42,17 +46,12 @@ protected String getResourceFolder() { protected final void setup(Settings settings) throws Exception { final Settings.Builder auditConfigSettings = Settings.builder(); final Settings.Builder defaultNodeSettings = Settings.builder(); - // Seperate the cluster defaults from audit settings that will be applied after the cluster is up - settings.keySet().stream().forEach(key -> { - final boolean isAuditLoaderConfigurationKey = "plugins.security.audit.type".equals(key); - if (isAuditLoaderConfigurationKey) { - defaultNodeSettings.put(key, settings.get(key)); - return; - } - - final boolean isAnAuditConfigSetting = key.contains("plugins.security.audit") - || key.contains("opendistro_security.audit"); - if (isAnAuditConfigSetting) { + // Separate the cluster defaults from audit settings that will be applied after the cluster is up + settings.keySet().forEach(key -> { + final boolean moveToAuditConfig = Arrays.stream(AuditConfig.Filter.FilterEntries.values()) + .anyMatch(entry -> entry.getKeyWithNamespace().equalsIgnoreCase(key) || entry.getLegacyKeyWithNamespace().equalsIgnoreCase(key)) + || DEPRECATED_KEYS.stream().anyMatch(key::equalsIgnoreCase); + if (moveToAuditConfig) { auditConfigSettings.put(key, settings.get(key)); } else { defaultNodeSettings.put(key, settings.get(key)); diff --git a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java index 3864a4e053..2a31f3924d 100644 --- a/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java +++ b/src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java @@ -225,5 +225,11 @@ public void fromSettingParseAuditCategory() { .put(entry.getLegacyKeyWithNamespace(), FAILED_LOGIN.name()) .build(); assertThat(parse.apply(settingMultipleValues), equalTo(ImmutableSet.of(AUTHENTICATED, BAD_HEADERS))); + + final Settings settingMultipleValuesString = Settings.builder() + .put(entry.getKeyWithNamespace(), AUTHENTICATED.name() + "," + BAD_HEADERS.name()) + .put(entry.getLegacyKeyWithNamespace(), FAILED_LOGIN.name()) + .build(); + assertThat(parse.apply(settingMultipleValues), equalTo(ImmutableSet.of(AUTHENTICATED, BAD_HEADERS))); } }