From 18ba96ed623f974ad67f4a6307413dff93f9ffc6 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 26 Jan 2024 09:25:35 -0500 Subject: [PATCH] [1.3] Remove json-smart and json-path dependencies (#3982) ### Description Selective manual backport of https://github.com/opensearch-project/security/pull/3262. The original PR cannot be backported as is because of the changes to how the AbstractAPI is written between the 2.x and 1.x line. This PR resolves the build error seen on https://github.com/opensearch-project/security/pull/3979. ### Check List - [ ] New functionality includes testing - [ ] New functionality has been documented - [ ] Commits are signed per the DCO using --signoff By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). --------- Signed-off-by: Craig Perkins --- build.gradle | 2 - .../dlic/rest/validation/RolesValidator.java | 24 ++++--- .../ssl/SecuritySSLCertsInfoActionTests.java | 15 +++-- .../SecuritySSLReloadCertsActionTests.java | 66 +++++++++++-------- 4 files changed, 63 insertions(+), 44 deletions(-) diff --git a/build.gradle b/build.gradle index 096e453813..9c50389621 100644 --- a/build.gradle +++ b/build.gradle @@ -78,7 +78,6 @@ configurations.all { force 'commons-codec:commons-codec:1.14' force 'org.apache.santuario:xmlsec:2.3.4' force 'org.cryptacular:cryptacular:1.2.4' - force 'net.minidev:json-smart:2.4.10' force 'commons-cli:commons-cli:1.3.1' force 'org.apache.httpcomponents:httpcore:4.4.12' force "org.apache.commons:commons-lang3:3.4" @@ -186,7 +185,6 @@ dependencies { } implementation 'commons-lang:commons-lang:2.4' implementation 'commons-collections:commons-collections:3.2.2' - implementation 'com.jayway.jsonpath:json-path:2.4.0' implementation 'org.apache.httpcomponents:httpclient:4.5.13' runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.10.8' runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.10.8' diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/RolesValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/RolesValidator.java index a1664e484a..5dbe8a9a87 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/validation/RolesValidator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/RolesValidator.java @@ -15,16 +15,18 @@ package org.opensearch.security.dlic.rest.validation; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; import org.opensearch.security.configuration.Salt; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.settings.Settings; import org.opensearch.rest.RestRequest; import org.opensearch.security.configuration.MaskedField; -import com.jayway.jsonpath.JsonPath; -import com.jayway.jsonpath.ReadContext; public class RolesValidator extends AbstractConfigurationValidator { @@ -51,15 +53,17 @@ public boolean validate() { if (this.content != null && this.content.length() > 0) { - final ReadContext ctx = JsonPath.parse(this.content.utf8ToString()); - final List maskedFields = ctx.read("$..masked_fields[*]"); - - if (maskedFields != null) { + ArrayNode indexPermissions = getContentAsNode().withArray("index_permissions"); + Set maskedFields = new HashSet<>(); + if (indexPermissions != null) { + for (JsonNode ip : indexPermissions) { + ip.withArray("masked_fields").forEach((n) -> maskedFields.add(n.asText())); + } + } - for (String mf : maskedFields) { - if (!validateMaskedFieldSyntax(mf)) { - valid = false; - } + for (String mf : maskedFields) { + if (!validateMaskedFieldSyntax(mf)) { + valid = false; } } } diff --git a/src/test/java/org/opensearch/security/ssl/SecuritySSLCertsInfoActionTests.java b/src/test/java/org/opensearch/security/ssl/SecuritySSLCertsInfoActionTests.java index 0538d5327c..47c0787384 100644 --- a/src/test/java/org/opensearch/security/ssl/SecuritySSLCertsInfoActionTests.java +++ b/src/test/java/org/opensearch/security/ssl/SecuritySSLCertsInfoActionTests.java @@ -15,6 +15,10 @@ package org.opensearch.security.ssl; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.ssl.util.SSLConfigConstants; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.test.SingleClusterTest; @@ -22,7 +26,6 @@ import org.opensearch.security.test.helper.rest.RestHelper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import net.minidev.json.JSONObject; import org.opensearch.common.settings.Settings; import org.junit.Assert; import org.junit.Test; @@ -52,11 +55,11 @@ public void testCertInfo_Pass() throws Exception { rh.sendAdminCertificate = true; rh.keystore = "kirk-keystore.jks"; - final RestHelper.HttpResponse transportInfoRestResponse = rh.executeGetRequest(ENDPOINT); - JSONObject expectedJsonResponse = new JSONObject(); - expectedJsonResponse.appendField("http_certificates_list", NODE_CERT_DETAILS); - expectedJsonResponse.appendField("transport_certificates_list", NODE_CERT_DETAILS); - Assert.assertEquals(expectedJsonResponse.toString(), transportInfoRestResponse.getBody()); + final String certInfoRestResponse = rh.executeSimpleRequest(ENDPOINT); + final ObjectNode expectedJsonResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedJsonResponse.set("http_certificates_list", SecuritySSLReloadCertsActionTests.buildCertsInfoNode(NODE_CERT_DETAILS)); + expectedJsonResponse.set("transport_certificates_list", SecuritySSLReloadCertsActionTests.buildCertsInfoNode(NODE_CERT_DETAILS)); + Assert.assertEquals(expectedJsonResponse, DefaultObjectMapper.readTree(certInfoRestResponse)); } @Test diff --git a/src/test/java/org/opensearch/security/ssl/SecuritySSLReloadCertsActionTests.java b/src/test/java/org/opensearch/security/ssl/SecuritySSLReloadCertsActionTests.java index 7020637c00..5c752bc90a 100644 --- a/src/test/java/org/opensearch/security/ssl/SecuritySSLReloadCertsActionTests.java +++ b/src/test/java/org/opensearch/security/ssl/SecuritySSLReloadCertsActionTests.java @@ -15,6 +15,10 @@ package org.opensearch.security.ssl; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.ssl.util.SSLConfigConstants; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.test.DynamicSecurityConfig; @@ -24,7 +28,6 @@ import org.opensearch.security.test.helper.rest.RestHelper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import net.minidev.json.JSONObject; import org.opensearch.common.settings.Settings; import org.junit.Assert; import org.junit.Rule; @@ -80,10 +83,10 @@ public void testReloadTransportSSLCertsPass() throws Exception { String certDetailsResponse = rh.executeSimpleRequest(GET_CERT_DETAILS_ENDPOINT); - JSONObject expectedJsonResponse = new JSONObject(); - expectedJsonResponse.appendField("http_certificates_list", NODE_CERT_DETAILS); - expectedJsonResponse.appendField("transport_certificates_list", NODE_CERT_DETAILS); - Assert.assertEquals(expectedJsonResponse.toString(), certDetailsResponse); + ObjectNode expectedJsonResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedJsonResponse.set("http_certificates_list", buildCertsInfoNode(NODE_CERT_DETAILS)); + expectedJsonResponse.set("transport_certificates_list", buildCertsInfoNode(NODE_CERT_DETAILS)); + Assert.assertEquals(expectedJsonResponse, DefaultObjectMapper.readTree(certDetailsResponse)); // Test Valid Case: Change transport file details to "ssl/pem/node-new.crt.pem" and "ssl/pem/node-new.key.pem" FileHelper.copyFileContents(FileHelper.getAbsoluteFilePathFromClassPath("ssl/reload/node-new.crt.pem").toString(), pemCertFilePath); @@ -91,14 +94,14 @@ public void testReloadTransportSSLCertsPass() throws Exception { RestHelper.HttpResponse reloadCertsResponse = rh.executePutRequest(RELOAD_TRANSPORT_CERTS_ENDPOINT, null); Assert.assertEquals(200, reloadCertsResponse.getStatusCode()); - expectedJsonResponse = new JSONObject(); - expectedJsonResponse.appendField("message", "updated transport certs"); + expectedJsonResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedJsonResponse.put("message", "updated transport certs"); Assert.assertEquals(expectedJsonResponse.toString(), reloadCertsResponse.getBody()); certDetailsResponse = rh.executeSimpleRequest(GET_CERT_DETAILS_ENDPOINT); - expectedJsonResponse = new JSONObject(); - expectedJsonResponse.appendField("http_certificates_list", NODE_CERT_DETAILS); - expectedJsonResponse.appendField("transport_certificates_list", NEW_NODE_CERT_DETAILS); + expectedJsonResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedJsonResponse.set("http_certificates_list", buildCertsInfoNode(NODE_CERT_DETAILS)); + expectedJsonResponse.set("transport_certificates_list", buildCertsInfoNode(NEW_NODE_CERT_DETAILS)); Assert.assertEquals(expectedJsonResponse.toString(), certDetailsResponse); } @@ -118,10 +121,11 @@ public void testReloadHttpSSLCertsPass() throws Exception { rh.keystore = "ssl/reload/kirk-keystore.jks"; String certDetailsResponse = rh.executeSimpleRequest(GET_CERT_DETAILS_ENDPOINT); - JSONObject expectedJsonResponse = new JSONObject(); - expectedJsonResponse.appendField("http_certificates_list", NODE_CERT_DETAILS); - expectedJsonResponse.appendField("transport_certificates_list", NODE_CERT_DETAILS); - Assert.assertEquals(expectedJsonResponse.toString(), certDetailsResponse); + + ObjectNode expectedJsonResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedJsonResponse.set("http_certificates_list", buildCertsInfoNode(NODE_CERT_DETAILS)); + expectedJsonResponse.set("transport_certificates_list", buildCertsInfoNode(NODE_CERT_DETAILS)); + Assert.assertEquals(expectedJsonResponse, DefaultObjectMapper.readTree(certDetailsResponse)); // Test Valid Case: Change rest file details to "ssl/pem/node-new.crt.pem" and "ssl/pem/node-new.key.pem" FileHelper.copyFileContents(FileHelper.getAbsoluteFilePathFromClassPath("ssl/reload/node-new.crt.pem").toString(), pemCertFilePath); @@ -129,14 +133,14 @@ public void testReloadHttpSSLCertsPass() throws Exception { RestHelper.HttpResponse reloadCertsResponse = rh.executePutRequest(RELOAD_HTTP_CERTS_ENDPOINT, null); Assert.assertEquals(200, reloadCertsResponse.getStatusCode()); - expectedJsonResponse = new JSONObject(); - expectedJsonResponse.appendField("message", "updated http certs"); + expectedJsonResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedJsonResponse.put("message", "updated http certs"); Assert.assertEquals(expectedJsonResponse.toString(), reloadCertsResponse.getBody()); certDetailsResponse = rh.executeSimpleRequest(GET_CERT_DETAILS_ENDPOINT); - expectedJsonResponse = new JSONObject(); - expectedJsonResponse.appendField("http_certificates_list", NEW_NODE_CERT_DETAILS); - expectedJsonResponse.appendField("transport_certificates_list", NODE_CERT_DETAILS); + expectedJsonResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedJsonResponse.set("http_certificates_list", buildCertsInfoNode(NEW_NODE_CERT_DETAILS)); + expectedJsonResponse.set("transport_certificates_list", buildCertsInfoNode(NODE_CERT_DETAILS)); Assert.assertEquals(expectedJsonResponse.toString(), certDetailsResponse); } @@ -157,7 +161,7 @@ public void testReloadHttpSSLCerts_FailWrongUri() throws Exception { rh.keystore = "ssl/reload/kirk-keystore.jks"; RestHelper.HttpResponse reloadCertsResponse = rh.executePutRequest("_opendistro/_security/api/ssl/wrong/reloadcerts", null); - JSONObject expectedResponse = new JSONObject(); + ObjectNode expectedResponse = DefaultObjectMapper.objectMapper.createObjectNode(); // Note: toString and toJSONString replace / with \/. This helps get rid of the additional \ character. expectedResponse.put("message", "invalid uri path, please use /_opendistro/_security/api/ssl/http/reload or /_opendistro/_security/api/ssl/transport/reload"); final String expectedResponseString = expectedResponse.toString().replace("\\", ""); @@ -207,8 +211,8 @@ public void testSSLReloadFail_InvalidDNAndDate() throws Exception { RestHelper.HttpResponse reloadCertsResponse = rh.executePutRequest(RELOAD_TRANSPORT_CERTS_ENDPOINT, null); Assert.assertEquals(500, reloadCertsResponse.getStatusCode()); - JSONObject expectedResponse = new JSONObject(); - expectedResponse.appendField("error", "OpenSearchSecurityException[Error while initializing transport SSL layer from PEM: java.lang.Exception: " + + ObjectNode expectedResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedResponse.put("error", "OpenSearchSecurityException[Error while initializing transport SSL layer from PEM: java.lang.Exception: " + "New Certs do not have valid Issuer DN, Subject DN or SAN.]; nested: Exception[New Certs do not have valid Issuer DN, Subject DN or SAN.];"); Assert.assertEquals(expectedResponse.toString(), reloadCertsResponse.getBody()); @@ -219,8 +223,8 @@ public void testSSLReloadFail_InvalidDNAndDate() throws Exception { reloadCertsResponse = rh.executePutRequest(RELOAD_TRANSPORT_CERTS_ENDPOINT, null); Assert.assertEquals(500, reloadCertsResponse.getStatusCode()); - expectedResponse = new JSONObject(); - expectedResponse.appendField("error", "OpenSearchSecurityException[Error while initializing transport SSL layer from PEM: java.lang.Exception: New certificates should not expire before the current ones.]; nested: Exception[New certificates should not expire before the current ones.];"); + expectedResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedResponse.put("error", "OpenSearchSecurityException[Error while initializing transport SSL layer from PEM: java.lang.Exception: New certificates should not expire before the current ones.]; nested: Exception[New certificates should not expire before the current ones.];"); Assert.assertEquals(expectedResponse.toString(), reloadCertsResponse.getBody()); } @@ -244,8 +248,8 @@ public void testSSLReloadFail_NoReloadSet() throws Exception { final RestHelper.HttpResponse reloadCertsResponse = rh.executePutRequest(RELOAD_TRANSPORT_CERTS_ENDPOINT, null); Assert.assertEquals(400, reloadCertsResponse.getStatusCode()); - JSONObject expectedResponse = new JSONObject(); - expectedResponse.appendField("error", "no handler found for uri [/_opendistro/_security/api/ssl/transport/reloadcerts] and method [PUT]"); + ObjectNode expectedResponse = DefaultObjectMapper.objectMapper.createObjectNode(); + expectedResponse.put("error", "no handler found for uri [/_opendistro/_security/api/ssl/transport/reloadcerts] and method [PUT]"); // Note: toString and toJSONString replace / with \/. This helps get rid of the additional \ character. final String expectedResponseString = expectedResponse.toString().replace("\\", ""); Assert.assertEquals(expectedResponseString, reloadCertsResponse.getBody()); @@ -287,4 +291,14 @@ private void initTestCluster(final String transportPemCertFilePath, final String setup(initTransportClientSettings, new DynamicSecurityConfig(), settings, true, ClusterConfiguration.DEFAULT); } + public static JsonNode buildCertsInfoNode(final List> certsInfo) { + final ArrayNode nodeCertDetailsArray = DefaultObjectMapper.objectMapper.createArrayNode(); + certsInfo.forEach(m -> { + final ObjectNode o = DefaultObjectMapper.objectMapper.createObjectNode(); + m.forEach(o::put); + nodeCertDetailsArray.add(o); + }); + return nodeCertDetailsArray; + } + }