Skip to content

Commit

Permalink
Enable warnings during compilation
Browse files Browse the repository at this point in the history 
Warnings the same as for OpenSearch main build
  • Loading branch information
@willyborankin
willyborankin committed Sep 18, 2023
1 parent 77641f3 commit c9cf995
Show file tree
Hide file tree
Showing 25 changed files with 126 additions and 104 deletions.
82 changes: 62 additions & 20 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,59 @@ java.targetCompatibility = JavaVersion.VERSION_11


compileJava {
options.compilerArgs = ["-Xlint:all"]
options.failOnError = false
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'
// options.compilerArgs << '-Xlint:removal' <<
}

compileTestJava {
options.compilerArgs = ["-Xlint:all"]
options.failOnError = false
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']
}


Expand Down Expand Up @@ -335,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'
Expand Down Expand Up @@ -395,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 {
Expand Down Expand Up @@ -534,7 +574,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.21.1'

compileOnly 'com.google.errorprone:error_prone_annotations:2.21.1'

runtimeOnly 'com.sun.istack:istack-commons-runtime:4.2.0'
runtimeOnly 'jakarta.xml.bind:jakarta.xml.bind-api:4.0.0'
runtimeOnly 'org.ow2.asm:asm:9.5'
Expand Down Expand Up @@ -566,7 +608,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.19'
runtimeOnly 'org.slf4j:slf4j-api:1.7.36'
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/security/DefaultObjectMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,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()
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,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
Expand All @@ -650,7 +651,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
Expand Down Expand Up @@ -735,6 +736,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
Expand Down Expand Up @@ -1866,6 +1868,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
Expand Down
21 changes: 4 additions & 17 deletions src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/opensearch/security/auditlog/impl/RequestResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ private static AuditMessage resolveInner(
return msg;
}

@SuppressWarnings("unchecked")
private static void addIndicesSourceSafe(
final AuditMessage msg,
final String[] indices,
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/opensearch/security/auditlog/routing/AuditMessageRouter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/security/auditlog/sink/InternalOpenSearchSink.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/org/opensearch/security/auth/BackendRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.SortedSet;
import java.util.concurrent.Callable;
Expand Down Expand Up @@ -62,7 +63,6 @@
import org.opensearch.security.http.OnBehalfOfAuthenticator;
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;
Expand Down Expand Up @@ -188,7 +188,7 @@ public void onDynamicConfigModelChanged(DynamicConfigModel dcm) {
public boolean authenticate(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) {
final boolean isDebugEnabled = log.isDebugEnabled();
if (request.getHttpChannel().getRemoteAddress() instanceof InetSocketAddress
&& isBlocked(((InetSocketAddress) request.getHttpChannel().getRemoteAddress()).getAddress())) {
&& isBlocked(request.getHttpChannel().getRemoteAddress().getAddress())) {
if (isDebugEnabled) {
log.debug("Rejecting REST request because of blocked address: {}", request.getHttpChannel().getRemoteAddress());
}
Expand Down Expand Up @@ -324,7 +324,7 @@ && isBlocked(((InetSocketAddress) request.getHttpChannel().getRemoteAddress()).g
)) {
authFailureListener.onAuthFailure(
(request.getHttpChannel().getRemoteAddress() instanceof InetSocketAddress)
? ((InetSocketAddress) request.getHttpChannel().getRemoteAddress()).getAddress()
? request.getHttpChannel().getRemoteAddress().getAddress()
: null,
ac,
request
Expand All @@ -345,7 +345,7 @@ && isBlocked(((InetSocketAddress) request.getHttpChannel().getRemoteAddress()).g
return false;
}

final String tenant = Utils.coalesce(request.header("securitytenant"), request.header("security_tenant"));
final String tenant = Optional.ofNullable(request.header("securitytenant")).orElse(request.header("security_tenant"));

if (isDebugEnabled) {
log.debug("Rest user '{}' is authenticated", authenticatedUser);
Expand Down Expand Up @@ -375,7 +375,7 @@ && isBlocked(((InetSocketAddress) request.getHttpChannel().getRemoteAddress()).g
}

if (authCredenetials == null && anonymousAuthEnabled) {
final String tenant = Utils.coalesce(request.header("securitytenant"), request.header("security_tenant"));
final String tenant = Optional.ofNullable(request.header("securitytenant")).orElse(request.header("security_tenant"));
User anonymousUser = new User(User.ANONYMOUS.getName(), new HashSet<String>(User.ANONYMOUS.getRoles()), null);
anonymousUser.setRequestedTenant(tenant);

Expand Down Expand Up @@ -427,7 +427,7 @@ && isBlocked(((InetSocketAddress) request.getHttpChannel().getRemoteAddress()).g
private void notifyIpAuthFailureListeners(RestRequest request, AuthCredentials authCredentials) {
notifyIpAuthFailureListeners(
(request.getHttpChannel().getRemoteAddress() instanceof InetSocketAddress)
? ((InetSocketAddress) request.getHttpChannel().getRemoteAddress()).getAddress()
? request.getHttpChannel().getRemoteAddress().getAddress()
: null,
authCredentials,
request
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@ private static <T> Field getField(Class<T> cls, String name) {
return AccessController.doPrivileged((PrivilegedAction<Field>) () -> getFieldPrivileged(cls, name));
}

@SuppressWarnings("unchecked")
private static <T, C> T getFieldValue(Field field, C c) {
try {
return (T) field.get(c);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public void onResponse(CreateIndexResponse response) {
}

br.execute(
new ConfigUpdatingActionListener(
new ConfigUpdatingActionListener<>(
cTypes.build().toArray(new String[0]),
client,
new ActionListener<BulkResponse>() {
Expand Down
8 changes: 3 additions & 5 deletions src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -247,10 +247,8 @@ private ValidationResult<JsonNode> 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");
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/opensearch/security/http/HTTPProxyAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,17 @@ public AuthCredentials extractCredentials(final RestRequest request, ThreadConte
log.debug("RolesHeader {}, value {}", rolesHeader, rolesHeader == null ? null : request.header(rolesHeader));
}

if (!Strings.isNullOrEmpty(userHeader) && !Strings.isNullOrEmpty((String) request.header(userHeader))) {
if (!Strings.isNullOrEmpty(userHeader) && !Strings.isNullOrEmpty(request.header(userHeader))) {

String[] backendRoles = null;

if (!Strings.isNullOrEmpty(rolesHeader) && !Strings.isNullOrEmpty((String) request.header(rolesHeader))) {
backendRoles = rolesSeparator.splitAsStream((String) request.header(rolesHeader))
if (!Strings.isNullOrEmpty(rolesHeader) && !Strings.isNullOrEmpty(request.header(rolesHeader))) {
backendRoles = rolesSeparator.splitAsStream(request.header(rolesHeader))
.map(String::trim)
.filter(Predicates.not(String::isEmpty))
.toArray(String[]::new);
}
return new AuthCredentials((String) request.header(userHeader), backendRoles).markComplete();
return new AuthCredentials(request.header(userHeader), backendRoles).markComplete();
} else {
log.trace("No '{}' header, send 401", userHeader);
return null;
Expand Down
Loading

0 comments on commit c9cf995

Please sign in to comment.