Merge remote-tracking branch 'upstream/main' into bump-jjwt

cwperks · Oct 16, 2023 · 904fe57 · 904fe57
2 parents e462672 + ece5e55
commit 904fe57
Show file tree

Hide file tree

Showing 60 changed files with 714 additions and 515 deletions.
diff --git a/build.gradle b/build.gradle
@@ -30,7 +30,7 @@ buildscript {
         open_saml_version = '4.3.0'
         one_login_java_saml = '2.9.0'
         jjwt_version = '0.12.3'
-        guava_version = '32.1.2-jre'
+        guava_version = '32.1.3-jre'
         jaxb_version = '2.3.8'
 
         if (buildVersionQualifier) {
@@ -95,6 +95,64 @@ 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
@@ -323,9 +381,17 @@ jacocoTestReport {
 
 checkstyle {
     toolVersion "10.3.3"
+    showViolations true
     configDirectory.set(rootProject.file("checkstyle/"))
 }
 
+tasks.withType(Checkstyle) {
+    reports {
+        ignoreFailures = false
+    }
+}
+
+
 opensearchplugin {
     name 'opensearch-security'
     description 'Provide access control related features for OpenSearch'
@@ -383,20 +449,6 @@ repositories {
     maven { url "https://build.shibboleth.net/nexus/content/repositories/releases" }
 }
 
-tasks.withType(Checkstyle) {
-    showViolations true
-    reports {
-        ignoreFailures = false
-    }
-}
-
-tasks.withType(JavaCompile) {
-    configure(options) {
-        options.encoding = 'UTF-8'
-        options.compilerArgs << '-Xlint:removal' << '-Werror'
-    }
-}
-
 tasks.test.finalizedBy(jacocoTestReport)  // report is always generated after tests run
 
 allprojects {
@@ -537,9 +589,9 @@ 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.6'
-    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.0'
+    runtimeOnly 'jakarta.xml.bind:jakarta.xml.bind-api:4.0.1'
     runtimeOnly 'org.ow2.asm:asm:9.6'
 
     testImplementation 'org.apache.camel:camel-xmlsecurity:3.21.1'
@@ -569,7 +621,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
@@ -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<? extends Plugin>... plugins) {
+    public PluginAwareNode(
+        boolean clusterManagerEligible,
+        final Settings preparedSettings,
+        final Collection<Class<? extends Plugin>> 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/security/CrossClusterSearchTests.java b/src/integrationTest/java/org/opensearch/security/CrossClusterSearchTests.java
@@ -12,6 +12,7 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
+import java.util.Arrays;
 
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
@@ -245,18 +246,17 @@ public void shouldSearchForDocumentOnBothClustersWhenIndexOnBothClusterArePointe
             SearchRequest searchRequest = SearchRequestFactory.searchAll(REMOTE_SONG_INDEX, SONG_INDEX_NAME);
             searchRequest.setCcsMinimizeRoundtrips(ccsMinimizeRoundtrips);
 
+            List<Pair<String, String>> documentIdsList = Arrays.asList(
+                Pair.of(SONG_INDEX_NAME, SONG_ID_1R),
+                Pair.of(SONG_INDEX_NAME, SONG_ID_2L),
+                Pair.of(SONG_INDEX_NAME, SONG_ID_6R)
+            );
+
             SearchResponse response = restHighLevelClient.search(searchRequest, DEFAULT);
 
             assertThat(response, isSuccessfulSearchResponse());
             assertThat(response, numberOfTotalHitsIsEqualTo(3));
-            assertThat(
-                response,
-                searchHitsContainDocumentsInAnyOrder(
-                    Pair.of(SONG_INDEX_NAME, SONG_ID_1R),
-                    Pair.of(SONG_INDEX_NAME, SONG_ID_2L),
-                    Pair.of(SONG_INDEX_NAME, SONG_ID_6R)
-                )
-            );
+            assertThat(response, searchHitsContainDocumentsInAnyOrder(documentIdsList));
         }
     }
 
@@ -286,19 +286,18 @@ public void shouldSearchViaAllAliasOnRemoteCluster_positive() throws IOException
         try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(ADMIN_USER)) {
             SearchRequest searchRequest = searchAll(REMOTE_CLUSTER_NAME + ":_all");
 
+            List<Pair<String, String>> documentIdsList = Arrays.asList(
+                Pair.of(SONG_INDEX_NAME, SONG_ID_1R),
+                Pair.of(SONG_INDEX_NAME, SONG_ID_6R),
+                Pair.of(PROHIBITED_SONG_INDEX_NAME, SONG_ID_3R),
+                Pair.of(LIMITED_USER_INDEX_NAME, SONG_ID_5R)
+            );
+
             SearchResponse response = restHighLevelClient.search(searchRequest, DEFAULT);
 
             assertThat(response, isSuccessfulSearchResponse());
             assertThat(response, numberOfTotalHitsIsEqualTo(4));
-            assertThat(
-                response,
-                searchHitsContainDocumentsInAnyOrder(
-                    Pair.of(SONG_INDEX_NAME, SONG_ID_1R),
-                    Pair.of(SONG_INDEX_NAME, SONG_ID_6R),
-                    Pair.of(PROHIBITED_SONG_INDEX_NAME, SONG_ID_3R),
-                    Pair.of(LIMITED_USER_INDEX_NAME, SONG_ID_5R)
-                )
-            );
+            assertThat(response, searchHitsContainDocumentsInAnyOrder(documentIdsList));
         }
     }
 
@@ -316,19 +315,18 @@ public void shouldSearchAllIndexOnRemoteClusterWhenStarIsUsedAsIndexName_positiv
         try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(ADMIN_USER)) {
             SearchRequest searchRequest = searchAll(REMOTE_CLUSTER_NAME + ":*");
 
+            List<Pair<String, String>> documentIdsList = Arrays.asList(
+                Pair.of(SONG_INDEX_NAME, SONG_ID_1R),
+                Pair.of(SONG_INDEX_NAME, SONG_ID_6R),
+                Pair.of(PROHIBITED_SONG_INDEX_NAME, SONG_ID_3R),
+                Pair.of(LIMITED_USER_INDEX_NAME, SONG_ID_5R)
+            );
+
             SearchResponse response = restHighLevelClient.search(searchRequest, DEFAULT);
 
             assertThat(response, isSuccessfulSearchResponse());
             assertThat(response, numberOfTotalHitsIsEqualTo(4));
-            assertThat(
-                response,
-                searchHitsContainDocumentsInAnyOrder(
-                    Pair.of(SONG_INDEX_NAME, SONG_ID_1R),
-                    Pair.of(SONG_INDEX_NAME, SONG_ID_6R),
-                    Pair.of(PROHIBITED_SONG_INDEX_NAME, SONG_ID_3R),
-                    Pair.of(LIMITED_USER_INDEX_NAME, SONG_ID_5R)
-                )
-            );
+            assertThat(response, searchHitsContainDocumentsInAnyOrder(documentIdsList));
         }
     }
 

diff --git 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<? extends Plugin>[] getPlugins() {
             return plugins.toArray(new Class[0]);
         }
 
-        public Class<? extends Plugin>[] pluginsWithAddition(List<Class<? extends Plugin>> additionalPlugins) {
-            return mergePlugins(plugins, additionalPlugins).toArray(Class[]::new);
+        public Collection<Class<? extends Plugin>> pluginsWithAddition(List<Class<? extends Plugin>> 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
@@ -407,7 +407,7 @@ boolean hasAssignedType(NodeType type) {
 
         CompletableFuture<StartStage> start() {
             CompletableFuture<StartStage> completableFuture = new CompletableFuture<>();
-            Class<? extends Plugin>[] mergedPlugins = nodeSettings.pluginsWithAddition(additionalPlugins);
+            final Collection<Class<? extends Plugin>> 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
@@ -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<String> getFields(Class cls) {
+    public static Set<String> 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
@@ -650,6 +650,7 @@ public void clear(String reason) {
 
                 @Override
                 public Weight doCache(Weight weight, QueryCachingPolicy policy) {
+                    @SuppressWarnings("unchecked")
                     final Map<String, Set<String>> allowedFlsFields = (Map<String, Set<String>>) HeaderHelper.deserializeSafeFromHeader(
                         threadPool.getThreadContext(),
                         ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_HEADER
@@ -658,7 +659,7 @@ public Weight doCache(Weight weight, QueryCachingPolicy policy) {
                     if (SecurityUtils.evalMap(allowedFlsFields, index().getName()) != null) {
                         return weight;
                     } else {
-
+                        @SuppressWarnings("unchecked")
                         final Map<String, Set<String>> maskedFieldsMap = (Map<String, Set<String>>) HeaderHelper.deserializeSafeFromHeader(
                             threadPool.getThreadContext(),
                             ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER
@@ -743,6 +744,7 @@ public void onQueryPhase(SearchContext searchContext, long tookInNanos) {
                         return;
                     }
 
+                    @SuppressWarnings("unchecked")
                     final Map<String, Set<String>> maskedFieldsMap = (Map<String, Set<String>>) HeaderHelper.deserializeSafeFromHeader(
                         threadPool.getThreadContext(),
                         ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER
@@ -1879,6 +1881,7 @@ public Function<String, Predicate<String>> getFieldFilter() {
             if (threadPool == null) {
                 return field -> true;
             }
+            @SuppressWarnings("unchecked")
             final Map<String, Set<String>> allowedFlsFields = (Map<String, Set<String>>) 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
@@ -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<String
                                 .collect(
                                     Collectors.toMap(
                                         entry -> "id",
-                                        entry -> new String(
-                                            BaseEncoding.base64().decode(((Entry<String, String>) 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<String, String> envAsMap = AccessController.doPrivileged(new PrivilegedAction<Map<String, String>>() {
-            @Override
-            public Map<String, String> run() {
-                return System.getenv();
-            }
-        });
+        final Map<String, String> envAsMap = AccessController.doPrivileged((PrivilegedAction<Map<String, String>>) System::getenv);
 
-        final Map propsAsMap = AccessController.doPrivileged(new PrivilegedAction<Map>() {
-            @Override
-            public Map run() {
-                return System.getProperties();
-            }
-        });
+        final Properties propsAsMap = AccessController.doPrivileged((PrivilegedAction<Properties>) 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
@@ -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<String, ?>) 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<String, ?>) 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
@@ -101,6 +101,7 @@ protected final void close(List<AuditLogSink> sinks) {
         }
     }
 
+    @SuppressWarnings("unchecked")
     public final void enableRoutes(Settings settings) {
         checkState(isEnabled(), "AuditMessageRouter is disabled");
         if (categorySinks != null) {