From c6b4467d7891bad4f7c27d1e22f0beccacb08bd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Wed, 2 May 2018 04:31:29 -0300 Subject: [PATCH] =?UTF-8?q?When=20creating=20a=20new=20account=20(via=20do?= =?UTF-8?q?main=20admin)=20it=20is=20possible=20to=20select=20=E2=80=9Croo?= =?UTF-8?q?t=20admin=E2=80=9D=20as=20the=20role=20for=20the=20new=20user?= =?UTF-8?q?=20(#2606)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * create account with domain admin showing 'root admin' role Domain admins should not be able to assign the role of root admin to new users. Therefore, the role ‘root admin’ (or any other of the same type) should not be visible to domain admins. * License and formatting * Break long sentence into multiple lines * Fix wording of method 'getCurrentAccount' * fix typo in variable name --- .../apache/cloudstack/acl/RoleService.java | 60 ++-- .../api/command/admin/acl/ListRolesCmd.java | 32 +- .../cloudstack/acl/RoleManagerImpl.java | 85 +++++- .../cloudstack/acl/RoleManagerImplTest.java | 288 ++++++++++++++++++ 4 files changed, 416 insertions(+), 49 deletions(-) create mode 100644 server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java 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 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 newOrder); - boolean updateRolePermission(final Role role, final RolePermission rolePermission, final Permission permission); - boolean deleteRolePermission(final RolePermission rolePermission); + boolean updateRolePermission(Role role, List 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 listRoles(); - List findRolesByName(final String name); - List findRolesByType(final RoleType roleType); - List 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 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 findRolesByType(RoleType roleType); + + List 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 @@ public class ListRolesCmd extends BaseCmd { } @Override - public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { - final List roles; + public void execute() { + List 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 @@ package org.apache.cloudstack.acl; 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.api.command.admin.acl.UpdateRolePermissionCmd; 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.cloud.utils.db.TransactionStatus; 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 class RoleManagerImpl extends ManagerBase implements RoleService, Configu } @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; } - return roleDao.findById(id); + RoleVO role = roleDao.findById(id); + if (role == null) { + logger.trace(String.format("Role not found [id=%s]", id)); + return null; + } + 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 class RoleManagerImpl extends ManagerBase implements RoleService, Configu 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 class RoleManagerImpl extends ManagerBase implements RoleService, Configu } @Override - public List findRolesByName(final String name) { + public List findRolesByName(String name) { List roles = null; - if (!Strings.isNullOrEmpty(name)) { + if (StringUtils.isNotBlank(name)) { roles = roleDao.findAllByName(name); } + removeRootAdminRolesIfNeeded(roles); return ListUtils.toListOfInterface(roles); } - @Override - public List findRolesByType(final RoleType roleType) { - List roles = null; - if (roleType != null) { - roles = roleDao.findAllByRoleType(roleType); + /** + * 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 roles) { + Account account = getCurrentAccount(); + if (!accountManager.isRootAdmin(account.getId())) { + removeRootAdminRoles(roles); } + } + + /** + * Remove all roles that have the {@link RoleType#Admin}. + */ + protected void removeRootAdminRoles(List roles) { + if (CollectionUtils.isEmpty(roles)) { + return; + } + Iterator rolesIterator = roles.iterator(); + while (rolesIterator.hasNext()) { + Role role = rolesIterator.next(); + if (RoleType.Admin == role.getRoleType()) { + rolesIterator.remove(); + } + } + + } + + @Override + public List findRolesByType(RoleType roleType) { + if (roleType == null || RoleType.Admin == roleType && !accountManager.isRootAdmin(getCurrentAccount().getId())) { + return Collections.emptyList(); + } + List roles = roleDao.findAllByRoleType(roleType); return ListUtils.toListOfInterface(roles); } @Override public List listRoles() { List roles = roleDao.listAll(); + removeRootAdminRolesIfNeeded(roles); return ListUtils.toListOfInterface(roles); } @@ -253,7 +312,7 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{RoleService.EnableDynamicApiChecker}; + return new ConfigKey[] {RoleService.EnableDynamicApiChecker}; } @Override @@ -269,4 +328,4 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu 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 rolesFound = roleManagerImpl.findRolesByName(null); + + Assert.assertTrue(CollectionUtils.isEmpty(rolesFound)); + } + + @Test + public void findRolesByNameTestEmptyRoleName() { + List rolesFound = roleManagerImpl.findRolesByName(""); + + Assert.assertTrue(CollectionUtils.isEmpty(rolesFound)); + } + + @Test + public void findRolesByNameTestBlankRoleName() { + List rolesFound = roleManagerImpl.findRolesByName(" "); + + Assert.assertTrue(CollectionUtils.isEmpty(rolesFound)); + } + + @Test + public void findRolesByNameTest() { + String roleName = "roleName"; + ArrayList 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 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 roles = new ArrayList<>(); + roleManagerImpl.removeRootAdminRolesIfNeeded(roles); + + Mockito.verify(roleManagerImpl, Mockito.times(1)).removeRootAdminRoles(roles); + } + + @Test + public void removeRootAdminRolesTest() { + List 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 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 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 roles = new ArrayList<>(); + roles.add(Mockito.mock(Role.class)); + Mockito.doReturn(roles).when(roleDaoMock).findAllByRoleType(RoleType.Admin); + List 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 roles = new ArrayList<>(); + roles.add(Mockito.mock(Role.class)); + Mockito.doReturn(roles).when(roleDaoMock).findAllByRoleType(RoleType.User); + List 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 roles = new ArrayList<>(); + roles.add(Mockito.mock(Role.class)); + + Mockito.doReturn(roles).when(roleDaoMock).listAll(); + Mockito.doNothing().when(roleManagerImpl).removeRootAdminRolesIfNeeded(roles); + + List returnedRoles = roleManagerImpl.listRoles(); + + Assert.assertEquals(roles.size(), returnedRoles.size()); + Mockito.verify(roleDaoMock).listAll(); + Mockito.verify(roleManagerImpl).removeRootAdminRolesIfNeeded(roles); + } +}