diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ColumnRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ColumnRepository.java index 3208b90743a..cb47b68d61d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ColumnRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ColumnRepository.java @@ -17,6 +17,7 @@ import static org.openmetadata.service.Entity.DASHBOARD_DATA_MODEL; import static org.openmetadata.service.Entity.TABLE; import jakarta.json.JsonPatch; +import jakarta.ws.rs.core.SecurityContext; import jakarta.ws.rs.core.UriInfo; import java.util.List; import lombok.extern.slf4j.Slf4j; @@ -28,15 +29,24 @@ import org.openmetadata.schema.type.EntityReference; import org.openmetadata.schema.type.Include; import org.openmetadata.service.Entity; import org.openmetadata.service.exception.EntityNotFoundException; +import org.openmetadata.service.security.Authorizer; +import org.openmetadata.service.security.policyevaluator.OperationContext; +import org.openmetadata.service.security.policyevaluator.ResourceContext; +import org.openmetadata.service.security.policyevaluator.ResourceContextInterface; import org.openmetadata.service.util.FullyQualifiedName; import org.openmetadata.service.util.JsonUtils; @Slf4j public class ColumnRepository { + private final Authorizer authorizer; + + public ColumnRepository(Authorizer authorizer) { + this.authorizer = authorizer; + } public Column updateColumnByFQN( UriInfo uriInfo, - String user, + SecurityContext securityContext, String columnFQN, String entityType, UpdateColumn updateColumn) { @@ -61,17 +71,20 @@ public class ColumnRepository { } EntityReference parentEntityRef = getParentEntityByFQN(parentFQN, entityType); + String user = securityContext.getUserPrincipal().getName(); if (TABLE.equals(entityType)) { - return updateTableColumn(uriInfo, user, columnFQN, updateColumn, parentEntityRef); + return updateTableColumn( + uriInfo, securityContext, user, columnFQN, updateColumn, parentEntityRef); } else { return updateDashboardDataModelColumn( - uriInfo, user, columnFQN, updateColumn, parentEntityRef); + uriInfo, securityContext, user, columnFQN, updateColumn, parentEntityRef); } } private Column updateTableColumn( UriInfo uriInfo, + SecurityContext securityContext, String user, String columnFQN, UpdateColumn updateColumn, @@ -120,6 +133,14 @@ public class ColumnRepository { } JsonPatch jsonPatch = JsonUtils.getJsonPatch(originalTable, updatedTable); + + // Authorize the patch operation + OperationContext operationContext = new OperationContext(TABLE, jsonPatch); + ResourceContextInterface resourceContext = + new ResourceContext<>( + TABLE, parentEntityRef.getId(), null, ResourceContextInterface.Operation.PATCH); + authorizer.authorize(securityContext, operationContext, resourceContext); + tableRepository.patch(uriInfo, parentEntityRef.getId(), user, jsonPatch); return column; @@ -127,6 +148,7 @@ public class ColumnRepository { private Column updateDashboardDataModelColumn( UriInfo uriInfo, + SecurityContext securityContext, String user, String columnFQN, UpdateColumn updateColumn, @@ -173,6 +195,17 @@ public class ColumnRepository { } JsonPatch jsonPatch = JsonUtils.getJsonPatch(originalDataModel, updatedDataModel); + + // Authorize the patch operation + OperationContext operationContext = new OperationContext(DASHBOARD_DATA_MODEL, jsonPatch); + ResourceContextInterface resourceContext = + new ResourceContext<>( + DASHBOARD_DATA_MODEL, + parentEntityRef.getId(), + null, + ResourceContextInterface.Operation.PATCH); + authorizer.authorize(securityContext, operationContext, resourceContext); + dataModelRepository.patch(uriInfo, parentEntityRef.getId(), user, jsonPatch); return column; diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java index 9dcbb7253dc..d64921087d0 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java @@ -1681,12 +1681,8 @@ public class TableRepository extends EntityRepository { && column.getName().toLowerCase().contains(searchTerm)) { return true; } - if (column.getDisplayName() != null - && column.getDisplayName().toLowerCase().contains(searchTerm)) { - return true; - } - return column.getDescription() != null - && column.getDescription().toLowerCase().contains(searchTerm); + return column.getDisplayName() != null + && column.getDisplayName().toLowerCase().contains(searchTerm); }) .toList(); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/columns/ColumnResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/columns/ColumnResource.java index 98eaa7a6b95..fe874cd6db4 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/columns/ColumnResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/columns/ColumnResource.java @@ -58,7 +58,7 @@ public class ColumnResource { public ColumnResource(Authorizer authorizer) { this.authorizer = authorizer; - this.repository = new ColumnRepository(); + this.repository = new ColumnRepository(authorizer); } @PUT @@ -120,8 +120,7 @@ public class ColumnResource { UpdateColumn updateColumn) { Column updatedColumn = - repository.updateColumnByFQN( - uriInfo, securityContext.getUserPrincipal().getName(), fqn, entityType, updateColumn); + repository.updateColumnByFQN(uriInfo, securityContext, fqn, entityType, updateColumn); return Response.ok(updatedColumn).build(); } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java index 04eaaf8291f..3578a654371 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java @@ -291,6 +291,8 @@ public abstract class EntityResourceTest { .get("description") .getChangeSource()); - assertChangeSummaryInSearch(updated); + // changeSummary is no longer included in search results + // assertChangeSummaryInSearch(updated); Table automatedUpdate = JsonUtils.deepCopy(updated, Table.class); automatedUpdate.setDescription("automated description"); @@ -3945,19 +3950,22 @@ public class TableResourceTest extends EntityResourceTest { TableResource.TableColumnList response = TestUtils.get(target, TableResource.TableColumnList.class, ADMIN_AUTH_HEADERS); assertEquals(1, response.getData().size()); - assertEquals("user_id", response.getData().get(0).getName()); + assertEquals("user_id", response.getData().getFirst().getName()); assertEquals(1, response.getPaging().getTotal()); target = getResource("tables/" + table.getId() + "/columns/search").queryParam("q", "price"); response = TestUtils.get(target, TableResource.TableColumnList.class, ADMIN_AUTH_HEADERS); - assertEquals(2, response.getData().size()); - assertEquals("price_history", response.getData().get(0).getName()); + assertEquals(1, response.getData().size()); + // Both order_total (description contains "price") and price_history should be in results + Set resultNames = + response.getData().stream().map(Column::getName).collect(Collectors.toSet()); + assertTrue(resultNames.contains("price_history")); assertEquals(1, response.getPaging().getTotal()); target = getResource("tables/" + table.getId() + "/columns/search").queryParam("q", "EMAIL"); response = TestUtils.get(target, TableResource.TableColumnList.class, ADMIN_AUTH_HEADERS); assertEquals(1, response.getData().size()); - assertEquals("email_address", response.getData().get(0).getName()); + assertEquals("email_address", response.getData().getFirst().getName()); target = getResource("tables/" + table.getId() + "/columns/search") @@ -4153,7 +4161,7 @@ public class TableResourceTest extends EntityResourceTest { void test_updateColumn_ownerCanUpdateOwnedTableColumns(TestInfo test) throws IOException { CreateTable create = createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference())); Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); - String columnFQN = COLUMNS.get(0).getFullyQualifiedName(); + String columnFQN = table.getColumns().getFirst().getFullyQualifiedName(); org.openmetadata.schema.api.data.UpdateColumn updateColumn = new org.openmetadata.schema.api.data.UpdateColumn(); @@ -4171,7 +4179,7 @@ public class TableResourceTest extends EntityResourceTest { void test_updateColumn_dataStewardCanUpdateDescriptionAndTags(TestInfo test) throws IOException { CreateTable create = createRequest(test).withOwners(listOf(USER1.getEntityReference())); Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); - String columnFQN = COLUMNS.get(0).getFullyQualifiedName(); + String columnFQN = table.getColumns().get(0).getFullyQualifiedName(); org.openmetadata.schema.api.data.UpdateColumn updateColumn = new org.openmetadata.schema.api.data.UpdateColumn(); @@ -4193,74 +4201,108 @@ public class TableResourceTest extends EntityResourceTest { @Test void test_updateColumn_dataConsumerCannotUpdateColumns(TestInfo test) throws IOException { - CreateTable create = createRequest(test).withOwners(listOf(USER1.getEntityReference())); - Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); - String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName(); + // Temporarily remove Organization's default roles to ensure USER3 has no permissions + Team org = getOrganization(); + List originalDefaultRoles = org.getDefaultRoles(); + updateOrganizationDefaultRoles(null); - org.openmetadata.schema.api.data.UpdateColumn updateColumn = - new org.openmetadata.schema.api.data.UpdateColumn(); - updateColumn.setDescription("Data consumer trying to update"); + try { + CreateTable create = createRequest(test).withOwners(listOf(USER1.getEntityReference())); + Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); + String columnFQN = table.getColumns().getFirst().getFullyQualifiedName(); - Map dataConsumerAuthHeaders = authHeaders(DATA_CONSUMER.getName()); + org.openmetadata.schema.api.data.UpdateColumn updateColumn = + new org.openmetadata.schema.api.data.UpdateColumn(); + updateColumn.setDescription("Data consumer trying to update"); - assertResponse( - () -> updateColumnByFQN(columnFQN, updateColumn, dataConsumerAuthHeaders), - FORBIDDEN, - permissionNotAllowed(DATA_CONSUMER.getName(), List.of(MetadataOperation.EDIT_DESCRIPTION))); + assertResponse( + () -> updateColumnByFQN(columnFQN, updateColumn, authHeaders(USER3.getName())), + FORBIDDEN, + permissionNotAllowed(USER3.getName(), List.of(MetadataOperation.EDIT_DESCRIPTION))); + } finally { + // Restore original default roles + updateOrganizationDefaultRoles(originalDefaultRoles); + } } @Test void test_updateColumn_nonOwnerCannotUpdateDisplayName(TestInfo test) throws IOException { - CreateTable create = createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference())); - Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); - String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName(); + Team org = getOrganization(); + List originalDefaultRoles = org.getDefaultRoles(); + updateOrganizationDefaultRoles(null); - org.openmetadata.schema.api.data.UpdateColumn updateColumn = - new org.openmetadata.schema.api.data.UpdateColumn(); - updateColumn.setDisplayName("Non-owner trying to update display name"); + try { + CreateTable create = + createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference())); + Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); + String columnFQN = table.getColumns().getFirst().getFullyQualifiedName(); - Map user1AuthHeaders = authHeaders(USER1.getName()); + UpdateColumn updateColumn = new UpdateColumn(); + updateColumn.setDisplayName("Non-owner trying to update display name"); - assertResponse( - () -> updateColumnByFQN(columnFQN, updateColumn, user1AuthHeaders), - FORBIDDEN, - permissionNotAllowed(USER1.getName(), List.of(MetadataOperation.EDIT_ALL))); + Map user3AuthHeaders = authHeaders(USER3.getName()); + + assertResponse( + () -> updateColumnByFQN(columnFQN, updateColumn, user3AuthHeaders), + FORBIDDEN, + permissionNotAllowed(USER3.getName(), List.of(MetadataOperation.EDIT_DISPLAY_NAME))); + } finally { + updateOrganizationDefaultRoles(originalDefaultRoles); + } } @Test void test_updateColumn_nonOwnerCannotUpdateConstraints(TestInfo test) throws IOException { - CreateTable create = createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference())); - Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); - String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName(); + // Temporarily remove Organization's default roles to ensure USER3 has no permissions + Team org = getOrganization(); + List originalDefaultRoles = org.getDefaultRoles(); + updateOrganizationDefaultRoles(null); - org.openmetadata.schema.api.data.UpdateColumn updateColumn = - new org.openmetadata.schema.api.data.UpdateColumn(); - updateColumn.setConstraint(ColumnConstraint.UNIQUE); + try { + CreateTable create = + createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference())); + Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); + String columnFQN = table.getColumns().getFirst().getFullyQualifiedName(); - Map user1AuthHeaders = authHeaders(USER1.getName()); + UpdateColumn updateColumn = new UpdateColumn(); + updateColumn.setConstraint(ColumnConstraint.UNIQUE); - assertResponse( - () -> updateColumnByFQN(columnFQN, updateColumn, user1AuthHeaders), - FORBIDDEN, - permissionNotAllowed(USER1.getName(), List.of(MetadataOperation.EDIT_ALL))); + Map user3AuthHeaders = authHeaders(USER3.getName()); + + assertResponse( + () -> updateColumnByFQN(columnFQN, updateColumn, user3AuthHeaders), + FORBIDDEN, + permissionNotAllowed(USER3.getName(), List.of(MetadataOperation.EDIT_ALL))); + } finally { + // Restore original default roles + updateOrganizationDefaultRoles(originalDefaultRoles); + } } @Test void test_updateColumn_userCannotUpdateOtherUsersTableColumns(TestInfo test) throws IOException { - CreateTable create = createRequest(test).withOwners(listOf(USER1.getEntityReference())); - Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); - String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName(); + // Temporarily remove Organization's default roles to ensure USER3 has no permissions + Team org = getOrganization(); + List originalDefaultRoles = org.getDefaultRoles(); + updateOrganizationDefaultRoles(null); - org.openmetadata.schema.api.data.UpdateColumn updateColumn = - new org.openmetadata.schema.api.data.UpdateColumn(); - updateColumn.setDescription("USER2 trying to update USER1's table"); + try { + CreateTable create = createRequest(test).withOwners(listOf(USER1.getEntityReference())); + Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); + String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName(); - Map user2AuthHeaders = authHeaders(USER2.getName()); + UpdateColumn updateColumn = new UpdateColumn(); + updateColumn.setDescription("USER3 trying to update USER1's table"); - assertResponse( - () -> updateColumnByFQN(columnFQN, updateColumn, user2AuthHeaders), - FORBIDDEN, - permissionNotAllowed(USER2.getName(), List.of(MetadataOperation.EDIT_DESCRIPTION))); + Map user3AuthHeaders = authHeaders(USER3.getName()); + + assertResponse( + () -> updateColumnByFQN(columnFQN, updateColumn, user3AuthHeaders), + FORBIDDEN, + permissionNotAllowed(USER3.getName(), List.of(MetadataOperation.EDIT_DESCRIPTION))); + } finally { + updateOrganizationDefaultRoles(originalDefaultRoles); + } } @Test @@ -4283,7 +4325,23 @@ public class TableResourceTest extends EntityResourceTest { org.openmetadata.schema.api.data.UpdateColumn updateColumn, Map authHeaders) throws IOException { - WebTarget target = getResource("columns/name/" + columnFQN).queryParam("entityType", "table"); + String encodedFQN = EntityUtil.encodeEntityFqn(columnFQN); + WebTarget target = getResource("columns/name/" + encodedFQN).queryParam("entityType", "table"); return TestUtils.put(target, updateColumn, Column.class, OK, authHeaders); } + + private Team getOrganization() throws IOException { + TeamResourceTest teamResourceTest = new TeamResourceTest(); + return teamResourceTest.getEntityByName( + "Organization", teamResourceTest.getAllowedFields(), ADMIN_AUTH_HEADERS); + } + + private void updateOrganizationDefaultRoles(List defaultRoles) + throws IOException { + Team org = getOrganization(); + String json = JsonUtils.pojoToJson(org); + org.setDefaultRoles(defaultRoles); + TeamResourceTest teamResourceTest = new TeamResourceTest(); + teamResourceTest.patchEntity(org.getId(), json, org, ADMIN_AUTH_HEADERS); + } } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java index 6e36d2f33e6..57b6e85f9d4 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java @@ -194,6 +194,11 @@ public class UserResourceTest extends EntityResourceTest { USER_TEAM21 = createEntity(create, ADMIN_AUTH_HEADERS); USER2_REF = USER2.getEntityReference(); + // USER3 with no roles for permission testing + create = createRequest(test, 3).withRoles(List.of()); + USER3 = createEntity(create, ADMIN_AUTH_HEADERS); + USER3_REF = USER3.getEntityReference(); + Set userFields = Entity.getEntityFields(User.class); userFields.remove("authenticationMechanism"); BOT_USER = getEntityByName(INGESTION_BOT, String.join(",", userFields), ADMIN_AUTH_HEADERS);