Revert "Add support for default role (#2631)" (#2651)

This reverts commit bfcd97ed0bae1770deb992c6e4ec518b1ea0b663.
This commit is contained in:
Matt 2022-02-07 01:54:26 -08:00 committed by GitHub
parent e38963d7c2
commit d2132551b1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 8 additions and 156 deletions

View File

@ -1,7 +0,0 @@
-- 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

@ -211,7 +211,6 @@ public final class Entity {
}
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(

View File

@ -991,12 +991,6 @@ public interface CollectionDAO {
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();

View File

@ -16,9 +16,7 @@ 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;
@ -37,8 +35,6 @@ import org.openmetadata.catalog.type.EntityReference;
import org.openmetadata.catalog.type.PolicyType;
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> {
@ -191,34 +187,6 @@ public class RoleRepository extends EntityRepository<Role> {
Relationship.CONTAINS.ordinal());
}
public ResultList<Role> getDefaultRolesResultList(Fields fields)
throws GeneralSecurityException, UnsupportedEncodingException {
List<Role> roles = getDefaultRoles(fields);
return new ResultList<>(roles, null, null, roles.size());
}
private List<Role> getDefaultRoles(Fields fields) {
List<Role> roles =
daoCollection.roleDAO().getDefaultRoles().stream()
.map(
json -> {
try {
return 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);
@ -340,10 +308,5 @@ public class RoleRepository extends EntityRepository<Role> {
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());
}
}
}

View File

@ -25,12 +25,9 @@ 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;
@ -141,15 +138,11 @@ public class UserRepository extends EntityRepository<User> {
}
public List<EntityReference> validateRoles(List<UUID> roleIds) throws IOException {
// Populate default roles.
Set<UUID> roleIdsSet =
daoCollection.roleDAO().getDefaultRolesIds().stream().map(UUID::fromString).collect(Collectors.toSet());
if (roleIds != null) {
roleIdsSet.addAll(new HashSet<>(roleIds));
if (roleIds == null) {
return Collections.emptyList(); // Return an empty roles list
}
List<EntityReference> validatedRoles = new ArrayList<>();
for (UUID roleId : roleIdsSet) {
for (UUID roleId : roleIds) {
validatedRoles.add(daoCollection.roleDAO().findEntityReferenceById(roleId));
}
return validatedRoles;
@ -202,6 +195,7 @@ public class UserRepository extends EntityRepository<User> {
}
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) {
daoCollection

View File

@ -124,9 +124,6 @@ public class RoleResource {
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))
@ -154,11 +151,6 @@ public class RoleResource {
RestUtil.validateCursors(before, after);
EntityUtil.Fields fields = new EntityUtil.Fields(FIELD_LIST, fieldsParam);
if (defaultParam) {
// The number of default roles is usually 1, and hence does not require pagination.
return dao.getDefaultRolesResultList(fields);
}
ResultList<Role> roles;
if (before != null) { // Reverse paging
roles = dao.listBefore(uriInfo, fields, null, limitParam, before, include); // Ask for one extra entry
@ -350,6 +342,7 @@ public class RoleResource {
}))
JsonPatch patch)
throws IOException, ParseException {
SecurityUtil.checkAdminOrBotRole(authorizer, securityContext);
PatchResponse<Role> response =
dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch);

View File

@ -6,7 +6,7 @@
"type": "object",
"definitions": {
"roleName": {
"description": "A unique name for the role.",
"description": "A unique name of the role.",
"type": "string",
"minLength": 1,
"maxLength": 128
@ -59,11 +59,6 @@
"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"],

View File

@ -16,20 +16,15 @@ package org.openmetadata.catalog.resources.teams;
import static javax.ws.rs.core.Response.Status.FORBIDDEN;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.openmetadata.catalog.security.SecurityUtil.authHeaders;
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;
@ -43,12 +38,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.ResultList;
import org.openmetadata.catalog.util.TestUtils;
@Slf4j
@ -58,40 +50,6 @@ public class RoleResourceTest extends EntityResourceTest<Role, CreateRole> {
super(Entity.ROLE, Role.class, RoleList.class, "roles", null, false, false, false, false);
}
@Test
void get_queryDefaultRole(TestInfo test) throws IOException {
Role defaultRole = createRolesAndSetDefault(test, 7);
ResultList<Role> rolesResponse = listEntities(Map.of("default", "true"), ADMIN_AUTH_HEADERS);
assertEquals(1, rolesResponse.getData().size());
assertEquals(defaultRole.getId(), rolesResponse.getData().get(0).getId());
}
/**
* 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
@ -121,6 +79,7 @@ public class RoleResourceTest extends EntityResourceTest<Role, CreateRole> {
@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);

View File

@ -45,7 +45,6 @@ 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;
@ -93,34 +92,6 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
// during first time login without being an admin
}
@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
@ -512,15 +483,6 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
// 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);

View File

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