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 aa3e0057efc..b5e1b976b21 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 @@ -23,7 +23,6 @@ import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; import java.io.IOException; -import java.util.Objects; import javax.validation.Valid; import javax.validation.constraints.Max; import javax.validation.constraints.Min; @@ -40,14 +39,18 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; +import javax.ws.rs.core.SecurityContext; import javax.ws.rs.core.UriInfo; +import lombok.NonNull; import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.api.lineage.AddLineage; import org.openmetadata.catalog.jdbi3.CollectionDAO; import org.openmetadata.catalog.jdbi3.LineageRepository; import org.openmetadata.catalog.resources.Collection; import org.openmetadata.catalog.security.Authorizer; +import org.openmetadata.catalog.security.SecurityUtil; import org.openmetadata.catalog.type.EntityLineage; +import org.openmetadata.catalog.type.MetadataOperation; @Path("/v1/lineage") @Api(value = "Lineage resource", tags = "Lineage resource") @@ -56,10 +59,11 @@ import org.openmetadata.catalog.type.EntityLineage; @Collection(name = "lineage") public class LineageResource { private final LineageRepository dao; + private final Authorizer authorizer; - public LineageResource(CollectionDAO dao, Authorizer authorizer) { - Objects.requireNonNull(dao, "LineageRepository must not be null"); + public LineageResource(@NonNull CollectionDAO dao, Authorizer authorizer) { this.dao = new LineageRepository(dao); + this.authorizer = authorizer; } @GET @@ -149,7 +153,11 @@ public class LineageResource { @ApiResponse(responseCode = "200"), @ApiResponse(responseCode = "404", description = "Entity for instance {id} is not found") }) - public Response addLineage(@Context UriInfo uriInfo, @Valid AddLineage addLineage) throws IOException { + public Response addLineage( + @Context UriInfo uriInfo, @Context SecurityContext securityContext, @Valid AddLineage addLineage) + throws IOException { + SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, null, MetadataOperation.UpdateLineage); + dao.addLineage(addLineage); return Response.status(Status.OK).build(); } @@ -166,6 +174,7 @@ public class LineageResource { }) public Response deleteLineage( @Context UriInfo uriInfo, + @Context SecurityContext securityContext, @Parameter( description = "Entity type of upstream entity of the edge", required = true, @@ -183,6 +192,8 @@ public class LineageResource { @Parameter(description = "Entity id", required = true, schema = @Schema(type = "string")) @PathParam("toId") String toId) throws IOException { + SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, null, MetadataOperation.UpdateLineage); + boolean deleted = dao.deleteLineage(fromEntity, fromId, toEntity, toId); if (!deleted) { return Response.status(NOT_FOUND) 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 34e27928c06..ccba3ee3110 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 @@ -133,13 +133,18 @@ public class DefaultAuthorizer implements Authorizer { AuthenticationContext ctx, EntityReference entityReference, MetadataOperation operation) { validateAuthenticationContext(ctx); try { + User user = getUserFromAuthenticationContext(ctx); + if (entityReference == null) { + // In some cases there is no specific entity being acted upon. Eg: Lineage. + return policyEvaluator.hasPermission(user, null, 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; 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 be4bf408895..9bf3d91ca99 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 @@ -55,6 +55,24 @@ public final class SecurityUtil { } } + /** This helper function checks if user has permission to perform the given metadata operation. */ + public static void checkAdminRoleOrPermissions( + Authorizer authorizer, + SecurityContext securityContext, + EntityReference entityReference, + MetadataOperation metadataOperation) { + Principal principal = securityContext.getUserPrincipal(); + AuthenticationContext authenticationCtx = SecurityUtil.getAuthenticationContext(principal); + + if (authorizer.isAdmin(authenticationCtx) || authorizer.isBot(authenticationCtx)) { + return; + } + + if (!authorizer.hasPermissions(authenticationCtx, entityReference, metadataOperation)) { + throw new AuthorizationException("Principal: " + principal + " does not have permission to " + metadataOperation); + } + } + /** * 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 @@ -65,7 +83,9 @@ public final class SecurityUtil { Principal principal = securityContext.getUserPrincipal(); AuthenticationContext authenticationCtx = SecurityUtil.getAuthenticationContext(principal); - if (authorizer.isAdmin(authenticationCtx) || authorizer.isBot(authenticationCtx)) return; + if (authorizer.isAdmin(authenticationCtx) || authorizer.isBot(authenticationCtx)) { + return; + } List metadataOperations = JsonPatchUtils.getMetadataOperations(patch); for (MetadataOperation metadataOperation : metadataOperations) { diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/AttributeBasedFacts.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/AttributeBasedFacts.java index a56962fcafb..2846f5b1dcc 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/AttributeBasedFacts.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/AttributeBasedFacts.java @@ -20,8 +20,8 @@ import org.openmetadata.catalog.util.EntityInterface; class AttributeBasedFacts { @NonNull private User user; - @NonNull private Object entity; @NonNull private MetadataOperation operation; + private Object entity; // Entity can be null in some cases, where the operation may not be on a specific entity. // Do not allow anything external or the builder itself change the value of facts. // Individual Fact(s) within facts may be changed by the RulesEngine. @@ -49,7 +49,10 @@ class AttributeBasedFacts { return user.getRoles().stream().map(EntityReference::getName).collect(Collectors.toList()); } - private List getEntityTags(@NonNull Object entity) { + private List getEntityTags(Object entity) { + if (entity == null) { + return Collections.emptyList(); + } List entityTags = null; try { EntityInterface entityInterface = Entity.getEntityInterface(entity); @@ -63,7 +66,10 @@ class AttributeBasedFacts { return entityTags.stream().map(TagLabel::getTagFQN).collect(Collectors.toList()); } - private static String getEntityType(@NonNull Object entity) { + private static String getEntityType(Object entity) { + if (entity == null) { + return ""; // Fact cannot be null. getFacts will throw NPE if this is null. + } String entityType = Entity.getEntityTypeFromObject(entity); if (entityType == null) { LOG.warn("could not find entity type for the given entity {}", entity); diff --git a/catalog-rest-service/src/main/resources/json/data/policy/DataStewardAccessControl.json b/catalog-rest-service/src/main/resources/json/data/policy/DataStewardAccessControl.json index 07f4358ae74..fa62d1688a2 100644 --- a/catalog-rest-service/src/main/resources/json/data/policy/DataStewardAccessControl.json +++ b/catalog-rest-service/src/main/resources/json/data/policy/DataStewardAccessControl.json @@ -15,6 +15,14 @@ "enabled": true, "priority": 1000 }, + { + "name": "DataStewardRoleAccessControlPolicy-UpdateLineage", + "userRoleAttr": "DataSteward", + "operation": "UpdateLineage", + "allow": true, + "enabled": true, + "priority": 1000 + }, { "name": "DataStewardRoleAccessControlPolicy-UpdateOwner", "userRoleAttr": "DataSteward", 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 c5572ebf310..1528e0c14ff 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 @@ -15,14 +15,16 @@ "SuggestTags", "UpdateDescription", "UpdateOwner", - "UpdateTags" + "UpdateTags", + "UpdateLineage" ], "javaEnums": [ {"name":"SuggestDescription"}, {"name":"SuggestTags"}, {"name":"UpdateDescription"}, {"name":"UpdateOwner"}, - {"name":"UpdateTags"} + {"name":"UpdateTags"}, + {"name":"UpdateLineage"} ] } }, diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/lineage/LineageResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/lineage/LineageResourceTest.java index 346da4f3a23..1338509e1de 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/lineage/LineageResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/lineage/LineageResourceTest.java @@ -13,10 +13,15 @@ package org.openmetadata.catalog.resources.lineage; +import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.openmetadata.catalog.security.SecurityUtil.authHeaders; +import static org.openmetadata.catalog.util.TestUtils.ADMIN_USER_NAME; import static org.openmetadata.catalog.util.TestUtils.adminAuthHeaders; +import static org.openmetadata.catalog.util.TestUtils.assertResponse; import java.io.IOException; import java.net.URISyntaxException; @@ -39,8 +44,13 @@ import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.api.data.CreateTable; import org.openmetadata.catalog.api.lineage.AddLineage; import org.openmetadata.catalog.entity.data.Table; +import org.openmetadata.catalog.entity.teams.Role; +import org.openmetadata.catalog.entity.teams.User; import org.openmetadata.catalog.jdbi3.TableRepository.TableEntityInterface; import org.openmetadata.catalog.resources.databases.TableResourceTest; +import org.openmetadata.catalog.resources.teams.RoleResource; +import org.openmetadata.catalog.resources.teams.RoleResourceTest; +import org.openmetadata.catalog.resources.teams.UserResourceTest; import org.openmetadata.catalog.type.Edge; import org.openmetadata.catalog.type.EntitiesEdge; import org.openmetadata.catalog.type.EntityLineage; @@ -53,6 +63,8 @@ public class LineageResourceTest extends CatalogApplicationTest { public static final List TABLES = new ArrayList<>(); public static final int TABLE_COUNT = 10; + private static final String DATA_STEWARD_ROLE_NAME = "DataSteward"; + @BeforeAll public static void setup(TestInfo test) throws IOException, URISyntaxException { // Create TABLE_COUNT number of tables @@ -64,6 +76,53 @@ public class LineageResourceTest extends CatalogApplicationTest { } } + @Test + void put_delete_lineage_withAuthorizer(TestInfo test) throws HttpResponseException { + // Random user cannot update lineage. + User RANDOM_USER = + UserResourceTest.createUser( + UserResourceTest.create(test.getDisplayName() + "_lineage_user"), adminAuthHeaders()); + + // User with Data Steward role. Data Steward role has a default policy to allow update for lineage. + Role dataStewardRole = + RoleResourceTest.getRoleByName(DATA_STEWARD_ROLE_NAME, RoleResource.FIELDS, adminAuthHeaders()); + User userWithDataStewardRole = + UserResourceTest.createUser( + UserResourceTest.create(test.getDisplayName() + "_lineage_user_data_steward") + .withRoles(List.of(dataStewardRole.getId())), + adminAuthHeaders()); + + // Admins are able to add or delete edges. + checkAuthorization(ADMIN_USER_NAME, false); + // User with Data Steward role is able to add or delete edges. + checkAuthorization(userWithDataStewardRole.getName(), false); + // Random user is not able to add or delete edges. + checkAuthorization(RANDOM_USER.getName(), true); + } + + private void checkAuthorization(String userName, boolean shouldThrowException) throws HttpResponseException { + Map authHeaders = authHeaders(userName + "@open-metadata.org"); + + if (shouldThrowException) { + HttpResponseException exception = + assertThrows(HttpResponseException.class, () -> addEdge(TABLES.get(1), TABLES.get(2), authHeaders)); + assertResponse( + exception, + FORBIDDEN, + String.format("Principal: CatalogPrincipal{name='%s'} does not have permission to UpdateLineage", userName)); + exception = + assertThrows(HttpResponseException.class, () -> deleteEdge(TABLES.get(1), TABLES.get(2), authHeaders)); + assertResponse( + exception, + FORBIDDEN, + String.format("Principal: CatalogPrincipal{name='%s'} does not have permission to UpdateLineage", userName)); + return; + } + + addEdge(TABLES.get(1), TABLES.get(2), authHeaders(userName + "@open-metadata.org")); + deleteEdge(TABLES.get(1), TABLES.get(2), authHeaders(userName + "@open-metadata.org")); + } + @Test void put_delete_lineage_200() throws HttpResponseException { // Add lineage table4-->table5 @@ -150,20 +209,28 @@ public class LineageResourceTest extends CatalogApplicationTest { } public void addEdge(Table from, Table to) throws HttpResponseException { + addEdge(from, to, adminAuthHeaders()); + } + + private void addEdge(Table from, Table to, Map authHeaders) throws HttpResponseException { EntitiesEdge edge = new EntitiesEdge() .withFromEntity(new TableEntityInterface(from).getEntityReference()) .withToEntity(new TableEntityInterface(to).getEntityReference()); AddLineage addLineage = new AddLineage().withEdge(edge); - addLineageAndCheck(addLineage, adminAuthHeaders()); + addLineageAndCheck(addLineage, authHeaders); } public void deleteEdge(Table from, Table to) throws HttpResponseException { + deleteEdge(from, to, adminAuthHeaders()); + } + + private void deleteEdge(Table from, Table to, Map authHeaders) throws HttpResponseException { EntitiesEdge edge = new EntitiesEdge() .withFromEntity(new TableEntityInterface(from).getEntityReference()) .withToEntity(new TableEntityInterface(to).getEntityReference()); - deleteLineageAndCheck(edge, adminAuthHeaders()); + deleteLineageAndCheck(edge, authHeaders); } public static void addLineageAndCheck(AddLineage addLineage, Map authHeaders) diff --git a/ingestion-core/src/metadata/_version.py b/ingestion-core/src/metadata/_version.py index f05ddb12e83..e9139b28269 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, 8, 0, dev=13) +__version__ = Version("metadata", 0, 8, 0, dev=14) __all__ = ["__version__"]