From f0804816ab50e7f9838b5e66f96d008a5a5cb767 Mon Sep 17 00:00:00 2001 From: Matt Date: Sat, 19 Feb 2022 11:02:21 -0800 Subject: [PATCH] Add support for default role (#2631) (#2676) - Add default field to role - Amend GET /roles to query default roles - Assign default role to new users being created --- .../mysql/v004__create_db_connection_info.sql | 12 +++++ .../java/org/openmetadata/catalog/Entity.java | 1 + .../catalog/jdbi3/CollectionDAO.java | 6 +++ .../catalog/jdbi3/RoleRepository.java | 38 ++++++++++++++++ .../catalog/jdbi3/UserRepository.java | 26 ++++++++++- .../catalog/resources/teams/RoleResource.java | 9 +++- .../json/schema/entity/teams/role.json | 7 ++- .../resources/teams/RoleResourceTest.java | 45 ++++++++++++++++++- .../resources/teams/UserResourceTest.java | 43 ++++++++++++++++++ ingestion-core/src/metadata/_version.py | 3 +- 10 files changed, 183 insertions(+), 7 deletions(-) diff --git a/bootstrap/sql/mysql/v004__create_db_connection_info.sql b/bootstrap/sql/mysql/v004__create_db_connection_info.sql index db93b6dde74..b0175c5c0b4 100644 --- a/bootstrap/sql/mysql/v004__create_db_connection_info.sql +++ b/bootstrap/sql/mysql/v004__create_db_connection_info.sql @@ -27,6 +27,7 @@ CREATE TABLE IF NOT EXISTS glossary_entity ( INDEX (updatedBy), INDEX (updatedAt) ); + CREATE TABLE IF NOT EXISTS glossary_term_entity ( id VARCHAR(36) GENERATED ALWAYS AS (json ->> '$.id') STORED NOT NULL, fullyQualifiedName VARCHAR(256) GENERATED ALWAYS AS (json ->> '$.fullyQualifiedName') NOT NULL, @@ -39,3 +40,14 @@ CREATE TABLE IF NOT EXISTS glossary_term_entity ( INDEX (updatedBy), INDEX (updatedAt) ); + + +-- Set default as false for all existing roles, to avoid unintended manipulation of roles during migration. + +UPDATE role_entity +SET json = JSON_SET(json, '$.default', FALSE); + +ALTER TABLE role_entity +ADD COLUMN `default` BOOLEAN GENERATED ALWAYS AS (JSON_EXTRACT(json, '$.default')), +ADD INDEX(`default`); + diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java index 024e7913ea1..1d765df65c6 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java @@ -229,6 +229,7 @@ public final class Entity { } public static EntityRepository getEntityRepositoryForClass(@NonNull Class clazz) { + @SuppressWarnings("unchecked") EntityRepository entityRepository = (EntityRepository) CLASS_ENTITY_REPOSITORY_MAP.get(clazz); if (entityRepository == null) { throw new UnhandledServerException( diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java index 590bcbc0b4c..a28dfa54ef7 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java @@ -1061,6 +1061,12 @@ public interface CollectionDAO { return "name"; } + @SqlQuery("SELECT id FROM role_entity WHERE `default` = TRUE") + List getDefaultRolesIds(); + + @SqlQuery("SELECT json FROM role_entity WHERE `default` = TRUE") + List getDefaultRoles(); + @Override default EntityReference getEntityReference(Role entity) { return new RoleEntityInterface(entity).getEntityReference(); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java index 8a99e0d3d54..d5d0f416421 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java @@ -16,12 +16,15 @@ package org.openmetadata.catalog.jdbi3; import static org.openmetadata.catalog.util.EntityUtil.toBoolean; import java.io.IOException; +import java.io.UnsupportedEncodingException; import java.net.URI; +import java.security.GeneralSecurityException; import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; +import javax.ws.rs.core.UriInfo; import lombok.NonNull; import lombok.extern.slf4j.Slf4j; import org.jdbi.v3.sqlobject.transaction.Transaction; @@ -36,6 +39,8 @@ import org.openmetadata.catalog.type.PolicyType; import org.openmetadata.catalog.type.Relationship; import org.openmetadata.catalog.util.EntityInterface; import org.openmetadata.catalog.util.EntityUtil.Fields; +import org.openmetadata.catalog.util.JsonUtils; +import org.openmetadata.catalog.util.ResultList; @Slf4j public class RoleRepository extends EntityRepository { @@ -174,6 +179,34 @@ public class RoleRepository extends EntityRepository { addRelationship(role.getId(), role.getPolicy().getId(), Entity.ROLE, Entity.POLICY, Relationship.CONTAINS); } + public ResultList getDefaultRolesResultList(UriInfo uriInfo, Fields fields) + throws GeneralSecurityException, UnsupportedEncodingException { + List roles = getDefaultRoles(uriInfo, fields); + return new ResultList<>(roles, null, null, roles.size()); + } + + private List getDefaultRoles(UriInfo uriInfo, Fields fields) { + List roles = + daoCollection.roleDAO().getDefaultRoles().stream() + .map( + json -> { + try { + return withHref(uriInfo, setFields(JsonUtils.readValue(json, Role.class), fields)); + } catch (IOException e) { + LOG.warn("Could not parse Role from json {}", json); + } + return null; + }) + .collect(Collectors.toList()); + if (roles.size() > 1) { + LOG.warn( + "{} roles {}, are registered as default. There SHOULD be only one role marked as default.", + roles.size(), + roles.stream().map(Role::getName).collect(Collectors.toList())); + } + return roles; + } + @Override public EntityUpdater getUpdater(Role original, Role updated, Operation operation) { return new RoleUpdater(original, updated, operation); @@ -305,5 +338,10 @@ public class RoleRepository extends EntityRepository { public RoleUpdater(Role original, Role updated, Operation operation) { super(original, updated, operation); } + + @Override + public void entitySpecificUpdate() throws IOException { + recordChange("default", original.getEntity().getDefault(), updated.getEntity().getDefault()); + } } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java index 32724b83f67..ce32c6bc444 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java @@ -22,9 +22,12 @@ import java.net.URI; import java.text.ParseException; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; 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; @@ -61,8 +64,28 @@ public class UserRepository extends EntityRepository { return new UserEntityInterface(entity); } + /** Ensures that the default roles are added for POST, PUT and PATCH operations. */ @Override - public void prepare(User entity) {} + public void prepare(User user) throws IOException { + List rolesRef = user.getRoles(); + Set existingRoleIds = new HashSet<>(); + if (rolesRef != null) { + existingRoleIds = user.getRoles().stream().map(EntityReference::getId).collect(Collectors.toSet()); + } + + // Find default roles to add. + Set defaultRoleIds = + daoCollection.roleDAO().getDefaultRolesIds().stream().map(UUID::fromString).collect(Collectors.toSet()); + defaultRoleIds.removeAll(existingRoleIds); + + if (rolesRef == null || rolesRef.size() == 0) { + rolesRef = new ArrayList<>(); + } + for (UUID roleId : defaultRoleIds) { + rolesRef.add(daoCollection.roleDAO().findEntityReferenceById(roleId)); + } + user.setRoles(rolesRef); + } @Override public void storeEntity(User user, boolean update) throws IOException { @@ -190,7 +213,6 @@ public class UserRepository extends EntityRepository { } private void assignTeams(User user, List teams) { - // Query - add team to the user teams = Optional.ofNullable(teams).orElse(Collections.emptyList()); for (EntityReference team : teams) { addRelationship(team.getId(), user.getId(), Entity.TEAM, Entity.USER, Relationship.HAS); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/RoleResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/RoleResource.java index 935bfc21b14..2640c022e2f 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/RoleResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/RoleResource.java @@ -124,6 +124,9 @@ public class RoleResource { public ResultList list( @Context UriInfo uriInfo, @Context SecurityContext securityContext, + @Parameter(description = "List only default role(s)", schema = @Schema(type = "boolean", example = "true")) + @QueryParam("default") + boolean defaultParam, @Parameter( description = "Fields requested in the returned resource", schema = @Schema(type = "string", example = FIELDS)) @@ -152,7 +155,10 @@ public class RoleResource { EntityUtil.Fields fields = new EntityUtil.Fields(FIELD_LIST, fieldsParam); ResultList roles; - if (before != null) { // Reverse paging + if (defaultParam) { + // The number of default roles is usually 1, and hence does not require pagination. + roles = dao.getDefaultRolesResultList(uriInfo, fields); + } else if (before != null) { // Reverse paging roles = dao.listBefore(uriInfo, fields, null, limitParam, before, include); // Ask for one extra entry } else { // Forward paging or first page roles = dao.listAfter(uriInfo, fields, null, limitParam, after, include); @@ -342,7 +348,6 @@ public class RoleResource { })) JsonPatch patch) throws IOException, ParseException { - SecurityUtil.checkAdminOrBotRole(authorizer, securityContext); PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json b/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json index d2639e972e3..a665c4afaa1 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json +++ b/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json @@ -6,7 +6,7 @@ "type": "object", "definitions": { "roleName": { - "description": "A unique name of the role.", + "description": "A unique name for the role.", "type": "string", "minLength": 1, "maxLength": 128 @@ -59,6 +59,11 @@ "users": { "description": "Users that have this role assigned.", "$ref": "../../type/entityReference.json#/definitions/entityReferenceList" + }, + "default": { + "description": "If `true`, this role is set as default and will be assigned to all users.", + "type": "boolean", + "default": false } }, "required": ["id", "name"], diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java index f68fafe332e..4d26e805f90 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java @@ -17,13 +17,17 @@ import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.openmetadata.catalog.util.TestUtils.ADMIN_AUTH_HEADERS; import static org.openmetadata.catalog.util.TestUtils.TEST_AUTH_HEADERS; +import static org.openmetadata.catalog.util.TestUtils.UpdateType.MINOR_UPDATE; import static org.openmetadata.catalog.util.TestUtils.assertListNotNull; import static org.openmetadata.catalog.util.TestUtils.assertResponse; import com.fasterxml.jackson.core.JsonProcessingException; import java.io.IOException; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Random; +import javax.validation.constraints.Positive; import lombok.extern.slf4j.Slf4j; import org.apache.http.client.HttpResponseException; import org.junit.jupiter.api.Test; @@ -37,7 +41,9 @@ import org.openmetadata.catalog.resources.EntityResourceTest; import org.openmetadata.catalog.resources.policies.PolicyResource; import org.openmetadata.catalog.resources.policies.PolicyResourceTest; import org.openmetadata.catalog.resources.teams.RoleResource.RoleList; +import org.openmetadata.catalog.type.ChangeDescription; import org.openmetadata.catalog.type.EntityReference; +import org.openmetadata.catalog.type.FieldChange; import org.openmetadata.catalog.util.EntityInterface; import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.TestUtils; @@ -49,6 +55,44 @@ public class RoleResourceTest extends EntityResourceTest { super(Entity.ROLE, Role.class, RoleList.class, "roles", null, false, false, false, false, false); } + @Test + void get_queryDefaultRole(TestInfo test) throws IOException { + Role defaultRole = createRolesAndSetDefault(test, 7); + List defaultRoles = getDefaultRoles(); + assertEquals(1, defaultRoles.size()); + assertEquals(defaultRole.getId(), defaultRoles.get(0).getId()); + } + + public List getDefaultRoles() throws HttpResponseException { + return listEntities(Map.of("default", "true"), ADMIN_AUTH_HEADERS).getData(); + } + + /** + * Creates the given number of roles and sets one of them as the default role. + * + * @return the default role + */ + public Role createRolesAndSetDefault(TestInfo test, @Positive int numberOfRoles) throws IOException { + // Create a set of roles. + for (int i = 0; i < numberOfRoles; i++) { + CreateRole create = createRequest(test, i + 1); + createAndCheckRole(create, ADMIN_AUTH_HEADERS); + } + + // Set one of the roles as default. + Role role = + getEntityByName( + getEntityName(test, new Random().nextInt(numberOfRoles)), + Collections.emptyMap(), + RoleResource.FIELDS, + ADMIN_AUTH_HEADERS); + String originalJson = JsonUtils.pojoToJson(role); + role.setDefault(true); + ChangeDescription change = getChangeDescription(role.getVersion()); + change.getFieldsUpdated().add(new FieldChange().withName("default").withOldValue(false).withNewValue(true)); + return patchEntityAndCheck(role, originalJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + } + @Test void post_validRoles_as_admin_200_OK(TestInfo test) throws IOException { // Create role with different optional fields @@ -78,7 +122,6 @@ public class RoleResourceTest extends EntityResourceTest { @Test void patch_roleAttributes_as_non_admin_403(TestInfo test) throws HttpResponseException, JsonProcessingException { - // Create table without any attributes Role role = createEntity(createRequest(test), ADMIN_AUTH_HEADERS); // Patching as a non-admin should is disallowed String originalJson = JsonUtils.pojoToJson(role); 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 8bab1020ae2..337b930ec4a 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 @@ -46,11 +46,15 @@ import java.util.Map; import java.util.Optional; import java.util.UUID; import java.util.function.Predicate; +import java.util.stream.Collectors; import javax.json.JsonPatch; import lombok.extern.slf4j.Slf4j; import org.apache.http.client.HttpResponseException; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.TestMethodOrder; import org.openmetadata.catalog.CatalogApplicationTest; import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.api.teams.CreateUser; @@ -79,6 +83,7 @@ import org.openmetadata.catalog.util.TestUtils; import org.openmetadata.catalog.util.TestUtils.UpdateType; @Slf4j +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class UserResourceTest extends EntityResourceTest { final Profile PROFILE = new Profile().withImages(new ImageList().withImage(URI.create("http://image.com"))); @@ -93,6 +98,35 @@ public class UserResourceTest extends EntityResourceTest { // during first time login without being an admin } + @Order(Integer.MAX_VALUE) // Run this test last to avoid side effects of default role creation to fail other tests. + @Test + void post_userWithDefaultRole(TestInfo test) throws IOException { + // Given no default role has been set, when a user is created, then no role should be assigned. + CreateUser create = createRequest(test, 1); + createUserAndCheckRoles(create, Collections.emptyList()); + + RoleResourceTest roleResourceTest = new RoleResourceTest(); + Role defaultRole = roleResourceTest.createRolesAndSetDefault(test, 7); + List roles = roleResourceTest.listEntities(Collections.emptyMap(), ADMIN_AUTH_HEADERS).getData(); + UUID nonDefaultRoleId = roles.stream().filter(role -> !role.getDefault()).findAny().orElseThrow().getId(); + UUID defaultRoleId = defaultRole.getId(); + + // Given a default role has been set, when a user is created without any roles, then the default role should be + // assigned. + create = createRequest(test, 2); + createUserAndCheckRoles(create, Arrays.asList(defaultRoleId)); + + // Given a default role has been set, when a user is created with a non default role, then the default role should + // be assigned along with the non default role. + create = createRequest(test, 3).withRoles(List.of(nonDefaultRoleId)); + createUserAndCheckRoles(create, Arrays.asList(nonDefaultRoleId, defaultRoleId)); + + // Given a default role has been set, when a user is created with both default and non-default role, then both + // roles should be assigned. + create = createRequest(test, 4).withRoles(List.of(nonDefaultRoleId, defaultRoleId)); + createUserAndCheckRoles(create, Arrays.asList(nonDefaultRoleId, defaultRoleId)); + } + @Test void post_userWithoutEmail_400_badRequest(TestInfo test) { // Create user with mandatory email field null @@ -479,6 +513,15 @@ public class UserResourceTest extends EntityResourceTest { // TODO deactivated user can't be made owner } + private void createUserAndCheckRoles(CreateUser create, List expectedRolesIds) throws HttpResponseException { + User user = createEntity(create, ADMIN_AUTH_HEADERS); + user = getEntity(user.getId(), ADMIN_AUTH_HEADERS); + List actualRolesIds = + user.getRoles().stream().map(EntityReference::getId).sorted().collect(Collectors.toList()); + Collections.sort(expectedRolesIds); + assertEquals(expectedRolesIds, actualRolesIds); + } + private User patchUser(UUID userId, String originalJson, User updated, Map headers) throws JsonProcessingException, HttpResponseException { String updatedJson = JsonUtils.pojoToJson(updated); diff --git a/ingestion-core/src/metadata/_version.py b/ingestion-core/src/metadata/_version.py index af65a6e6a60..321bf467368 100644 --- a/ingestion-core/src/metadata/_version.py +++ b/ingestion-core/src/metadata/_version.py @@ -7,5 +7,6 @@ Provides metadata version information. from incremental import Version -__version__ = Version("metadata", 0, 9, 0, dev=8) + +__version__ = Version("metadata", 0, 9, 0, dev=9) __all__ = ["__version__"]