Skip to content

Commit

Permalink
[Backport 2.x] [Refactor] Remove json-path from deps and use JsonPoin…
Browse files Browse the repository at this point in the history
…ter instead #3262  (#3284)

Backport 14574dd from #3262
  • Loading branch information
willyborankin authored Sep 1, 2023
1 parent b856568 commit 4c9e6eb
Show file tree
Hide file tree
Showing 6 changed files with 322 additions and 59 deletions.
4 changes: 0 additions & 4 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -509,10 +509,6 @@ dependencies {
//Password generation
implementation 'org.passay:passay:1.6.3'

//JSON path
implementation 'com.jayway.jsonpath:json-path:2.8.0'
implementation 'net.minidev:json-smart:2.5.0'

implementation "org.apache.kafka:kafka-clients:${kafka_version}"

runtimeOnly 'net.minidev:accessors-smart:2.4.7'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.rest.RestStatus;
Expand Down Expand Up @@ -137,7 +136,7 @@ public class AuditApiAction extends AbstractApiAction {
private final List<String> readonlyFields;

public static class AuditRequestContentValidator extends RequestContentValidator {
private static final Set<AuditCategory> DISABLED_REST_CATEGORIES = ImmutableSet.of(
public static final Set<AuditCategory> DISABLED_REST_CATEGORIES = Set.of(
AuditCategory.BAD_HEADERS,
AuditCategory.SSL_EXCEPTION,
AuditCategory.AUTHENTICATED,
Expand All @@ -146,7 +145,7 @@ public static class AuditRequestContentValidator extends RequestContentValidator
AuditCategory.MISSING_PRIVILEGES
);

private static final Set<AuditCategory> DISABLED_TRANSPORT_CATEGORIES = ImmutableSet.of(
public static final Set<AuditCategory> DISABLED_TRANSPORT_CATEGORIES = Set.of(
AuditCategory.BAD_HEADERS,
AuditCategory.SSL_EXCEPTION,
AuditCategory.AUTHENTICATED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@

package org.opensearch.security.dlic.rest.api;

import com.fasterxml.jackson.core.JsonPointer;
import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.jayway.jsonpath.JsonPath;
import com.jayway.jsonpath.ReadContext;
import org.apache.commons.lang3.tuple.Pair;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.settings.Settings;
Expand All @@ -34,6 +34,8 @@
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.StreamSupport;

import static org.opensearch.security.dlic.rest.api.RequestHandler.methodNotImplementedHandler;
import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
Expand All @@ -51,11 +53,11 @@ public class RolesApiAction extends AbstractApiAction {
)
);

public static class RoleValidator extends RequestContentValidator {
public static class RoleRequestContentValidator extends RequestContentValidator {

private static final Salt SALT = new Salt(new byte[] { 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 6 });

protected RoleValidator(ValidationContext validationContext) {
protected RoleRequestContentValidator(ValidationContext validationContext) {
super(validationContext);
}

Expand All @@ -70,27 +72,29 @@ public ValidationResult<JsonNode> validate(RestRequest request, JsonNode jsonCon
}

private ValidationResult<JsonNode> validateMaskedFields(final JsonNode content) {
final ReadContext ctx = JsonPath.parse(content.toString());
final List<String> maskedFields = ctx.read("$..masked_fields[*]");
if (maskedFields != null) {
for (String mf : maskedFields) {
if (!validateMaskedFieldSyntax(mf)) {
this.validationError = ValidationError.WRONG_DATATYPE;
return ValidationResult.error(RestStatus.BAD_REQUEST, this);
}
}
StreamSupport.stream(content.withArray(JsonPointer.compile("/index_permissions")).spliterator(), false)
.flatMap(
indexPermissionsNode -> StreamSupport.stream(indexPermissionsNode.withArray("/masked_fields").spliterator(), false)
)
.map(this::validateMaskedFieldSyntax)
.filter(Objects::nonNull)
.forEach(wrongMaskedField -> {
this.validationError = ValidationError.WRONG_DATATYPE;
wrongDataTypes.put("Masked field not valid: " + wrongMaskedField.getLeft(), wrongMaskedField.getRight());
});
if (validationError != ValidationError.NONE) {
return ValidationResult.error(RestStatus.BAD_REQUEST, this);
}
return ValidationResult.success(content);
}

private boolean validateMaskedFieldSyntax(String mf) {
private Pair<String, String> validateMaskedFieldSyntax(final JsonNode maskedFieldNode) {
try {
new MaskedField(mf, SALT).isValid();
new MaskedField(maskedFieldNode.asText(), SALT).isValid();
} catch (Exception e) {
wrongDataTypes.put("Masked field not valid: " + mf, e.getMessage());
return false;
return Pair.of(maskedFieldNode.asText(), e.getMessage());
}
return true;
return null;
}

}
Expand Down Expand Up @@ -146,7 +150,7 @@ public ValidationResult<SecurityConfiguration> isAllowedToChangeImmutableEntity(

@Override
public RequestContentValidator createRequestContentValidator(Object... params) {
return new RoleValidator(new RequestContentValidator.ValidationContext() {
return new RoleRequestContentValidator(new RequestContentValidator.ValidationContext() {
@Override
public Object[] params() {
return params;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.dlic.rest.api;

import com.fasterxml.jackson.databind.InjectableValues;
import org.junit.Test;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.auditlog.config.AuditConfig;
import org.opensearch.security.auditlog.impl.AuditCategory;
import org.opensearch.security.compliance.ComplianceConfig;
import org.opensearch.security.util.FakeRestRequest;

import java.io.IOException;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;

public class AuditApiActionRequestContentValidatorTest extends AbstractApiActionValidationTest {

@Test
public void validateAuditDisabledRestCategories() throws IOException {
InjectableValues.Std injectableValues = new InjectableValues.Std();
injectableValues.addValue(Settings.class, Settings.EMPTY);
DefaultObjectMapper.inject(injectableValues);
final var auditApiActionRequestContentValidator = new AuditApiAction(clusterService, threadPool, securityApiDependencies)
.createEndpointValidator()
.createRequestContentValidator();

final var disabledTransportCategories = AuditApiAction.AuditRequestContentValidator.DISABLED_TRANSPORT_CATEGORIES.stream()
.map(Enum::name)
.collect(Collectors.toList());
final var auditConfig = new AuditConfig(
true,
AuditConfig.Filter.from(Map.of("disabled_rest_categories", disabledTransportCategories)),
ComplianceConfig.DEFAULT
);
final var content = DefaultObjectMapper.writeValueAsString(objectMapper.valueToTree(auditConfig), false);
var result = auditApiActionRequestContentValidator.validate(FakeRestRequest.builder().withContent(new BytesArray(content)).build());
assertFalse(result.isValid());
assertEquals(RestStatus.BAD_REQUEST, result.status());
}

@Test
public void validateAuditDisabledTransportCategories() throws IOException {
InjectableValues.Std injectableValues = new InjectableValues.Std();
injectableValues.addValue(Settings.class, Settings.EMPTY);
DefaultObjectMapper.inject(injectableValues);
final var auditApiActionRequestContentValidator = new AuditApiAction(clusterService, threadPool, securityApiDependencies)
.createEndpointValidator()
.createRequestContentValidator();

final var disabledRestCategories = Stream.of(AuditCategory.COMPLIANCE_DOC_WRITE, AuditCategory.COMPLIANCE_DOC_READ)
.map(Enum::name)
.collect(Collectors.toList());
final var auditConfig = new AuditConfig(
true,
AuditConfig.Filter.from(Map.of("disabled_transport_categories", disabledRestCategories)),
ComplianceConfig.DEFAULT
);
final var content = DefaultObjectMapper.writeValueAsString(objectMapper.valueToTree(auditConfig), false);
var result = auditApiActionRequestContentValidator.validate(FakeRestRequest.builder().withContent(new BytesArray(content)).build());
assertFalse(result.isValid());
assertEquals(RestStatus.BAD_REQUEST, result.status());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.dlic.rest.api;

import com.fasterxml.jackson.databind.node.ObjectNode;
import org.junit.Test;
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.security.util.FakeRestRequest;

import java.io.IOException;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class RolesApiActionRequestContentValidatorTest extends AbstractApiActionValidationTest {

@Test
public void doesNotValidateMaskedFields() throws IOException {

final var requestContentValidator = new RolesApiAction(clusterService, threadPool, securityApiDependencies)
.createEndpointValidator()
.createRequestContentValidator();

// no masked fields
final var noMaskedFields = objectMapper.createObjectNode()
.set(
"index_permissions",
objectMapper.createArrayNode()
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("a*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.set("allowed_actions", objectMapper.createArrayNode().add("read"))
)
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("b*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.set("allowed_actions", objectMapper.createArrayNode().add("write"))
)
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("c*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.set("allowed_actions", objectMapper.createArrayNode().add("read").add("write"))

)
);

var result = requestContentValidator.validate(
FakeRestRequest.builder().withContent(new BytesArray(noMaskedFields.toString())).build()
);
assertTrue(result.isValid());
result = requestContentValidator.validate(
FakeRestRequest.builder().withContent(new BytesArray(noMaskedFields.toString())).build(),
noMaskedFields
);
assertTrue(result.isValid());
}

@Test
public void validateOnlySpecifiedMaskedFields() throws IOException {
final var requestContentValidator = new RolesApiAction(clusterService, threadPool, securityApiDependencies)
.createEndpointValidator()
.createRequestContentValidator();
final var specifiedMaskedFields = objectMapper.createObjectNode()
.set(
"index_permissions",
objectMapper.createArrayNode()
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("a*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.<ObjectNode>set("masked_fields", objectMapper.nullNode())
.set("allowed_actions", objectMapper.createArrayNode().add("read"))
)
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("b*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.set("allowed_actions", objectMapper.createArrayNode().add("write"))
)
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("c*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.<ObjectNode>set("masked_fields", objectMapper.createArrayNode().add("aaa::").add("bbb"))
.set("allowed_actions", objectMapper.createArrayNode().add("read").add("write"))

)
);
var result = requestContentValidator.validate(
FakeRestRequest.builder().withContent(new BytesArray(specifiedMaskedFields.toString())).build()
);
assertFalse(result.isValid());
var errorMessage = xContentToJsonNode(result.errorMessage());
assertTrue(errorMessage.toString(), errorMessage.toString().contains("aaa::"));

result = requestContentValidator.validate(
FakeRestRequest.builder().withContent(new BytesArray(specifiedMaskedFields.toString())).build(),
specifiedMaskedFields
);
assertFalse(result.isValid());
errorMessage = xContentToJsonNode(result.errorMessage());
assertTrue(errorMessage.toString(), errorMessage.toString().contains("aaa::"));
}

@Test
public void validateAllMaskedFields() throws IOException {
final var requestContentValidator = new RolesApiAction(clusterService, threadPool, securityApiDependencies)
.createEndpointValidator()
.createRequestContentValidator();
final var invalidMaskedFields = objectMapper.createObjectNode()
.set(
"index_permissions",
objectMapper.createArrayNode()
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("a*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.<ObjectNode>set("masked_fields", objectMapper.createArrayNode().add("aaa").add("bbb"))
.set("allowed_actions", objectMapper.createArrayNode().add("read"))
)
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("b*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.<ObjectNode>set("masked_fields", objectMapper.createArrayNode().add("aaa::").add("bbb::").add("ccc:::"))
.set("allowed_actions", objectMapper.createArrayNode().add("write"))
)
.add(
objectMapper.createObjectNode()
.<ObjectNode>set("index_patterns", objectMapper.createArrayNode().add("c*"))
.put("dls", "")
.<ObjectNode>set("fls", objectMapper.createArrayNode())
.<ObjectNode>set("masked_fields", objectMapper.createArrayNode().add("ddd::").add("eee"))
.set("allowed_actions", objectMapper.createArrayNode().add("read").add("write"))

)
);
var result = requestContentValidator.validate(
FakeRestRequest.builder().withContent(new BytesArray(invalidMaskedFields.toString())).build()
);
assertFalse(result.isValid());
var errorMessage = xContentToJsonNode(result.errorMessage()).toString();
assertTrue(errorMessage, errorMessage.contains("aaa::"));
assertTrue(errorMessage, errorMessage.contains("bbb::"));
assertTrue(errorMessage, errorMessage.contains("ccc:::"));
assertTrue(errorMessage, errorMessage.contains("ddd::"));

result = requestContentValidator.validate(
FakeRestRequest.builder().withContent(new BytesArray(invalidMaskedFields.toString())).build(),
invalidMaskedFields
);
assertFalse(result.isValid());
errorMessage = xContentToJsonNode(result.errorMessage()).toString();
assertTrue(errorMessage, errorMessage.contains("aaa::"));
assertTrue(errorMessage, errorMessage.contains("bbb::"));
assertTrue(errorMessage, errorMessage.contains("ccc:::"));
assertTrue(errorMessage, errorMessage.contains("ddd::"));
}
}
Loading

0 comments on commit 4c9e6eb

Please sign in to comment.