You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2018/05/02 07:31:31 UTC

[GitHub] DaanHoogland closed pull request #2606: When creating a new account (via domain admin) it is possible to select “root admin” as the role for the new user

DaanHoogland closed pull request #2606: When creating a new account (via domain admin) it is possible to select “root admin” as the role for the new user
URL: https://github.com/apache/cloudstack/pull/2606
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/api/src/main/java/org/apache/cloudstack/acl/RoleService.java b/api/src/main/java/org/apache/cloudstack/acl/RoleService.java
index 721bef08c20..6130c62a7d4 100644
--- a/api/src/main/java/org/apache/cloudstack/acl/RoleService.java
+++ b/api/src/main/java/org/apache/cloudstack/acl/RoleService.java
@@ -17,38 +17,64 @@
 
 package org.apache.cloudstack.acl;
 
+import java.util.List;
+
 import org.apache.cloudstack.acl.RolePermission.Permission;
 import org.apache.cloudstack.framework.config.ConfigKey;
 
-import java.util.List;
-
 public interface RoleService {
 
     ConfigKey<Boolean> EnableDynamicApiChecker = new ConfigKey<>("Advanced", Boolean.class, "dynamic.apichecker.enabled", "false",
-            "If set to true, this enables the dynamic role-based api access checker and disables the default static role-based api access checker.",
-            true);
+            "If set to true, this enables the dynamic role-based api access checker and disables the default static role-based api access checker.", true);
 
     boolean isEnabled();
-    Role findRole(final Long id);
-    Role createRole(final String name, final RoleType roleType, final String description);
-    Role updateRole(final Role role, final String name, final RoleType roleType, final String description);
-    boolean deleteRole(final Role role);
 
-    RolePermission findRolePermission(final Long id);
-    RolePermission findRolePermissionByUuid(final String uuid);
+    /**
+     *  Searches for a role with the given ID. If the ID is null or less than zero, this method will return null.
+     *  This method will also return null if no role is found with the provided ID.
+     *  Moreover, we will check if the requested role is of 'Admin' type; roles with 'Admin' type should only be visible to 'root admins'.
+     *  Therefore, if a non-'root admin' user tries to search for an 'Admin' role, this method will return null.
+     */
+    Role findRole(Long id);
+
+    Role createRole(String name, RoleType roleType, String description);
+
+    Role updateRole(Role role, String name, RoleType roleType, String description);
+
+    boolean deleteRole(Role role);
+
+    RolePermission findRolePermission(Long id);
+
+    RolePermission findRolePermissionByUuid(String uuid);
+
+    RolePermission createRolePermission(Role role, Rule rule, Permission permission, String description);
 
-    RolePermission createRolePermission(final Role role, final Rule rule, final Permission permission, final String description);
     /**
      * updateRolePermission updates the order/position of an role permission
      * @param role The role whose permissions needs to be re-ordered
      * @param newOrder The new list of ordered role permissions
      */
-    boolean updateRolePermission(final Role role, final List<RolePermission> newOrder);
-    boolean updateRolePermission(final Role role, final RolePermission rolePermission, final Permission permission);
-    boolean deleteRolePermission(final RolePermission rolePermission);
+    boolean updateRolePermission(Role role, List<RolePermission> newOrder);
 
+    boolean updateRolePermission(Role role, RolePermission rolePermission, Permission permission);
+
+    boolean deleteRolePermission(RolePermission rolePermission);
+
+    /**
+     *  List all roles configured in the database. Roles that have the type {@link RoleType#Admin} will not be shown for users that are not 'root admin'.
+     */
     List<Role> listRoles();
-    List<Role> findRolesByName(final String name);
-    List<Role> findRolesByType(final RoleType roleType);
-    List<RolePermission> findAllPermissionsBy(final Long roleId);
+
+    /**
+     *  Find all roles that have the giving {@link String} as part of their name.
+     *  If the user calling the method is not a 'root admin', roles of type {@link RoleType#Admin} wil lbe removed of the returned list.
+     */
+    List<Role> findRolesByName(String name);
+
+    /**
+     *  Find all roles by {@link RoleType}. If the role type is {@link RoleType#Admin}, the calling account must be a root admin, otherwise we return an empty list.
+     */
+    List<Role> findRolesByType(RoleType roleType);
+
+    List<RolePermission> findAllPermissionsBy(Long roleId);
 }
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java
index 5cf870bfc06..9025e89a93c 100644
--- a/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java
@@ -17,31 +17,25 @@
 
 package org.apache.cloudstack.api.command.admin.acl;
 
-import com.cloud.exception.ConcurrentOperationException;
-import com.cloud.exception.InsufficientCapacityException;
-import com.cloud.exception.NetworkRuleConflictException;
-import com.cloud.exception.ResourceAllocationException;
-import com.cloud.exception.ResourceUnavailableException;
-import com.cloud.user.Account;
-import com.google.common.base.Strings;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
 import org.apache.cloudstack.acl.Role;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.BaseCmd;
 import org.apache.cloudstack.api.Parameter;
-import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.response.ListResponse;
 import org.apache.cloudstack.api.response.RoleResponse;
+import org.apache.commons.lang3.StringUtils;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import com.cloud.user.Account;
+import com.google.common.base.Strings;
 
-@APICommand(name = ListRolesCmd.APINAME, description = "Lists dynamic roles in CloudStack", responseObject = RoleResponse.class,
-        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
-        since = "4.9.0",
-        authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin})
+@APICommand(name = ListRolesCmd.APINAME, description = "Lists dynamic roles in CloudStack", responseObject = RoleResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.9.0", authorized = {
+        RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin})
 public class ListRolesCmd extends BaseCmd {
     public static final String APINAME = "listRoles";
 
@@ -112,13 +106,13 @@ private void setupResponse(final List<Role> roles) {
     }
 
     @Override
-    public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
-        final List<Role> roles;
+    public void execute() {
+        List<Role> roles;
         if (getId() != null && getId() > 0L) {
             roles = Collections.singletonList(roleService.findRole(getId()));
-        } else if (!Strings.isNullOrEmpty(getName())) {
+        } else if (StringUtils.isNotBlank(getName())) {
             roles = roleService.findRolesByName(getName());
-        } else if (getRoleType() != null){
+        } else if (getRoleType() != null) {
             roles = roleService.findRolesByType(getRoleType());
         } else {
             roles = roleService.listRoles();
diff --git a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java
index b1e13313117..ae471b2486c 100644
--- a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java
@@ -18,6 +18,7 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 
 import javax.inject.Inject;
@@ -37,11 +38,15 @@
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.log4j.Logger;
 
 import com.cloud.event.ActionEvent;
 import com.cloud.event.EventTypes;
 import com.cloud.exception.PermissionDeniedException;
 import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.utils.ListUtils;
 import com.cloud.utils.component.ManagerBase;
@@ -52,18 +57,23 @@
 import com.google.common.base.Strings;
 
 public class RoleManagerImpl extends ManagerBase implements RoleService, Configurable, PluggableService {
+
+    private Logger logger = Logger.getLogger(getClass());
+
     @Inject
     private AccountDao accountDao;
     @Inject
     private RoleDao roleDao;
     @Inject
     private RolePermissionsDao rolePermissionsDao;
+    @Inject
+    private AccountManager accountManager;
 
     private void checkCallerAccess() {
         if (!isEnabled()) {
             throw new PermissionDeniedException("Dynamic api checker is not enabled, aborting role operation");
         }
-        Account caller = CallContext.current().getCallingAccount();
+        Account caller = getCurrentAccount();
         if (caller == null || caller.getRoleId() == null) {
             throw new PermissionDeniedException("Restricted API called by an invalid user account");
         }
@@ -79,11 +89,30 @@ public boolean isEnabled() {
     }
 
     @Override
-    public Role findRole(final Long id) {
+    public Role findRole(Long id) {
         if (id == null || id < 1L) {
+            logger.trace(String.format("Role ID is invalid [%s]", id));
+            return null;
+        }
+        RoleVO role = roleDao.findById(id);
+        if (role == null) {
+            logger.trace(String.format("Role not found [id=%s]", id));
             return null;
         }
-        return roleDao.findById(id);
+        Account account = getCurrentAccount();
+        if (!accountManager.isRootAdmin(account.getId()) && RoleType.Admin == role.getRoleType()) {
+            logger.debug(String.format("Role [id=%s, name=%s] is of 'Admin' type and is only visible to 'Root admins'.", id, role.getName()));
+            return null;
+        }
+        return role;
+    }
+
+    /**
+     * Simple call to {@link CallContext#current()} to retrieve the current calling account.
+     * This method facilitates unit testing, it avoids mocking static methods.
+     */
+    protected Account getCurrentAccount() {
+        return CallContext.current().getCallingAccount();
     }
 
     @Override
@@ -125,7 +154,7 @@ public Role updateRole(final Role role, final String name, final RoleType roleTy
         if (roleType != null && roleType == RoleType.Unknown) {
             throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unknown is not a valid role type");
         }
-        RoleVO roleVO = (RoleVO) role;
+        RoleVO roleVO = (RoleVO)role;
         if (!Strings.isNullOrEmpty(name)) {
             roleVO.setName(name);
         }
@@ -214,26 +243,56 @@ public boolean deleteRolePermission(final RolePermission rolePermission) {
     }
 
     @Override
-    public List<Role> findRolesByName(final String name) {
+    public List<Role> findRolesByName(String name) {
         List<? extends Role> roles = null;
-        if (!Strings.isNullOrEmpty(name)) {
+        if (StringUtils.isNotBlank(name)) {
             roles = roleDao.findAllByName(name);
         }
+        removeRootAdminRolesIfNeeded(roles);
         return ListUtils.toListOfInterface(roles);
     }
 
+    /**
+     *  Removes roles of the given list that have the type '{@link RoleType#Admin}' if the user calling the method is not a 'root admin'.
+     *  The actual removal is executed via {@link #removeRootAdminRoles(List)}. Therefore, if the method is called by a 'root admin', we do nothing here.
+     */
+    protected void removeRootAdminRolesIfNeeded(List<? extends Role> roles) {
+        Account account = getCurrentAccount();
+        if (!accountManager.isRootAdmin(account.getId())) {
+            removeRootAdminRoles(roles);
+        }
+    }
+
+    /**
+     * Remove all roles that have the {@link RoleType#Admin}.
+     */
+    protected void removeRootAdminRoles(List<? extends Role> roles) {
+        if (CollectionUtils.isEmpty(roles)) {
+            return;
+        }
+        Iterator<? extends Role> rolesIterator = roles.iterator();
+        while (rolesIterator.hasNext()) {
+            Role role = rolesIterator.next();
+            if (RoleType.Admin == role.getRoleType()) {
+                rolesIterator.remove();
+            }
+        }
+
+    }
+
     @Override
-    public List<Role> findRolesByType(final RoleType roleType) {
-        List<? extends Role> roles = null;
-        if (roleType != null) {
-            roles = roleDao.findAllByRoleType(roleType);
+    public List<Role> findRolesByType(RoleType roleType) {
+        if (roleType == null || RoleType.Admin == roleType && !accountManager.isRootAdmin(getCurrentAccount().getId())) {
+            return Collections.emptyList();
         }
+        List<? extends Role> roles = roleDao.findAllByRoleType(roleType);
         return ListUtils.toListOfInterface(roles);
     }
 
     @Override
     public List<Role> listRoles() {
         List<? extends Role> roles = roleDao.listAll();
+        removeRootAdminRolesIfNeeded(roles);
         return ListUtils.toListOfInterface(roles);
     }
 
@@ -253,7 +312,7 @@ public String getConfigComponentName() {
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[]{RoleService.EnableDynamicApiChecker};
+        return new ConfigKey<?>[] {RoleService.EnableDynamicApiChecker};
     }
 
     @Override
@@ -269,4 +328,4 @@ public String getConfigComponentName() {
         cmdList.add(DeleteRolePermissionCmd.class);
         return cmdList;
     }
- }
+}
diff --git a/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java
new file mode 100644
index 00000000000..bc50f340898
--- /dev/null
+++ b/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java
@@ -0,0 +1,288 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.acl;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.cloudstack.acl.dao.RoleDao;
+import org.apache.commons.collections.CollectionUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
+
+@RunWith(MockitoJUnitRunner.class)
+public class RoleManagerImplTest {
+
+    @Spy
+    @InjectMocks
+    private RoleManagerImpl roleManagerImpl;
+    @Mock
+    private AccountManager accountManagerMock;
+    @Mock
+    private RoleDao roleDaoMock;
+
+    @Mock
+    private Account accountMock;
+    private long accountMockId = 100l;
+
+    @Mock
+    private RoleVO roleVoMock;
+    private long roleMockId = 1l;
+
+    @Before
+    public void beforeTest() {
+        Mockito.doReturn(accountMockId).when(accountMock).getId();
+        Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount();
+
+        Mockito.doReturn(roleMockId).when(roleVoMock).getId();
+    }
+
+    @Test
+    public void findRoleTestIdNull() {
+        Role returnedRole = roleManagerImpl.findRole(null);
+        Assert.assertNull(returnedRole);
+    }
+
+    @Test
+    public void findRoleTestIdZero() {
+        Role returnedRole = roleManagerImpl.findRole(0l);
+        Assert.assertNull(returnedRole);
+    }
+
+    @Test
+    public void findRoleTestIdNegative() {
+        Role returnedRole = roleManagerImpl.findRole(-1l);
+        Assert.assertNull(returnedRole);
+    }
+
+    @Test
+    public void findRoleTestRoleNotFound() {
+        Mockito.doReturn(null).when(roleDaoMock).findById(roleMockId);
+        Role returnedRole = roleManagerImpl.findRole(roleMockId);
+        Assert.assertNull(returnedRole);
+    }
+
+    @Test
+    public void findRoleTestNotRootAdminAndNotRoleAdminType() {
+        Mockito.doReturn(RoleType.DomainAdmin).when(roleVoMock).getRoleType();
+        Mockito.doReturn(roleVoMock).when(roleDaoMock).findById(roleMockId);
+        Mockito.doReturn(false).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        Role returnedRole = roleManagerImpl.findRole(roleMockId);
+
+        Assert.assertEquals(roleMockId, returnedRole.getId());
+        Mockito.verify(accountManagerMock).isRootAdmin(accountMockId);
+        Mockito.verify(roleVoMock, Mockito.times(1)).getRoleType();
+    }
+
+    @Test
+    public void findRoleTestRootAdminAndNotRoleAdminType() {
+        Mockito.doReturn(RoleType.DomainAdmin).when(roleVoMock).getRoleType();
+        Mockito.doReturn(roleVoMock).when(roleDaoMock).findById(roleMockId);
+        Mockito.doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        Role returnedRole = roleManagerImpl.findRole(roleMockId);
+
+        Assert.assertEquals(roleMockId, returnedRole.getId());
+        Mockito.verify(accountManagerMock).isRootAdmin(accountMockId);
+        Mockito.verify(roleVoMock, Mockito.times(0)).getRoleType();
+    }
+
+    @Test
+    public void findRoleTestRootAdminAndRoleAdminType() {
+        Mockito.doReturn(RoleType.Admin).when(roleVoMock).getRoleType();
+        Mockito.doReturn(roleVoMock).when(roleDaoMock).findById(roleMockId);
+        Mockito.doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        Role returnedRole = roleManagerImpl.findRole(roleMockId);
+
+        Assert.assertEquals(roleMockId, returnedRole.getId());
+        Mockito.verify(accountManagerMock).isRootAdmin(accountMockId);
+        Mockito.verify(roleVoMock, Mockito.times(0)).getRoleType();
+    }
+
+    @Test
+    public void findRoleTestNotRootAdminAndRoleAdminType() {
+        Mockito.doReturn(RoleType.Admin).when(roleVoMock).getRoleType();
+        Mockito.doReturn(roleVoMock).when(roleDaoMock).findById(roleMockId);
+        Mockito.doReturn(false).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        Role returnedRole = roleManagerImpl.findRole(roleMockId);
+
+        Assert.assertNull(returnedRole);
+        Mockito.verify(accountManagerMock).isRootAdmin(accountMockId);
+        Mockito.verify(roleVoMock, Mockito.times(1)).getRoleType();
+    }
+
+    @Test
+    public void findRolesByNameTestNullRoleName() {
+        List<Role> rolesFound = roleManagerImpl.findRolesByName(null);
+
+        Assert.assertTrue(CollectionUtils.isEmpty(rolesFound));
+    }
+
+    @Test
+    public void findRolesByNameTestEmptyRoleName() {
+        List<Role> rolesFound = roleManagerImpl.findRolesByName("");
+
+        Assert.assertTrue(CollectionUtils.isEmpty(rolesFound));
+    }
+
+    @Test
+    public void findRolesByNameTestBlankRoleName() {
+        List<Role> rolesFound = roleManagerImpl.findRolesByName("      ");
+
+        Assert.assertTrue(CollectionUtils.isEmpty(rolesFound));
+    }
+
+    @Test
+    public void findRolesByNameTest() {
+        String roleName = "roleName";
+        ArrayList<Role> toBeReturned = new ArrayList<>();
+        Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByName(roleName);
+
+        roleManagerImpl.findRolesByName(roleName);
+
+        Mockito.verify(roleManagerImpl).removeRootAdminRolesIfNeeded(toBeReturned);
+    }
+
+    @Test
+    public void removeRootAdminRolesIfNeededTestRootAdmin() {
+        Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount();
+        Mockito.doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        List<Role> roles = new ArrayList<>();
+        roleManagerImpl.removeRootAdminRolesIfNeeded(roles);
+
+        Mockito.verify(roleManagerImpl, Mockito.times(0)).removeRootAdminRoles(roles);
+    }
+
+    @Test
+    public void removeRootAdminRolesIfNeededTestNonRootAdminUser() {
+        Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount();
+        Mockito.doReturn(false).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        List<Role> roles = new ArrayList<>();
+        roleManagerImpl.removeRootAdminRolesIfNeeded(roles);
+
+        Mockito.verify(roleManagerImpl, Mockito.times(1)).removeRootAdminRoles(roles);
+    }
+
+    @Test
+    public void removeRootAdminRolesTest() {
+        List<Role> roles = new ArrayList<>();
+        Role roleRootAdmin = Mockito.mock(Role.class);
+        Mockito.doReturn(RoleType.Admin).when(roleRootAdmin).getRoleType();
+
+        Role roleDomainAdmin = Mockito.mock(Role.class);
+        Mockito.doReturn(RoleType.DomainAdmin).when(roleDomainAdmin).getRoleType();
+
+        Role roleResourceAdmin = Mockito.mock(Role.class);
+        Mockito.doReturn(RoleType.ResourceAdmin).when(roleResourceAdmin).getRoleType();
+
+        Role roleUser = Mockito.mock(Role.class);
+        Mockito.doReturn(RoleType.User).when(roleUser).getRoleType();
+
+        roles.add(roleRootAdmin);
+        roles.add(roleDomainAdmin);
+        roles.add(roleResourceAdmin);
+        roles.add(roleUser);
+
+        roleManagerImpl.removeRootAdminRoles(roles);
+
+        Assert.assertEquals(3, roles.size());
+        Assert.assertEquals(roleDomainAdmin, roles.get(0));
+        Assert.assertEquals(roleResourceAdmin, roles.get(1));
+        Assert.assertEquals(roleUser, roles.get(2));
+    }
+
+    @Test
+    public void findRolesByTypeTestNullRoleType() {
+        List<Role> returnedRoles = roleManagerImpl.findRolesByType(null);
+
+        Assert.assertEquals(0, returnedRoles.size());
+        Mockito.verify(accountManagerMock, Mockito.times(0)).isRootAdmin(Mockito.anyLong());
+    }
+
+    @Test
+    public void findRolesByTypeTestAdminRoleNonRootAdminUser() {
+        Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount();
+        Mockito.doReturn(false).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        List<Role> returnedRoles = roleManagerImpl.findRolesByType(RoleType.Admin);
+
+        Assert.assertEquals(0, returnedRoles.size());
+        Mockito.verify(accountManagerMock, Mockito.times(1)).isRootAdmin(Mockito.anyLong());
+        Mockito.verify(roleDaoMock, Mockito.times(0)).findAllByRoleType(Mockito.any(RoleType.class));
+    }
+
+    @Test
+    public void findRolesByTypeTestAdminRoleRootAdminUser() {
+        Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount();
+        Mockito.doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        List<Role> roles = new ArrayList<>();
+        roles.add(Mockito.mock(Role.class));
+        Mockito.doReturn(roles).when(roleDaoMock).findAllByRoleType(RoleType.Admin);
+        List<Role> returnedRoles = roleManagerImpl.findRolesByType(RoleType.Admin);
+
+        Assert.assertEquals(1, returnedRoles.size());
+        Mockito.verify(accountManagerMock, Mockito.times(1)).isRootAdmin(Mockito.anyLong());
+        Mockito.verify(roleDaoMock, Mockito.times(1)).findAllByRoleType(Mockito.any(RoleType.class));
+    }
+
+    @Test
+    public void findRolesByTypeTestNonAdminRoleRootAdminUser() {
+        Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount();
+        Mockito.doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId);
+
+        List<Role> roles = new ArrayList<>();
+        roles.add(Mockito.mock(Role.class));
+        Mockito.doReturn(roles).when(roleDaoMock).findAllByRoleType(RoleType.User);
+        List<Role> returnedRoles = roleManagerImpl.findRolesByType(RoleType.User);
+
+        Assert.assertEquals(1, returnedRoles.size());
+        Mockito.verify(accountManagerMock, Mockito.times(0)).isRootAdmin(Mockito.anyLong());
+        Mockito.verify(roleDaoMock, Mockito.times(1)).findAllByRoleType(Mockito.any(RoleType.class));
+    }
+
+    @Test
+    public void listRolesTest() {
+        List<Role> roles = new ArrayList<>();
+        roles.add(Mockito.mock(Role.class));
+
+        Mockito.doReturn(roles).when(roleDaoMock).listAll();
+        Mockito.doNothing().when(roleManagerImpl).removeRootAdminRolesIfNeeded(roles);
+
+        List<Role> returnedRoles = roleManagerImpl.listRoles();
+
+        Assert.assertEquals(roles.size(), returnedRoles.size());
+        Mockito.verify(roleDaoMock).listAll();
+        Mockito.verify(roleManagerImpl).removeRootAdminRolesIfNeeded(roles);
+    }
+}


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services