etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

server: simplify role change validation (#9173) 

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Co-authored-by: dahn <daan.hoogland@gmail.com>
This commit is contained in:
Abhishek Kumar 2024-12-15 00:56:32 +05:30 committed by GitHub
parent a278849507
commit 8639ba8b01
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 145 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
server/src/main/java/com/cloud/user/AccountManagerImpl.java
View File

@ -120,8 +120,6 @@ import com.cloud.network.IpAddress;
import com.cloud.network.IpAddressManager;
import com.cloud.network.Network;
import com.cloud.network.NetworkModel;
import com.cloud.network.security.SecurityGroupService;
import com.cloud.network.security.SecurityGroupVO;
import com.cloud.network.VpnUserVO;
import com.cloud.network.as.AutoScaleManager;
import com.cloud.network.dao.AccountGuestVlanMapDao;
@ -135,6 +133,8 @@ import com.cloud.network.dao.RemoteAccessVpnVO;
import com.cloud.network.dao.VpnUserDao;
import com.cloud.network.router.VirtualRouter;
import com.cloud.network.security.SecurityGroupManager;
import com.cloud.network.security.SecurityGroupService;
import com.cloud.network.security.SecurityGroupVO;
import com.cloud.network.security.dao.SecurityGroupDao;
import com.cloud.network.vpc.Vpc;
import com.cloud.network.vpc.VpcManager;
@ -1301,16 +1301,33 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
return _userAccountDao.findById(userId);
}
private boolean isValidRoleChange(Account account, Role role) {
Long currentAccRoleId = account.getRoleId();
Role currentRole = roleService.findRole(currentAccRoleId);
if (role.getRoleType().ordinal() < currentRole.getRoleType().ordinal() && ((account.getType() == Account.Type.NORMAL && role.getRoleType().getAccountType().ordinal() > Account.Type.NORMAL.ordinal()) ||
account.getType().ordinal() > Account.Type.NORMAL.ordinal() && role.getRoleType().getAccountType().ordinal() < account.getType().ordinal() && role.getRoleType().getAccountType().ordinal() > 0)) {
throw new PermissionDeniedException(String.format("Unable to update account role to %s as you are " +
"attempting to escalate the account %s to account type %s which has higher privileges", role.getName(), account.getAccountName(), role.getRoleType().getAccountType().name()));
/*
Role change should follow the below conditions:
- Caller should not be of Unknown role type
- New role's type should not be Unknown
- Caller should not be able to escalate or de-escalate an account's role which is of higher role type
- New role should not be of type Admin with domain other than ROOT domain
*/
protected void validateRoleChange(Account account, Role role, Account caller) {
Role currentRole = roleService.findRole(account.getRoleId());
Role callerRole = roleService.findRole(caller.getRoleId());
String errorMsg = String.format("Unable to update account role to %s, ", role.getName());
if (RoleType.Unknown.equals(callerRole.getRoleType())) {
throw new PermissionDeniedException(String.format("%s as the caller privileges are unknown", errorMsg));
}
if (RoleType.Unknown.equals(role.getRoleType())) {
throw new PermissionDeniedException(String.format("%s as the new role privileges are unknown", errorMsg));
}
if (!callerRole.getRoleType().equals(RoleType.Admin) &&
(role.getRoleType().ordinal() < callerRole.getRoleType().ordinal() ||
currentRole.getRoleType().ordinal() < callerRole.getRoleType().ordinal())) {
throw new PermissionDeniedException(String.format("%s as either current or new role has higher " +
"privileges than the caller", errorMsg));
}
if (role.getRoleType().equals(RoleType.Admin) && account.getDomainId() != Domain.ROOT_DOMAIN) {
throw new PermissionDeniedException(String.format("%s as the user does not belong to the ROOT domain",
errorMsg));
}
return true;
}
/**
@ -2053,7 +2070,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
}
Role role = roleService.findRole(roleId);
isValidRoleChange(account, role);
validateRoleChange(account, role, caller);
acctForUpdate.setRoleId(roleId);
acctForUpdate.setType(role.getRoleType().getAccountType());
checkRoleEscalation(getCurrentCallingAccount(), acctForUpdate);

118
server/src/test/java/com/cloud/user/AccountManagerImplTest.java
View File

@ -16,15 +16,20 @@
// under the License.
package com.cloud.user;
import static org.mockito.ArgumentMatchers.nullable;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.HashMap;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.Role;
import org.apache.cloudstack.acl.RoleService;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd;
@ -42,7 +47,6 @@ import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
import static org.mockito.ArgumentMatchers.nullable;
import com.cloud.acl.DomainChecker;
import com.cloud.api.auth.SetupUserTwoFactorAuthenticationCmd;
@ -102,6 +106,8 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
@Mock
ConfigKey<Boolean> enableUserTwoFactorAuthenticationMock;
@Mock
RoleService roleService;
@Before
public void setUp() throws Exception {
@ -1086,4 +1092,112 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
Mockito.verify(_projectAccountDao).removeUserFromProjects(userId);
}
@Test(expected = PermissionDeniedException.class)
public void testValidateRoleChangeUnknownCaller() {
Account account = Mockito.mock(Account.class);
Mockito.when(account.getRoleId()).thenReturn(1L);
Role role = Mockito.mock(Role.class);
Mockito.when(role.getRoleType()).thenReturn(RoleType.Unknown);
Account caller = Mockito.mock(Account.class);
Mockito.when(caller.getRoleId()).thenReturn(2L);
Mockito.when(roleService.findRole(2L)).thenReturn(role);
accountManagerImpl.validateRoleChange(account, Mockito.mock(Role.class), caller);
}
@Test(expected = PermissionDeniedException.class)
public void testValidateRoleChangeUnknownNewRole() {
Account account = Mockito.mock(Account.class);
Mockito.when(account.getRoleId()).thenReturn(1L);
Role newRole = Mockito.mock(Role.class);
Mockito.when(newRole.getRoleType()).thenReturn(RoleType.Unknown);
Role callerRole = Mockito.mock(Role.class);
Mockito.when(callerRole.getRoleType()).thenReturn(RoleType.DomainAdmin);
Account caller = Mockito.mock(Account.class);
Mockito.when(caller.getRoleId()).thenReturn(2L);
Mockito.when(roleService.findRole(2L)).thenReturn(callerRole);
accountManagerImpl.validateRoleChange(account, newRole, caller);
}
@Test
public void testValidateRoleNewRoleSameCaller() {
Account account = Mockito.mock(Account.class);
Mockito.when(account.getRoleId()).thenReturn(1L);
Role currentRole = Mockito.mock(Role.class);
Mockito.when(currentRole.getRoleType()).thenReturn(RoleType.User);
Mockito.when(roleService.findRole(1L)).thenReturn(currentRole);
Role newRole = Mockito.mock(Role.class);
Mockito.when(newRole.getRoleType()).thenReturn(RoleType.DomainAdmin);
Role callerRole = Mockito.mock(Role.class);
Mockito.when(callerRole.getRoleType()).thenReturn(RoleType.DomainAdmin);
Account caller = Mockito.mock(Account.class);
Mockito.when(caller.getRoleId()).thenReturn(2L);
Mockito.when(roleService.findRole(2L)).thenReturn(callerRole);
accountManagerImpl.validateRoleChange(account, newRole, caller);
}
@Test
public void testValidateRoleCurrentRoleSameCaller() {
Account account = Mockito.mock(Account.class);
Mockito.when(account.getRoleId()).thenReturn(1L);
Role accountRole = Mockito.mock(Role.class);
Mockito.when(accountRole.getRoleType()).thenReturn(RoleType.DomainAdmin);
Role newRole = Mockito.mock(Role.class);
Mockito.when(newRole.getRoleType()).thenReturn(RoleType.User);
Role callerRole = Mockito.mock(Role.class);
Mockito.when(callerRole.getRoleType()).thenReturn(RoleType.DomainAdmin);
Account caller = Mockito.mock(Account.class);
Mockito.when(caller.getRoleId()).thenReturn(2L);
Mockito.when(roleService.findRole(1L)).thenReturn(accountRole);
Mockito.when(roleService.findRole(2L)).thenReturn(callerRole);
accountManagerImpl.validateRoleChange(account, newRole, caller);
}
@Test(expected = PermissionDeniedException.class)
public void testValidateRoleNewRoleHigherCaller() {
Account account = Mockito.mock(Account.class);
Mockito.when(account.getRoleId()).thenReturn(1L);
Role newRole = Mockito.mock(Role.class);
Mockito.when(newRole.getRoleType()).thenReturn(RoleType.Admin);
Role callerRole = Mockito.mock(Role.class);
Mockito.when(callerRole.getRoleType()).thenReturn(RoleType.DomainAdmin);
Account caller = Mockito.mock(Account.class);
Mockito.when(caller.getRoleId()).thenReturn(2L);
Mockito.when(roleService.findRole(2L)).thenReturn(callerRole);
accountManagerImpl.validateRoleChange(account, newRole, caller);
}
@Test
public void testValidateRoleNewRoleLowerCaller() {
Account account = Mockito.mock(Account.class);
Mockito.when(account.getRoleId()).thenReturn(1L);
Role newRole = Mockito.mock(Role.class);
Mockito.when(newRole.getRoleType()).thenReturn(RoleType.User);
Role accountRole = Mockito.mock(Role.class);
Mockito.when(accountRole.getRoleType()).thenReturn(RoleType.User);
Role callerRole = Mockito.mock(Role.class);
Mockito.when(callerRole.getRoleType()).thenReturn(RoleType.DomainAdmin);
Account caller = Mockito.mock(Account.class);
Mockito.when(caller.getRoleId()).thenReturn(2L);
Mockito.when(roleService.findRole(1L)).thenReturn(accountRole);
Mockito.when(roleService.findRole(2L)).thenReturn(callerRole);
accountManagerImpl.validateRoleChange(account, newRole, caller);
}
@Test(expected = PermissionDeniedException.class)
public void testValidateRoleAdminCannotEscalateAdminFromNonRootDomain() {
Account account = Mockito.mock(Account.class);
Mockito.when(account.getRoleId()).thenReturn(1L);
Mockito.when(account.getDomainId()).thenReturn(2L);
Role newRole = Mockito.mock(Role.class);
Mockito.when(newRole.getRoleType()).thenReturn(RoleType.Admin);
Role accountRole = Mockito.mock(Role.class);
Role callerRole = Mockito.mock(Role.class);
Mockito.when(callerRole.getRoleType()).thenReturn(RoleType.Admin);
Account caller = Mockito.mock(Account.class);
Mockito.when(caller.getRoleId()).thenReturn(2L);
Mockito.when(roleService.findRole(1L)).thenReturn(accountRole);
Mockito.when(roleService.findRole(2L)).thenReturn(callerRole);
accountManagerImpl.validateRoleChange(account, newRole, caller);
}
}