From 27f4d9799e876c1b95d275e2ad6237e65fa04be6 Mon Sep 17 00:00:00 2001 From: Pere Miquel Brull Date: Tue, 9 Jul 2024 11:13:57 +0200 Subject: [PATCH] REL #460-CL - Metadata Actions Create Permissions (#16955) * REL #460-CL - Metadata Actions Create Permissions * format --- .../native/1.5.0/mysql/schemaChanges.sql | 2 +- .../native/1.5.0/postgres/schemaChanges.sql | 2 +- .../jdbi3/IngestionPipelineRepository.java | 19 +++++++ .../IngestionPipelineResource.java | 41 +++++++++++++- .../util/OpenMetadataConnectionBuilder.java | 10 +--- .../json/data/policy/DefaultBotPolicy.json | 2 +- .../IngestionPipelineResourceTest.java | 55 +++++++++++++++++++ .../accessControl/resourceDescriptor.json | 2 + 8 files changed, 121 insertions(+), 12 deletions(-) diff --git a/bootstrap/sql/migrations/native/1.5.0/mysql/schemaChanges.sql b/bootstrap/sql/migrations/native/1.5.0/mysql/schemaChanges.sql index bf4a95a4c1a..adcd400cbfc 100644 --- a/bootstrap/sql/migrations/native/1.5.0/mysql/schemaChanges.sql +++ b/bootstrap/sql/migrations/native/1.5.0/mysql/schemaChanges.sql @@ -27,7 +27,7 @@ SET json = JSON_ARRAY_APPEND( "name": "BotRule-IngestionPipeline", "description": "A bot can Edit ingestion pipelines to pass the status", "resources": ["ingestionPipeline"], - "operations": ["ViewAll","EditAll"], + "operations": ["ViewAll","EditIngestionPipelineStatus"], "effect": "allow" }' AS JSON) ) diff --git a/bootstrap/sql/migrations/native/1.5.0/postgres/schemaChanges.sql b/bootstrap/sql/migrations/native/1.5.0/postgres/schemaChanges.sql index 1e3d1b9f2e6..260ed74eaae 100644 --- a/bootstrap/sql/migrations/native/1.5.0/postgres/schemaChanges.sql +++ b/bootstrap/sql/migrations/native/1.5.0/postgres/schemaChanges.sql @@ -22,7 +22,7 @@ SET json = jsonb_set( 'name', 'BotRule-IngestionPipeline', 'description', 'A bot can Edit ingestion pipelines to pass the status', 'resources', jsonb_build_array('ingestionPipeline'), - 'operations', jsonb_build_array('ViewAll', 'EditAll'), + 'operations', jsonb_build_array('ViewAll', 'EditIngestionPipelineStatus'), 'effect', 'allow' ) ]), diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java index d53be60b318..a71cf3c97bd 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java @@ -25,9 +25,12 @@ import lombok.Setter; import org.jdbi.v3.sqlobject.transaction.Transaction; import org.json.JSONObject; import org.openmetadata.schema.EntityInterface; +import org.openmetadata.schema.entity.applications.configuration.ApplicationConfig; import org.openmetadata.schema.entity.services.ingestionPipelines.AirflowConfig; import org.openmetadata.schema.entity.services.ingestionPipelines.IngestionPipeline; import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineStatus; +import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineType; +import org.openmetadata.schema.metadataIngestion.ApplicationPipeline; import org.openmetadata.schema.metadataIngestion.LogLevels; import org.openmetadata.schema.services.connections.metadata.OpenMetadataConnection; import org.openmetadata.schema.type.ChangeDescription; @@ -348,4 +351,20 @@ public class IngestionPipelineRepository extends EntityRepository getEntitySpecificOperations() { + return listOf( + MetadataOperation.CREATE_INGESTION_PIPELINE_AUTOMATOR, + MetadataOperation.EDIT_INGESTION_PIPELINE_STATUS); + } + public static class IngestionPipelineList extends ResultList { /* Required for serde */ } + /** + * Handle permissions based on the pipeline type + */ + @Override + public Response create( + UriInfo uriInfo, SecurityContext securityContext, IngestionPipeline entity) { + OperationContext operationContext = + new OperationContext(entityType, getOperationForPipelineType(entity)); + CreateResourceContext createResourceContext = + new CreateResourceContext<>(entityType, entity); + limits.enforceLimits(securityContext, createResourceContext, operationContext); + authorizer.authorize(securityContext, operationContext, createResourceContext); + entity = addHref(uriInfo, repository.create(uriInfo, entity)); + return Response.created(entity.getHref()).entity(entity).build(); + } + + /** + * Dynamically get the MetadataOperation based on the pipelineType (or application Type). + * E.g., for the Automator, the Operation will be `CREATE_INGESTION_PIPELINE_AUTOMATOR`. + */ + private MetadataOperation getOperationForPipelineType(IngestionPipeline ingestionPipeline) { + String pipelineType = IngestionPipelineRepository.getPipelineWorkflowType(ingestionPipeline); + try { + return MetadataOperation.valueOf( + String.format("CREATE_INGESTION_PIPELINE_%s", pipelineType.toUpperCase())); + } catch (IllegalArgumentException | NullPointerException e) { + return CREATE; + } + } + @GET @Valid @Operation( @@ -794,7 +833,7 @@ public class IngestionPipelineResource String fqn, @Valid PipelineStatus pipelineStatus) { OperationContext operationContext = - new OperationContext(entityType, MetadataOperation.EDIT_ALL); + new OperationContext(entityType, MetadataOperation.EDIT_INGESTION_PIPELINE_STATUS); authorizer.authorize(securityContext, operationContext, getResourceContextByName(fqn)); return repository.addPipelineStatus(uriInfo, fqn, pipelineStatus).toResponse(); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataConnectionBuilder.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataConnectionBuilder.java index 3a1e876a6fc..a78b84ea3ec 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataConnectionBuilder.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataConnectionBuilder.java @@ -20,11 +20,9 @@ import org.openmetadata.schema.api.configuration.pipelineServiceClient.PipelineS import org.openmetadata.schema.auth.JWTAuthMechanism; import org.openmetadata.schema.auth.SSOAuthMechanism; import org.openmetadata.schema.entity.Bot; -import org.openmetadata.schema.entity.applications.configuration.ApplicationConfig; import org.openmetadata.schema.entity.services.ingestionPipelines.IngestionPipeline; import org.openmetadata.schema.entity.teams.AuthenticationMechanism; import org.openmetadata.schema.entity.teams.User; -import org.openmetadata.schema.metadataIngestion.ApplicationPipeline; import org.openmetadata.schema.security.client.OpenMetadataJWTClientConfig; import org.openmetadata.schema.security.secrets.SecretsManagerClientLoader; import org.openmetadata.schema.security.secrets.SecretsManagerProvider; @@ -36,6 +34,7 @@ import org.openmetadata.service.Entity; import org.openmetadata.service.OpenMetadataApplicationConfig; import org.openmetadata.service.exception.EntityNotFoundException; import org.openmetadata.service.jdbi3.BotRepository; +import org.openmetadata.service.jdbi3.IngestionPipelineRepository; import org.openmetadata.service.jdbi3.UserRepository; import org.openmetadata.service.secrets.SecretsManagerFactory; import org.openmetadata.service.util.EntityUtil.Fields; @@ -87,12 +86,7 @@ public class OpenMetadataConnectionBuilder { switch (ingestionPipeline.getPipelineType()) { case METADATA, DBT -> botName = Entity.INGESTION_BOT_NAME; case APPLICATION -> { - ApplicationPipeline applicationPipeline = - JsonUtils.convertValue( - ingestionPipeline.getSourceConfig().getConfig(), ApplicationPipeline.class); - ApplicationConfig appConfig = - JsonUtils.convertValue(applicationPipeline.getAppConfig(), ApplicationConfig.class); - String type = (String) appConfig.getAdditionalProperties().get("type"); + String type = IngestionPipelineRepository.getPipelineWorkflowType(ingestionPipeline); botName = String.format("%sApplicationBot", type); } // TODO: Remove this once we internalize the DataInsights app diff --git a/openmetadata-service/src/main/resources/json/data/policy/DefaultBotPolicy.json b/openmetadata-service/src/main/resources/json/data/policy/DefaultBotPolicy.json index 102b110fcb2..c037b6b8d8c 100644 --- a/openmetadata-service/src/main/resources/json/data/policy/DefaultBotPolicy.json +++ b/openmetadata-service/src/main/resources/json/data/policy/DefaultBotPolicy.json @@ -18,7 +18,7 @@ "name": "BotRule-IngestionPipeline", "description" : "A bot can Edit ingestion pipelines to pass the status", "resources" : ["ingestionPipeline"], - "operations": ["ViewAll","EditAll"], + "operations": ["ViewAll","EditIngestionPipelineStatus"], "effect": "allow" } ] diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/ingestionpipelines/IngestionPipelineResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/ingestionpipelines/IngestionPipelineResourceTest.java index c57cf83800a..86fd0d6b47d 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/ingestionpipelines/IngestionPipelineResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/ingestionpipelines/IngestionPipelineResourceTest.java @@ -14,11 +14,13 @@ package org.openmetadata.service.resources.services.ingestionpipelines; import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static javax.ws.rs.core.Response.Status.OK; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.openmetadata.service.Entity.FIELD_OWNER; +import static org.openmetadata.service.exception.CatalogExceptionMessage.permissionNotAllowed; import static org.openmetadata.service.security.SecurityUtil.authHeaders; import static org.openmetadata.service.util.EntityUtil.fieldAdded; import static org.openmetadata.service.util.TestUtils.ADMIN_AUTH_HEADERS; @@ -26,6 +28,7 @@ import static org.openmetadata.service.util.TestUtils.INGESTION_BOT_AUTH_HEADERS import static org.openmetadata.service.util.TestUtils.UpdateType.MINOR_UPDATE; import static org.openmetadata.service.util.TestUtils.assertListNotNull; import static org.openmetadata.service.util.TestUtils.assertListNull; +import static org.openmetadata.service.util.TestUtils.assertResponse; import static org.openmetadata.service.util.TestUtils.assertResponseContains; import java.io.IOException; @@ -51,6 +54,8 @@ import org.openmetadata.schema.api.services.CreateDashboardService; import org.openmetadata.schema.api.services.CreateDatabaseService; import org.openmetadata.schema.api.services.DatabaseConnection; import org.openmetadata.schema.api.services.ingestionPipelines.CreateIngestionPipeline; +import org.openmetadata.schema.entity.app.external.AutomatorAppConfig; +import org.openmetadata.schema.entity.app.external.Resource; import org.openmetadata.schema.entity.services.DashboardService; import org.openmetadata.schema.entity.services.DatabaseService; import org.openmetadata.schema.entity.services.ingestionPipelines.AirflowConfig; @@ -58,6 +63,7 @@ import org.openmetadata.schema.entity.services.ingestionPipelines.IngestionPipel import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineStatus; import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineStatusType; import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineType; +import org.openmetadata.schema.metadataIngestion.ApplicationPipeline; import org.openmetadata.schema.metadataIngestion.DashboardServiceMetadataPipeline; import org.openmetadata.schema.metadataIngestion.DatabaseServiceMetadataPipeline; import org.openmetadata.schema.metadataIngestion.DatabaseServiceQueryUsagePipeline; @@ -73,6 +79,7 @@ import org.openmetadata.schema.services.connections.database.ConnectionArguments import org.openmetadata.schema.services.connections.database.ConnectionOptions; import org.openmetadata.schema.type.ChangeDescription; import org.openmetadata.schema.type.EntityReference; +import org.openmetadata.schema.type.MetadataOperation; import org.openmetadata.service.Entity; import org.openmetadata.service.resources.EntityResourceTest; import org.openmetadata.service.resources.services.DashboardServiceResourceTest; @@ -782,6 +789,54 @@ public class IngestionPipelineResourceTest TestUtils.readResponse(response, PipelineStatus.class, Status.NO_CONTENT.getStatusCode()); } + @Test + void put_pipelineStatus_403(TestInfo test) throws IOException { + CreateIngestionPipeline requestPipeline = createRequest(getEntityName(test)); + IngestionPipeline ingestionPipeline = createAndCheckEntity(requestPipeline, ADMIN_AUTH_HEADERS); + + String runId = UUID.randomUUID().toString(); + + // Create a status without having the EDIT_INGESTION_PIPELINE_STATUS permission + assertResponse( + () -> + TestUtils.put( + getPipelineStatusTarget(ingestionPipeline.getFullyQualifiedName()), + new PipelineStatus() + .withPipelineState(PipelineStatusType.RUNNING) + .withRunId(runId) + .withTimestamp(3L), + Response.Status.CREATED, + authHeaders(USER2.getName())), + FORBIDDEN, + permissionNotAllowed( + USER2.getName(), List.of(MetadataOperation.EDIT_INGESTION_PIPELINE_STATUS))); + } + + @Test + void post_ingestionPipeline_403(TestInfo test) throws HttpResponseException { + CreateIngestionPipeline create = createRequest(getEntityName(test)); + create + .withPipelineType(PipelineType.APPLICATION) + .withSourceConfig( + new SourceConfig() + .withConfig( + new ApplicationPipeline() + .withAppConfig( + new AutomatorAppConfig() + .withResources(new Resource().withQueryFilter("")) + .withActions(List.of())))); + + // Create ingestion pipeline without having the CREATE_INGESTION_PIPELINE_AUTOMATOR permission + assertResponse( + () -> createEntity(create, authHeaders(USER1.getName())), + FORBIDDEN, + permissionNotAllowed( + USER1.getName(), List.of(MetadataOperation.CREATE_INGESTION_PIPELINE_AUTOMATOR))); + + // Admin has permissions and can create it + createEntity(create, ADMIN_AUTH_HEADERS); + } + @Test void testInheritedPermissionFromParent(TestInfo test) throws IOException { // Create a dashboard service with owner data consumer diff --git a/openmetadata-spec/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json b/openmetadata-spec/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json index cbfd206b4a6..2d14f89f46d 100644 --- a/openmetadata-spec/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json +++ b/openmetadata-spec/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json @@ -13,6 +13,7 @@ "enum": [ "All", "Create", + "CreateIngestionPipelineAutomator", "Delete", "ViewAll", "ViewBasic", @@ -44,6 +45,7 @@ "EditLifeCycle", "EditKnowledgePanel", "EditPage", + "EditIngestionPipelineStatus", "DeleteTestCaseFailedRowsSample", "Deploy", "Trigger",