From 023c1a4995b204b32de647d5be09894df6668219 Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Mon, 16 Oct 2023 15:54:58 +0200 Subject: [PATCH] [Backport 2.x] [Improvement] Enable warnings during compilation (#3358) Backport 40efea6c7633002cd6775320d07efaaeafbf535d from 3358 Signed-off-by: Andrey Pleskach --- build.gradle | 60 ++++++++++++++++++- .../org/opensearch/node/PluginAwareNode.java | 11 ++-- .../framework/cluster/ClusterManager.java | 4 +- .../cluster/LocalOpenSearchCluster.java | 2 +- .../security/DefaultObjectMapper.java | 4 +- .../security/OpenSearchSecurityPlugin.java | 5 +- .../auditlog/impl/AbstractAuditLog.java | 21 ++----- .../auditlog/impl/RequestResolver.java | 6 +- .../auditlog/routing/AuditMessageRouter.java | 1 + .../auditlog/sink/InternalOpenSearchSink.java | 2 +- .../security/auth/BackendRegistry.java | 9 ++- .../security/auth/UserInjector.java | 21 ++++++- .../configuration/DlsFlsValveImpl.java | 1 + .../dlic/rest/api/MigrateApiAction.java | 2 +- .../validation/RequestContentValidator.java | 8 +-- .../security/http/HTTPProxyAuthenticator.java | 37 ++++++------ .../opensearch/security/http/XFFResolver.java | 2 +- .../security/securityconf/ConfigModelV6.java | 13 ++-- .../security/securityconf/ConfigModelV7.java | 5 +- .../securityconf/DynamicConfigFactory.java | 2 + .../security/securityconf/Migration.java | 1 + .../impl/SecurityDynamicConfiguration.java | 3 +- .../security/ssl/DefaultSecurityKeyStore.java | 8 +-- .../transport/SecuritySSLRequestHandler.java | 2 +- .../opensearch/security/ssl/util/Utils.java | 22 +------ .../security/support/SecurityUtils.java | 20 ------- .../security/support/WildcardMatcher.java | 4 +- .../opensearch/security/user/UserService.java | 7 ++- .../dlic/auth/ldap/LdapBackendTest.java | 36 +++++------ .../ldap/LdapBackendTestNewStyleConfig.java | 29 +++++---- .../ldap2/LdapBackendTestNewStyleConfig2.java | 5 +- .../ldap2/LdapBackendTestOldStyleConfig2.java | 6 +- .../org/opensearch/node/PluginAwareNode.java | 15 +++-- .../org/opensearch/security/ConfigTests.java | 1 + .../InitializationIntegrationTests.java | 6 +- .../security/RolesInjectorIntegTest.java | 13 ++-- .../security/RolesValidationIntegTest.java | 13 ++-- .../security/SlowIntegrationTests.java | 16 ++++- .../TransportUserInjectorIntegTest.java | 36 +++++++---- .../org/opensearch/security/UtilTests.java | 30 ---------- .../security/auth/UserInjectorTest.java | 32 ++++++++++ .../ccstest/CrossClusterSearchTests.java | 9 +-- .../dlic/dlsfls/CCReplicationTest.java | 17 ++---- .../rest/api/AbstractRestApiUnitTest.java | 39 +----------- .../security/dlic/rest/api/RolesApiTest.java | 2 +- .../security/dlic/rest/api/UserApiTest.java | 27 ++++++++- .../multitenancy/test/MultitenancyTests.java | 25 ++++---- .../opensearch/security/ssl/OpenSSLTest.java | 6 +- .../org/opensearch/security/ssl/SSLTest.java | 11 +++- .../security/support/Base64HelperTest.java | 2 - .../security/support/Base64JDKHelperTest.java | 37 ++++++++---- .../helper/cluster/ClusterConfiguration.java | 6 +- .../security/test/helper/rest/RestHelper.java | 21 ------- .../transport/SecurityInterceptorTests.java | 1 + 54 files changed, 392 insertions(+), 332 deletions(-) diff --git a/build.gradle b/build.gradle index d5c5025701..9a6b731389 100644 --- a/build.gradle +++ b/build.gradle @@ -95,6 +95,62 @@ spotbugsTest { java.sourceCompatibility = JavaVersion.VERSION_11 java.targetCompatibility = JavaVersion.VERSION_11 +compileJava { + options.compilerArgs = [ + '-Xlint:auxiliaryclass', + '-Xlint:cast', + '-Xlint:classfile', + '-Xlint:dep-ann', + '-Xlint:divzero', + '-Xlint:empty', + '-Xlint:exports', + '-Xlint:fallthrough', + '-Xlint:finally', + '-Xlint:module', + '-Xlint:opens', + '-Xlint:overloads', + '-Xlint:overrides', + '-Xlint:-processing', + '-Xlint:rawtypes', + '-Xlint:removal', + '-Xlint:requires-automatic', + '-Xlint:requires-transitive-automatic', + '-Xlint:static', + '-Xlint:unchecked', + '-Xlint:varargs', + '-Xlint:preview', + '-Werror'] + options.encoding = 'UTF-8' +} + +compileTestJava { + options.compilerArgs = [ + '-Xlint:auxiliaryclass', + '-Xlint:cast', + '-Xlint:classfile', + '-Xlint:dep-ann', + '-Xlint:divzero', + '-Xlint:empty', + '-Xlint:exports', + '-Xlint:fallthrough', + '-Xlint:finally', + '-Xlint:module', + '-Xlint:opens', + '-Xlint:overloads', + '-Xlint:overrides', + '-Xlint:-processing', + '-Xlint:rawtypes', + '-Xlint:removal', + '-Xlint:requires-automatic', + '-Xlint:requires-transitive-automatic', + '-Xlint:static', + '-Xlint:unchecked', + '-Xlint:varargs', + '-Xlint:preview', + '-Werror'] + options.encoding = 'UTF-8' +} + licenseHeaders.enabled = true // The following check that have never be enabled in security @@ -539,7 +595,7 @@ dependencies { runtimeOnly 'com.eclipsesource.minimal-json:minimal-json:0.9.5' runtimeOnly 'commons-codec:commons-codec:1.16.0' runtimeOnly 'org.cryptacular:cryptacular:1.2.5' - runtimeOnly 'com.google.errorprone:error_prone_annotations:2.22.0' + compileOnly 'com.google.errorprone:error_prone_annotations:2.22.0' runtimeOnly 'com.sun.istack:istack-commons-runtime:4.2.0' runtimeOnly 'jakarta.xml.bind:jakarta.xml.bind-api:4.0.1' runtimeOnly 'org.ow2.asm:asm:9.6' @@ -571,7 +627,7 @@ dependencies { runtimeOnly 'org.apache.commons:commons-text:1.10.0' runtimeOnly "org.glassfish.jaxb:jaxb-runtime:${jaxb_version}" runtimeOnly 'com.google.j2objc:j2objc-annotations:2.8' - runtimeOnly 'com.google.code.findbugs:jsr305:3.0.2' + compileOnly 'com.google.code.findbugs:jsr305:3.0.2' runtimeOnly 'org.lz4:lz4-java:1.8.0' runtimeOnly 'io.dropwizard.metrics:metrics-core:4.2.21' runtimeOnly 'org.slf4j:slf4j-api:1.7.36' diff --git a/src/integrationTest/java/org/opensearch/node/PluginAwareNode.java b/src/integrationTest/java/org/opensearch/node/PluginAwareNode.java index 53e44496ca..191c32646a 100644 --- a/src/integrationTest/java/org/opensearch/node/PluginAwareNode.java +++ b/src/integrationTest/java/org/opensearch/node/PluginAwareNode.java @@ -26,7 +26,7 @@ package org.opensearch.node; -import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import org.opensearch.common.settings.Settings; @@ -36,11 +36,14 @@ public class PluginAwareNode extends Node { private final boolean clusterManagerEligible; - @SafeVarargs - public PluginAwareNode(boolean clusterManagerEligible, final Settings preparedSettings, final Class... plugins) { + public PluginAwareNode( + boolean clusterManagerEligible, + final Settings preparedSettings, + final Collection> plugins + ) { super( InternalSettingsPreparer.prepareEnvironment(preparedSettings, Collections.emptyMap(), null, () -> System.getenv("HOSTNAME")), - Arrays.asList(plugins), + plugins, true ); this.clusterManagerEligible = clusterManagerEligible; diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/ClusterManager.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/ClusterManager.java index db786a65e9..0bf50c7a4d 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/ClusterManager.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/ClusterManager.java @@ -165,8 +165,8 @@ public Class[] getPlugins() { return plugins.toArray(new Class[0]); } - public Class[] pluginsWithAddition(List> additionalPlugins) { - return mergePlugins(plugins, additionalPlugins).toArray(Class[]::new); + public Collection> pluginsWithAddition(List> additionalPlugins) { + return mergePlugins(plugins, additionalPlugins); } } } diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java index 77890a4645..189ef79f7c 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java @@ -407,7 +407,7 @@ boolean hasAssignedType(NodeType type) { CompletableFuture start() { CompletableFuture completableFuture = new CompletableFuture<>(); - Class[] mergedPlugins = nodeSettings.pluginsWithAddition(additionalPlugins); + final Collection> mergedPlugins = nodeSettings.pluginsWithAddition(additionalPlugins); this.node = new PluginAwareNode(nodeSettings.containRole(NodeRole.CLUSTER_MANAGER), getOpenSearchSettings(), mergedPlugins); new Thread(new Runnable() { diff --git a/src/main/java/org/opensearch/security/DefaultObjectMapper.java b/src/main/java/org/opensearch/security/DefaultObjectMapper.java index 69e1d0ac83..f8564cb21b 100644 --- a/src/main/java/org/opensearch/security/DefaultObjectMapper.java +++ b/src/main/java/org/opensearch/security/DefaultObjectMapper.java @@ -70,6 +70,8 @@ public class DefaultObjectMapper { YAML_MAPPER.enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION); } + private DefaultObjectMapper() {} + public static void inject(final InjectableValues.Std injectableValues) { objectMapper.setInjectableValues(injectableValues); YAML_MAPPER.setInjectableValues(injectableValues); @@ -220,7 +222,7 @@ public static TypeFactory getTypeFactory() { return objectMapper.getTypeFactory(); } - public static Set getFields(Class cls) { + public static Set getFields(Class cls) { return objectMapper.getSerializationConfig() .introspect(getTypeFactory().constructType(cls)) .findProperties() diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 667f7f9a28..f3e4ae33cf 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -634,6 +634,7 @@ public void clear(String reason) { @Override public Weight doCache(Weight weight, QueryCachingPolicy policy) { + @SuppressWarnings("unchecked") final Map> allowedFlsFields = (Map>) HeaderHelper.deserializeSafeFromHeader( threadPool.getThreadContext(), ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER @@ -642,7 +643,7 @@ public Weight doCache(Weight weight, QueryCachingPolicy policy) { if (SecurityUtils.evalMap(allowedFlsFields, index().getName()) != null) { return weight; } else { - + @SuppressWarnings("unchecked") final Map> maskedFieldsMap = (Map>) HeaderHelper.deserializeSafeFromHeader( threadPool.getThreadContext(), ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER @@ -727,6 +728,7 @@ public void onQueryPhase(SearchContext searchContext, long tookInNanos) { return; } + @SuppressWarnings("unchecked") final Map> maskedFieldsMap = (Map>) HeaderHelper.deserializeSafeFromHeader( threadPool.getThreadContext(), ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER @@ -1864,6 +1866,7 @@ public Function> getFieldFilter() { if (threadPool == null) { return field -> true; } + @SuppressWarnings("unchecked") final Map> allowedFlsFields = (Map>) HeaderHelper.deserializeSafeFromHeader( threadPool.getThreadContext(), ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java index 0b2f5cbc7a..0e5709b2d8 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java @@ -22,7 +22,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; +import java.util.Properties; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; @@ -507,10 +507,7 @@ public void logDocumentRead(String index, String id, ShardId shardId, Map "id", - entry -> new String( - BaseEncoding.base64().decode(((Entry) entry).getValue()), - StandardCharsets.UTF_8 - ) + entry -> new String(BaseEncoding.base64().decode(entry.getValue()), StandardCharsets.UTF_8) ) ); msg.addSecurityConfigMapToRequestBody(Utils.convertJsonToxToStructuredMap(map.get("id")), id); @@ -711,19 +708,9 @@ protected void logExternalConfig() { sm.checkPermission(new SpecialPermission()); } - final Map envAsMap = AccessController.doPrivileged(new PrivilegedAction>() { - @Override - public Map run() { - return System.getenv(); - } - }); + final Map envAsMap = AccessController.doPrivileged((PrivilegedAction>) System::getenv); - final Map propsAsMap = AccessController.doPrivileged(new PrivilegedAction() { - @Override - public Map run() { - return System.getProperties(); - } - }); + final Properties propsAsMap = AccessController.doPrivileged((PrivilegedAction) System::getProperties); final String sha256 = DigestUtils.sha256Hex(configAsMap.toString() + envAsMap.toString() + propsAsMap.toString()); AuditMessage msg = new AuditMessage(AuditCategory.COMPLIANCE_EXTERNAL_CONFIG, clusterService, null, null); diff --git a/src/main/java/org/opensearch/security/auditlog/impl/RequestResolver.java b/src/main/java/org/opensearch/security/auditlog/impl/RequestResolver.java index 6a300b7e87..8a1177ec60 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/RequestResolver.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/RequestResolver.java @@ -376,6 +376,7 @@ private static AuditMessage resolveInner( return msg; } + @SuppressWarnings("unchecked") private static void addIndicesSourceSafe( final AuditMessage msg, final String[] indices, @@ -425,14 +426,15 @@ private static void addIndicesSourceSafe( if (source instanceof BytesReference) { msg.addTupleToRequestBody(convertSource(mediaType, (BytesReference) source)); } else { - msg.addMapToRequestBody((Map) source); + msg.addMapToRequestBody((Map) source); } } } else if (source != null) { if (source instanceof BytesReference) { msg.addTupleToRequestBody(convertSource(mediaType, (BytesReference) source)); } else { - msg.addMapToRequestBody((Map) source); + // noinspection unchecked + msg.addMapToRequestBody((Map) source); } } } diff --git a/src/main/java/org/opensearch/security/auditlog/routing/AuditMessageRouter.java b/src/main/java/org/opensearch/security/auditlog/routing/AuditMessageRouter.java index af120daf35..b56ac42a2a 100644 --- a/src/main/java/org/opensearch/security/auditlog/routing/AuditMessageRouter.java +++ b/src/main/java/org/opensearch/security/auditlog/routing/AuditMessageRouter.java @@ -101,6 +101,7 @@ protected final void close(List sinks) { } } + @SuppressWarnings("unchecked") public final void enableRoutes(Settings settings) { checkState(isEnabled(), "AuditMessageRouter is disabled"); if (categorySinks != null) { diff --git a/src/main/java/org/opensearch/security/auditlog/sink/InternalOpenSearchSink.java b/src/main/java/org/opensearch/security/auditlog/sink/InternalOpenSearchSink.java index 4a4af8d1fe..f01051c70f 100644 --- a/src/main/java/org/opensearch/security/auditlog/sink/InternalOpenSearchSink.java +++ b/src/main/java/org/opensearch/security/auditlog/sink/InternalOpenSearchSink.java @@ -71,7 +71,7 @@ public void close() throws IOException { public boolean doStore(final AuditMessage msg) { if (Boolean.parseBoolean( - (String) HeaderHelper.getSafeFromHeader(threadPool.getThreadContext(), ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER) + HeaderHelper.getSafeFromHeader(threadPool.getThreadContext(), ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER) )) { if (log.isTraceEnabled()) { log.trace("audit log of audit log will not be executed"); diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 980dc64094..118c82c7c5 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -63,7 +63,6 @@ import org.opensearch.security.filter.SecurityResponse; import org.opensearch.security.http.XFFResolver; import org.opensearch.security.securityconf.DynamicConfigModel; -import org.opensearch.security.ssl.util.Utils; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; @@ -364,7 +363,7 @@ public boolean authenticate(final SecurityRequestChannel request) { return false; } - final String tenant = Utils.coalesce(request.header("securitytenant"), request.header("security_tenant")); + final String tenant = resolveTenantFrom(request); if (isDebugEnabled) { log.debug("Rest user '{}' is authenticated", authenticatedUser); @@ -392,7 +391,7 @@ public boolean authenticate(final SecurityRequestChannel request) { } if (authCredentials == null && anonymousAuthEnabled) { - final String tenant = Utils.coalesce(request.header("securitytenant"), request.header("security_tenant")); + final String tenant = resolveTenantFrom(request); User anonymousUser = new User(User.ANONYMOUS.getName(), new HashSet(User.ANONYMOUS.getRoles()), null); anonymousUser.setRequestedTenant(tenant); @@ -437,6 +436,10 @@ public boolean authenticate(final SecurityRequestChannel request) { return authenticated; } + private String resolveTenantFrom(final SecurityRequest request) { + return Optional.ofNullable(request.header("securitytenant")).orElse(request.header("security_tenant")); + } + private void notifyIpAuthFailureListeners(SecurityRequestChannel request, AuthCredentials authCredentials) { notifyIpAuthFailureListeners(request.getRemoteAddress().map(InetSocketAddress::getAddress).orElse(null), authCredentials, request); } diff --git a/src/main/java/org/opensearch/security/auth/UserInjector.java b/src/main/java/org/opensearch/security/auth/UserInjector.java index 57750bfa9a..351afde0f1 100644 --- a/src/main/java/org/opensearch/security/auth/UserInjector.java +++ b/src/main/java/org/opensearch/security/auth/UserInjector.java @@ -31,6 +31,7 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.util.Arrays; +import java.util.HashMap; import java.util.Map; import com.google.common.base.Strings; @@ -45,7 +46,6 @@ import org.opensearch.security.filter.SecurityRequestChannel; import org.opensearch.security.http.XFFResolver; import org.opensearch.security.support.ConfigConstants; -import org.opensearch.security.support.SecurityUtils; import org.opensearch.security.user.User; import org.opensearch.threadpool.ThreadPool; @@ -150,7 +150,7 @@ public InjectedUser getInjectedUser() { // custom attributes if (parts.length > 3 && !Strings.isNullOrEmpty(parts[3])) { - Map attributes = SecurityUtils.mapFromArray((parts[3].split(","))); + Map attributes = mapFromArray((parts[3].split(","))); if (attributes == null) { log.error("Could not parse custom attributes {}, user injection failed.", parts[3]); return null; @@ -204,4 +204,21 @@ boolean injectUser(SecurityRequestChannel request) { return true; } + + protected Map mapFromArray(String... keyValues) { + if (keyValues == null) { + return Map.of(); + } + if (keyValues.length % 2 != 0) { + log.error("Expected even number of key/value pairs, got {}.", Arrays.toString(keyValues)); + return null; + } + Map map = new HashMap<>(); + + for (int i = 0; i < keyValues.length; i += 2) { + map.put(keyValues[i], keyValues[i + 1]); + } + return map; + } + } diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index b35137a35d..06c94c26b5 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -671,6 +671,7 @@ private static Field getField(Class cls, String name) { return AccessController.doPrivileged((PrivilegedAction) () -> getFieldPrivileged(cls, name)); } + @SuppressWarnings("unchecked") private static T getFieldValue(Field field, C c) { try { return (T) field.get(c); diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java index 5402c0ac16..f098de5fb9 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java @@ -222,7 +222,7 @@ public void onResponse(CreateIndexResponse response) { } br.execute( - new ConfigUpdatingActionListener( + new ConfigUpdatingActionListener<>( cTypes.build().toArray(new String[0]), client, new ActionListener() { diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java index d1aafa37e3..5889bf5504 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java @@ -23,13 +23,13 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.rest.RestRequest; import org.opensearch.security.DefaultObjectMapper; -import org.opensearch.security.ssl.util.Utils; import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Optional; import java.util.Set; import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE; @@ -247,10 +247,8 @@ private ValidationResult validatePassword(final RestRequest request, f this.validationError = ValidationError.INVALID_PASSWORD_TOO_SHORT; return ValidationResult.error(RestStatus.BAD_REQUEST, this); } - final String username = Utils.coalesce( - request.param("name"), - validationContext.hasParams() ? (String) validationContext.params()[0] : null - ); + final String username = Optional.ofNullable(request.param("name")) + .orElseGet(() -> validationContext.hasParams() ? (String) validationContext.params()[0] : null); if (Strings.isNullOrEmpty(username)) { if (LOGGER.isDebugEnabled()) { LOGGER.debug("Unable to validate username because no user is given"); diff --git a/src/main/java/org/opensearch/security/http/HTTPProxyAuthenticator.java b/src/main/java/org/opensearch/security/http/HTTPProxyAuthenticator.java index de57e2f9e6..e49248a0dc 100644 --- a/src/main/java/org/opensearch/security/http/HTTPProxyAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/HTTPProxyAuthenticator.java @@ -30,20 +30,20 @@ import java.util.Optional; import java.util.regex.Pattern; -import com.google.common.base.Predicates; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.OpenSearchSecurityException; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.core.common.Strings; import org.opensearch.security.auth.HTTPAuthenticator; import org.opensearch.security.filter.SecurityRequest; import org.opensearch.security.filter.SecurityResponse; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.AuthCredentials; +import static java.util.function.Predicate.not; + public class HTTPProxyAuthenticator implements HTTPAuthenticator { protected final Logger log = LogManager.getLogger(this.getClass()); @@ -63,30 +63,29 @@ public AuthCredentials extractCredentials(final SecurityRequest request, final T throw new OpenSearchSecurityException("xff not done"); } - final String userHeader = settings.get("user_header"); - final String rolesHeader = settings.get("roles_header"); + final Optional requestUserHeader = Optional.ofNullable(settings.get("user_header")) + .flatMap(userHeader -> Optional.ofNullable(request.header(userHeader))); + final Optional requestRolesHeader = Optional.ofNullable(settings.get("roles_header")) + .flatMap(rolesHeader -> Optional.ofNullable(request.header(rolesHeader))); if (log.isDebugEnabled()) { log.debug("Headers {}", request.getHeaders()); - log.debug("UserHeader {}, value {}", userHeader, userHeader == null ? null : request.header(userHeader)); - log.debug("RolesHeader {}, value {}", rolesHeader, rolesHeader == null ? null : request.header(rolesHeader)); + log.debug("UserHeader {}, value {}", settings.get("user_header"), requestUserHeader.orElse(null)); + log.debug("RolesHeader {}, value {}", settings.get("roles_header"), requestRolesHeader.orElse(null)); } - if (!Strings.isNullOrEmpty(userHeader) && !Strings.isNullOrEmpty((String) request.header(userHeader))) { - - String[] backendRoles = null; - - if (!Strings.isNullOrEmpty(rolesHeader) && !Strings.isNullOrEmpty((String) request.header(rolesHeader))) { - backendRoles = rolesSeparator.splitAsStream((String) request.header(rolesHeader)) + return requestUserHeader.map(userHeader -> { + final String[] backendRoles = requestRolesHeader.map( + rolesHeader -> rolesSeparator.splitAsStream(rolesHeader) .map(String::trim) - .filter(Predicates.not(String::isEmpty)) - .toArray(String[]::new); - } - return new AuthCredentials((String) request.header(userHeader), backendRoles).markComplete(); - } else { - log.trace("No '{}' header, send 401", userHeader); + .filter(not(String::isEmpty)) + .toArray(String[]::new) + ).orElse(null); + return new AuthCredentials(userHeader, backendRoles).markComplete(); + }).orElseGet(() -> { + log.trace("No '{}' header, send 401", settings.get("user_header")); return null; - } + }); } @Override diff --git a/src/main/java/org/opensearch/security/http/XFFResolver.java b/src/main/java/org/opensearch/security/http/XFFResolver.java index 64c5f4b60c..7fcbfad888 100644 --- a/src/main/java/org/opensearch/security/http/XFFResolver.java +++ b/src/main/java/org/opensearch/security/http/XFFResolver.java @@ -78,7 +78,7 @@ public TransportAddress resolve(final SecurityRequest request) throws OpenSearch if (isTraceEnabled) { log.trace("no xff done (enabled or no netty request) {},{},{},{}", enabled, request.getClass()); } - return new TransportAddress((InetSocketAddress) request.getRemoteAddress().get()); + return new TransportAddress(request.getRemoteAddress().get()); } else { throw new OpenSearchSecurityException( "Cannot handle this request. Remote address is " diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java index 84109c845e..545d383ced 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java @@ -155,19 +155,20 @@ private Set resolve(final SecurityDynamicConfiguration actionGroups, final Object actionGroupAsObject = actionGroups.getCEntries().get(entry); - if (actionGroupAsObject != null && actionGroupAsObject instanceof List) { - - for (final String perm : ((List) actionGroupAsObject)) { - if (actionGroups.getCEntries().keySet().contains(perm)) { + if (actionGroupAsObject instanceof List) { + @SuppressWarnings("unchecked") + final List actionGroupPermissions = (List) actionGroupAsObject; + for (final String perm : actionGroupPermissions) { + if (actionGroups.getCEntries().containsKey(perm)) { ret.addAll(resolve(actionGroups, perm)); } else { ret.add(perm); } } - } else if (actionGroupAsObject != null && actionGroupAsObject instanceof ActionGroupsV6) { + } else if (actionGroupAsObject instanceof ActionGroupsV6) { for (final String perm : ((ActionGroupsV6) actionGroupAsObject).getPermissions()) { - if (actionGroups.getCEntries().keySet().contains(perm)) { + if (actionGroups.getCEntries().containsKey(perm)) { ret.addAll(resolve(actionGroups, perm)); } else { ret.add(perm); diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java index fe6bb9ff4f..0ea8d4013d 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java @@ -136,6 +136,7 @@ private Set getGroupMembers(final String groupname) { return Collections.unmodifiableSet(resolve(actionGroups, groupname)); } + @SuppressWarnings("unchecked") private Set resolve(final SecurityDynamicConfiguration actionGroups, final String entry) { // SG5 format, plain array @@ -153,7 +154,7 @@ private Set resolve(final SecurityDynamicConfiguration actionGroups, final Object actionGroupAsObject = actionGroups.getCEntries().get(entry); - if (actionGroupAsObject != null && actionGroupAsObject instanceof List) { + if (actionGroupAsObject instanceof List) { for (final String perm : ((List) actionGroupAsObject)) { if (actionGroups.getCEntries().keySet().contains(perm)) { @@ -163,7 +164,7 @@ private Set resolve(final SecurityDynamicConfiguration actionGroups, } } - } else if (actionGroupAsObject != null && actionGroupAsObject instanceof ActionGroupsV7) { + } else if (actionGroupAsObject instanceof ActionGroupsV7) { for (final String perm : ((ActionGroupsV7) actionGroupAsObject).getAllowed_actions()) { if (actionGroups.getCEntries().keySet().contains(perm)) { ret.addAll(resolve(actionGroups, perm)); diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java index 106bd7956b..ecf91fb674 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java @@ -168,6 +168,7 @@ public DynamicConfigFactory( } @Override + @SuppressWarnings("unchecked") public void onChange(Map> typeToConfig) { SecurityDynamicConfiguration actionGroups = cr.getConfiguration(CType.ACTIONGROUPS); @@ -462,6 +463,7 @@ private static class NodesDnModelImpl extends NodesDnModel { SecurityDynamicConfiguration configuration; + @SuppressWarnings("unchecked") public NodesDnModelImpl(SecurityDynamicConfiguration configuration) { super(); this.configuration = null == configuration.getCType() diff --git a/src/main/java/org/opensearch/security/securityconf/Migration.java b/src/main/java/org/opensearch/security/securityconf/Migration.java index 4eeaba5c68..b5e3ef973d 100644 --- a/src/main/java/org/opensearch/security/securityconf/Migration.java +++ b/src/main/java/org/opensearch/security/securityconf/Migration.java @@ -200,6 +200,7 @@ public static SecurityDynamicConfiguration migrateInternalUsers( return i7; } + @SuppressWarnings("unchecked") public static SecurityDynamicConfiguration migrateActionGroups(SecurityDynamicConfiguration r6as) throws MigrationException { diff --git a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java index 4d10cb8ad6..5c5e9b8ecd 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java @@ -109,7 +109,7 @@ public static SecurityDynamicConfiguration fromJson( return sdc; } - public static void validate(SecurityDynamicConfiguration sdc, int version, CType ctype) throws IOException { + public static void validate(SecurityDynamicConfiguration sdc, int version, CType ctype) throws IOException { if (version < 2 && sdc.get_meta() != null) { throw new IOException("A version of " + version + " can not have a _meta key for " + ctype); } @@ -199,6 +199,7 @@ public T putCEntry(String key, T value) { } @JsonIgnore + @SuppressWarnings("unchecked") public void putCObject(String key, Object value) { centries.put(key, (T) value); } diff --git a/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java b/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java index d85e6a1d32..87a83d7d5b 100644 --- a/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java +++ b/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java @@ -883,17 +883,17 @@ private void initEnabledSSLCiphers() { } if (OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && OpenSsl.isAvailable() && OpenSsl.version() > 0x10101009L) { - enabledHttpProtocolsOpenSSLProvider = new ArrayList(Arrays.asList("TLSv1.3", "TLSv1.2", "TLSv1.1", "TLSv1")); + enabledHttpProtocolsOpenSSLProvider = new ArrayList<>(Arrays.asList("TLSv1.3", "TLSv1.2", "TLSv1.1", "TLSv1")); enabledHttpProtocolsOpenSSLProvider.retainAll(secureHttpSSLProtocols); - enabledTransportProtocolsOpenSSLProvider = new ArrayList(Arrays.asList("TLSv1.3", "TLSv1.2", "TLSv1.1")); + enabledTransportProtocolsOpenSSLProvider = new ArrayList<>(Arrays.asList("TLSv1.3", "TLSv1.2", "TLSv1.1")); enabledTransportProtocolsOpenSSLProvider.retainAll(secureTransportSSLProtocols); log.info("OpenSSL supports TLSv1.3"); } else if (OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && OpenSsl.isAvailable()) { - enabledHttpProtocolsOpenSSLProvider = new ArrayList(Arrays.asList("TLSv1.2", "TLSv1.1", "TLSv1")); + enabledHttpProtocolsOpenSSLProvider = new ArrayList<>(Arrays.asList("TLSv1.2", "TLSv1.1", "TLSv1")); enabledHttpProtocolsOpenSSLProvider.retainAll(secureHttpSSLProtocols); - enabledTransportProtocolsOpenSSLProvider = new ArrayList(Arrays.asList("TLSv1.2", "TLSv1.1")); + enabledTransportProtocolsOpenSSLProvider = new ArrayList<>(Arrays.asList("TLSv1.2", "TLSv1.1")); enabledTransportProtocolsOpenSSLProvider.retainAll(secureTransportSSLProtocols); } else { enabledHttpProtocolsOpenSSLProvider = Collections.emptyList(); diff --git a/src/main/java/org/opensearch/security/ssl/transport/SecuritySSLRequestHandler.java b/src/main/java/org/opensearch/security/ssl/transport/SecuritySSLRequestHandler.java index c67579e30f..63148d4ce6 100644 --- a/src/main/java/org/opensearch/security/ssl/transport/SecuritySSLRequestHandler.java +++ b/src/main/java/org/opensearch/security/ssl/transport/SecuritySSLRequestHandler.java @@ -190,7 +190,7 @@ public final void messageReceived(T request, TransportChannel channel, Task task protected TransportChannel getInnerChannel(TransportChannel transportChannel) throws Exception { try { - Class wrappedChannelCls = transportChannel.getClass(); + Class wrappedChannelCls = transportChannel.getClass(); Method getInnerChannel = wrappedChannelCls.getMethod("getInnerChannel"); TransportChannel innerChannel = (TransportChannel) (getInnerChannel.invoke(transportChannel)); log.debug("Using inner transport channel " + innerChannel.getChannelType()); diff --git a/src/main/java/org/opensearch/security/ssl/util/Utils.java b/src/main/java/org/opensearch/security/ssl/util/Utils.java index 00771d7804..6f2ea9de3f 100644 --- a/src/main/java/org/opensearch/security/ssl/util/Utils.java +++ b/src/main/java/org/opensearch/security/ssl/util/Utils.java @@ -17,27 +17,11 @@ package org.opensearch.security.ssl.util; -public class Utils { - public static T coalesce(T first, T... more) { - if (first != null) { - return first; - } - - if (more == null || more.length == 0) { - return null; - } - - for (int i = 0; i < more.length; i++) { - T t = more[i]; - if (t != null) { - return t; - } - } +import org.opensearch.core.common.Strings; - return null; - } +public class Utils { public static char[] toCharArray(String str) { - return (str == null || str.length() == 0) ? null : str.toCharArray(); + return Strings.isNullOrEmpty(str) ? null : str.toCharArray(); } } diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java index aa25bf7e89..1df5f23637 100644 --- a/src/main/java/org/opensearch/security/support/SecurityUtils.java +++ b/src/main/java/org/opensearch/security/support/SecurityUtils.java @@ -27,10 +27,7 @@ package org.opensearch.security.support; import java.nio.charset.StandardCharsets; -import java.util.Arrays; import java.util.Base64; -import java.util.Collections; -import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.Set; @@ -90,23 +87,6 @@ public static String evalMap(final Map> map, final String in return map.keySet().stream().filter(key -> WildcardMatcher.from(key).test(index)).findAny().orElse(null); } - @SafeVarargs - public static Map mapFromArray(T... keyValues) { - if (keyValues == null) { - return Collections.emptyMap(); - } - if (keyValues.length % 2 != 0) { - log.error("Expected even number of key/value pairs, got {}.", Arrays.toString(keyValues)); - return null; - } - Map map = new HashMap<>(); - - for (int i = 0; i < keyValues.length; i += 2) { - map.put(keyValues[i], keyValues[i + 1]); - } - return map; - } - public static String replaceEnvVars(String in, Settings settings) { if (in == null || in.isEmpty()) { return in; diff --git a/src/main/java/org/opensearch/security/support/WildcardMatcher.java b/src/main/java/org/opensearch/security/support/WildcardMatcher.java index 4d0fe4cfad..99c34b53ba 100644 --- a/src/main/java/org/opensearch/security/support/WildcardMatcher.java +++ b/src/main/java/org/opensearch/security/support/WildcardMatcher.java @@ -56,7 +56,7 @@ public boolean matchAny(Collection candidates) { } @Override - public boolean matchAny(String[] candidates) { + public boolean matchAny(String... candidates) { return true; } @@ -104,7 +104,7 @@ public boolean matchAny(Collection candidates) { } @Override - public boolean matchAny(String[] candidates) { + public boolean matchAny(String... candidates) { return false; } diff --git a/src/main/java/org/opensearch/security/user/UserService.java b/src/main/java/org/opensearch/security/user/UserService.java index c8733cb773..95241070ae 100644 --- a/src/main/java/org/opensearch/security/user/UserService.java +++ b/src/main/java/org/opensearch/security/user/UserService.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.ImmutableList; import org.apache.logging.log4j.LogManager; @@ -59,7 +60,7 @@ public class UserService { protected final Logger log = LogManager.getLogger(this.getClass()); ClusterService clusterService; - static ConfigurationRepository configurationRepository; + protected final ConfigurationRepository configurationRepository; String securityIndex; Client client; @@ -101,7 +102,7 @@ public UserService(ClusterService clusterService, ConfigurationRepository config * @param config CType whose data is to be loaded in-memory * @return configuration loaded with given CType data */ - protected static final SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent) { + protected final SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent) { SecurityDynamicConfiguration loaded = configurationRepository.getConfigurationsFromIndex( Collections.singleton(config), logComplianceEvent @@ -251,7 +252,7 @@ public String generateAuthToken(String accountName) throws IOException { String authToken = null; try { - DefaultObjectMapper mapper = new DefaultObjectMapper(); + final ObjectMapper mapper = DefaultObjectMapper.objectMapper; JsonNode accountDetails = mapper.readTree(internalUsersConfiguration.getCEntry(accountName).toString()); final ObjectNode contentAsNode = (ObjectNode) accountDetails; SecurityJsonNode securityJsonNode = new SecurityJsonNode(contentAsNode); diff --git a/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTest.java b/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTest.java index a24275590e..4bbf94f729 100755 --- a/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTest.java +++ b/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTest.java @@ -13,10 +13,9 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Paths; -import java.util.ArrayList; import java.util.Arrays; -import java.util.TreeSet; +import org.hamcrest.MatcherAssert; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -39,6 +38,8 @@ import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; +import static org.hamcrest.Matchers.hasItem; + public class LdapBackendTest { static { @@ -387,7 +388,7 @@ public void testLdapAuthorization() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("cn=Michael Jackson,ou=people,o=TEST", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo")); Assert.assertEquals(user.getName(), user.getUserEntry().getDn()); } @@ -507,7 +508,7 @@ public void testLdapAuthorizationRoleSearchUsername() throws Exception { Assert.assertEquals("Michael Jackson", user.getOriginalUsername()); Assert.assertEquals("cn=Michael Jackson,ou=people,o=TEST", user.getUserEntry().getDn()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo")); Assert.assertEquals(user.getName(), user.getUserEntry().getDn()); } @@ -530,7 +531,7 @@ public void testLdapAuthorizationOnly() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("jacksonm", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo")); } @Test @@ -552,7 +553,7 @@ public void testLdapAuthorizationNonDNEntry() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("jacksonm", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo-ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo-ceo")); } @Test @@ -575,7 +576,7 @@ public void testLdapAuthorizationNested() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(4, user.getRoles().size()); - Assert.assertEquals("nested1", new ArrayList(new TreeSet(user.getRoles())).get(1)); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested1")); } @Test @@ -599,8 +600,8 @@ public void testLdapAuthorizationNestedFilter() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); - Assert.assertEquals("nested2", new ArrayList(new TreeSet(user.getRoles())).get(1)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo")); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested2")); } @Test @@ -623,7 +624,7 @@ public void testLdapAuthorizationDnNested() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(4, user.getRoles().size()); - Assert.assertEquals("cn=nested1,ou=groups,o=TEST", new ArrayList(new TreeSet(user.getRoles())).get(1)); + MatcherAssert.assertThat(user.getRoles(), hasItem("cn=nested1,ou=groups,o=TEST")); } @Test @@ -647,7 +648,7 @@ public void testLdapAuthorizationDn() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("jacksonm", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("cn=ceo,ou=groups,o=TEST", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("cn=ceo,ou=groups,o=TEST")); } @Test @@ -761,8 +762,8 @@ public void testLdapAuthorizationNestedAttr() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(8, user.getRoles().size()); - Assert.assertEquals("nested3", new ArrayList(new TreeSet(user.getRoles())).get(4)); - Assert.assertEquals("rolemo4", new ArrayList(new TreeSet(user.getRoles())).get(7)); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested3")); + MatcherAssert.assertThat(user.getRoles(), hasItem("rolemo4")); } @Test @@ -788,8 +789,8 @@ public void testLdapAuthorizationNestedAttrFilter() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(6, user.getRoles().size()); - Assert.assertEquals("role2", new ArrayList(new TreeSet(user.getRoles())).get(4)); - Assert.assertEquals("nested1", new ArrayList(new TreeSet(user.getRoles())).get(2)); + MatcherAssert.assertThat(user.getRoles(), hasItem("role2")); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested1")); } @@ -867,8 +868,8 @@ public void testLdapAuthorizationNestedAttrNoRoleSearch() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(3, user.getRoles().size()); - Assert.assertEquals("nested3", new ArrayList(new TreeSet(user.getRoles())).get(1)); - Assert.assertEquals("rolemo4", new ArrayList(new TreeSet(user.getRoles())).get(2)); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested3")); + MatcherAssert.assertThat(user.getRoles(), hasItem("rolemo4")); } @Test @@ -1091,4 +1092,5 @@ public static void tearDown() throws Exception { } } + } diff --git a/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTestNewStyleConfig.java b/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTestNewStyleConfig.java index 8bc13eec48..3026ffcd61 100644 --- a/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTestNewStyleConfig.java +++ b/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTestNewStyleConfig.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.TreeSet; +import org.hamcrest.MatcherAssert; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -37,6 +38,8 @@ import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; +import static org.hamcrest.Matchers.hasItem; + public class LdapBackendTestNewStyleConfig { static { @@ -481,7 +484,7 @@ public void testLdapAuthorizationRoleSearchUsername() throws Exception { Assert.assertEquals("Michael Jackson", user.getOriginalUsername()); Assert.assertEquals("cn=Michael Jackson,ou=people,o=TEST", user.getUserEntry().getDn()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo")); Assert.assertEquals(user.getName(), user.getUserEntry().getDn()); } @@ -504,7 +507,7 @@ public void testLdapAuthorizationOnly() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("jacksonm", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo")); } @Test @@ -527,7 +530,7 @@ public void testLdapAuthorizationNested() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(4, user.getRoles().size()); - Assert.assertEquals("nested1", new ArrayList(new TreeSet(user.getRoles())).get(1)); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested1")); } @Test @@ -551,8 +554,8 @@ public void testLdapAuthorizationNestedFilter() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); - Assert.assertEquals("nested2", new ArrayList(new TreeSet(user.getRoles())).get(1)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo")); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested2")); } @Test @@ -575,7 +578,7 @@ public void testLdapAuthorizationDnNested() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(4, user.getRoles().size()); - Assert.assertEquals("cn=nested1,ou=groups,o=TEST", new ArrayList(new TreeSet(user.getRoles())).get(1)); + MatcherAssert.assertThat(user.getRoles(), hasItem("cn=nested1,ou=groups,o=TEST")); } @Test @@ -599,7 +602,7 @@ public void testLdapAuthorizationDn() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("jacksonm", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("cn=ceo,ou=groups,o=TEST", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("cn=ceo,ou=groups,o=TEST")); } @Test @@ -688,8 +691,8 @@ public void testLdapAuthorizationNestedAttr() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(8, user.getRoles().size()); - Assert.assertEquals("nested3", new ArrayList(new TreeSet(user.getRoles())).get(4)); - Assert.assertEquals("rolemo4", new ArrayList(new TreeSet(user.getRoles())).get(7)); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested3")); + MatcherAssert.assertThat(user.getRoles(), hasItem("rolemo4")); } @Test @@ -715,8 +718,8 @@ public void testLdapAuthorizationNestedAttrFilter() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(6, user.getRoles().size()); - Assert.assertEquals("role2", new ArrayList(new TreeSet(user.getRoles())).get(4)); - Assert.assertEquals("nested1", new ArrayList(new TreeSet(user.getRoles())).get(2)); + MatcherAssert.assertThat(user.getRoles(), hasItem("role2")); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested1")); } @@ -795,8 +798,8 @@ public void testLdapAuthorizationNestedAttrNoRoleSearch() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("spock", user.getName()); Assert.assertEquals(3, user.getRoles().size()); - Assert.assertEquals("nested3", new ArrayList(new TreeSet(user.getRoles())).get(1)); - Assert.assertEquals("rolemo4", new ArrayList(new TreeSet(user.getRoles())).get(2)); + MatcherAssert.assertThat(user.getRoles(), hasItem("nested3")); + MatcherAssert.assertThat(user.getRoles(), hasItem("rolemo4")); } @Test diff --git a/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestNewStyleConfig2.java b/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestNewStyleConfig2.java index a53e73772d..4bd9f92083 100644 --- a/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestNewStyleConfig2.java +++ b/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestNewStyleConfig2.java @@ -18,6 +18,7 @@ import java.util.TreeSet; import org.apache.commons.lang3.exception.ExceptionUtils; +import org.hamcrest.MatcherAssert; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -46,6 +47,8 @@ import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; +import static org.hamcrest.Matchers.hasItem; + @RunWith(Parameterized.class) public class LdapBackendTestNewStyleConfig2 { @@ -1048,7 +1051,7 @@ public void testLdapAuthorizationNonDNEntry() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("jacksonm", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo-ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo-ceo")); } @Test diff --git a/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestOldStyleConfig2.java b/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestOldStyleConfig2.java index 8c6f4dc85d..7f7d6646b2 100755 --- a/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestOldStyleConfig2.java +++ b/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestOldStyleConfig2.java @@ -18,6 +18,7 @@ import java.util.TreeSet; import org.apache.commons.lang3.exception.ExceptionUtils; +import org.hamcrest.MatcherAssert; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -46,6 +47,8 @@ import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; +import static org.hamcrest.Matchers.hasItem; + @RunWith(Parameterized.class) public class LdapBackendTestOldStyleConfig2 { @@ -972,7 +975,7 @@ public void testLdapAuthorizationNonDNEntry() throws Exception { Assert.assertNotNull(user); Assert.assertEquals("jacksonm", user.getName()); Assert.assertEquals(2, user.getRoles().size()); - Assert.assertEquals("ceo-ceo", new ArrayList(new TreeSet(user.getRoles())).get(0)); + MatcherAssert.assertThat(user.getRoles(), hasItem("ceo-ceo")); } @Test @@ -1098,4 +1101,5 @@ public static void tearDown() throws Exception { } } + } diff --git a/src/test/java/org/opensearch/node/PluginAwareNode.java b/src/test/java/org/opensearch/node/PluginAwareNode.java index 4cd7ff9247..19cda27e81 100644 --- a/src/test/java/org/opensearch/node/PluginAwareNode.java +++ b/src/test/java/org/opensearch/node/PluginAwareNode.java @@ -26,21 +26,24 @@ package org.opensearch.node; -import java.util.Arrays; -import java.util.Collections; - import org.opensearch.common.settings.Settings; import org.opensearch.plugins.Plugin; +import java.util.Collection; +import java.util.Collections; + public class PluginAwareNode extends Node { private final boolean clusterManagerEligible; - @SafeVarargs - public PluginAwareNode(boolean clusterManagerEligible, final Settings preparedSettings, final Class... plugins) { + public PluginAwareNode( + boolean clusterManagerEligible, + final Settings preparedSettings, + final Collection> plugins + ) { super( InternalSettingsPreparer.prepareEnvironment(preparedSettings, Collections.emptyMap(), null, () -> System.getenv("HOSTNAME")), - Arrays.asList(plugins), + plugins, true ); this.clusterManagerEligible = clusterManagerEligible; diff --git a/src/test/java/org/opensearch/security/ConfigTests.java b/src/test/java/org/opensearch/security/ConfigTests.java index dfebd635fc..4815f5a9b7 100644 --- a/src/test/java/org/opensearch/security/ConfigTests.java +++ b/src/test/java/org/opensearch/security/ConfigTests.java @@ -55,6 +55,7 @@ public void testEmptyConfig() throws Exception { } @Test + @SuppressWarnings("unchecked") public void testMigrate() throws Exception { Tuple, SecurityDynamicConfiguration> rolesResult = Migration.migrateRoles( diff --git a/src/test/java/org/opensearch/security/InitializationIntegrationTests.java b/src/test/java/org/opensearch/security/InitializationIntegrationTests.java index ed531b4111..7b74846906 100644 --- a/src/test/java/org/opensearch/security/InitializationIntegrationTests.java +++ b/src/test/java/org/opensearch/security/InitializationIntegrationTests.java @@ -154,7 +154,7 @@ public void testConfigHotReload() throws Exception { Header spock = encodeBasicHeader("spock", "spock"); for (Iterator iterator = clusterInfo.httpAdresses.iterator(); iterator.hasNext();) { - TransportAddress TransportAddress = (TransportAddress) iterator.next(); + TransportAddress TransportAddress = iterator.next(); HttpResponse res = rh.executeRequest( new HttpGet( "http://" @@ -186,7 +186,7 @@ public void testConfigHotReload() throws Exception { } for (Iterator iterator = clusterInfo.httpAdresses.iterator(); iterator.hasNext();) { - TransportAddress TransportAddress = (TransportAddress) iterator.next(); + TransportAddress TransportAddress = iterator.next(); log.debug("http://" + TransportAddress.getAddress() + ":" + TransportAddress.getPort()); HttpResponse res = rh.executeRequest( new HttpGet( @@ -218,7 +218,7 @@ public void testConfigHotReload() throws Exception { } for (Iterator iterator = clusterInfo.httpAdresses.iterator(); iterator.hasNext();) { - TransportAddress TransportAddress = (TransportAddress) iterator.next(); + TransportAddress TransportAddress = iterator.next(); HttpResponse res = rh.executeRequest( new HttpGet( "http://" diff --git a/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java b/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java index ec39e11345..6fd353f4d7 100644 --- a/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java +++ b/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java @@ -20,6 +20,7 @@ import java.util.Collection; import java.util.function.Supplier; +import com.google.common.collect.Lists; import org.junit.Assert; import org.junit.Test; @@ -117,9 +118,7 @@ public void testRolesInject() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - RolesInjectorPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesInjectorPlugin.class) ).start() ) { waitForInit(node.client()); @@ -137,9 +136,7 @@ public void testRolesInject() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - RolesInjectorPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesInjectorPlugin.class) ).start() ) { waitForInit(node.client()); @@ -159,9 +156,7 @@ public void testRolesInject() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - RolesInjectorPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesInjectorPlugin.class) ).start() ) { waitForInit(node.client()); diff --git a/src/test/java/org/opensearch/security/RolesValidationIntegTest.java b/src/test/java/org/opensearch/security/RolesValidationIntegTest.java index 0aa24b1f59..5e65a4438b 100644 --- a/src/test/java/org/opensearch/security/RolesValidationIntegTest.java +++ b/src/test/java/org/opensearch/security/RolesValidationIntegTest.java @@ -16,6 +16,7 @@ import java.util.Collection; import java.util.function.Supplier; +import com.google.common.collect.Lists; import org.junit.Assert; import org.junit.Test; @@ -100,9 +101,7 @@ public void testRolesValidation() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - RolesValidationPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesValidationPlugin.class) ).start() ) { waitForInit(node.client()); @@ -119,9 +118,7 @@ public void testRolesValidation() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - RolesValidationPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesValidationPlugin.class) ).start() ) { waitForInit(node.client()); @@ -138,9 +135,7 @@ public void testRolesValidation() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - RolesValidationPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesValidationPlugin.class) ).start() ) { waitForInit(node.client()); diff --git a/src/test/java/org/opensearch/security/SlowIntegrationTests.java b/src/test/java/org/opensearch/security/SlowIntegrationTests.java index 15640757d2..9fa97c3d36 100644 --- a/src/test/java/org/opensearch/security/SlowIntegrationTests.java +++ b/src/test/java/org/opensearch/security/SlowIntegrationTests.java @@ -28,6 +28,7 @@ import java.io.IOException; +import com.google.common.collect.Lists; import org.apache.http.HttpStatus; import org.junit.Assert; import org.junit.Test; @@ -108,7 +109,10 @@ public void testNodeClientAllowedWithServerCertificate() throws Exception { log.debug("Start node client"); - try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class).start()) { + try ( + Node node = new PluginAwareNode(false, tcSettings, Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class)) + .start() + ) { Assert.assertFalse( node.client() .admin() @@ -157,7 +161,10 @@ public void testNodeClientDisallowedWithNonServerCertificate() throws Exception log.debug("Start node client"); - try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class).start()) { + try ( + Node node = new PluginAwareNode(false, tcSettings, Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class)) + .start() + ) { Thread.sleep(10000); Assert.assertEquals(1, node.client().admin().cluster().nodesInfo(new NodesInfoRequest()).actionGet().getNodes().size()); } catch (Exception e) { @@ -199,7 +206,10 @@ public void testNodeClientDisallowedWithNonServerCertificate2() throws Exception log.debug("Start node client"); - try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class).start()) { + try ( + Node node = new PluginAwareNode(false, tcSettings, Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class)) + .start() + ) { Thread.sleep(10000); Assert.assertEquals(1, node.client().admin().cluster().nodesInfo(new NodesInfoRequest()).actionGet().getNodes().size()); } catch (Exception e) { diff --git a/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java b/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java index 4bfb0a675c..c693113d67 100644 --- a/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java +++ b/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java @@ -16,6 +16,7 @@ import java.util.Collection; import java.util.function.Supplier; +import com.google.common.collect.Lists; import org.junit.Assert; import org.junit.Test; @@ -93,8 +94,11 @@ public void testSecurityUserInjection() throws Exception { // 1. without user injection try ( - Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) - .start() + Node node = new PluginAwareNode( + false, + tcSettings, + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) + ).start() ) { waitForInit(node.client()); CreateIndexResponse cir = node.client().admin().indices().create(new CreateIndexRequest("captain-logs-1")).actionGet(); @@ -105,8 +109,11 @@ public void testSecurityUserInjection() throws Exception { UserInjectorPlugin.injectedUser = "ttt|kkk"; Exception exception = null; try ( - Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) - .start() + Node node = new PluginAwareNode( + false, + tcSettings, + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) + ).start() ) { waitForInit(node.client()); CreateIndexResponse cir = node.client().admin().indices().create(new CreateIndexRequest("captain-logs-2")).actionGet(); @@ -121,8 +128,11 @@ public void testSecurityUserInjection() throws Exception { // 3. with valid backend roles for injected user UserInjectorPlugin.injectedUser = "injectedadmin|injecttest"; try ( - Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) - .start() + Node node = new PluginAwareNode( + false, + tcSettings, + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) + ).start() ) { waitForInit(node.client()); CreateIndexResponse cir = node.client().admin().indices().create(new CreateIndexRequest("captain-logs-2")).actionGet(); @@ -151,8 +161,11 @@ public void testSecurityUserInjectionWithConfigDisabled() throws Exception { // 1. without user injection try ( - Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) - .start() + Node node = new PluginAwareNode( + false, + tcSettings, + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) + ).start() ) { waitForInit(node.client()); CreateIndexResponse cir = node.client().admin().indices().create(new CreateIndexRequest("captain-logs-1")).actionGet(); @@ -162,8 +175,11 @@ public void testSecurityUserInjectionWithConfigDisabled() throws Exception { // with invalid backend roles UserInjectorPlugin.injectedUser = "ttt|kkk"; try ( - Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) - .start() + Node node = new PluginAwareNode( + false, + tcSettings, + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class) + ).start() ) { waitForInit(node.client()); CreateIndexResponse cir = node.client().admin().indices().create(new CreateIndexRequest("captain-logs-2")).actionGet(); diff --git a/src/test/java/org/opensearch/security/UtilTests.java b/src/test/java/org/opensearch/security/UtilTests.java index 2127ada55f..f64c905667 100644 --- a/src/test/java/org/opensearch/security/UtilTests.java +++ b/src/test/java/org/opensearch/security/UtilTests.java @@ -38,7 +38,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; public class UtilTests { @@ -106,35 +105,6 @@ public void testWildcardMatchers() { assertTrue(!WildcardMatcher.from("ABC").test("abc")); } - @Test - public void testMapFromArray() { - Map map = SecurityUtils.mapFromArray((Object) null); - assertTrue(map == null); - - map = SecurityUtils.mapFromArray("key"); - assertTrue(map == null); - - map = SecurityUtils.mapFromArray("key", "value", "otherkey"); - assertTrue(map == null); - - map = SecurityUtils.mapFromArray("key", "value"); - assertNotNull(map); - assertEquals(1, map.size()); - assertEquals("value", map.get("key")); - - map = SecurityUtils.mapFromArray("key", "value", "key", "value"); - assertNotNull(map); - assertEquals(1, map.size()); - assertEquals("value", map.get("key")); - - map = SecurityUtils.mapFromArray("key1", "value1", "key2", "value2"); - assertNotNull(map); - assertEquals(2, map.size()); - assertEquals("value1", map.get("key1")); - assertEquals("value2", map.get("key2")); - - } - @Test public void testEnvReplace() { Settings settings = Settings.EMPTY; diff --git a/src/test/java/org/opensearch/security/auth/UserInjectorTest.java b/src/test/java/org/opensearch/security/auth/UserInjectorTest.java index e9570b1455..df89b09981 100644 --- a/src/test/java/org/opensearch/security/auth/UserInjectorTest.java +++ b/src/test/java/org/opensearch/security/auth/UserInjectorTest.java @@ -13,6 +13,7 @@ import java.util.Arrays; import java.util.HashSet; +import java.util.Map; import org.junit.Before; import org.junit.Test; @@ -29,6 +30,7 @@ import org.opensearch.transport.TransportRequest; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.mockito.Mockito.mock; @@ -75,4 +77,34 @@ public void testEmptyInjectUserHeader() { User injectedUser = userInjector.getInjectedUser(); assertNull(injectedUser); } + + @Test + public void testMapFromArray() { + Map map = userInjector.mapFromArray((String) null); + assertNull(map); + + map = userInjector.mapFromArray("key"); + assertNull(map); + + map = userInjector.mapFromArray("key", "value", "otherkey"); + assertNull(map); + + map = userInjector.mapFromArray("key", "value"); + assertNotNull(map); + assertEquals(1, map.size()); + assertEquals("value", map.get("key")); + + map = userInjector.mapFromArray("key", "value", "key", "value"); + assertNotNull(map); + assertEquals(1, map.size()); + assertEquals("value", map.get("key")); + + map = userInjector.mapFromArray("key1", "value1", "key2", "value2"); + assertNotNull(map); + assertEquals(2, map.size()); + assertEquals("value1", map.get("key1")); + assertEquals("value2", map.get("key2")); + + } + } diff --git a/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java b/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java index ea9d684ca7..dde353446f 100644 --- a/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java +++ b/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java @@ -26,6 +26,7 @@ package org.opensearch.security.ccstest; +import com.google.common.collect.Lists; import org.apache.http.HttpStatus; import org.junit.After; import org.junit.Assert; @@ -1458,9 +1459,7 @@ public void testCcsWithRoleInjection() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - RolesInjectorIntegTest.RolesInjectorPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesInjectorIntegTest.RolesInjectorPlugin.class) ).start() ) { waitForInit(node.client()); @@ -1485,9 +1484,7 @@ public void testCcsWithRoleInjection() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - RolesInjectorIntegTest.RolesInjectorPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesInjectorIntegTest.RolesInjectorPlugin.class) ).start() ) { waitForInit(node.client()); diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java index c9b76beae0..411a98af1a 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.function.Supplier; +import com.google.common.collect.Lists; import org.junit.Assert; import org.junit.Test; @@ -231,9 +232,7 @@ public void testReplication() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - MockReplicationPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class) ).start() ) { waitOrThrow(node.client(), "hr-dls"); @@ -251,9 +250,7 @@ public void testReplication() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - MockReplicationPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class) ).start() ) { waitOrThrow(node.client(), "hr-fls"); @@ -271,9 +268,7 @@ public void testReplication() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - MockReplicationPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class) ).start() ) { waitOrThrow(node.client(), "hr-masking"); @@ -291,9 +286,7 @@ public void testReplication() throws Exception { Node node = new PluginAwareNode( false, tcSettings, - Netty4Plugin.class, - OpenSearchSecurityPlugin.class, - MockReplicationPlugin.class + Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class) ).start() ) { waitOrThrow(node.client(), "hr-normal"); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java index 2314e32e89..2a0c5913c2 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java @@ -11,18 +11,11 @@ package org.opensearch.security.dlic.rest.api; -import java.io.IOException; -import java.util.Arrays; -import java.util.Collection; -import java.util.HashMap; import java.util.Map; import java.util.Objects; import java.util.stream.Stream; -import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import org.apache.http.Header; @@ -30,7 +23,6 @@ import org.junit.Assert; import org.opensearch.common.settings.Settings; -import org.opensearch.plugins.Plugin; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.auditlog.AuditTestUtils; import org.opensearch.security.dlic.rest.validation.PasswordValidator; @@ -256,41 +248,14 @@ protected void assertHealthy() throws Exception { ); } - protected Settings defaultNodeSettings(boolean enableRestSSL) { - Settings.Builder builder = Settings.builder(); - - if (enableRestSSL) { - builder.put("plugins.security.ssl.http.enabled", true) - .put( - "plugins.security.ssl.http.keystore_filepath", - FileHelper.getAbsoluteFilePathFromClassPath("restapi/node-0-keystore.jks") - ) - .put( - "plugins.security.ssl.http.truststore_filepath", - FileHelper.getAbsoluteFilePathFromClassPath("restapi/truststore.jks") - ); - } - return builder.build(); - } - - protected Map jsonStringToMap(String json) throws JsonParseException, JsonMappingException, IOException { - TypeReference> typeRef = new TypeReference>() { - }; - return DefaultObjectMapper.objectMapper.readValue(json, typeRef); - } - - protected static Collection> asCollection(Class... plugins) { - return Arrays.asList(plugins); - } - String createRestAdminPermissionsPayload(String... additionPerms) throws JsonProcessingException { - final ObjectNode rootNode = (ObjectNode) DefaultObjectMapper.objectMapper.createObjectNode(); + final ObjectNode rootNode = DefaultObjectMapper.objectMapper.createObjectNode(); rootNode.set("cluster_permissions", clusterPermissionsForRestAdmin(additionPerms)); return DefaultObjectMapper.objectMapper.writeValueAsString(rootNode); } ArrayNode clusterPermissionsForRestAdmin(String... additionPerms) { - final ArrayNode permissionsArray = (ArrayNode) DefaultObjectMapper.objectMapper.createArrayNode(); + final ArrayNode permissionsArray = DefaultObjectMapper.objectMapper.createArrayNode(); for (final Map.Entry< Endpoint, RestApiAdminPrivilegesEvaluator.PermissionBuilder> entry : RestApiAdminPrivilegesEvaluator.ENDPOINTS_WITH_PERMISSIONS diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java index dd945494ec..eb2991b251 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java @@ -776,7 +776,7 @@ public void testDeleteRestApiAdminRoleForbiddenForNonSuperAdmin() throws Excepti } private String createPatchRestAdminPermissionsPayload(final String op) throws JsonProcessingException { - final ArrayNode rootNode = (ArrayNode) DefaultObjectMapper.objectMapper.createArrayNode(); + final ArrayNode rootNode = DefaultObjectMapper.objectMapper.createArrayNode(); final ObjectNode opAddObjectNode = DefaultObjectMapper.objectMapper.createObjectNode(); final ObjectNode clusterPermissionsNode = DefaultObjectMapper.objectMapper.createObjectNode(); clusterPermissionsNode.set("cluster_permissions", clusterPermissionsForRestAdmin("cluster/*")); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java index 85a8dc24f0..b331743816 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java @@ -12,8 +12,12 @@ package org.opensearch.security.dlic.rest.api; import java.net.URLEncoder; +import java.util.ArrayList; import java.util.Base64; import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import org.apache.http.Header; import org.apache.http.HttpStatus; @@ -118,7 +122,7 @@ public void testParallelPutRequests() throws Exception { rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; - HttpResponse[] responses = rh.executeMultipleAsyncPutRequest( + HttpResponse[] responses = executeMultipleAsyncPutRequest( 10, ENDPOINT + "/internalusers/test1", "{\"password\":\"test1test1test1test1test1test1\"}" @@ -141,6 +145,27 @@ public void testParallelPutRequests() throws Exception { deleteUser("test1"); } + private HttpResponse[] executeMultipleAsyncPutRequest(final int numOfRequests, final String request, String body) throws Exception { + final ExecutorService executorService = Executors.newFixedThreadPool(numOfRequests); + try { + List> futures = new ArrayList<>(numOfRequests); + for (int i = 0; i < numOfRequests; i++) { + futures.add(executorService.submit(() -> rh.executePutRequest(request, body))); + } + return futures.stream().map(this::from).toArray(HttpResponse[]::new); + } finally { + executorService.shutdown(); + } + } + + private HttpResponse from(Future future) { + try { + return future.get(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + @Test public void testUserApi() throws Exception { diff --git a/src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java b/src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java index e31e4de425..0da733f5a2 100644 --- a/src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java +++ b/src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java @@ -11,7 +11,6 @@ package org.opensearch.security.multitenancy.test; -import java.util.HashMap; import java.util.Map; import org.apache.http.Header; @@ -336,12 +335,12 @@ public void testMtMulti() throws Exception { + "\"index-pattern\" : {" + "\"title\" : \"humanresources\"" + "}}"; - Map indexSettings = new HashMap(); - indexSettings.put("number_of_shards", 1); - indexSettings.put("number_of_replicas", 0); tc.admin() .indices() - .create(new CreateIndexRequest(dashboardsIndex).settings(indexSettings).alias(new Alias(".kibana_92668751_admin"))) + .create( + new CreateIndexRequest(dashboardsIndex).settings(Map.of("number_of_shards", 1, "number_of_replicas", 0)) + .alias(new Alias(".kibana_92668751_admin")) + ) .actionGet(); tc.index( @@ -479,12 +478,12 @@ public void testDashboardsAlias() throws Exception { try (Client tc = getClient()) { String body = "{\"buildNum\": 15460, \"defaultIndex\": \"humanresources\", \"tenant\": \"human_resources\"}"; - Map indexSettings = new HashMap(); - indexSettings.put("number_of_shards", 1); - indexSettings.put("number_of_replicas", 0); tc.admin() .indices() - .create(new CreateIndexRequest(".kibana-6").alias(new Alias(".kibana")).settings(indexSettings)) + .create( + new CreateIndexRequest(".kibana-6").alias(new Alias(".kibana")) + .settings(Map.of("number_of_shards", 1, "number_of_replicas", 0)) + ) .actionGet(); tc.index(new IndexRequest(".kibana-6").id("6.2.2").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source(body, XContentType.JSON)) @@ -512,12 +511,12 @@ public void testDashboardsAlias65() throws Exception { try (Client tc = getClient()) { String body = "{\"buildNum\": 15460, \"defaultIndex\": \"humanresources\", \"tenant\": \"human_resources\"}"; - Map indexSettings = new HashMap(); - indexSettings.put("number_of_shards", 1); - indexSettings.put("number_of_replicas", 0); tc.admin() .indices() - .create(new CreateIndexRequest(".kibana_1").alias(new Alias(".kibana")).settings(indexSettings)) + .create( + new CreateIndexRequest(".kibana_1").alias(new Alias(".kibana")) + .settings(Map.of("number_of_shards", 1, "number_of_replicas", 0)) + ) .actionGet(); tc.index(new IndexRequest(".kibana_1").id("6.2.2").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source(body, XContentType.JSON)) diff --git a/src/test/java/org/opensearch/security/ssl/OpenSSLTest.java b/src/test/java/org/opensearch/security/ssl/OpenSSLTest.java index 30a2f4b5ee..2a60ebb8e7 100644 --- a/src/test/java/org/opensearch/security/ssl/OpenSSLTest.java +++ b/src/test/java/org/opensearch/security/ssl/OpenSSLTest.java @@ -21,6 +21,7 @@ import java.util.Random; import java.util.Set; +import com.google.common.collect.Lists; import io.netty.handler.ssl.OpenSsl; import io.netty.util.internal.PlatformDependent; import org.junit.AfterClass; @@ -206,7 +207,10 @@ public void testNodeClientSSLwithOpenSslTLSv13() throws Exception { .put(settings)// ----- .build(); - try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class).start()) { + try ( + Node node = new PluginAwareNode(false, tcSettings, Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class)) + .start() + ) { ClusterHealthResponse res = node.client() .admin() .cluster() diff --git a/src/test/java/org/opensearch/security/ssl/SSLTest.java b/src/test/java/org/opensearch/security/ssl/SSLTest.java index 937d2d73b1..c8f215bd57 100644 --- a/src/test/java/org/opensearch/security/ssl/SSLTest.java +++ b/src/test/java/org/opensearch/security/ssl/SSLTest.java @@ -30,6 +30,7 @@ import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; +import com.google.common.collect.Lists; import io.netty.util.internal.PlatformDependent; import org.apache.http.NoHttpResponseException; import org.apache.lucene.util.Constants; @@ -746,7 +747,10 @@ public void testNodeClientSSL() throws Exception { .put(settings)// ----- .build(); - try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class).start()) { + try ( + Node node = new PluginAwareNode(false, tcSettings, Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class)) + .start() + ) { ClusterHealthResponse res = node.client() .admin() .cluster() @@ -991,7 +995,10 @@ public void testNodeClientSSLwithJavaTLSv13() throws Exception { .put(settings)// ----- .build(); - try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class).start()) { + try ( + Node node = new PluginAwareNode(false, tcSettings, Lists.newArrayList(Netty4Plugin.class, OpenSearchSecurityPlugin.class)) + .start() + ) { ClusterHealthResponse res = node.client() .admin() .cluster() diff --git a/src/test/java/org/opensearch/security/support/Base64HelperTest.java b/src/test/java/org/opensearch/security/support/Base64HelperTest.java index f55581c7e7..3bc81aaebc 100644 --- a/src/test/java/org/opensearch/security/support/Base64HelperTest.java +++ b/src/test/java/org/opensearch/security/support/Base64HelperTest.java @@ -47,7 +47,5 @@ public void testEnsureJDKSerialized() { String customSerialized = Base64Helper.serializeObject(test, false); Assert.assertEquals(jdkSerialized, Base64Helper.ensureJDKSerialized(jdkSerialized)); Assert.assertEquals(jdkSerialized, Base64Helper.ensureJDKSerialized(customSerialized)); - } - } diff --git a/src/test/java/org/opensearch/security/support/Base64JDKHelperTest.java b/src/test/java/org/opensearch/security/support/Base64JDKHelperTest.java index 704f1dc1d7..341c4a8659 100644 --- a/src/test/java/org/opensearch/security/support/Base64JDKHelperTest.java +++ b/src/test/java/org/opensearch/security/support/Base64JDKHelperTest.java @@ -11,8 +11,17 @@ package org.opensearch.security.support; -import com.amazon.dlic.auth.ldap.LdapUser; -import com.google.common.io.BaseEncoding; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertThrows; +import static org.hamcrest.Matchers.containsString; + +import java.io.ByteArrayOutputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; +import java.net.InetSocketAddress; +import java.util.ArrayList; +import java.util.HashMap; + import org.junit.Assert; import org.junit.Test; import org.ldaptive.LdapEntry; @@ -22,12 +31,8 @@ import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; -import java.io.ByteArrayOutputStream; -import java.io.ObjectOutputStream; -import java.io.Serializable; -import java.net.InetSocketAddress; -import java.util.ArrayList; -import java.util.HashMap; +import com.amazon.dlic.auth.ldap.LdapUser; +import com.google.common.io.BaseEncoding; public class Base64JDKHelperTest { private static final class NotSafeSerializable implements Serializable { @@ -88,18 +93,26 @@ public void testArrayList() { Assert.assertEquals(list, ds(list)); } - @Test(expected = OpenSearchException.class) + @Test public void notSafeSerializable() { - Base64JDKHelper.serializeObject(new NotSafeSerializable()); + final OpenSearchException exception = assertThrows( + OpenSearchException.class, + () -> Base64JDKHelper.serializeObject(new NotSafeSerializable()) + ); + assertThat(exception.getMessage(), containsString("NotSafeSerializable is not serializable")); } - @Test(expected = OpenSearchException.class) + @Test public void notSafeDeserializable() throws Exception { final ByteArrayOutputStream bos = new ByteArrayOutputStream(); try (final ObjectOutputStream out = new ObjectOutputStream(bos)) { out.writeObject(new NotSafeSerializable()); } - Base64JDKHelper.deserializeObject(BaseEncoding.base64().encode(bos.toByteArray())); + final OpenSearchException exception = assertThrows( + OpenSearchException.class, + () -> Base64JDKHelper.deserializeObject(BaseEncoding.base64().encode(bos.toByteArray())) + ); + assertThat(exception.getMessage(), containsString("Unauthorized deserialization attempt")); } @Test diff --git a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java index f8bea4c476..d8d51196e2 100644 --- a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java +++ b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java @@ -27,6 +27,7 @@ package org.opensearch.security.test.helper.cluster; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -117,6 +118,7 @@ public int getClientNodes() { public static class NodeSettings { public boolean clusterManagerNode; public boolean dataNode; + public List> plugins = Lists.newArrayList( Netty4Plugin.class, OpenSearchSecurityPlugin.class, @@ -143,8 +145,8 @@ public NodeSettings removePluginIfPresent(Class pluginToRemove return this; } - public Class[] getPlugins() { - return plugins.toArray(new Class[0]); + public Collection> getPlugins() { + return List.copyOf(plugins); } } } diff --git a/src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java b/src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java index b80741bfd5..40c733c7fc 100644 --- a/src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java +++ b/src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java @@ -35,9 +35,6 @@ import java.util.Collections; import java.util.List; import java.util.Scanner; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -131,16 +128,6 @@ public String executeSimpleRequest(final String request) throws Exception { } } - public HttpResponse[] executeMultipleAsyncPutRequest(final int numOfRequests, final String request, String body) throws Exception { - final ExecutorService executorService = Executors.newFixedThreadPool(numOfRequests); - Future[] futures = new Future[numOfRequests]; - for (int i = 0; i < numOfRequests; i++) { - futures[i] = executorService.submit(() -> executePutRequest(request, body, new Header[0])); - } - executorService.shutdown(); - return Arrays.stream(futures).map(HttpResponse::from).toArray(s -> new HttpResponse[s]); - } - public HttpResponse executeGetRequest(final String request, Header... header) { return executeRequest(new HttpGet(getHttpServerUri() + "/" + request), header); } @@ -485,14 +472,6 @@ public String findValueInJson(final String jsonDotPath) { return currentNode.asText(); } } - - private static HttpResponse from(Future future) { - try { - return future.get(); - } catch (Exception e) { - throw new RuntimeException(e); - } - } } } diff --git a/src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java b/src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java index abc0e314ef..ea97a5897a 100644 --- a/src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java +++ b/src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java @@ -139,6 +139,7 @@ private void testSendRequestDecorate(Version remoteNodeVersion) { String action = "testAction"; TransportRequest request = mock(TransportRequest.class); TransportRequestOptions options = mock(TransportRequestOptions.class); + @SuppressWarnings("unchecked") TransportResponseHandler handler = mock(TransportResponseHandler.class); InetAddress localAddress = null;