diff --git a/core/src/main/java/com/datastrato/gravitino/authorization/AccessControlManager.java b/core/src/main/java/com/datastrato/gravitino/authorization/AccessControlManager.java index eb7dbfb04ef..26ec14a7e7b 100644 --- a/core/src/main/java/com/datastrato/gravitino/authorization/AccessControlManager.java +++ b/core/src/main/java/com/datastrato/gravitino/authorization/AccessControlManager.java @@ -53,7 +53,7 @@ public class AccessControlManager { public AccessControlManager(EntityStore store, IdGenerator idGenerator, Config config) { this.adminManager = new AdminManager(store, idGenerator, config); this.roleManager = new RoleManager(store, idGenerator, config); - this.userGroupManager = new UserGroupManager(store, idGenerator, roleManager); + this.userGroupManager = new UserGroupManager(store, idGenerator); this.permissionManager = new PermissionManager(store, roleManager); } diff --git a/core/src/main/java/com/datastrato/gravitino/authorization/PermissionManager.java b/core/src/main/java/com/datastrato/gravitino/authorization/PermissionManager.java index f30f418771b..9bbb51fbaed 100644 --- a/core/src/main/java/com/datastrato/gravitino/authorization/PermissionManager.java +++ b/core/src/main/java/com/datastrato/gravitino/authorization/PermissionManager.java @@ -67,9 +67,10 @@ User grantRolesToUser(String metalake, List roles, String user) { UserEntity.class, Entity.EntityType.USER, userEntity -> { - List roleEntities = - roleManager.getValidRoles(metalake, userEntity.roleNames(), userEntity.roleIds()); - + List roleEntities = Lists.newArrayList(); + for (String role : userEntity.roleNames()) { + roleEntities.add(roleManager.getRole(metalake, role)); + } List roleNames = Lists.newArrayList(toRoleNames(roleEntities)); List roleIds = Lists.newArrayList(toRoleIds(roleEntities)); @@ -129,8 +130,10 @@ Group grantRolesToGroup(String metalake, List roles, String group) { GroupEntity.class, Entity.EntityType.GROUP, groupEntity -> { - List roleEntities = - roleManager.getValidRoles(metalake, groupEntity.roleNames(), groupEntity.roleIds()); + List roleEntities = Lists.newArrayList(); + for (String role : groupEntity.roleNames()) { + roleEntities.add(roleManager.getRole(metalake, role)); + } List roleNames = Lists.newArrayList(toRoleNames(roleEntities)); List roleIds = Lists.newArrayList(toRoleIds(roleEntities)); @@ -190,8 +193,10 @@ Group revokeRolesFromGroup(String metalake, List roles, String group) { GroupEntity.class, Entity.EntityType.GROUP, groupEntity -> { - List roleEntities = - roleManager.getValidRoles(metalake, groupEntity.roleNames(), groupEntity.roleIds()); + List roleEntities = Lists.newArrayList(); + for (String role : groupEntity.roleNames()) { + roleEntities.add(roleManager.getRole(metalake, role)); + } List roleNames = Lists.newArrayList(toRoleNames(roleEntities)); List roleIds = Lists.newArrayList(toRoleIds(roleEntities)); @@ -252,8 +257,10 @@ User revokeRolesFromUser(String metalake, List roles, String user) { UserEntity.class, Entity.EntityType.USER, userEntity -> { - List roleEntities = - roleManager.getValidRoles(metalake, userEntity.roleNames(), userEntity.roleIds()); + List roleEntities = Lists.newArrayList(); + for (String role : userEntity.roleNames()) { + roleEntities.add(roleManager.getRole(metalake, role)); + } List roleNames = Lists.newArrayList(toRoleNames(roleEntities)); List roleIds = Lists.newArrayList(toRoleIds(roleEntities)); diff --git a/core/src/main/java/com/datastrato/gravitino/authorization/RoleManager.java b/core/src/main/java/com/datastrato/gravitino/authorization/RoleManager.java index 96fc7170250..9d717b4eb9c 100644 --- a/core/src/main/java/com/datastrato/gravitino/authorization/RoleManager.java +++ b/core/src/main/java/com/datastrato/gravitino/authorization/RoleManager.java @@ -36,7 +36,6 @@ import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.Scheduler; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Lists; import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.io.IOException; import java.time.Instant; @@ -161,28 +160,4 @@ private RoleEntity getRoleEntity(NameIdentifier identifier) { Cache getCache() { return cache; } - - List getValidRoles(String metalake, List roleNames, List roleIds) { - List roleEntities = Lists.newArrayList(); - if (roleNames == null || roleNames.isEmpty()) { - return roleEntities; - } - - int index = 0; - for (String role : roleNames) { - try { - - RoleEntity roleEntity = getRoleEntity(AuthorizationUtils.ofRole(metalake, role)); - - if (roleEntity.id().equals(roleIds.get(index))) { - roleEntities.add(roleEntity); - } - index++; - - } catch (NoSuchEntityException nse) { - // ignore this entity - } - } - return roleEntities; - } } diff --git a/core/src/main/java/com/datastrato/gravitino/authorization/UserGroupManager.java b/core/src/main/java/com/datastrato/gravitino/authorization/UserGroupManager.java index c21b060a304..b92ab7d5a1d 100644 --- a/core/src/main/java/com/datastrato/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/com/datastrato/gravitino/authorization/UserGroupManager.java @@ -28,7 +28,6 @@ import com.datastrato.gravitino.exceptions.UserAlreadyExistsException; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.GroupEntity; -import com.datastrato.gravitino.meta.RoleEntity; import com.datastrato.gravitino.meta.UserEntity; import com.datastrato.gravitino.storage.IdGenerator; import com.datastrato.gravitino.utils.PrincipalUtils; @@ -36,8 +35,6 @@ import java.io.IOException; import java.time.Instant; import java.util.Collections; -import java.util.List; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,12 +49,10 @@ class UserGroupManager { private final EntityStore store; private final IdGenerator idGenerator; - private final RoleManager roleManager; - UserGroupManager(EntityStore store, IdGenerator idGenerator, RoleManager roleManager) { + UserGroupManager(EntityStore store, IdGenerator idGenerator) { this.store = store; this.idGenerator = idGenerator; - this.roleManager = roleManager; } User addUser(String metalake, String name) throws UserAlreadyExistsException { @@ -102,20 +97,9 @@ boolean removeUser(String metalake, String user) { User getUser(String metalake, String user) throws NoSuchUserException { try { AuthorizationUtils.checkMetalakeExists(metalake); - UserEntity entity = - store.get( - AuthorizationUtils.ofUser(metalake, user), Entity.EntityType.USER, UserEntity.class); + return store.get( + AuthorizationUtils.ofUser(metalake, user), Entity.EntityType.USER, UserEntity.class); - List roleEntities = - roleManager.getValidRoles(metalake, entity.roles(), entity.roleIds()); - - return UserEntity.builder() - .withId(entity.id()) - .withName(entity.name()) - .withAuditInfo(entity.auditInfo()) - .withNamespace(entity.namespace()) - .withRoleNames(roleEntities.stream().map(RoleEntity::name).collect(Collectors.toList())) - .build(); } catch (NoSuchEntityException e) { LOG.warn("User {} does not exist in the metalake {}", user, metalake, e); throw new NoSuchUserException(AuthorizationUtils.USER_DOES_NOT_EXIST_MSG, user, metalake); @@ -171,22 +155,8 @@ Group getGroup(String metalake, String group) { try { AuthorizationUtils.checkMetalakeExists(metalake); - GroupEntity entity = - store.get( - AuthorizationUtils.ofGroup(metalake, group), - Entity.EntityType.GROUP, - GroupEntity.class); - - List roleEntities = - roleManager.getValidRoles(metalake, entity.roles(), entity.roleIds()); - - return GroupEntity.builder() - .withId(entity.id()) - .withName(entity.name()) - .withAuditInfo(entity.auditInfo()) - .withNamespace(entity.namespace()) - .withRoleNames(roleEntities.stream().map(RoleEntity::name).collect(Collectors.toList())) - .build(); + return store.get( + AuthorizationUtils.ofGroup(metalake, group), Entity.EntityType.GROUP, GroupEntity.class); } catch (NoSuchEntityException e) { LOG.warn("Group {} does not exist in the metalake {}", group, metalake, e); throw new NoSuchGroupException(AuthorizationUtils.GROUP_DOES_NOT_EXIST_MSG, group, metalake);