Use JSON schema constraints to validate policies and other misc cleanup (#6451)

This commit is contained in:
Suresh Srinivas 2022-07-30 15:23:18 -07:00 committed by GitHub
parent afab066f3e
commit 109b937fd4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 140 additions and 103 deletions

View File

@ -380,7 +380,7 @@
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt</artifactId>
<version>0.9.1</version>
<version>${jjwt.version}</version>
</dependency>
@ -388,14 +388,14 @@
<dependency>
<groupId>com.auth0</groupId>
<artifactId>java-jwt</artifactId>
<version>3.19.1</version>
<version>${java-jwt.version}</version>
</dependency>
<!-- https://mvnrepository.com/artifact/com.auth0/jwks-rsa -->
<dependency>
<groupId>com.auth0</groupId>
<artifactId>jwks-rsa</artifactId>
<version>0.21.1</version>
<version>${jwks-rsa.version}</version>
</dependency>
@ -408,7 +408,7 @@
<dependency>
<groupId>io.github.artsok</groupId>
<artifactId>rerunner-jupiter</artifactId>
<version>2.1.6</version>
<version>${rerunner-jupiter.version}</version>
<scope>test</scope>
</dependency>
<dependency>
@ -451,6 +451,11 @@
<artifactId>websocket-server</artifactId>
<version>9.4.46.v20220331</version>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-expression</artifactId>
<version>${spring-expression.version}</version>
</dependency>
</dependencies>
<build>

View File

@ -21,19 +21,15 @@ import static org.openmetadata.catalog.util.EntityUtil.entityReferenceMatch;
import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import lombok.extern.slf4j.Slf4j;
import org.jdbi.v3.sqlobject.transaction.Transaction;
import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.entity.data.Location;
import org.openmetadata.catalog.entity.policies.Policy;
import org.openmetadata.catalog.entity.policies.accessControl.Rule;
import org.openmetadata.catalog.exception.CatalogExceptionMessage;
import org.openmetadata.catalog.resources.policies.PolicyResource;
import org.openmetadata.catalog.type.EntityReference;
import org.openmetadata.catalog.type.MetadataOperation;
import org.openmetadata.catalog.type.PolicyType;
import org.openmetadata.catalog.type.Relationship;
import org.openmetadata.catalog.util.EntityUtil;
@ -87,7 +83,7 @@ public class PolicyRepository extends EntityRepository<Policy> {
@Override
public void prepare(Policy policy) throws IOException {
setFullyQualifiedName(policy);
isValid(policy);
validateRules(policy);
policy.setLocation(getLocationReference(policy));
// Check if owner is valid and set the relationship
populateOwner(policy.getOwner());
@ -122,37 +118,11 @@ public class PolicyRepository extends EntityRepository<Policy> {
return new PolicyUpdater(original, updated, operation);
}
/**
* Validates policy schema beyond what json-schema validation supports. If policy is invalid, throws {@link
* IllegalArgumentException}
*
* <p>Example of validation that jsonschema2pojo does not support: <code>
* "anyOf": [
* {"required": ["entityTypeAttr"]},
* {"required": ["entityTagAttr"]},
* {"required": ["userRoleAttr"]}
* ]
* </code>
*/
public void isValid(Policy policy) throws IOException {
public void validateRules(Policy policy) throws IOException {
if (!policy.getPolicyType().equals(PolicyType.AccessControl)) {
return;
}
LOG.debug("Validating rules for {} policy: {}", PolicyType.AccessControl, policy.getName());
Set<MetadataOperation> operations = new HashSet<>();
List<Rule> rules = EntityUtil.resolveRules(policy.getRules());
for (Rule rule : rules) {
if (rule.getOperation() == null) {
throw new IllegalArgumentException(
CatalogExceptionMessage.invalidPolicyOperationNull(rule.getName(), policy.getName()));
}
if (!operations.add(rule.getOperation())) {
throw new IllegalArgumentException(
CatalogExceptionMessage.invalidPolicyDuplicateOperation(rule.getOperation().value(), policy.getName()));
}
}
EntityUtil.resolveRules(policy.getRules()); // Resolve rules performs JSON schema constraint based validation
}
public List<Policy> getAccessControlPolicies() throws IOException {

View File

@ -246,5 +246,5 @@ public class LineageResource {
public EntityInterface getEntity() throws IOException {
return null;
}
};
}
}

View File

@ -13,6 +13,8 @@
package org.openmetadata.catalog.resources.policies;
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
import io.swagger.annotations.Api;
import io.swagger.v3.oas.annotations.ExternalDocumentation;
import io.swagger.v3.oas.annotations.Operation;
@ -94,24 +96,7 @@ public class PolicyResource extends EntityResource<Policy, PolicyRepository> {
// Load any existing rules from database, before loading seed data.
dao.initSeedDataFromResources();
initResourceDescriptors();
}
/** Initialize list of resources and the corresponding operations */
private void initResourceDescriptors() throws IOException {
List<String> jsonDataFiles = EntityUtil.getJsonDataResources(".*json/data/ResourceDescriptors.json$");
if (jsonDataFiles.size() != 1) {
LOG.warn("Invalid number of jsonDataFiles {}. Only one expected.", jsonDataFiles.size());
return;
}
String jsonDataFile = jsonDataFiles.get(0);
try {
String json = IOUtil.toString(getClass().getClassLoader().getResourceAsStream(jsonDataFile));
List<ResourceDescriptor> resourceDetails = JsonUtils.readObjects(json, ResourceDescriptor.class);
ResourceRegistry.add(resourceDetails);
} catch (Exception e) {
LOG.warn("Failed to initialize the {} from file {}", entityType, jsonDataFile, e);
}
ResourceRegistry.add(listOrEmpty(PolicyResource.getResourceDescriptors()));
}
public static class PolicyList extends ResultList<Policy> {
@ -425,4 +410,20 @@ public class PolicyResource extends EntityResource<Policy, PolicyRepository> {
}
return policy;
}
public static List<ResourceDescriptor> getResourceDescriptors() throws IOException {
List<String> jsonDataFiles = EntityUtil.getJsonDataResources(".*json/data/ResourceDescriptors.json$");
if (jsonDataFiles.size() != 1) {
LOG.warn("Invalid number of jsonDataFiles {}. Only one expected.", jsonDataFiles.size());
return null;
}
String jsonDataFile = jsonDataFiles.get(0);
try {
String json = IOUtil.toString(PolicyResource.class.getClassLoader().getResourceAsStream(jsonDataFile));
return JsonUtils.readObjects(json, ResourceDescriptor.class);
} catch (Exception e) {
LOG.warn("Failed to initialize the resource descriptors from file {}", jsonDataFile, e);
}
return null;
}
}

View File

@ -57,7 +57,9 @@ public class NoopAuthorizer implements Authorizer {
SecurityContext securityContext,
OperationContext operationContext,
ResourceContextInterface resourceContext,
boolean allowBots) {}
boolean allowBots) {
/* Always authorize */
}
private void addAnonymousUser() {
String username = "anonymous";
@ -90,5 +92,7 @@ public class NoopAuthorizer implements Authorizer {
}
@Override
public void authorizeAdmin(SecurityContext securityContext, boolean allowBots) {}
public void authorizeAdmin(SecurityContext securityContext, boolean allowBots) {
/* Always authorize */
}
}

View File

@ -11,24 +11,47 @@ import org.openmetadata.catalog.util.JsonPatchUtils;
/** OperationContext for Access Control Policy */
public class OperationContext {
@Getter @NonNull private final String resource;
@NonNull private final List<MetadataOperation> operations = new ArrayList<>();
List<MetadataOperation> operations;
@Getter private JsonPatch patch;
public OperationContext(String resource, MetadataOperation... operations) {
public OperationContext(@NonNull String resource, MetadataOperation... operations) {
this.resource = resource;
this.operations = new ArrayList<>();
this.operations.addAll(List.of(operations));
}
public OperationContext(String resource, JsonPatch patch) {
public OperationContext(@NonNull String resource, JsonPatch patch) {
this.resource = resource;
this.patch = patch;
}
public List<MetadataOperation> getOperations() {
// Lazy resolve patch operations
if (patch != null) {
if (operations != null) {
return operations;
}
if (patch != null) { // Lazy initialize operations for PATCH
operations = new ArrayList<>();
operations.addAll(JsonPatchUtils.getMetadataOperations(patch));
}
return operations;
}
public boolean isEditOperation() {
for (MetadataOperation operation : operations) {
if (!operation.value().startsWith("Edit")) {
return false;
}
}
return true;
}
public boolean isViewOperation() {
for (MetadataOperation operation : operations) {
if (!operation.value().startsWith("View")) {
return false;
}
}
return true;
}
}

View File

@ -19,7 +19,6 @@ import java.util.List;
import java.util.stream.Collectors;
import lombok.NonNull;
import lombok.extern.slf4j.Slf4j;
import org.jeasy.rules.api.Rule;
import org.jeasy.rules.api.Rules;
import org.jeasy.rules.api.RulesEngine;
import org.jeasy.rules.api.RulesEngineParameters;

View File

@ -383,7 +383,8 @@ public final class EntityUtil {
List<Rule> resolvedRules = new ArrayList<>();
for (Object ruleObject : rules) {
// Cast to access control policy Rule.
resolvedRules.add(JsonUtils.readValue(JsonUtils.getJsonStructure(ruleObject).toString(), Rule.class));
resolvedRules.add(
JsonUtils.readValueWithValidation(JsonUtils.getJsonStructure(ruleObject).toString(), Rule.class));
}
return resolvedRules;
}

View File

@ -36,6 +36,7 @@ import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import javax.json.Json;
import javax.json.JsonArray;
import javax.json.JsonArrayBuilder;
@ -44,6 +45,9 @@ import javax.json.JsonPatch;
import javax.json.JsonReader;
import javax.json.JsonStructure;
import javax.json.JsonValue;
import javax.validation.ConstraintViolation;
import javax.validation.Validation;
import javax.validation.Validator;
import lombok.extern.slf4j.Slf4j;
import org.openmetadata.catalog.entity.Type;
import org.openmetadata.catalog.entity.type.Category;
@ -54,6 +58,7 @@ public final class JsonUtils {
public static final String ENTITY_TYPE_ANNOTATION = "@om-entity-type";
public static final String JSON_FILE_EXTENSION = ".json";
private static final ObjectMapper OBJECT_MAPPER;
private static final Validator VALIDATOR = Validation.buildDefaultValidatorFactory().getValidator();
private static final JsonSchemaFactory schemaFactory = JsonSchemaFactory.getInstance(VersionFlag.V7);
static {
@ -94,6 +99,22 @@ public final class JsonUtils {
return OBJECT_MAPPER.readValue(json, clz);
}
public static <T> T readValueWithValidation(String json, Class<T> clz) throws IOException {
T pojo = readValue(json, clz);
if (pojo == null) {
return null;
}
Set<ConstraintViolation<T>> violations = VALIDATOR.validate(pojo);
if (violations.size() > 0) {
List<String> errors = new ArrayList<>();
for (ConstraintViolation<T> violation : violations) {
errors.add(violation.getPropertyPath().toString() + " " + violation.getMessage());
}
throw new IllegalArgumentException(errors.toString());
}
return pojo;
}
public static <T> T readValue(String json, TypeReference<T> valueTypeRef) throws IOException {
if (json == null) {
return null;

View File

@ -1,4 +1,31 @@
[
{
"name" : "all",
"operations" : [
"All",
"Create",
"Delete",
"ViewAll",
"ViewUsage",
"ViewTests",
"ViewQueries",
"ViewDataProfile",
"ViewSampleData",
"EditAll",
"EditDescription",
"EditTags",
"EditOwner",
"EditTier",
"EditCustomFields",
"EditLineage",
"EditReviewers",
"EditTests",
"EditQueries",
"EditDataProfile",
"EditSampleData",
"EditUsers"
]
},
{
"name" : "bot",
"operations" : [

View File

@ -11,6 +11,7 @@
"description": "This schema defines all possible operations on metadata of entities in OpenMetadata.",
"type": "string",
"enum": [
"All",
"Create",
"Delete",
"ViewAll",

View File

@ -7,7 +7,7 @@
"javaType": "org.openmetadata.catalog.entity.policies.accessControl.Rule",
"properties": {
"name": {
"description": "Name for this Rule.",
"description": "Name of this Rule.",
"type": "string"
},
"fullyQualifiedName": {
@ -40,8 +40,6 @@
"default": false
}
},
"required": [
"name"
],
"required": ["name", "operation"],
"additionalProperties": false
}

View File

@ -105,6 +105,6 @@
"default": false
}
},
"required": ["id", "name", "policyType"],
"required": ["id", "name", "policyType", "rules"],
"additionalProperties": false
}

View File

@ -85,7 +85,7 @@
"pattern": "^<#E::\\S+::\\S+>$"
},
"entityName": {
"description": "Name that identifies this dashboard service.",
"description": "Name that identifies a entity.",
"type": "string",
"minLength": 1,
"maxLength": 128
@ -107,7 +107,11 @@
},
"markdown": {
"$comment" : "@om-field-type",
"description": "Text in Markdown format",
"description": "Text in Markdown format.",
"type": "string"
},
"expression": {
"description": "Expression in SpEL.",
"type": "string"
},
"jsonSchema": {

View File

@ -21,7 +21,6 @@ import static org.openmetadata.catalog.util.TestUtils.UpdateType.MINOR_UPDATE;
import static org.openmetadata.catalog.util.TestUtils.assertListNotNull;
import static org.openmetadata.catalog.util.TestUtils.assertListNull;
import static org.openmetadata.catalog.util.TestUtils.assertResponse;
import static org.openmetadata.catalog.util.TestUtils.assertResponseContains;
import java.io.IOException;
import java.net.URI;
@ -43,7 +42,6 @@ import org.openmetadata.catalog.api.policies.CreatePolicy;
import org.openmetadata.catalog.entity.data.Location;
import org.openmetadata.catalog.entity.policies.Policy;
import org.openmetadata.catalog.entity.policies.accessControl.Rule;
import org.openmetadata.catalog.exception.CatalogExceptionMessage;
import org.openmetadata.catalog.resources.EntityResourceTest;
import org.openmetadata.catalog.resources.locations.LocationResourceTest;
import org.openmetadata.catalog.resources.policies.PolicyResource.PolicyList;
@ -136,7 +134,7 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
void post_AccessControlPolicyWithValidRules_200_ok(TestInfo test) throws IOException {
List<Rule> rules = new ArrayList<>();
rules.add(PolicyUtils.accessControlRule(null, null, MetadataOperation.EDIT_DESCRIPTION, true));
rules.add(PolicyUtils.accessControlRule(null, null, "DataConsumer", MetadataOperation.EDIT_TAGS, true));
rules.add(PolicyUtils.accessControlRule(null, "DataConsumer", MetadataOperation.EDIT_TAGS, true));
CreatePolicy create = createAccessControlPolicyWithRules(getEntityName(test), rules);
createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
}
@ -149,27 +147,7 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
List<Rule> rules = new ArrayList<>();
rules.add(PolicyUtils.accessControlRule(ruleName, null, null, null, true));
CreatePolicy create = createAccessControlPolicyWithRules(policyName, rules);
assertResponse(
() -> createEntity(create, ADMIN_AUTH_HEADERS),
BAD_REQUEST,
CatalogExceptionMessage.invalidPolicyOperationNull(ruleName, policyName));
}
@Test
void post_AccessControlPolicyWithDuplicateRules_400_error(TestInfo test) {
List<Rule> rules = new ArrayList<>();
rules.add(PolicyUtils.accessControlRule("rule1", null, null, MetadataOperation.EDIT_DESCRIPTION, true));
rules.add(PolicyUtils.accessControlRule("rule2", null, null, MetadataOperation.EDIT_TAGS, true));
rules.add(PolicyUtils.accessControlRule("rule3", null, null, MetadataOperation.EDIT_TAGS, true));
String policyName = getEntityName(test);
CreatePolicy create = createAccessControlPolicyWithRules(policyName, rules);
assertResponseContains(
() -> createEntity(create, ADMIN_AUTH_HEADERS),
BAD_REQUEST,
String.format(
"Found multiple rules with operation EditTags within policy %s. "
+ "Please ensure that operation across all rules within the policy are distinct",
policyName));
assertResponse(() -> createEntity(create, ADMIN_AUTH_HEADERS), BAD_REQUEST, "[operation must not be null]");
}
@Test

View File

@ -91,7 +91,7 @@
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt</artifactId>
<version>0.9.1</version>
<version>${jjwt.version}</version>
</dependency>
@ -99,14 +99,14 @@
<dependency>
<groupId>com.auth0</groupId>
<artifactId>java-jwt</artifactId>
<version>3.19.2</version>
<version>${java-jwt.version}</version>
</dependency>
<!-- https://mvnrepository.com/artifact/com.auth0/jwks-rsa -->
<dependency>
<groupId>com.auth0</groupId>
<artifactId>jwks-rsa</artifactId>
<version>0.21.1</version>
<version>${jwks-rsa.version}</version>
</dependency>
<!--test dependencies-->

View File

@ -7,5 +7,5 @@ Provides metadata version information.
from incremental import Version
__version__ = Version("metadata", 0, 12, 0, dev=5)
__version__ = Version("metadata", 0, 12, 0, dev=6)
__all__ = ["__version__"]

View File

@ -102,7 +102,7 @@
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt</artifactId>
<version>0.9.1</version>
<version>${jjwt.version}</version>
</dependency>
@ -110,14 +110,14 @@
<dependency>
<groupId>com.auth0</groupId>
<artifactId>java-jwt</artifactId>
<version>3.19.1</version>
<version>${java-jwt.version}</version>
</dependency>
<!-- https://mvnrepository.com/artifact/com.auth0/jwks-rsa -->
<dependency>
<groupId>com.auth0</groupId>
<artifactId>jwks-rsa</artifactId>
<version>0.21.1</version>
<version>${jwks-rsa.version}</version>
</dependency>
<!--test dependencies-->

View File

@ -31,7 +31,7 @@
<dependency>
<groupId>io.github.artsok</groupId>
<artifactId>rerunner-jupiter</artifactId>
<version>2.1.6</version>
<version>${rerunner-jupiter.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

View File

@ -130,6 +130,11 @@
<testng.version>7.6.0</testng.version>
<dropwizard-micrometer.version>2.0.5</dropwizard-micrometer.version>
<json-schema-validator.version>1.0.70</json-schema-validator.version>
<spring-expression.version>5.3.21</spring-expression.version>
<java-jwt.version>3.19.2</java-jwt.version>
<jwks-rsa.version>0.21.1</jwks-rsa.version>
<jjwt.version>0.9.1</jjwt.version>
<rerunner-jupiter.version>2.1.6</rerunner-jupiter.version>
</properties>
<dependencyManagement>
<dependencies>