From c6d42558ecc53be486a5aabf1ca140187188b3d6 Mon Sep 17 00:00:00 2001 From: Lukasz Soszynski Date: Thu, 13 Oct 2022 17:25:19 +0200 Subject: [PATCH] Remarks gathered during code review applied to tests related to audit logs. Signed-off-by: Lukasz Soszynski --- .../security/SearchOperationTest.java | 6 +- .../test/framework/AuditCompliance.java | 106 ++++++++ .../test/framework/AuditConfiguration.java | 55 ++++ .../test/framework/AuditFilters.java | 121 +++++++++ .../test/framework/TestSecurityConfig.java | 246 +----------------- .../test/framework/cluster/LocalCluster.java | 2 +- 6 files changed, 293 insertions(+), 243 deletions(-) create mode 100644 src/integrationTest/java/org/opensearch/test/framework/AuditCompliance.java create mode 100644 src/integrationTest/java/org/opensearch/test/framework/AuditConfiguration.java create mode 100644 src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java diff --git a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java index c20dca26cb..520837f6bb 100644 --- a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java +++ b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java @@ -73,9 +73,9 @@ import org.opensearch.repositories.RepositoryMissingException; import org.opensearch.rest.RestStatus; import org.opensearch.search.builder.SearchSourceBuilder; -import org.opensearch.test.framework.TestSecurityConfig.AuditCompliance; -import org.opensearch.test.framework.TestSecurityConfig.AuditConfiguration; -import org.opensearch.test.framework.TestSecurityConfig.AuditFilters; +import org.opensearch.test.framework.AuditCompliance; +import org.opensearch.test.framework.AuditConfiguration; +import org.opensearch.test.framework.AuditFilters; import org.opensearch.test.framework.TestSecurityConfig.Role; import org.opensearch.test.framework.TestSecurityConfig.User; import org.opensearch.test.framework.audit.AuditLogsRule; diff --git a/src/integrationTest/java/org/opensearch/test/framework/AuditCompliance.java b/src/integrationTest/java/org/opensearch/test/framework/AuditCompliance.java new file mode 100644 index 0000000000..fdf3cdd6f1 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/test/framework/AuditCompliance.java @@ -0,0 +1,106 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +* +*/ +package org.opensearch.test.framework; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import org.opensearch.common.xcontent.ToXContentObject; +import org.opensearch.common.xcontent.XContentBuilder; + +public class AuditCompliance implements ToXContentObject { + + private boolean enabled = false; + + private Boolean writeLogDiffs; + + private List readIgnoreUsers; + + private List writeWatchedIndices; + + private List writeIgnoreUsers; + + private Boolean readMetadataOnly; + + private Boolean writeMetadataOnly; + + private Boolean externalConfig; + + private Boolean internalConfig; + + public AuditCompliance enabled(boolean enabled) { + this.enabled = enabled; + this.writeLogDiffs = false; + this.readIgnoreUsers = Collections.emptyList(); + this.writeWatchedIndices = Collections.emptyList(); + this.writeIgnoreUsers = Collections.emptyList(); + this.readMetadataOnly = false; + this.writeMetadataOnly = false; + this.externalConfig = false; + this.internalConfig = false; + return this; + } + + public AuditCompliance writeLogDiffs(boolean writeLogDiffs) { + this.writeLogDiffs = writeLogDiffs; + return this; + } + + public AuditCompliance readIgnoreUsers(List list) { + this.readIgnoreUsers = list; + return this; + } + + public AuditCompliance writeWatchedIndices(List list) { + this.writeWatchedIndices = list; + return this; + } + + public AuditCompliance writeIgnoreUsers(List list) { + this.writeIgnoreUsers = list; + return this; + } + + public AuditCompliance readMetadataOnly(boolean readMetadataOnly) { + this.readMetadataOnly = readMetadataOnly; + return this; + } + + public AuditCompliance writeMetadataOnly(boolean writeMetadataOnly) { + this.writeMetadataOnly = writeMetadataOnly; + return this; + } + + public AuditCompliance externalConfig(boolean externalConfig) { + this.externalConfig = externalConfig; + return this; + } + + public AuditCompliance internalConfig(boolean internalConfig) { + this.internalConfig = internalConfig; + return this; + } + + @Override public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { + xContentBuilder.startObject(); + xContentBuilder.field("enabled", enabled); + xContentBuilder.field("write_log_diffs", writeLogDiffs); + xContentBuilder.field("read_ignore_users", readIgnoreUsers); + xContentBuilder.field("write_watched_indices", writeWatchedIndices); + xContentBuilder.field("write_ignore_users", writeIgnoreUsers); + xContentBuilder.field("read_metadata_only", readMetadataOnly); + xContentBuilder.field("write_metadata_only", writeMetadataOnly); + xContentBuilder.field("external_config", externalConfig); + xContentBuilder.field("internal_config", internalConfig); + xContentBuilder.endObject(); + return xContentBuilder; + } +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/AuditConfiguration.java b/src/integrationTest/java/org/opensearch/test/framework/AuditConfiguration.java new file mode 100644 index 0000000000..762b27085a --- /dev/null +++ b/src/integrationTest/java/org/opensearch/test/framework/AuditConfiguration.java @@ -0,0 +1,55 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +* +*/ +package org.opensearch.test.framework; + +import java.io.IOException; + +import org.opensearch.common.xcontent.ToXContentObject; +import org.opensearch.common.xcontent.XContentBuilder; + +public class AuditConfiguration implements ToXContentObject { + private final boolean enabled; + + private AuditFilters filters; + + private AuditCompliance compliance; + + public AuditConfiguration(boolean enabled) { + this.filters = new AuditFilters(); + this.compliance = new AuditCompliance(); + this.enabled = enabled; + } + + public boolean isEnabled() { + return enabled; + } + + public AuditConfiguration filters(AuditFilters filters) { + this.filters = filters; + return this; + } + + public AuditConfiguration compliance(AuditCompliance auditCompliance) { + this.compliance = auditCompliance; + return this; + } + + @Override public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { + // json built here must be deserialized to org.opensearch.security.auditlog.config.AuditConfig + xContentBuilder.startObject(); + xContentBuilder.field("enabled", enabled); + + xContentBuilder.field("audit", filters); + xContentBuilder.field("compliance", compliance); + + xContentBuilder.endObject(); + return xContentBuilder; + } +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java b/src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java new file mode 100644 index 0000000000..24dc3449b9 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/test/framework/AuditFilters.java @@ -0,0 +1,121 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +* +*/ +package org.opensearch.test.framework; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import org.opensearch.common.xcontent.ToXContentObject; +import org.opensearch.common.xcontent.XContentBuilder; + +public class AuditFilters implements ToXContentObject { + + private Boolean enabledRest; + + private Boolean enabledTransport; + + private Boolean logRequestBody; + + private Boolean resolveIndices; + + private Boolean resolveBulkRequests; + + private Boolean excludeSensitiveHeaders; + + private List ignoreUsers; + + private List ignoreRequests; + + private List disabledRestCategories; + + private List disabledTransportCategories; + + public AuditFilters() { + this.enabledRest = false; + this.enabledTransport = false; + + this.logRequestBody = true; + this.resolveIndices = true; + this.resolveBulkRequests = false; + this.excludeSensitiveHeaders = true; + + this.ignoreUsers = Collections.emptyList(); + this.ignoreRequests = Collections.emptyList(); + this.disabledRestCategories = Collections.emptyList(); + this.disabledTransportCategories = Collections.emptyList(); + } + + public AuditFilters enabledRest(boolean enabled) { + this.enabledRest = enabled; + return this; + } + + public AuditFilters enabledTransport(boolean enabled) { + this.enabledTransport = enabled; + return this; + } + + public AuditFilters logRequestBody(boolean logRequestBody) { + this.logRequestBody = logRequestBody; + return this; + } + + public AuditFilters resolveIndices(boolean resolveIndices) { + this.resolveIndices = resolveIndices; + return this; + } + + public AuditFilters resolveBulkRequests(boolean resolveBulkRequests) { + this.resolveBulkRequests = resolveBulkRequests; + return this; + } + + public AuditFilters excludeSensitiveHeaders(boolean excludeSensitiveHeaders) { + this.excludeSensitiveHeaders = excludeSensitiveHeaders; + return this; + } + + public AuditFilters ignoreUsers(List ignoreUsers) { + this.ignoreUsers = ignoreUsers; + return this; + } + + public AuditFilters ignoreRequests(List ignoreRequests) { + this.ignoreRequests = ignoreRequests; + return this; + } + + public AuditFilters disabledRestCategories(List disabledRestCategories) { + this.disabledRestCategories = disabledRestCategories; + return this; + } + + public AuditFilters disabledTransportCategories(List disabledTransportCategories) { + this.disabledTransportCategories = disabledTransportCategories; + return this; + } + + @Override public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { + xContentBuilder.startObject(); + xContentBuilder.field("enable_rest", enabledRest); + xContentBuilder.field("enable_transport", enabledTransport); + xContentBuilder.field("resolve_indices", resolveIndices); + xContentBuilder.field("log_request_body", logRequestBody); + xContentBuilder.field("resolve_bulk_requests", resolveBulkRequests); + xContentBuilder.field("exclude_sensitive_headers", excludeSensitiveHeaders); + xContentBuilder.field("ignore_users", ignoreUsers); + xContentBuilder.field("ignore_requests", ignoreRequests); + xContentBuilder.field("disabled_rest_categories", disabledRestCategories); + xContentBuilder.field("disabled_transport_categories", disabledTransportCategories); + xContentBuilder.endObject(); + return xContentBuilder; + } +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java index 2c7d469549..613092d586 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java +++ b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java @@ -164,9 +164,9 @@ public static class User implements UserCredentialsHolder, ToXContentObject { public final static TestSecurityConfig.User USER_ADMIN = new TestSecurityConfig.User("admin") .roles(new Role("allaccess").indexPermissions("*").on("*").clusterPermissions("*")); - private String name; + String name; private String password; - private List roles = new ArrayList<>(); + List roles = new ArrayList<>(); private Map attributes = new HashMap<>(); public User(String name) { @@ -181,8 +181,8 @@ public User password(String password) { public User roles(Role... roles) { // We scope the role names by user to keep tests free of potential side effects - String roleNamePrefix = "user_" + this.name + "__"; - this.roles.addAll(Arrays.asList(roles).stream().map((r) -> r.clone().name(roleNamePrefix + r.name)).collect(Collectors.toSet())); + String roleNamePrefix = "user_" + this.getName() + "__"; + this.roles.addAll(Arrays.asList(roles).stream().map((r) -> r.clone().name(roleNamePrefix + r.getName())).collect(Collectors.toSet())); return this; } @@ -222,241 +222,9 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params xContentBuilder.endObject(); return xContentBuilder; } - } - - public static class AuditFilters implements ToXContentObject { - - private Boolean enabledRest; - - private Boolean enabledTransport; - - private Boolean logRequestBody; - - private Boolean resolveIndices; - - private Boolean resolveBulkRequests; - - private Boolean excludeSensitiveHeaders; - - private List ignoreUsers; - - private List ignoreRequests; - - private List disabledRestCategories; - - private List disabledTransportCategories; - - public AuditFilters(){ - this.enabledRest = false; - this.enabledTransport = false; - - this.logRequestBody = true; - this.resolveIndices = true; - this.resolveBulkRequests = false; - this.excludeSensitiveHeaders = true; - - this.ignoreUsers = Collections.emptyList(); - this.ignoreRequests = Collections.emptyList(); - this.disabledRestCategories = Collections.emptyList(); - this.disabledTransportCategories = Collections.emptyList(); - } - - public AuditFilters enabledRest(boolean enabled) { - this.enabledRest = enabled; - return this; - } - - public AuditFilters enabledTransport(boolean enabled) { - this.enabledTransport = enabled; - return this; - } - - public AuditFilters logRequestBody(boolean logRequestBody){ - this.logRequestBody = logRequestBody; - return this; - } - - public AuditFilters resolveIndices(boolean resolveIndices) { - this.resolveIndices = resolveIndices; - return this; - } - - public AuditFilters resolveBulkRequests(boolean resolveBulkRequests) { - this.resolveBulkRequests = resolveBulkRequests; - return this; - } - - public AuditFilters excludeSensitiveHeaders(boolean excludeSensitiveHeaders) { - this.excludeSensitiveHeaders = excludeSensitiveHeaders; - return this; - } - - public AuditFilters ignoreUsers(List ignoreUsers) { - this.ignoreUsers = ignoreUsers; - return this; - } - - public AuditFilters ignoreRequests(List ignoreRequests) { - this.ignoreRequests =ignoreRequests; - return this; - } - - public AuditFilters disabledRestCategories(List disabledRestCategories) { - this.disabledRestCategories = disabledRestCategories; - return this; - } - - public AuditFilters disabledTransportCategories(List disabledTransportCategories) { - this.disabledTransportCategories = disabledTransportCategories; - return this; - } - - @Override - public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { - xContentBuilder.startObject(); - xContentBuilder.field("enable_rest", enabledRest); - xContentBuilder.field("enable_transport", enabledTransport); - xContentBuilder.field("resolve_indices", resolveIndices); - xContentBuilder.field("log_request_body", logRequestBody); - xContentBuilder.field("resolve_bulk_requests", resolveBulkRequests); - xContentBuilder.field("exclude_sensitive_headers", excludeSensitiveHeaders); - xContentBuilder.field("ignore_users", ignoreUsers); - xContentBuilder.field("ignore_requests", ignoreRequests); - xContentBuilder.field("disabled_rest_categories", disabledRestCategories); - xContentBuilder.field("disabled_transport_categories", disabledTransportCategories); - xContentBuilder.endObject(); - return xContentBuilder; - } - } - - public static class AuditCompliance implements ToXContentObject { - - private boolean enabled = false; - - private Boolean writeLogDiffs; - - private List readIgnoreUsers; - - private List writeWatchedIndices; - - private List writeIgnoreUsers; - - private Boolean readMetadataOnly; - - private Boolean writeMetadataOnly; - - private Boolean externalConfig; - - private Boolean internalConfig; - - public AuditCompliance enabled(boolean enabled) { - this.enabled = enabled; - this.writeLogDiffs = false; - this.readIgnoreUsers = Collections.emptyList(); - this.writeWatchedIndices = Collections.emptyList(); - this.writeIgnoreUsers = Collections.emptyList(); - this.readMetadataOnly = false; - this.writeMetadataOnly = false; - this.externalConfig = false; - this.internalConfig = false; - return this; - } - - public AuditCompliance writeLogDiffs(boolean writeLogDiffs) { - this.writeLogDiffs = writeLogDiffs; - return this; - } - - public AuditCompliance readIgnoreUsers(List list) { - this.readIgnoreUsers = list; - return this; - } - - public AuditCompliance writeWatchedIndices(List list) { - this.writeWatchedIndices = list; - return this; - } - - public AuditCompliance writeIgnoreUsers(List list) { - this.writeIgnoreUsers = list; - return this; - } - - public AuditCompliance readMetadataOnly(boolean readMetadataOnly) { - this.readMetadataOnly = readMetadataOnly; - return this; - } - - public AuditCompliance writeMetadataOnly(boolean writeMetadataOnly) { - this.writeMetadataOnly = writeMetadataOnly; - return this; - } - - public AuditCompliance externalConfig(boolean externalConfig) { - this.externalConfig = externalConfig; - return this; - } - - public AuditCompliance internalConfig(boolean internalConfig) { - this.internalConfig = internalConfig; - return this; - } - - @Override - public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { - xContentBuilder.startObject(); - xContentBuilder.field("enabled", enabled); - xContentBuilder.field("write_log_diffs", writeLogDiffs); - xContentBuilder.field("read_ignore_users", readIgnoreUsers); - xContentBuilder.field("write_watched_indices", writeWatchedIndices); - xContentBuilder.field("write_ignore_users", writeIgnoreUsers); - xContentBuilder.field("read_metadata_only", readMetadataOnly); - xContentBuilder.field("write_metadata_only", writeMetadataOnly); - xContentBuilder.field("external_config", externalConfig); - xContentBuilder.field("internal_config", internalConfig); - xContentBuilder.endObject(); - return xContentBuilder; - } - } - - public static class AuditConfiguration implements ToXContentObject { - private final boolean enabled; - - private AuditFilters filters; - - private AuditCompliance compliance; - - public AuditConfiguration(boolean enabled) { - this.filters = new AuditFilters(); - this.compliance = new AuditCompliance(); - this.enabled = enabled; - } - public boolean isEnabled() { - return enabled; - } - - public AuditConfiguration filters(AuditFilters filters) { - this.filters = filters; - return this; - } - - public AuditConfiguration compliance(AuditCompliance auditCompliance) { - this.compliance = auditCompliance; - return this; - } - - @Override - public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { - // json built here must be deserialized to org.opensearch.security.auditlog.config.AuditConfig - xContentBuilder.startObject(); - xContentBuilder.field("enabled", enabled); - - xContentBuilder.field("audit", filters); - xContentBuilder.field("compliance", compliance); - - xContentBuilder.endObject(); - return xContentBuilder; + @Override public int hashCode() { + return super.hashCode(); } } @@ -755,7 +523,7 @@ public void initIndex(Client client) { } - private static String hash(final char[] clearTextPassword) { + static String hash(final char[] clearTextPassword) { final byte[] salt = new byte[16]; new SecureRandom().nextBytes(salt); final String hash = OpenBSDBCrypt.generate((Objects.requireNonNull(clearTextPassword)), salt, 12); diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java index 79df891303..9650ac1b1d 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java @@ -48,9 +48,9 @@ import org.opensearch.node.PluginAwareNode; import org.opensearch.plugins.Plugin; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.AuditConfiguration; import org.opensearch.test.framework.TestIndex; import org.opensearch.test.framework.TestSecurityConfig; -import org.opensearch.test.framework.TestSecurityConfig.AuditConfiguration; import org.opensearch.test.framework.TestSecurityConfig.Role; import org.opensearch.test.framework.audit.TestRuleAuditLogSink; import org.opensearch.test.framework.certificate.TestCertificates;