From a6438c6347b055a94c2e1973c86489b7ee38dfb8 Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Thu, 27 Oct 2022 15:37:44 -0700 Subject: [PATCH] Fixes #8401 Remove lingering authorizeAdmin in TagResource (#8403) --- .../service/resources/EntityResource.java | 6 +-- .../service/resources/tags/TagResource.java | 52 +++++++++++-------- .../service/resources/teams/UserResource.java | 6 +-- .../openmetadata/service/util/EntityUtil.java | 5 ++ 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java index df88919e930..d38a4bca2dd 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java @@ -2,7 +2,7 @@ package org.openmetadata.service.resources; import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; import static org.openmetadata.schema.type.MetadataOperation.CREATE; -import static org.openmetadata.schema.type.MetadataOperation.EDIT_ALL; +import static org.openmetadata.service.util.EntityUtil.createOrUpdateOperation; import java.io.IOException; import java.util.List; @@ -196,9 +196,7 @@ public abstract class EntityResource response = dao.createOrUpdate(uriInfo, entity); addHref(uriInfo, response.getEntity()); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/tags/TagResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/tags/TagResource.java index a7ce5164d16..b056d666d7d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/tags/TagResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/tags/TagResource.java @@ -15,6 +15,9 @@ package org.openmetadata.service.resources.tags; import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; import static org.openmetadata.service.Entity.ADMIN_USER_NAME; +import static org.openmetadata.service.Entity.TAG; +import static org.openmetadata.service.Entity.TAG_CATEGORY; +import static org.openmetadata.service.util.EntityUtil.createOrUpdateOperation; import io.swagger.annotations.Api; import io.swagger.annotations.ApiOperation; @@ -94,7 +97,7 @@ public class TagResource { public void initialize(OpenMetadataApplicationConfig config) throws IOException { // Find tag definitions and load tag categories from the json file, if necessary List tagCategories = - dao.getEntitiesFromSeedData(Entity.TAG_CATEGORY, ".*json/data/tags/.*\\.json$", TagCategory.class); + dao.getEntitiesFromSeedData(TAG_CATEGORY, ".*json/data/tags/.*\\.json$", TagCategory.class); for (TagCategory tagCategory : tagCategories) { long now = System.currentTimeMillis(); tagCategory.withId(UUID.randomUUID()).withUpdatedBy(ADMIN_USER_NAME).withUpdatedAt(now); @@ -279,8 +282,8 @@ public class TagResource { public Response createCategory( @Context UriInfo uriInfo, @Context SecurityContext securityContext, @Valid CreateTagCategory create) throws IOException { - OperationContext operationContext = new OperationContext(Entity.TAG_CATEGORY, MetadataOperation.CREATE); - ResourceContext resourceContext = EntityResource.getResourceContext(Entity.TAG_CATEGORY, daoCategory).build(); + OperationContext operationContext = new OperationContext(TAG_CATEGORY, MetadataOperation.CREATE); + ResourceContext resourceContext = EntityResource.getResourceContext(TAG_CATEGORY, daoCategory).build(); authorizer.authorize(securityContext, operationContext, resourceContext); TagCategory category = getTagCategory(securityContext, create); category = addHref(uriInfo, daoCategory.create(uriInfo, category)); @@ -308,8 +311,8 @@ public class TagResource { String category, @Valid CreateTag create) throws IOException { - OperationContext operationContext = new OperationContext(Entity.TAG, MetadataOperation.CREATE); - ResourceContext resourceContext = EntityResource.getResourceContext(Entity.TAG, dao).build(); + OperationContext operationContext = new OperationContext(TAG, MetadataOperation.CREATE); + ResourceContext resourceContext = EntityResource.getResourceContext(TAG, dao).build(); authorizer.authorize(securityContext, operationContext, resourceContext); Tag tag = getTag(securityContext, create, FullyQualifiedName.build(category)); URI categoryHref = RestUtil.getHref(uriInfo, TAG_COLLECTION_PATH, category); @@ -346,8 +349,8 @@ public class TagResource { String primaryTag, @Valid CreateTag create) throws IOException { - OperationContext operationContext = new OperationContext(Entity.TAG, MetadataOperation.CREATE); - ResourceContext resourceContext = EntityResource.getResourceContext(Entity.TAG, dao).build(); + OperationContext operationContext = new OperationContext(TAG, MetadataOperation.CREATE); + ResourceContext resourceContext = EntityResource.getResourceContext(TAG, dao).build(); authorizer.authorize(securityContext, operationContext, resourceContext); Tag tag = getTag(securityContext, create, FullyQualifiedName.build(category, primaryTag)); URI categoryHref = RestUtil.getHref(uriInfo, TAG_COLLECTION_PATH, category); @@ -370,11 +373,12 @@ public class TagResource { String categoryName, @Valid CreateTagCategory create) throws IOException { - OperationContext operationContext = new OperationContext(Entity.TAG_CATEGORY, MetadataOperation.EDIT_ALL); - ResourceContext resourceContext = - EntityResource.getResourceContext(Entity.TAG_CATEGORY, daoCategory).name(categoryName).build(); - authorizer.authorize(securityContext, operationContext, resourceContext); TagCategory category = getTagCategory(securityContext, create); + ResourceContext resourceContext = + EntityResource.getResourceContext(TAG_CATEGORY, daoCategory).name(categoryName).build(); + OperationContext operationContext = new OperationContext(TAG_CATEGORY, createOrUpdateOperation(resourceContext)); + + authorizer.authorize(securityContext, operationContext, resourceContext); // TODO clean this up if (categoryName.equals(create.getName())) { // Not changing the name category = addHref(uriInfo, daoCategory.createOrUpdate(uriInfo, category).getEntity()); @@ -409,8 +413,8 @@ public class TagResource { throws IOException { Tag tag = getTag(securityContext, create, FullyQualifiedName.build(categoryName)); - OperationContext operationContext = new OperationContext(Entity.TAG, MetadataOperation.EDIT_ALL); - ResourceContext resourceContext = EntityResource.getResourceContext(Entity.TAG, dao).name(categoryName).build(); + ResourceContext resourceContext = EntityResource.getResourceContext(TAG, dao).name(categoryName).build(); + OperationContext operationContext = new OperationContext(TAG, createOrUpdateOperation(resourceContext)); authorizer.authorize(securityContext, operationContext, resourceContext); URI categoryHref = RestUtil.getHref(uriInfo, TAG_COLLECTION_PATH, categoryName); @@ -455,10 +459,14 @@ public class TagResource { String secondaryTag, @Valid CreateTag create) throws IOException { - authorizer.authorizeAdmin(securityContext); Tag tag = getTag(securityContext, create, FullyQualifiedName.build(categoryName, primaryTag)); - URI categoryHref = RestUtil.getHref(uriInfo, TAG_COLLECTION_PATH, categoryName); - URI parentHRef = RestUtil.getHref(categoryHref, primaryTag); + + // If entity does not exist, this is a create operation, else update operation + ResourceContext resourceContext = + EntityResource.getResourceContext(TAG, dao).name(tag.getFullyQualifiedName()).build(); + OperationContext operationContext = new OperationContext(TAG, createOrUpdateOperation(resourceContext)); + authorizer.authorize(securityContext, operationContext, resourceContext); + RestUtil.PutResponse response; // TODO clean this up if (secondaryTag.equals(create.getName())) { // Not changing the name @@ -468,6 +476,9 @@ public class TagResource { getTag(securityContext, create, FullyQualifiedName.build(categoryName, primaryTag)).withName(secondaryTag); response = dao.createOrUpdate(uriInfo, origTag, tag); } + + URI categoryHref = RestUtil.getHref(uriInfo, TAG_COLLECTION_PATH, categoryName); + URI parentHRef = RestUtil.getHref(categoryHref, primaryTag); addHref(parentHRef, (Tag) response.getEntity()); return response.toResponse(); } @@ -484,9 +495,8 @@ public class TagResource { @Context SecurityContext securityContext, @Parameter(description = "Tag category id", schema = @Schema(type = "UUID")) @PathParam("id") UUID id) throws IOException { - OperationContext operationContext = new OperationContext(Entity.TAG_CATEGORY, MetadataOperation.EDIT_ALL); - ResourceContext resourceContext = - EntityResource.getResourceContext(Entity.TAG_CATEGORY, daoCategory).id(id).build(); + OperationContext operationContext = new OperationContext(TAG_CATEGORY, MetadataOperation.DELETE); + ResourceContext resourceContext = EntityResource.getResourceContext(TAG_CATEGORY, daoCategory).id(id).build(); authorizer.authorize(securityContext, operationContext, resourceContext); TagCategory tagCategory = daoCategory.delete(uriInfo, id); addHref(uriInfo, tagCategory); @@ -506,8 +516,8 @@ public class TagResource { @Parameter(description = "Tag id", schema = @Schema(type = "string")) @PathParam("category") String category, @Parameter(description = "Tag id", schema = @Schema(type = "UUID")) @PathParam("id") UUID id) throws IOException { - OperationContext operationContext = new OperationContext(Entity.TAG, MetadataOperation.EDIT_ALL); - ResourceContext resourceContext = EntityResource.getResourceContext(Entity.TAG, dao).id(id).build(); + OperationContext operationContext = new OperationContext(TAG, MetadataOperation.DELETE); + ResourceContext resourceContext = EntityResource.getResourceContext(TAG, dao).id(id).build(); authorizer.authorize(securityContext, operationContext, resourceContext); Tag tag = dao.delete(uriInfo, id); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java index bef48f9f40e..7c4778c0e70 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java @@ -19,8 +19,6 @@ import static org.openmetadata.schema.api.teams.CreateUser.CreatePasswordType.AD import static org.openmetadata.schema.auth.ChangePasswordRequest.RequestType.SELF; import static org.openmetadata.schema.entity.teams.AuthenticationMechanism.AuthType.BASIC; import static org.openmetadata.schema.entity.teams.AuthenticationMechanism.AuthType.JWT; -import static org.openmetadata.schema.type.MetadataOperation.CREATE; -import static org.openmetadata.schema.type.MetadataOperation.EDIT_ALL; import static org.openmetadata.service.exception.CatalogExceptionMessage.EMAIL_SENDING_ISSUE; import at.favre.lib.crypto.bcrypt.BCrypt; @@ -522,7 +520,6 @@ public class UserResource extends EntityResource { // If entity does not exist, this is a create operation, else update operation ResourceContext resourceContext = getResourceContextByName(user.getFullyQualifiedName()); - MetadataOperation operation = resourceContext.getEntity() == null ? CREATE : EDIT_ALL; dao.prepare(user); if (Boolean.TRUE.equals(create.getIsAdmin()) || Boolean.TRUE.equals(create.getIsBot())) { @@ -530,7 +527,8 @@ public class UserResource extends EntityResource { } else if (!securityContext.getUserPrincipal().getName().equals(user.getName())) { // doing authorization check outside of authorizer here. We are checking if the logged-in user same as the user // we are trying to update. One option is to set users.owner as user, however that is not supported for User. - OperationContext createOperationContext = new OperationContext(entityType, operation); + OperationContext createOperationContext = + new OperationContext(entityType, EntityUtil.createOrUpdateOperation(resourceContext)); authorizer.authorize(securityContext, createOperationContext, resourceContext); } if (Boolean.TRUE.equals(create.getIsBot())) { // TODO expect bot to be created separately diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java index b2cc6ccfb08..c7cac036356 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java @@ -70,6 +70,7 @@ import org.openmetadata.service.jdbi3.CollectionDAO.EntityRelationshipRecord; import org.openmetadata.service.jdbi3.CollectionDAO.EntityVersionPair; import org.openmetadata.service.jdbi3.CollectionDAO.UsageDAO; import org.openmetadata.service.resources.feeds.MessageParser.EntityLink; +import org.openmetadata.service.security.policyevaluator.ResourceContext; @Slf4j public final class EntityUtil { @@ -461,4 +462,8 @@ public final class EntityUtil { FieldChange fieldChange = new FieldChange().withName(fieldName).withOldValue(oldValue).withNewValue(newValue); change.getFieldsUpdated().add(fieldChange); } + + public static MetadataOperation createOrUpdateOperation(ResourceContext resourceContext) throws IOException { + return resourceContext.getEntity() == null ? MetadataOperation.CREATE : MetadataOperation.EDIT_ALL; + } }