diff --git a/catalog-rest-service/pom.xml b/catalog-rest-service/pom.xml index 61b9cf916fe..f9580a4a866 100644 --- a/catalog-rest-service/pom.xml +++ b/catalog-rest-service/pom.xml @@ -380,7 +380,7 @@ io.jsonwebtoken jjwt - 0.9.1 + ${jjwt.version} @@ -388,14 +388,14 @@ com.auth0 java-jwt - 3.19.1 + ${java-jwt.version} com.auth0 jwks-rsa - 0.21.1 + ${jwks-rsa.version} @@ -408,7 +408,7 @@ io.github.artsok rerunner-jupiter - 2.1.6 + ${rerunner-jupiter.version} test @@ -451,6 +451,11 @@ websocket-server 9.4.46.v20220331 + + org.springframework + spring-expression + ${spring-expression.version} + diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java index c44c0c55730..bbc7d13e122 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java @@ -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 { @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 { return new PolicyUpdater(original, updated, operation); } - /** - * Validates policy schema beyond what json-schema validation supports. If policy is invalid, throws {@link - * IllegalArgumentException} - * - *

Example of validation that jsonschema2pojo does not support: - * "anyOf": [ - * {"required": ["entityTypeAttr"]}, - * {"required": ["entityTagAttr"]}, - * {"required": ["userRoleAttr"]} - * ] - * - */ - 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 operations = new HashSet<>(); - List 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 getAccessControlPolicies() throws IOException { diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/lineage/LineageResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/lineage/LineageResource.java index 94040f4057a..a4cac9a981c 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/lineage/LineageResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/lineage/LineageResource.java @@ -246,5 +246,5 @@ public class LineageResource { public EntityInterface getEntity() throws IOException { return null; } - }; + } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/policies/PolicyResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/policies/PolicyResource.java index f2b144e268e..97df5814445 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/policies/PolicyResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/policies/PolicyResource.java @@ -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 { // 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 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 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 { @@ -425,4 +410,20 @@ public class PolicyResource extends EntityResource { } return policy; } + + public static List getResourceDescriptors() throws IOException { + List 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; + } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/NoopAuthorizer.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/NoopAuthorizer.java index 9c62cfb953f..782e2935813 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/NoopAuthorizer.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/NoopAuthorizer.java @@ -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 */ + } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/OperationContext.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/OperationContext.java index 20ec85c048c..dd9c32c2d2d 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/OperationContext.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/OperationContext.java @@ -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 operations = new ArrayList<>(); + List 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 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; + } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/PolicyEvaluator.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/PolicyEvaluator.java index fb4ba640acf..ce8a2b6feb8 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/PolicyEvaluator.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/PolicyEvaluator.java @@ -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; diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java index 40de5422b6f..68866a25886 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java @@ -383,7 +383,8 @@ public final class EntityUtil { List 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; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java index 4eb5c155f76..de0ad9b7e8e 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java @@ -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 readValueWithValidation(String json, Class clz) throws IOException { + T pojo = readValue(json, clz); + if (pojo == null) { + return null; + } + Set> violations = VALIDATOR.validate(pojo); + if (violations.size() > 0) { + List errors = new ArrayList<>(); + for (ConstraintViolation violation : violations) { + errors.add(violation.getPropertyPath().toString() + " " + violation.getMessage()); + } + throw new IllegalArgumentException(errors.toString()); + } + return pojo; + } + public static T readValue(String json, TypeReference valueTypeRef) throws IOException { if (json == null) { return null; diff --git a/catalog-rest-service/src/main/resources/json/data/ResourceDescriptors.json b/catalog-rest-service/src/main/resources/json/data/ResourceDescriptors.json index f8f92a86a95..85e3350b728 100644 --- a/catalog-rest-service/src/main/resources/json/data/ResourceDescriptors.json +++ b/catalog-rest-service/src/main/resources/json/data/ResourceDescriptors.json @@ -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" : [ diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json b/catalog-rest-service/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json index cf4bf519d9b..d908abf64d1 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json +++ b/catalog-rest-service/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json @@ -11,6 +11,7 @@ "description": "This schema defines all possible operations on metadata of entities in OpenMetadata.", "type": "string", "enum": [ + "All", "Create", "Delete", "ViewAll", diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/policies/accessControl/rule.json b/catalog-rest-service/src/main/resources/json/schema/entity/policies/accessControl/rule.json index 24558af30c8..8a7903ff77d 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/policies/accessControl/rule.json +++ b/catalog-rest-service/src/main/resources/json/schema/entity/policies/accessControl/rule.json @@ -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 } diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/policies/policy.json b/catalog-rest-service/src/main/resources/json/schema/entity/policies/policy.json index c085b608f27..2236f0a2c3e 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/policies/policy.json +++ b/catalog-rest-service/src/main/resources/json/schema/entity/policies/policy.json @@ -105,6 +105,6 @@ "default": false } }, - "required": ["id", "name", "policyType"], + "required": ["id", "name", "policyType", "rules"], "additionalProperties": false } diff --git a/catalog-rest-service/src/main/resources/json/schema/type/basic.json b/catalog-rest-service/src/main/resources/json/schema/type/basic.json index 12695c34015..124747ed8e6 100644 --- a/catalog-rest-service/src/main/resources/json/schema/type/basic.json +++ b/catalog-rest-service/src/main/resources/json/schema/type/basic.json @@ -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": { diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java index 8c4af10b7ea..3731e77013e 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java @@ -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 void post_AccessControlPolicyWithValidRules_200_ok(TestInfo test) throws IOException { List 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 List 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 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 diff --git a/common/pom.xml b/common/pom.xml index 5e435ec6bcb..791f0fb8f8a 100644 --- a/common/pom.xml +++ b/common/pom.xml @@ -91,7 +91,7 @@ io.jsonwebtoken jjwt - 0.9.1 + ${jjwt.version} @@ -99,14 +99,14 @@ com.auth0 java-jwt - 3.19.2 + ${java-jwt.version} com.auth0 jwks-rsa - 0.21.1 + ${jwks-rsa.version} diff --git a/ingestion-core/src/metadata/_version.py b/ingestion-core/src/metadata/_version.py index 370d1db9b13..ae0e8e4c4a7 100644 --- a/ingestion-core/src/metadata/_version.py +++ b/ingestion-core/src/metadata/_version.py @@ -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__"] diff --git a/openmetadata-core/pom.xml b/openmetadata-core/pom.xml index 801dea8dda3..66374846195 100644 --- a/openmetadata-core/pom.xml +++ b/openmetadata-core/pom.xml @@ -102,7 +102,7 @@ io.jsonwebtoken jjwt - 0.9.1 + ${jjwt.version} @@ -110,14 +110,14 @@ com.auth0 java-jwt - 3.19.1 + ${java-jwt.version} com.auth0 jwks-rsa - 0.21.1 + ${jwks-rsa.version} diff --git a/openmetadata-ui/pom.xml b/openmetadata-ui/pom.xml index 8e6484f6b41..2b8d3321b66 100644 --- a/openmetadata-ui/pom.xml +++ b/openmetadata-ui/pom.xml @@ -31,7 +31,7 @@ io.github.artsok rerunner-jupiter - 2.1.6 + ${rerunner-jupiter.version} test diff --git a/pom.xml b/pom.xml index 56732deb999..c6c82cc0a3c 100644 --- a/pom.xml +++ b/pom.xml @@ -130,6 +130,11 @@ 7.6.0 2.0.5 1.0.70 + 5.3.21 + 3.19.2 + 0.21.1 + 0.9.1 + 2.1.6