Skip to content

Commit

Permalink
Add support for default role (#2631) (#2676)
Browse files Browse the repository at this point in the history
- Add default field to role
- Amend GET /roles to query default roles
- Assign default role to new users being created
  • Loading branch information
mithmatt committed Feb 19, 2022
1 parent d2c6400 commit f080481
Show file tree
Hide file tree
Showing 10 changed files with 183 additions and 7 deletions.
12 changes: 12 additions & 0 deletions bootstrap/sql/mysql/v004__create_db_connection_info.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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`);

Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ public static void deleteEntity(String updatedBy, String entity, UUID entityId,
}

public static <T> EntityRepository<T> getEntityRepositoryForClass(@NonNull Class<T> clazz) {
@SuppressWarnings("unchecked")
EntityRepository<T> entityRepository = (EntityRepository<T>) CLASS_ENTITY_REPOSITORY_MAP.get(clazz);
if (entityRepository == null) {
throw new UnhandledServerException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,12 @@ default String getNameColumn() {
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
default EntityReference getEntityReference(Role entity) {
return new RoleEntityInterface(entity).getEntityReference();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@
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;
Expand All @@ -36,6 +39,8 @@
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<Role> {
Expand Down Expand Up @@ -174,6 +179,34 @@ public void storeRelationships(Role role) {
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
public EntityUpdater getUpdater(Role original, Role updated, Operation operation) {
return new RoleUpdater(original, updated, operation);
Expand Down Expand Up @@ -305,5 +338,10 @@ public class RoleUpdater extends EntityUpdater {
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());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@
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;
Expand Down Expand Up @@ -61,8 +64,28 @@ public EntityInterface<User> getEntityInterface(User entity) {
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<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
public void storeEntity(User user, boolean update) throws IOException {
Expand Down Expand Up @@ -190,7 +213,6 @@ private void assignRoles(User user, List<EntityReference> roles) {
}

private void assignTeams(User user, List<EntityReference> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ public RoleList(List<Role> roles, String beforeCursor, String afterCursor, int t
public ResultList<Role> 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))
Expand Down Expand Up @@ -152,7 +155,10 @@ public ResultList<Role> list(
EntityUtil.Fields fields = new EntityUtil.Fields(FIELD_LIST, fieldsParam);

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
} else { // Forward paging or first page
roles = dao.listAfter(uriInfo, fields, null, limitParam, after, include);
Expand Down Expand Up @@ -342,7 +348,6 @@ public Response patch(
}))
JsonPatch patch)
throws IOException, ParseException {

SecurityUtil.checkAdminOrBotRole(authorizer, securityContext);
PatchResponse<Role> response =
dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@
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;
Expand All @@ -37,7 +41,9 @@
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;
Expand All @@ -49,6 +55,44 @@ public RoleResourceTest() {
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
void post_validRoles_as_admin_200_OK(TestInfo test) throws IOException {
// Create role with different optional fields
Expand Down Expand Up @@ -78,7 +122,6 @@ private Role createAndCheckRole(CreateRole create, Map<String, String> authHeade

@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,15 @@
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;
Expand Down Expand Up @@ -79,6 +83,7 @@
import org.openmetadata.catalog.util.TestUtils.UpdateType;

@Slf4j
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
final Profile PROFILE = new Profile().withImages(new ImageList().withImage(URI.create("http://image.com")));

Expand All @@ -93,6 +98,35 @@ public void post_entity_as_non_admin_401(TestInfo test) {
// 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
void post_userWithoutEmail_400_badRequest(TestInfo test) {
// Create user with mandatory email field null
Expand Down Expand Up @@ -479,6 +513,15 @@ void delete_validUser_as_admin_200(TestInfo test) throws IOException {
// 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)
throws JsonProcessingException, HttpResponseException {
String updatedJson = JsonUtils.pojoToJson(updated);
Expand Down
3 changes: 2 additions & 1 deletion ingestion-core/src/metadata/_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@

from incremental import Version

__version__ = Version("metadata", 0, 9, 0, dev=8)

__version__ = Version("metadata", 0, 9, 0, dev=9)
__all__ = ["__version__"]

0 comments on commit f080481

Please sign in to comment.