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 126a9bbb444..1dbd3dbda16 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 @@ -20,6 +20,7 @@ import java.net.URI; import java.util.ArrayList; import java.util.List; import java.util.UUID; +import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; import org.jdbi.v3.sqlobject.transaction.Transaction; import org.openmetadata.catalog.Entity; @@ -43,6 +44,7 @@ public class PolicyRepository extends EntityRepository { new Fields(PolicyResource.FIELD_LIST, "displayName,description,owner,policyUrl,enabled,rules,location"); private static final Fields POLICY_PATCH_FIELDS = new Fields(PolicyResource.FIELD_LIST, "displayName,description,owner,policyUrl,enabled,rules,location"); + public static final String ENABLED = "enabled"; public PolicyRepository(CollectionDAO dao) { super( @@ -91,7 +93,7 @@ public class PolicyRepository extends EntityRepository { policy.setDescription(fields.contains("description") ? policy.getDescription() : null); policy.setOwner(fields.contains("owner") ? getOwner(policy) : null); policy.setPolicyUrl(fields.contains("policyUrl") ? policy.getPolicyUrl() : null); - policy.setEnabled(fields.contains("enabled") ? policy.getEnabled() : null); + policy.setEnabled(fields.contains(ENABLED) ? policy.getEnabled() : null); policy.setRules(fields.contains("rules") ? policy.getRules() : null); policy.setLocation(fields.contains("location") ? getLocationForPolicy(policy) : null); return policy; @@ -194,12 +196,12 @@ public class PolicyRepository extends EntityRepository { } private List getAccessControlPolicies() throws IOException { - EntityUtil.Fields fields = new EntityUtil.Fields(List.of("policyType", "rules")); + EntityUtil.Fields fields = new EntityUtil.Fields(List.of("policyType", "rules", ENABLED)); List jsons = daoCollection.policyDAO().listAfter(null, Integer.MAX_VALUE, "", Include.NON_DELETED); List policies = new ArrayList<>(jsons.size()); for (String json : jsons) { Policy policy = setFields(JsonUtils.readValue(json, Policy.class), fields); - if (policy.getPolicyType() != PolicyType.AccessControl) { + if (!policy.getPolicyType().equals(PolicyType.AccessControl)) { continue; } policies.add(policy); @@ -207,10 +209,17 @@ public class PolicyRepository extends EntityRepository { return policies; } + /** + * Helper method to get Access Control Policies Rules. This method returns only rules for policies that are enabled. + */ public List getAccessControlPolicyRules() throws IOException { List policies = getAccessControlPolicies(); List rules = new ArrayList<>(); for (Policy policy : policies) { + if (!Boolean.TRUE.equals(policy.getEnabled())) { + // Skip if policy is not enabled. + continue; + } List ruleObjects = policy.getRules(); for (Object ruleObject : ruleObjects) { Rule rule = JsonUtils.readValue(JsonUtils.getJsonStructure(ruleObject).toString(), Rule.class); @@ -220,6 +229,10 @@ public class PolicyRepository extends EntityRepository { return rules; } + public static List getRuleObjects(List rules) { + return rules.stream().map(Object.class::cast).collect(Collectors.toList()); + } + private void setLocation(Policy policy, EntityReference location) { if (location == null || location.getId() == null) { return; @@ -375,7 +388,7 @@ public class PolicyRepository extends EntityRepository { throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute(Entity.POLICY, "policyType")); } recordChange("policyUrl", original.getEntity().getPolicyUrl(), updated.getEntity().getPolicyUrl()); - recordChange("enabled", original.getEntity().getEnabled(), updated.getEntity().getEnabled()); + recordChange(ENABLED, original.getEntity().getEnabled(), updated.getEntity().getEnabled()); recordChange("rules", original.getEntity().getRules(), updated.getEntity().getRules()); updateLocation(original.getEntity(), updated.getEntity()); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/charts/ChartResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/charts/ChartResource.java index c257dbecf5d..1400146a026 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/charts/ChartResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/charts/ChartResource.java @@ -55,7 +55,6 @@ import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.api.data.CreateChart; import org.openmetadata.catalog.entity.data.Chart; import org.openmetadata.catalog.jdbi3.ChartRepository; -import org.openmetadata.catalog.jdbi3.ChartRepository.ChartEntityInterface; import org.openmetadata.catalog.jdbi3.CollectionDAO; import org.openmetadata.catalog.resources.Collection; import org.openmetadata.catalog.security.Authorizer; @@ -328,7 +327,8 @@ public class ChartResource { Fields fields = new Fields(FIELD_LIST, FIELDS); Chart chart = dao.get(uriInfo, id, fields); SecurityUtil.checkAdminRoleOrPermissions( - authorizer, securityContext, new ChartEntityInterface(chart).getEntityReference()); + authorizer, securityContext, dao.getEntityInterface(chart).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/config/ConfigResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/config/ConfigResource.java index be3e6d28b52..df60e396975 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/config/ConfigResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/config/ConfigResource.java @@ -28,6 +28,7 @@ import javax.ws.rs.core.UriInfo; import org.openmetadata.catalog.CatalogApplicationConfig; import org.openmetadata.catalog.resources.Collection; import org.openmetadata.catalog.security.AuthenticationConfiguration; +import org.openmetadata.catalog.security.AuthorizerConfiguration; @Path("/v1/config") @Api(value = "Get configuration") @@ -61,4 +62,27 @@ public class ConfigResource { } return authenticationConfiguration; } + + @GET + @Path(("/authorizer")) + @Operation( + summary = "Get authorizer configuration", + tags = "general", + responses = { + @ApiResponse( + responseCode = "200", + description = "Authorizer configuration", + content = + @Content( + mediaType = "application/json", + schema = @Schema(implementation = AuthorizerConfiguration.class))) + }) + public AuthorizerConfiguration getAuthorizerConfig( + @Context UriInfo uriInfo, @Context SecurityContext securityContext) { + AuthorizerConfiguration authorizerConfiguration = new AuthorizerConfiguration(); + if (catalogApplicationConfig.getAuthorizerConfiguration() != null) { + authorizerConfiguration = catalogApplicationConfig.getAuthorizerConfiguration(); + } + return authorizerConfiguration; + } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/dashboards/DashboardResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/dashboards/DashboardResource.java index bb3e741c19d..838877d3522 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/dashboards/DashboardResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/dashboards/DashboardResource.java @@ -333,7 +333,9 @@ public class DashboardResource { throws IOException, ParseException { Fields fields = new Fields(FIELD_LIST, FIELDS); Dashboard dashboard = dao.get(uriInfo, id, fields); - SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(dashboard)); + SecurityUtil.checkAdminRoleOrPermissions( + authorizer, securityContext, dao.getEntityInterface(dashboard).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/DatabaseResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/DatabaseResource.java index d898d44e19b..9d23fbbb7bd 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/DatabaseResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/DatabaseResource.java @@ -56,7 +56,6 @@ import org.openmetadata.catalog.api.data.CreateDatabase; import org.openmetadata.catalog.entity.data.Database; import org.openmetadata.catalog.jdbi3.CollectionDAO; import org.openmetadata.catalog.jdbi3.DatabaseRepository; -import org.openmetadata.catalog.jdbi3.DatabaseRepository.DatabaseEntityInterface; import org.openmetadata.catalog.resources.Collection; import org.openmetadata.catalog.security.Authorizer; import org.openmetadata.catalog.security.SecurityUtil; @@ -344,10 +343,13 @@ public class DatabaseResource { })) JsonPatch patch) throws IOException, ParseException { + Fields fields = new Fields(FIELD_LIST, FIELDS); + Database database = dao.get(uriInfo, id, fields); + SecurityUtil.checkAdminRoleOrPermissions( + authorizer, securityContext, dao.getEntityInterface(database).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); - SecurityUtil.checkAdminRoleOrPermissions( - authorizer, securityContext, new DatabaseEntityInterface(response.getEntity()).getEntityReference()); addHref(uriInfo, response.getEntity()); return response.toResponse(); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java index 4984d6f83a3..00024e81c0b 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java @@ -349,7 +349,9 @@ public class TableResource { throws IOException, ParseException { Fields fields = new Fields(FIELD_LIST, FIELDS); Table table = dao.get(uriInfo, id, fields); - SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(table)); + SecurityUtil.checkAdminRoleOrPermissions( + authorizer, securityContext, dao.getEntityInterface(table).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/locations/LocationResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/locations/LocationResource.java index 1845d257dc1..89d5a12acac 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/locations/LocationResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/locations/LocationResource.java @@ -400,7 +400,9 @@ public class LocationResource { throws IOException, ParseException { Fields fields = new Fields(FIELD_LIST, FIELDS); Location location = dao.get(uriInfo, id, fields); - SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(location)); + SecurityUtil.checkAdminRoleOrPermissions( + authorizer, securityContext, dao.getEntityInterface(location).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/mlmodels/MlModelResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/mlmodels/MlModelResource.java index 8f6348366cb..343504f80bc 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/mlmodels/MlModelResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/mlmodels/MlModelResource.java @@ -272,7 +272,9 @@ public class MlModelResource { throws IOException, ParseException { Fields fields = new Fields(FIELD_LIST, FIELDS); MlModel mlModel = dao.get(uriInfo, id, fields); - SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(mlModel)); + SecurityUtil.checkAdminRoleOrPermissions( + authorizer, securityContext, dao.getEntityInterface(mlModel).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/operations/IngestionResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/operations/IngestionResource.java index 1c1930fad90..5a23fac76ce 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/operations/IngestionResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/operations/IngestionResource.java @@ -355,7 +355,9 @@ public class IngestionResource { throws IOException, ParseException { Fields fields = new Fields(FIELD_LIST, FIELDS); Ingestion ingestion = dao.get(uriInfo, id, fields); - SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(ingestion)); + SecurityUtil.checkAdminRoleOrPermissions( + authorizer, securityContext, dao.getEntityInterface(ingestion).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/pipelines/PipelineResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/pipelines/PipelineResource.java index e56c9156113..5ec12a9344f 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/pipelines/PipelineResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/pipelines/PipelineResource.java @@ -332,7 +332,9 @@ public class PipelineResource { throws IOException, ParseException { Fields fields = new Fields(FIELD_LIST, FIELDS); Pipeline pipeline = dao.get(uriInfo, id, fields); - SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(pipeline)); + SecurityUtil.checkAdminRoleOrPermissions( + authorizer, securityContext, dao.getEntityInterface(pipeline).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); 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 edfde5fc4b5..ffeb6562bdd 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 @@ -379,7 +379,8 @@ public class PolicyResource { .withPolicyType(create.getPolicyType()) .withUpdatedBy(securityContext.getUserPrincipal().getName()) .withUpdatedAt(System.currentTimeMillis()) - .withRules(create.getRules()); + .withRules(create.getRules()) + .withEnabled(create.getEnabled()); if (create.getLocation() != null) { policy = policy.withLocation(new EntityReference().withId(create.getLocation())); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/topics/TopicResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/topics/TopicResource.java index 29cf4e8275c..993b3a36e37 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/topics/TopicResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/topics/TopicResource.java @@ -330,7 +330,9 @@ public class TopicResource { throws IOException, ParseException { Fields fields = new Fields(FIELD_LIST, FIELDS); Topic topic = dao.get(uriInfo, id, fields); - SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(topic)); + SecurityUtil.checkAdminRoleOrPermissions( + authorizer, securityContext, dao.getEntityInterface(topic).getEntityReference(), patch); + PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); addHref(uriInfo, response.getEntity()); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java index f780039adf6..dd743f4f379 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java @@ -21,6 +21,9 @@ import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.UUID; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import org.apache.commons.lang3.exception.ExceptionUtils; import org.jdbi.v3.core.Jdbi; import org.openmetadata.catalog.Entity; @@ -44,10 +47,16 @@ public class DefaultAuthorizer implements Authorizer { private Set botUsers; private String principalDomain; + private CollectionDAO collectionDAO; private UserRepository userRepository; - private PolicyEvaluator policyEvaluator; + + // policyEvaluator has to be thread-safe. A background thread will be updating the value. + private volatile PolicyEvaluator policyEvaluator; private static final String fieldsParam = "roles,teams"; + public static final int POLICY_LOADER_INITIAL_DELAY = 5; // seconds. + private static final int POLICY_LOADER_SCHEDULE_INTERVAL = 300; // seconds. + @Override public void init(AuthorizerConfiguration config, Jdbi dbi) throws IOException { LOG.debug("Initializing DefaultAuthorizer with config {}", config); @@ -55,12 +64,28 @@ public class DefaultAuthorizer implements Authorizer { this.botUsers = new HashSet<>(config.getBotPrincipals()); this.principalDomain = config.getPrincipalDomain(); LOG.debug("Admin users: {}", adminUsers); - CollectionDAO collectionDAO = dbi.onDemand(CollectionDAO.class); + this.collectionDAO = dbi.onDemand(CollectionDAO.class); this.userRepository = new UserRepository(collectionDAO); mayBeAddAdminUsers(); mayBeAddBotUsers(); - // Load all rules from access control policies at once during init. - this.policyEvaluator = new PolicyEvaluator(new PolicyRepository(collectionDAO).getAccessControlPolicyRules()); + + // Use a 15-min schedule to refresh policies. This should be replaced by a better solution which can load policies + // only when a policy change event occurs. + ScheduledExecutorService scheduleService = Executors.newSingleThreadScheduledExecutor(); + scheduleService.scheduleWithFixedDelay( + new PolicyLoader(), POLICY_LOADER_INITIAL_DELAY, POLICY_LOADER_SCHEDULE_INTERVAL, TimeUnit.SECONDS); + } + + private class PolicyLoader implements Runnable { + @Override + public void run() { + LOG.info("Loading access control policies for DefaultAuthorizer Policy Evaluator"); + try { + policyEvaluator = new PolicyEvaluator(new PolicyRepository(collectionDAO).getAccessControlPolicyRules()); + } catch (IOException e) { + LOG.warn("Could not update access control policies for DefaultAuthorizer Policy Evaluator: {}", e.getMessage()); + } + } } private void mayBeAddAdminUsers() { @@ -124,16 +149,7 @@ public class DefaultAuthorizer implements Authorizer { } try { User user = getUserFromAuthenticationContext(ctx); - if (owner.getType().equals(Entity.TEAM)) { - for (EntityReference team : user.getTeams()) { - if (team.getName().equals(owner.getName())) { - return true; - } - } - } else if (owner.getType().equals(Entity.USER)) { - return user.getName().equals(owner.getName()); - } - return false; + return isOwnedByUser(user, owner); } catch (IOException | EntityNotFoundException | ParseException ex) { return false; } @@ -144,15 +160,40 @@ public class DefaultAuthorizer implements Authorizer { AuthenticationContext ctx, EntityReference entityReference, MetadataOperation operation) { validateAuthenticationContext(ctx); try { - return policyEvaluator.hasPermission( - getUserFromAuthenticationContext(ctx), - Entity.getEntity(entityReference, new EntityUtil.Fields(List.of("tags"))), - operation); + Object entity = Entity.getEntity(entityReference, new EntityUtil.Fields(List.of("tags", "owner"))); + EntityReference owner = Entity.getEntityInterface(entity).getOwner(); + if (owner == null) { + // Entity does not have an owner. + return true; + } + User user = getUserFromAuthenticationContext(ctx); + if (isOwnedByUser(user, owner)) { + // Entity is owned by the user. + return true; + } + return policyEvaluator.hasPermission(user, entity, operation); } catch (IOException | EntityNotFoundException | ParseException ex) { return false; } } + /** Checks if the user is same as owner or part of the team that is the owner. */ + private boolean isOwnedByUser(User user, EntityReference owner) { + if (owner.getType().equals(Entity.USER) && owner.getName().equals(user.getName())) { + // Owner is same as user. + return true; + } + if (owner.getType().equals(Entity.TEAM)) { + for (EntityReference userTeam : user.getTeams()) { + if (userTeam.getName().equals(owner.getName())) { + // Owner is a team, and the user is part of this team. + return true; + } + } + } + return false; + } + @Override public boolean isAdmin(AuthenticationContext ctx) { validateAuthenticationContext(ctx); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/SecurityUtil.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/SecurityUtil.java index f152749e368..be4bf408895 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/SecurityUtil.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/SecurityUtil.java @@ -15,18 +15,17 @@ package org.openmetadata.catalog.security; import java.security.Principal; import java.util.HashMap; +import java.util.List; import java.util.Map; +import javax.json.JsonPatch; import javax.ws.rs.client.Invocation; import javax.ws.rs.client.WebTarget; import javax.ws.rs.core.SecurityContext; import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.type.MetadataOperation; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.openmetadata.catalog.util.JsonPatchUtils; public final class SecurityUtil { - private static final Logger LOG = LoggerFactory.getLogger(SecurityUtil.class); - private SecurityUtil() {} public static void checkAdminRole(Authorizer authorizer, SecurityContext securityContext) { @@ -56,17 +55,24 @@ public final class SecurityUtil { } } + /** + * Most REST API requests should yield in a single metadata operation. There are cases where the JSON patch request + * may yield multiple metadata operations. This helper function checks if user has permission to perform the given set + * of metadata operations. + */ public static void checkAdminRoleOrPermissions( - Authorizer authorizer, - SecurityContext securityContext, - EntityReference entityReference, - MetadataOperation metadataOperation) { + Authorizer authorizer, SecurityContext securityContext, EntityReference entityReference, JsonPatch patch) { Principal principal = securityContext.getUserPrincipal(); AuthenticationContext authenticationCtx = SecurityUtil.getAuthenticationContext(principal); - if (!authorizer.isAdmin(authenticationCtx) - && !authorizer.isBot(authenticationCtx) - && !authorizer.hasPermissions(authenticationCtx, entityReference, metadataOperation)) { - throw new AuthorizationException("Principal: " + principal + " does not have permissions"); + + if (authorizer.isAdmin(authenticationCtx) || authorizer.isBot(authenticationCtx)) return; + + List metadataOperations = JsonPatchUtils.getMetadataOperations(patch); + for (MetadataOperation metadataOperation : metadataOperations) { + if (!authorizer.hasPermissions(authenticationCtx, entityReference, metadataOperation)) { + throw new AuthorizationException( + "Principal: " + principal + " does not have permission to " + metadataOperation); + } } } 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 df79f9b2886..aedce54b6ec 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 @@ -37,6 +37,7 @@ public class PolicyEvaluator { public PolicyEvaluator(List rules) { this.rules = new Rules(); rules.stream() + // Add rules only if they are enabled. .filter(org.openmetadata.catalog.entity.policies.accessControl.Rule::getEnabled) .map(this::convertRule) .forEach(this.rules::register); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonPatchUtils.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonPatchUtils.java new file mode 100644 index 00000000000..3f0e304778e --- /dev/null +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonPatchUtils.java @@ -0,0 +1,38 @@ +package org.openmetadata.catalog.util; + +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; +import javax.json.JsonPatch; +import org.openmetadata.catalog.type.MetadataOperation; + +public class JsonPatchUtils { + private JsonPatchUtils() {} + + public static List getMetadataOperations(JsonPatch jsonPatch) { + return jsonPatch.toJsonArray().stream() + .map(JsonPatchUtils::getMetadataOperation) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + } + + private static MetadataOperation getMetadataOperation(Object jsonPatchObject) { + Map jsonPatchMap = JsonUtils.getMap(jsonPatchObject); + String path = jsonPatchMap.get("path").toString(); + + // To get operation, use the following: + // JsonPatch.Operation op = JsonPatch.Operation.fromOperationName(jsonPatchMap.get("op").toString()); + + if (path.contains("description")) { + return MetadataOperation.UpdateDescription; + } + if (path.contains("tags")) { + return MetadataOperation.UpdateTags; + } + if (path.contains("owner")) { + return MetadataOperation.UpdateOwner; + } + return null; + } +} diff --git a/catalog-rest-service/src/main/resources/json/schema/api/policies/createPolicy.json b/catalog-rest-service/src/main/resources/json/schema/api/policies/createPolicy.json index 3da6ad02cfa..5982eac6977 100644 --- a/catalog-rest-service/src/main/resources/json/schema/api/policies/createPolicy.json +++ b/catalog-rest-service/src/main/resources/json/schema/api/policies/createPolicy.json @@ -32,6 +32,11 @@ "rules": { "$ref": "../../entity/policies/policy.json#/definitions/rules" }, + "enabled": { + "description": "Is the policy enabled.", + "type": "boolean", + "default": true + }, "location" : { "description": "UUID of Location where this policy is applied", "$ref": "../../type/basic.json#/definitions/uuid", diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityOperationsResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityOperationsResourceTest.java index 131d6c17735..58e61fe0543 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityOperationsResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityOperationsResourceTest.java @@ -25,7 +25,8 @@ public abstract class EntityOperationsResourceTest extends EntityResourceTest String fields, boolean supportsFollowers, boolean supportsOwner, - boolean supportsTags) { + boolean supportsTags, + boolean supportsAuthorizedMetadataOperations) { super( entityName, entityClass, @@ -34,7 +35,8 @@ public abstract class EntityOperationsResourceTest extends EntityResourceTest fields, supportsFollowers, supportsOwner, - supportsTags); + supportsTags, + supportsAuthorizedMetadataOperations); } // Override the resource path name of regular entities api/v1/ to api/operations/v1/ diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java index 8d4f70f6c8b..5fb86812a51 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java @@ -16,6 +16,7 @@ package org.openmetadata.catalog.resources; import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static javax.ws.rs.core.Response.Status.CONFLICT; import static javax.ws.rs.core.Response.Status.CREATED; +import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static javax.ws.rs.core.Response.Status.NOT_FOUND; import static javax.ws.rs.core.Response.Status.OK; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -25,6 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.openmetadata.catalog.exception.CatalogExceptionMessage.ENTITY_ALREADY_EXISTS; import static org.openmetadata.catalog.exception.CatalogExceptionMessage.entityNotFound; import static org.openmetadata.catalog.security.SecurityUtil.authHeaders; @@ -68,6 +70,7 @@ import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestInstance; import org.openmetadata.catalog.CatalogApplicationTest; import org.openmetadata.catalog.Entity; +import org.openmetadata.catalog.api.policies.CreatePolicy; import org.openmetadata.catalog.api.services.CreateDatabaseService; import org.openmetadata.catalog.api.services.CreateDatabaseService.DatabaseServiceType; import org.openmetadata.catalog.api.services.CreateMessagingService; @@ -75,23 +78,28 @@ import org.openmetadata.catalog.api.services.CreateMessagingService.MessagingSer import org.openmetadata.catalog.api.services.CreatePipelineService; import org.openmetadata.catalog.api.services.CreatePipelineService.PipelineServiceType; import org.openmetadata.catalog.api.services.CreateStorageService; +import org.openmetadata.catalog.entity.policies.accessControl.Rule; import org.openmetadata.catalog.entity.services.DatabaseService; import org.openmetadata.catalog.entity.services.MessagingService; import org.openmetadata.catalog.entity.services.PipelineService; import org.openmetadata.catalog.entity.services.StorageService; +import org.openmetadata.catalog.entity.teams.Role; import org.openmetadata.catalog.entity.teams.Team; import org.openmetadata.catalog.entity.teams.User; import org.openmetadata.catalog.exception.CatalogExceptionMessage; import org.openmetadata.catalog.jdbi3.DatabaseServiceRepository.DatabaseServiceEntityInterface; import org.openmetadata.catalog.jdbi3.MessagingServiceRepository.MessagingServiceEntityInterface; import org.openmetadata.catalog.jdbi3.PipelineServiceRepository.PipelineServiceEntityInterface; +import org.openmetadata.catalog.jdbi3.PolicyRepository; import org.openmetadata.catalog.resources.events.EventResource.ChangeEventList; import org.openmetadata.catalog.resources.events.WebhookResourceTest; +import org.openmetadata.catalog.resources.policies.PolicyResourceTest; import org.openmetadata.catalog.resources.services.DatabaseServiceResourceTest; import org.openmetadata.catalog.resources.services.MessagingServiceResourceTest; import org.openmetadata.catalog.resources.services.PipelineServiceResourceTest; import org.openmetadata.catalog.resources.services.StorageServiceResourceTest; import org.openmetadata.catalog.resources.tags.TagResourceTest; +import org.openmetadata.catalog.resources.teams.RoleResourceTest; import org.openmetadata.catalog.resources.teams.TeamResourceTest; import org.openmetadata.catalog.resources.teams.UserResourceTest; import org.openmetadata.catalog.type.ChangeDescription; @@ -100,12 +108,15 @@ import org.openmetadata.catalog.type.EntityHistory; import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.type.EventType; import org.openmetadata.catalog.type.FieldChange; +import org.openmetadata.catalog.type.MetadataOperation; +import org.openmetadata.catalog.type.PolicyType; import org.openmetadata.catalog.type.StorageServiceType; import org.openmetadata.catalog.type.Tag; import org.openmetadata.catalog.type.TagLabel; import org.openmetadata.catalog.util.EntityInterface; import org.openmetadata.catalog.util.EntityUtil; import org.openmetadata.catalog.util.JsonUtils; +import org.openmetadata.catalog.util.PolicyUtils; import org.openmetadata.catalog.util.ResultList; import org.openmetadata.catalog.util.TestUtils; @@ -121,11 +132,21 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { private final boolean supportsOwner; private final boolean supportsTags; protected boolean supportsPatch = true; + private boolean supportsAuthorizedMetadataOperations; + + public static final String DATA_STEWARD_ROLE_NAME = "DataSteward"; + public static final String DATA_CONSUMER_ROLE_NAME = "DataConsumer"; public static User USER1; public static EntityReference USER_OWNER1; public static Team TEAM1; public static EntityReference TEAM_OWNER1; + public static User USER_WITH_DATA_STEWARD_ROLE; + public static Role DATA_STEWARD_ROLE; + public static EntityReference DATA_STEWARD_ROLE_REFERENCE; + public static User USER_WITH_DATA_CONSUMER_ROLE; + public static Role DATA_CONSUMER_ROLE; + public static EntityReference DATA_CONSUMER_ROLE_REFERENCE; public static EntityReference SNOWFLAKE_REFERENCE; public static EntityReference REDSHIFT_REFERENCE; @@ -155,7 +176,8 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { String fields, boolean supportsFollowers, boolean supportsOwner, - boolean supportsTags) { + boolean supportsTags, + boolean supportsAuthorizedMetadataOperations) { this.entityName = entityName; this.entityClass = entityClass; this.entityListClass = entityListClass; @@ -164,6 +186,7 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { this.supportsFollowers = supportsFollowers; this.supportsOwner = supportsOwner; this.supportsTags = supportsTags; + this.supportsAuthorizedMetadataOperations = supportsAuthorizedMetadataOperations; ENTITY_RESOURCE_TEST_MAP.put(entityName, this); } @@ -175,13 +198,37 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { webhookResourceTest.startWebhookEntitySubscriptions(entityName); UserResourceTest userResourceTest = new UserResourceTest(); - USER1 = UserResourceTest.createUser(userResourceTest.create(test), authHeaders("test@open-metadata.org")); + USER1 = UserResourceTest.createUser(userResourceTest.create(test), adminAuthHeaders()); USER_OWNER1 = new EntityReference().withId(USER1.getId()).withType("user"); + RoleResourceTest roleResourceTest = new RoleResourceTest(); + DATA_STEWARD_ROLE = + RoleResourceTest.createRole(roleResourceTest.create(DATA_STEWARD_ROLE_NAME), adminAuthHeaders()); + DATA_STEWARD_ROLE_REFERENCE = new EntityReference().withId(DATA_STEWARD_ROLE.getId()).withType("role"); + USER_WITH_DATA_STEWARD_ROLE = + UserResourceTest.createUser( + userResourceTest.create("user-data-steward").withRoles(List.of(DATA_STEWARD_ROLE.getId())), + adminAuthHeaders()); + DATA_CONSUMER_ROLE = + RoleResourceTest.createRole(roleResourceTest.create(DATA_CONSUMER_ROLE_NAME), adminAuthHeaders()); + DATA_CONSUMER_ROLE_REFERENCE = new EntityReference().withId(DATA_CONSUMER_ROLE.getId()).withType("role"); + USER_WITH_DATA_CONSUMER_ROLE = + UserResourceTest.createUser( + userResourceTest.create("user-data-consumer").withRoles(List.of(DATA_CONSUMER_ROLE.getId())), + adminAuthHeaders()); + TeamResourceTest teamResourceTest = new TeamResourceTest(); TEAM1 = TeamResourceTest.createTeam(teamResourceTest.create(test), adminAuthHeaders()); TEAM_OWNER1 = new EntityReference().withId(TEAM1.getId()).withType("team"); + PolicyResourceTest.createPolicy(createAccessControlPolicies(), adminAuthHeaders()); + // Ensure that DefaultAuthorizer gets enough time to load policies before running tests. + try { + Thread.sleep(8000); + } catch (InterruptedException e) { + fail(); + } + // Create snowflake database service DatabaseServiceResourceTest databaseServiceResourceTest = new DatabaseServiceResourceTest(); CreateDatabaseService createDatabaseService = @@ -549,7 +596,7 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { } @Test - void post_chartWithInvalidOwnerType_4xx(TestInfo test) throws URISyntaxException { + void post_entityWithInvalidOwnerType_4xx(TestInfo test) throws URISyntaxException { if (!supportsOwner) { return; } @@ -793,6 +840,47 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { /////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Common entity tests for PATCH operations /////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + @Test + void patch_entityDescriptionAndTestAuthorizer(TestInfo test) throws IOException, URISyntaxException { + if (!supportsPatch || !supportsAuthorizedMetadataOperations) { + return; + } + + T entity = createEntity(createRequest(getEntityName(test), "description", null, null), adminAuthHeaders()); + + // Anyone can update description on unowned entity. + entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), TestUtils.ADMIN_USER_NAME, false); + entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER1.getName(), false); + entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_STEWARD_ROLE.getName(), false); + entity = + patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_CONSUMER_ROLE.getName(), false); + + EntityInterface entityInterface = getEntityInterface(entity); + + if (!supportsOwner) { + return; + } + + // Set the owner for the table. + String originalJson = JsonUtils.pojoToJson(entity); + ChangeDescription change = getChangeDescription(entityInterface.getVersion()); + change.getFieldsAdded().add(new FieldChange().withName("owner").withNewValue(USER_OWNER1)); + entityInterface.setOwner(USER_OWNER1); + entity = + patchEntityAndCheck( + entityInterface.getEntity(), + originalJson, + authHeaders(USER1.getName() + "@open-metadata.org"), + MINOR_UPDATE, + change); + + // Admin, owner (USER1) and user with DataSteward role can update description on entity owned by USER1. + entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), TestUtils.ADMIN_USER_NAME, false); + entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER1.getName(), false); + entity = patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_STEWARD_ROLE.getName(), false); + patchEntityAndCheckAuthorization(getEntityInterface(entity), USER_WITH_DATA_CONSUMER_ROLE.getName(), true); + } + @Test void patch_entityAttributes_200_ok(TestInfo test) throws IOException, URISyntaxException { if (!supportsPatch) { @@ -1136,6 +1224,40 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { return returned; } + T patchEntityAndCheckAuthorization(EntityInterface entityInterface, String userName, boolean shouldThrowException) + throws IOException { + T entity = entityInterface.getEntity(); + String originalJson = JsonUtils.pojoToJson(entity); + + String originalDescription = entityInterface.getDescription(); + String newDescription = String.format("Description added by %s", userName); + ChangeDescription change = getChangeDescription(entityInterface.getVersion()); + change + .getFieldsUpdated() + .add(new FieldChange().withName("description").withOldValue(originalDescription).withNewValue(newDescription)); + + entityInterface.setDescription(newDescription); + + if (shouldThrowException) { + HttpResponseException exception = + assertThrows( + HttpResponseException.class, + () -> + patchEntity( + entityInterface.getId(), originalJson, entity, authHeaders(userName + "@open-metadata.org"))); + assertResponse( + exception, + FORBIDDEN, + String.format( + "Principal: CatalogPrincipal{name='%s'} does not have permission to UpdateDescription", userName)); + // Revert to original. + entityInterface.setDescription(originalDescription); + return entityInterface.getEntity(); + } + return patchEntityAndCheck( + entity, originalJson, authHeaders(userName + "@open-metadata.org"), MINOR_UPDATE, change); + } + protected final void validateCommonEntityFields( EntityInterface entity, String expectedDescription, @@ -1448,4 +1570,24 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { public final String getEntityName(TestInfo test, int index) { return String.format("%s_%d_%s", entityName, index, test.getDisplayName()); } + + private CreatePolicy createAccessControlPolicies() { + List rules = new ArrayList<>(); + rules.add( + PolicyUtils.accessControlRule( + null, + Entity.getEntityNameFromClass(entityClass), + DATA_STEWARD_ROLE_NAME, + MetadataOperation.UpdateDescription, + true, + 1, + true)); + + return new CreatePolicy() + .withName("test-acp") + .withDescription("description") + .withPolicyType(PolicyType.AccessControl) + .withRules(PolicyRepository.getRuleObjects(rules)) + .withOwner(USER_OWNER1); + } } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/charts/ChartResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/charts/ChartResourceTest.java index 1d33e0a615a..bdc3e57e735 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/charts/ChartResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/charts/ChartResourceTest.java @@ -58,7 +58,7 @@ public class ChartResourceTest extends EntityResourceTest { public static EntityReference LOOKER_REFERENCE; public ChartResourceTest() { - super(Entity.CHART, Chart.class, ChartList.class, "charts", ChartResource.FIELDS, true, true, true); + super(Entity.CHART, Chart.class, ChartList.class, "charts", ChartResource.FIELDS, true, true, true, true); } @BeforeAll diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java index 07e509983ce..488e460094b 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java @@ -81,6 +81,7 @@ public class DashboardResourceTest extends EntityResourceTest { DashboardResource.FIELDS, true, true, + true, true); } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/DatabaseResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/DatabaseResourceTest.java index 35b8fe67fc0..c026122b3c3 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/DatabaseResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/DatabaseResourceTest.java @@ -53,7 +53,15 @@ import org.openmetadata.catalog.util.TestUtils; public class DatabaseResourceTest extends EntityResourceTest { public DatabaseResourceTest() { super( - Entity.DATABASE, Database.class, DatabaseList.class, "databases", DatabaseResource.FIELDS, false, true, false); + Entity.DATABASE, + Database.class, + DatabaseList.class, + "databases", + DatabaseResource.FIELDS, + false, + true, + false, + true); } @BeforeAll diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java index fbc0563b829..53e3e965608 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java @@ -120,7 +120,7 @@ public class TableResourceTest extends EntityResourceTest
{ getColumn("c3", BIGINT, USER_BANK_ACCOUNT_TAG_LABEL)); public TableResourceTest() { - super(Entity.TABLE, Table.class, TableList.class, "tables", TableResource.FIELDS, true, true, true); + super(Entity.TABLE, Table.class, TableList.class, "tables", TableResource.FIELDS, true, true, true, true); } @BeforeAll @@ -1069,9 +1069,6 @@ public class TableResourceTest extends EntityResourceTest
{ .withConstraintType(ConstraintType.UNIQUE) .withColumns(List.of(COLUMNS.get(0).getName()))); - // - // Add description, tableType, and tableConstraints when previously they were null - // String originalJson = JsonUtils.pojoToJson(table); ChangeDescription change = getChangeDescription(table.getVersion()); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/events/WebhookResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/events/WebhookResourceTest.java index 0e3e49d7272..c02a96d179b 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/events/WebhookResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/events/WebhookResourceTest.java @@ -60,7 +60,7 @@ public class WebhookResourceTest extends EntityResourceTest { } public WebhookResourceTest() { - super(Entity.WEBHOOK, Webhook.class, WebhookList.class, "webhook", "", false, false, false); + super(Entity.WEBHOOK, Webhook.class, WebhookList.class, "webhook", "", false, false, false, false); supportsPatch = false; } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java index 4c1124325ae..07fb531ad0e 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java @@ -54,7 +54,16 @@ import org.openmetadata.catalog.util.TestUtils; public class LocationResourceTest extends EntityResourceTest { public LocationResourceTest() { - super(Entity.LOCATION, Location.class, LocationList.class, "locations", LocationResource.FIELDS, true, true, true); + super( + Entity.LOCATION, + Location.class, + LocationList.class, + "locations", + LocationResource.FIELDS, + true, + true, + true, + true); } @BeforeAll diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/mlmodels/MlModelResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/mlmodels/MlModelResourceTest.java index 1ae61f0d7e7..f5e8c3fd5e6 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/mlmodels/MlModelResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/mlmodels/MlModelResourceTest.java @@ -116,7 +116,7 @@ public class MlModelResourceTest extends EntityResourceTest { new MlHyperParameter().withName("random").withValue("hello")); public MlModelResourceTest() { - super(Entity.MLMODEL, MlModel.class, MlModelList.class, "mlmodels", MlModelResource.FIELDS, true, true, true); + super(Entity.MLMODEL, MlModel.class, MlModelList.class, "mlmodels", MlModelResource.FIELDS, true, true, true, true); } @BeforeAll diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/operations/IngestionResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/operations/IngestionResourceTest.java index def5b7d9e9a..2192de69d7d 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/operations/IngestionResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/operations/IngestionResourceTest.java @@ -58,6 +58,7 @@ public class IngestionResourceTest extends EntityOperationsResourceTest { public static List TASKS; public PipelineResourceTest() { - super(Entity.PIPELINE, Pipeline.class, PipelineList.class, "pipelines", PipelineResource.FIELDS, true, true, true); + super( + Entity.PIPELINE, + Pipeline.class, + PipelineList.class, + "pipelines", + PipelineResource.FIELDS, + true, + true, + true, + true); } @BeforeAll 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 a71ba0348ba..792399874f5 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 @@ -72,7 +72,7 @@ public class PolicyResourceTest extends EntityResourceTest { private static Location location; public PolicyResourceTest() { - super(Entity.POLICY, Policy.class, PolicyList.class, "policies", PolicyResource.FIELDS, false, true, false); + super(Entity.POLICY, Policy.class, PolicyList.class, "policies", PolicyResource.FIELDS, false, true, false, false); } @BeforeAll diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/services/DashboardServiceResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/services/DashboardServiceResourceTest.java index 659f1257500..5db9ac4d4c8 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/services/DashboardServiceResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/services/DashboardServiceResourceTest.java @@ -60,6 +60,7 @@ public class DashboardServiceResourceTest extends EntityResourceTest { public RoleResourceTest() { - super(Entity.ROLE, Role.class, RoleList.class, "roles", null, false, false, false); + super(Entity.ROLE, Role.class, RoleList.class, "roles", null, false, false, false, false); } @Test diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java index cfccbef8c3f..2a093078483 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java @@ -64,7 +64,7 @@ public class TeamResourceTest extends EntityResourceTest { final Profile PROFILE = new Profile().withImages(new ImageList().withImage(URI.create("http://image.com"))); public TeamResourceTest() { - super(Entity.TEAM, Team.class, TeamList.class, "teams", TeamResource.FIELDS, false, false, false); + super(Entity.TEAM, Team.class, TeamList.class, "teams", TeamResource.FIELDS, false, false, false, false); } @Test diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java index fa4c3a9f409..eded542eed1 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java @@ -82,7 +82,7 @@ public class UserResourceTest extends EntityResourceTest { final Profile PROFILE = new Profile().withImages(new ImageList().withImage(URI.create("http://image.com"))); public UserResourceTest() { - super(Entity.USER, User.class, UserList.class, "users", UserResource.FIELDS, false, false, false); + super(Entity.USER, User.class, UserList.class, "users", UserResource.FIELDS, false, false, false, false); } @Test diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java index 1fd529f83fd..9604acb0fc0 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java @@ -56,7 +56,7 @@ import org.openmetadata.catalog.util.TestUtils.UpdateType; public class TopicResourceTest extends EntityResourceTest { public TopicResourceTest() { - super(Entity.TOPIC, Topic.class, TopicList.class, "topics", TopicResource.FIELDS, true, true, true); + super(Entity.TOPIC, Topic.class, TopicList.class, "topics", TopicResource.FIELDS, true, true, true, true); } @Test diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java index 7f0a18a76d4..5fcb3977d45 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java @@ -59,6 +59,7 @@ public final class TestUtils { LONG_ENTITY_NAME = "1".repeat(ENTITY_NAME_MAX_LEN + 1); } + public static final String ADMIN_USER_NAME = "admin"; public static final String ENTITY_NAME_LENGTH_ERROR = String.format("[name size must be between 1 and %d]", ENTITY_NAME_MAX_LEN); @@ -267,7 +268,7 @@ public final class TestUtils { } public static Map adminAuthHeaders() { - return SecurityUtil.authHeaders("admin@open-metadata.org"); + return SecurityUtil.authHeaders(ADMIN_USER_NAME + "@open-metadata.org"); } public static Map userAuthHeaders() { diff --git a/ingestion/examples/sample_data/policies/access_control.json b/ingestion/examples/sample_data/policies/access_control.json index 3824625675b..961eb694d1d 100644 --- a/ingestion/examples/sample_data/policies/access_control.json +++ b/ingestion/examples/sample_data/policies/access_control.json @@ -11,46 +11,22 @@ "name": "update-description", "userRoleAttr": "DataSteward", "operation": "UpdateDescription", - "allow": true + "allow": true, + "enabled": true + }, + { + "name": "update-tags", + "userRoleAttr": "DataSteward", + "operation": "UpdateOwner", + "allow": true, + "enabled": true }, { "name": "update-tags", "userRoleAttr": "DataSteward", "operation": "UpdateTags", - "allow": true - } - ] - }, - { - "name": "data-consumer-role", - "displayName": "Data Consumer Role Policy", - "description": "Policy for Data Consumer Role to perform operations on metadata entities", - "policyType": "AccessControl", - "enabled": true, - "rules": [ - { - "name": "suggest-description", - "userRoleAttr": "DataConsumer", - "operation": "SuggestDescription", - "allow": true - }, - { - "name": "suggest-tags", - "userRoleAttr": "DataConsumer", - "operation": "SuggestTags", - "allow": true - }, - { - "name": "update-description", - "userRoleAttr": "DataConsumer", - "operation": "UpdateDescription", - "allow": false - }, - { - "name": "update-tags", - "userRoleAttr": "DataConsumer", - "operation": "UpdateTags", - "allow": false + "allow": true, + "enabled": true } ] }