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
This commit is contained in:
Matt 2022-02-19 11:02:21 -08:00 committed by GitHub
parent d2c64007cb
commit f0804816ab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 183 additions and 7 deletions

View File

@ -27,6 +27,7 @@ CREATE TABLE IF NOT EXISTS glossary_entity (
INDEX (updatedBy), INDEX (updatedBy),
INDEX (updatedAt) INDEX (updatedAt)
); );
CREATE TABLE IF NOT EXISTS glossary_term_entity ( CREATE TABLE IF NOT EXISTS glossary_term_entity (
id VARCHAR(36) GENERATED ALWAYS AS (json ->> '$.id') STORED NOT NULL, id VARCHAR(36) GENERATED ALWAYS AS (json ->> '$.id') STORED NOT NULL,
fullyQualifiedName VARCHAR(256) GENERATED ALWAYS AS (json ->> '$.fullyQualifiedName') 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 (updatedBy),
INDEX (updatedAt) 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`);

View File

@ -229,6 +229,7 @@ public final class Entity {
} }
public static <T> EntityRepository<T> getEntityRepositoryForClass(@NonNull Class<T> clazz) { public static <T> EntityRepository<T> getEntityRepositoryForClass(@NonNull Class<T> clazz) {
@SuppressWarnings("unchecked")
EntityRepository<T> entityRepository = (EntityRepository<T>) CLASS_ENTITY_REPOSITORY_MAP.get(clazz); EntityRepository<T> entityRepository = (EntityRepository<T>) CLASS_ENTITY_REPOSITORY_MAP.get(clazz);
if (entityRepository == null) { if (entityRepository == null) {
throw new UnhandledServerException( throw new UnhandledServerException(

View File

@ -1061,6 +1061,12 @@ public interface CollectionDAO {
return "name"; return "name";
} }
@SqlQuery("SELECT id FROM role_entity WHERE `default` = TRUE")
List<String> getDefaultRolesIds();
@SqlQuery("SELECT json FROM role_entity WHERE `default` = TRUE")
List<String> getDefaultRoles();
@Override @Override
default EntityReference getEntityReference(Role entity) { default EntityReference getEntityReference(Role entity) {
return new RoleEntityInterface(entity).getEntityReference(); return new RoleEntityInterface(entity).getEntityReference();

View File

@ -16,12 +16,15 @@ package org.openmetadata.catalog.jdbi3;
import static org.openmetadata.catalog.util.EntityUtil.toBoolean; import static org.openmetadata.catalog.util.EntityUtil.toBoolean;
import java.io.IOException; import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URI; import java.net.URI;
import java.security.GeneralSecurityException;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.UUID; import java.util.UUID;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import javax.ws.rs.core.UriInfo;
import lombok.NonNull; import lombok.NonNull;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.jdbi.v3.sqlobject.transaction.Transaction; 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.type.Relationship;
import org.openmetadata.catalog.util.EntityInterface; import org.openmetadata.catalog.util.EntityInterface;
import org.openmetadata.catalog.util.EntityUtil.Fields; import org.openmetadata.catalog.util.EntityUtil.Fields;
import org.openmetadata.catalog.util.JsonUtils;
import org.openmetadata.catalog.util.ResultList;
@Slf4j @Slf4j
public class RoleRepository extends EntityRepository<Role> { public class RoleRepository extends EntityRepository<Role> {
@ -174,6 +179,34 @@ public class RoleRepository extends EntityRepository<Role> {
addRelationship(role.getId(), role.getPolicy().getId(), Entity.ROLE, Entity.POLICY, Relationship.CONTAINS); addRelationship(role.getId(), role.getPolicy().getId(), Entity.ROLE, Entity.POLICY, Relationship.CONTAINS);
} }
public ResultList<Role> getDefaultRolesResultList(UriInfo uriInfo, Fields fields)
throws GeneralSecurityException, UnsupportedEncodingException {
List<Role> roles = getDefaultRoles(uriInfo, fields);
return new ResultList<>(roles, null, null, roles.size());
}
private List<Role> getDefaultRoles(UriInfo uriInfo, Fields fields) {
List<Role> 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 @Override
public EntityUpdater getUpdater(Role original, Role updated, Operation operation) { public EntityUpdater getUpdater(Role original, Role updated, Operation operation) {
return new RoleUpdater(original, updated, operation); return new RoleUpdater(original, updated, operation);
@ -305,5 +338,10 @@ public class RoleRepository extends EntityRepository<Role> {
public RoleUpdater(Role original, Role updated, Operation operation) { public RoleUpdater(Role original, Role updated, Operation operation) {
super(original, updated, operation); super(original, updated, operation);
} }
@Override
public void entitySpecificUpdate() throws IOException {
recordChange("default", original.getEntity().getDefault(), updated.getEntity().getDefault());
}
} }
} }

View File

@ -22,9 +22,12 @@ import java.net.URI;
import java.text.ParseException; import java.text.ParseException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.Set;
import java.util.UUID; import java.util.UUID;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.jdbi.v3.sqlobject.transaction.Transaction; import org.jdbi.v3.sqlobject.transaction.Transaction;
import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.Entity;
@ -61,8 +64,28 @@ public class UserRepository extends EntityRepository<User> {
return new UserEntityInterface(entity); return new UserEntityInterface(entity);
} }
/** Ensures that the default roles are added for POST, PUT and PATCH operations. */
@Override @Override
public void prepare(User entity) {} public void prepare(User user) throws IOException {
List<EntityReference> rolesRef = user.getRoles();
Set<UUID> existingRoleIds = new HashSet<>();
if (rolesRef != null) {
existingRoleIds = user.getRoles().stream().map(EntityReference::getId).collect(Collectors.toSet());
}
// Find default roles to add.
Set<UUID> 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 @Override
public void storeEntity(User user, boolean update) throws IOException { public void storeEntity(User user, boolean update) throws IOException {
@ -190,7 +213,6 @@ public class UserRepository extends EntityRepository<User> {
} }
private void assignTeams(User user, List<EntityReference> teams) { private void assignTeams(User user, List<EntityReference> teams) {
// Query - add team to the user
teams = Optional.ofNullable(teams).orElse(Collections.emptyList()); teams = Optional.ofNullable(teams).orElse(Collections.emptyList());
for (EntityReference team : teams) { for (EntityReference team : teams) {
addRelationship(team.getId(), user.getId(), Entity.TEAM, Entity.USER, Relationship.HAS); addRelationship(team.getId(), user.getId(), Entity.TEAM, Entity.USER, Relationship.HAS);

View File

@ -124,6 +124,9 @@ public class RoleResource {
public ResultList<Role> list( public ResultList<Role> list(
@Context UriInfo uriInfo, @Context UriInfo uriInfo,
@Context SecurityContext securityContext, @Context SecurityContext securityContext,
@Parameter(description = "List only default role(s)", schema = @Schema(type = "boolean", example = "true"))
@QueryParam("default")
boolean defaultParam,
@Parameter( @Parameter(
description = "Fields requested in the returned resource", description = "Fields requested in the returned resource",
schema = @Schema(type = "string", example = FIELDS)) schema = @Schema(type = "string", example = FIELDS))
@ -152,7 +155,10 @@ public class RoleResource {
EntityUtil.Fields fields = new EntityUtil.Fields(FIELD_LIST, fieldsParam); EntityUtil.Fields fields = new EntityUtil.Fields(FIELD_LIST, fieldsParam);
ResultList<Role> roles; ResultList<Role> 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 roles = dao.listBefore(uriInfo, fields, null, limitParam, before, include); // Ask for one extra entry
} else { // Forward paging or first page } else { // Forward paging or first page
roles = dao.listAfter(uriInfo, fields, null, limitParam, after, include); roles = dao.listAfter(uriInfo, fields, null, limitParam, after, include);
@ -342,7 +348,6 @@ public class RoleResource {
})) }))
JsonPatch patch) JsonPatch patch)
throws IOException, ParseException { throws IOException, ParseException {
SecurityUtil.checkAdminOrBotRole(authorizer, securityContext); SecurityUtil.checkAdminOrBotRole(authorizer, securityContext);
PatchResponse<Role> response = PatchResponse<Role> response =
dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch);

View File

@ -6,7 +6,7 @@
"type": "object", "type": "object",
"definitions": { "definitions": {
"roleName": { "roleName": {
"description": "A unique name of the role.", "description": "A unique name for the role.",
"type": "string", "type": "string",
"minLength": 1, "minLength": 1,
"maxLength": 128 "maxLength": 128
@ -59,6 +59,11 @@
"users": { "users": {
"description": "Users that have this role assigned.", "description": "Users that have this role assigned.",
"$ref": "../../type/entityReference.json#/definitions/entityReferenceList" "$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"], "required": ["id", "name"],

View File

@ -17,13 +17,17 @@ import static javax.ws.rs.core.Response.Status.FORBIDDEN;
import static org.junit.jupiter.api.Assertions.assertEquals; 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.ADMIN_AUTH_HEADERS;
import static org.openmetadata.catalog.util.TestUtils.TEST_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.assertListNotNull;
import static org.openmetadata.catalog.util.TestUtils.assertResponse; import static org.openmetadata.catalog.util.TestUtils.assertResponse;
import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.JsonProcessingException;
import java.io.IOException; import java.io.IOException;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Random;
import javax.validation.constraints.Positive;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.apache.http.client.HttpResponseException; import org.apache.http.client.HttpResponseException;
import org.junit.jupiter.api.Test; 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.PolicyResource;
import org.openmetadata.catalog.resources.policies.PolicyResourceTest; import org.openmetadata.catalog.resources.policies.PolicyResourceTest;
import org.openmetadata.catalog.resources.teams.RoleResource.RoleList; 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.EntityReference;
import org.openmetadata.catalog.type.FieldChange;
import org.openmetadata.catalog.util.EntityInterface; import org.openmetadata.catalog.util.EntityInterface;
import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.JsonUtils;
import org.openmetadata.catalog.util.TestUtils; import org.openmetadata.catalog.util.TestUtils;
@ -49,6 +55,44 @@ public class RoleResourceTest extends EntityResourceTest<Role, CreateRole> {
super(Entity.ROLE, Role.class, RoleList.class, "roles", null, false, false, false, false, false); 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<Role> defaultRoles = getDefaultRoles();
assertEquals(1, defaultRoles.size());
assertEquals(defaultRole.getId(), defaultRoles.get(0).getId());
}
public List<Role> 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 @Test
void post_validRoles_as_admin_200_OK(TestInfo test) throws IOException { void post_validRoles_as_admin_200_OK(TestInfo test) throws IOException {
// Create role with different optional fields // Create role with different optional fields
@ -78,7 +122,6 @@ public class RoleResourceTest extends EntityResourceTest<Role, CreateRole> {
@Test @Test
void patch_roleAttributes_as_non_admin_403(TestInfo test) throws HttpResponseException, JsonProcessingException { 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); Role role = createEntity(createRequest(test), ADMIN_AUTH_HEADERS);
// Patching as a non-admin should is disallowed // Patching as a non-admin should is disallowed
String originalJson = JsonUtils.pojoToJson(role); String originalJson = JsonUtils.pojoToJson(role);

View File

@ -46,11 +46,15 @@ import java.util.Map;
import java.util.Optional; import java.util.Optional;
import java.util.UUID; import java.util.UUID;
import java.util.function.Predicate; import java.util.function.Predicate;
import java.util.stream.Collectors;
import javax.json.JsonPatch; import javax.json.JsonPatch;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.apache.http.client.HttpResponseException; 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.Test;
import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.api.TestMethodOrder;
import org.openmetadata.catalog.CatalogApplicationTest; import org.openmetadata.catalog.CatalogApplicationTest;
import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.api.teams.CreateUser; import org.openmetadata.catalog.api.teams.CreateUser;
@ -79,6 +83,7 @@ import org.openmetadata.catalog.util.TestUtils;
import org.openmetadata.catalog.util.TestUtils.UpdateType; import org.openmetadata.catalog.util.TestUtils.UpdateType;
@Slf4j @Slf4j
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class UserResourceTest extends EntityResourceTest<User, CreateUser> { public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
final Profile PROFILE = new Profile().withImages(new ImageList().withImage(URI.create("http://image.com"))); final Profile PROFILE = new Profile().withImages(new ImageList().withImage(URI.create("http://image.com")));
@ -93,6 +98,35 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
// during first time login without being an admin // 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<Role> 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 @Test
void post_userWithoutEmail_400_badRequest(TestInfo test) { void post_userWithoutEmail_400_badRequest(TestInfo test) {
// Create user with mandatory email field null // Create user with mandatory email field null
@ -479,6 +513,15 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
// TODO deactivated user can't be made owner // TODO deactivated user can't be made owner
} }
private void createUserAndCheckRoles(CreateUser create, List<UUID> expectedRolesIds) throws HttpResponseException {
User user = createEntity(create, ADMIN_AUTH_HEADERS);
user = getEntity(user.getId(), ADMIN_AUTH_HEADERS);
List<UUID> 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<String, String> headers) private User patchUser(UUID userId, String originalJson, User updated, Map<String, String> headers)
throws JsonProcessingException, HttpResponseException { throws JsonProcessingException, HttpResponseException {
String updatedJson = JsonUtils.pojoToJson(updated); String updatedJson = JsonUtils.pojoToJson(updated);

View File

@ -7,5 +7,6 @@ Provides metadata version information.
from incremental import Version from incremental import Version
__version__ = Version("metadata", 0, 9, 0, dev=8)
__version__ = Version("metadata", 0, 9, 0, dev=9)
__all__ = ["__version__"] __all__ = ["__version__"]