Adding privilege checks on user and account operations

Co-authored-by: Harikrishna <harikrishna.patnala@gmail.com>
This commit is contained in:
nvazquez 2025-05-11 22:48:41 -03:00 committed by Daan Hoogland
parent e2f187912c
commit a0080a04fe
10 changed files with 321 additions and 10 deletions

View File

@ -71,6 +71,7 @@ public interface Account extends ControlledEntity, InternalIdentity, Identity {
} }
public static final long ACCOUNT_ID_SYSTEM = 1; public static final long ACCOUNT_ID_SYSTEM = 1;
public static final long ACCOUNT_ID_ADMIN = 2;
public String getAccountName(); public String getAccountName();

View File

@ -521,4 +521,8 @@ public class MockAccountManager extends ManagerBase implements AccountManager {
public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) { public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
return null; return null;
} }
@Override
public void verifyCallerPrivilegeForUserOrAccountOperations(Account userAccount) {
}
} }

View File

@ -47,11 +47,13 @@ import javax.naming.ConfigurationException;
import com.cloud.hypervisor.HypervisorGuru; import com.cloud.hypervisor.HypervisorGuru;
import com.cloud.user.AccountManagerImpl;
import com.cloud.utils.crypt.DBEncryptionUtil; import com.cloud.utils.crypt.DBEncryptionUtil;
import com.cloud.host.HostTagVO; import com.cloud.host.HostTagVO;
import com.cloud.storage.StoragePoolTagVO; import com.cloud.storage.StoragePoolTagVO;
import com.cloud.storage.VolumeApiServiceImpl; import com.cloud.storage.VolumeApiServiceImpl;
import com.googlecode.ipv6.IPv6Address; import com.googlecode.ipv6.IPv6Address;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.AffinityGroup;
import org.apache.cloudstack.affinity.AffinityGroupService; import org.apache.cloudstack.affinity.AffinityGroupService;
@ -470,6 +472,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
private long _defaultPageSize = Long.parseLong(Config.DefaultPageSize.getDefaultValue()); private long _defaultPageSize = Long.parseLong(Config.DefaultPageSize.getDefaultValue());
private static final String DOMAIN_NAME_PATTERN = "^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\\.)+[A-Za-z]{1,63}$"; private static final String DOMAIN_NAME_PATTERN = "^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\\.)+[A-Za-z]{1,63}$";
private Set<String> configValuesForValidation = new HashSet<String>(); private Set<String> configValuesForValidation = new HashSet<String>();
private Set<String> configKeysAllowedOnlyForDefaultAdmin = new HashSet<String>();
private Set<String> weightBasedParametersForValidation = new HashSet<String>(); private Set<String> weightBasedParametersForValidation = new HashSet<String>();
private Set<String> overprovisioningFactorsForValidation = new HashSet<String>(); private Set<String> overprovisioningFactorsForValidation = new HashSet<String>();
@ -533,6 +536,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
populateConfigValuesForValidationSet(); populateConfigValuesForValidationSet();
weightBasedParametersForValidation(); weightBasedParametersForValidation();
overProvisioningFactorsForValidation(); overProvisioningFactorsForValidation();
populateConfigKeysAllowedOnlyForDefaultAdmin();
initMessageBusListener(); initMessageBusListener();
return true; return true;
} }
@ -596,6 +600,11 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
overprovisioningFactorsForValidation.add(CapacityManager.StorageOverprovisioningFactor.key()); overprovisioningFactorsForValidation.add(CapacityManager.StorageOverprovisioningFactor.key());
} }
protected void populateConfigKeysAllowedOnlyForDefaultAdmin() {
configKeysAllowedOnlyForDefaultAdmin.add(AccountManagerImpl.listOfRoleTypesAllowedForOperationsOfSameRoleType.key());
configKeysAllowedOnlyForDefaultAdmin.add(AccountManagerImpl.allowOperationsOnUsersInSameAccount.key());
}
private void initMessageBusListener() { private void initMessageBusListener() {
messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, new MessageSubscriber() { messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, new MessageSubscriber() {
@Override @Override
@ -1183,6 +1192,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
s_logger.error("Missing configuration variable " + name + " in configuration table"); s_logger.error("Missing configuration variable " + name + " in configuration table");
return "Invalid configuration variable."; return "Invalid configuration variable.";
} }
validateConfigurationAllowedOnlyForDefaultAdmin(name, value);
final String configScope = cfg.getScope(); final String configScope = cfg.getScope();
if (scope != null) { if (scope != null) {
@ -1347,6 +1357,33 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
return String.format("Invalid value for configuration [%s].", name); return String.format("Invalid value for configuration [%s].", name);
} }
protected void validateConfigurationAllowedOnlyForDefaultAdmin(String configName, String value) {
if (configKeysAllowedOnlyForDefaultAdmin.contains(configName)) {
final Long userId = CallContext.current().getCallingUserId();
if (userId != User.UID_ADMIN) {
throw new CloudRuntimeException("Only default admin is allowed to change this setting");
}
if (AccountManagerImpl.listOfRoleTypesAllowedForOperationsOfSameRoleType.key().equals(configName)) {
if (value != null && !value.isBlank()) {
List<String> validRoleTypes = Arrays.stream(RoleType.values())
.map(Enum::name)
.collect(Collectors.toList());
boolean allValid = Arrays.stream(value.split(","))
.map(String::trim)
.allMatch(validRoleTypes::contains);
if (!allValid) {
throw new CloudRuntimeException("Invalid role types provided in value");
}
} else {
throw new CloudRuntimeException("Value for role types must not be empty");
}
}
}
}
/** /**
* A valid value should be an integer between min and max (the values from the range). * A valid value should be an integer between min and max (the values from the range).
*/ */

View File

@ -784,6 +784,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim
} else { } else {
_accountMgr.checkAccess(caller, null, true, account); _accountMgr.checkAccess(caller, null, true, account);
} }
_accountMgr.verifyCallerPrivilegeForUserOrAccountOperations(account);
ownerType = ResourceOwnerType.Account; ownerType = ResourceOwnerType.Account;
ownerId = accountId; ownerId = accountId;
@ -853,6 +854,11 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim
throw new InvalidParameterValueException("Please specify a valid domain ID."); throw new InvalidParameterValueException("Please specify a valid domain ID.");
} }
_accountMgr.checkAccess(callerAccount, domain); _accountMgr.checkAccess(callerAccount, domain);
Account account = _entityMgr.findById(Account.class, accountId);
if (account == null) {
throw new InvalidParameterValueException("Unable to find account " + accountId);
}
_accountMgr.verifyCallerPrivilegeForUserOrAccountOperations(account);
if (resourceType != null) { if (resourceType != null) {
resourceTypes.add(resourceType); resourceTypes.add(resourceType);

View File

@ -200,4 +200,5 @@ public interface AccountManager extends AccountService, Configurable {
UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user); UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user);
void verifyCallerPrivilegeForUserOrAccountOperations(Account userAccount);
} }

View File

@ -20,6 +20,7 @@ import java.net.InetAddress;
import java.net.URLEncoder; import java.net.URLEncoder;
import java.security.NoSuchAlgorithmException; import java.security.NoSuchAlgorithmException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
import java.util.HashMap; import java.util.HashMap;
@ -386,6 +387,22 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
true, true,
ConfigKey.Scope.Domain); ConfigKey.Scope.Domain);
public static ConfigKey<String> listOfRoleTypesAllowedForOperationsOfSameRoleType = new ConfigKey<>("Hidden",
String.class,
"role.types.allowed.for.operations.on.accounts.of.same.role.type",
"Admin, ResourceAdmin, DomainAdmin",
"Comma separated list of role types that are allowed to do operations on accounts or users of the same role type within a domain.",
true,
ConfigKey.Scope.Domain);
public static ConfigKey<Boolean> allowOperationsOnUsersInSameAccount = new ConfigKey<>("Hidden",
Boolean.class,
"allow.operations.on.users.in.same.account",
"true",
"Allow operations on users among them in the same account",
true,
ConfigKey.Scope.Domain);
protected AccountManagerImpl() { protected AccountManagerImpl() {
super(); super();
} }
@ -1369,7 +1386,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
/** /**
* if there is any permission under the requested role that is not permitted for the caller, refuse * if there is any permission under the requested role that is not permitted for the caller, refuse
*/ */
private void checkRoleEscalation(Account caller, Account requested) { protected void checkRoleEscalation(Account caller, Account requested) {
if (s_logger.isDebugEnabled()) { if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Checking if user of account %s [%s] with role-id [%d] can create an account of type %s [%s] with role-id [%d]", s_logger.debug(String.format("Checking if user of account %s [%s] with role-id [%d] can create an account of type %s [%s] with role-id [%d]",
caller.getAccountName(), caller.getAccountName(),
@ -1471,7 +1488,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
// users can't exist in same account // users can't exist in same account
assertUserNotAlreadyInAccount(duplicatedUser, account); assertUserNotAlreadyInAccount(duplicatedUser, account);
} }
verifyCallerPrivilegeForUserOrAccountOperations(account);
UserVO user = null; UserVO user = null;
user = createUser(account.getId(), userName, password, firstName, lastName, email, timeZone, userUUID, source); user = createUser(account.getId(), userName, password, firstName, lastName, email, timeZone, userUUID, source);
return user; return user;
@ -1488,10 +1505,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User") @ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User")
public UserAccount updateUser(UpdateUserCmd updateUserCmd) { public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
UserVO user = retrieveAndValidateUser(updateUserCmd); UserVO user = retrieveAndValidateUser(updateUserCmd);
Account account = retrieveAndValidateAccount(user);
User caller = CallContext.current().getCallingUser();
checkAccess(caller, account);
verifyCallerPrivilegeForUserOrAccountOperations(user);
s_logger.debug("Updating user with Id: " + user.getUuid()); s_logger.debug("Updating user with Id: " + user.getUuid());
validateAndUpdateApiAndSecretKeyIfNeeded(updateUserCmd, user); validateAndUpdateApiAndSecretKeyIfNeeded(updateUserCmd, user);
Account account = retrieveAndValidateAccount(user);
validateAndUpdateFirstNameIfNeeded(updateUserCmd, user); validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
validateAndUpdateLastNameIfNeeded(updateUserCmd, user); validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
@ -1514,6 +1535,85 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
return _userAccountDao.findById(user.getId()); return _userAccountDao.findById(user.getId());
} }
@Override
public void verifyCallerPrivilegeForUserOrAccountOperations(Account userAccount) {
s_logger.debug(String.format("Verifying whether the caller has the correct privileges based on the user's role type and API permissions: %s", userAccount));
checkCallerRoleTypeAllowedForUserOrAccountOperations(userAccount, null);
checkCallerApiPermissionsForUserOrAccountOperations(userAccount);
}
protected void verifyCallerPrivilegeForUserOrAccountOperations(User user) {
s_logger.debug(String.format("Verifying whether the caller has the correct privileges based on the user's role type and API permissions: %s", user));
Account userAccount = getAccount(user.getAccountId());
checkCallerRoleTypeAllowedForUserOrAccountOperations(userAccount, user);
checkCallerApiPermissionsForUserOrAccountOperations(userAccount);
}
protected void checkCallerRoleTypeAllowedForUserOrAccountOperations(Account userAccount, User user) {
Account callingAccount = getCurrentCallingAccount();
RoleType callerRoleType = getRoleType(callingAccount);
RoleType userAccountRoleType = getRoleType(userAccount);
if (RoleType.Unknown == callerRoleType || RoleType.Unknown == userAccountRoleType) {
String errMsg = String.format("The role type of account [%s, %s] or [%s, %s] is unknown",
callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid());
throw new PermissionDeniedException(errMsg);
}
boolean isCallerSystemOrDefaultAdmin = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || callingAccount.getId() == Account.ACCOUNT_ID_ADMIN;
if (isCallerSystemOrDefaultAdmin) {
s_logger.trace(String.format("Admin account [%s, %s] performing this operation for user account [%s, %s] ", callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid()));
} else if (callerRoleType.getId() < userAccountRoleType.getId()) {
s_logger.trace(String.format("The calling account [%s] has a higher role type than the user account [%s]",
callingAccount, userAccount));
} else if (callerRoleType.getId() == userAccountRoleType.getId()) {
if (callingAccount.getId() != userAccount.getId()) {
String allowedRoleTypes = listOfRoleTypesAllowedForOperationsOfSameRoleType.valueInDomain(callingAccount.getDomainId());
boolean updateAllowed = allowedRoleTypes != null &&
Arrays.stream(allowedRoleTypes.split(","))
.map(String::trim)
.anyMatch(role -> role.equals(callerRoleType.toString()));
if (BooleanUtils.isFalse(updateAllowed)) {
String errMsg = String.format("The calling account [%s, %s] is not allowed to perform this operation on users from other accounts " +
"of the same role type within the domain", callingAccount.getName(), callingAccount.getUuid());
s_logger.error(errMsg);
throw new PermissionDeniedException(errMsg);
}
} else if ((callingAccount.getId() == userAccount.getId()) && user != null) {
Boolean allowOperationOnUsersinSameAccount = allowOperationsOnUsersInSameAccount.valueInDomain(callingAccount.getDomainId());
User callingUser = CallContext.current().getCallingUser();
if (callingUser.getId() != user.getId() && BooleanUtils.isFalse(allowOperationOnUsersinSameAccount)) {
String errMsg = "The user operations are not allowed by the users in the same account";
s_logger.error(errMsg);
throw new PermissionDeniedException(errMsg);
}
}
} else {
String errMsg = String.format("The calling account [%s, %s] has a lower role type than the user account [%s, %s]",
callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid());
throw new PermissionDeniedException(errMsg);
}
}
protected void checkCallerApiPermissionsForUserOrAccountOperations(Account userAccount) {
Account callingAccount = getCurrentCallingAccount();
boolean isCallerRootAdmin = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(callingAccount.getId());
if (isCallerRootAdmin) {
s_logger.trace(String.format("Admin account [%s, %s] performing this operation for user account [%s, %s] ", callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid()));
} else if (isRootAdmin(userAccount.getAccountId())) {
String errMsg = String.format("Account [%s, %s] cannot perform this operation for user account [%s, %s] ", callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid());
s_logger.error(errMsg);
throw new PermissionDeniedException(errMsg);
} else {
s_logger.debug(String.format("Checking calling account [%s, %s] permission to perform this operation for user account [%s, %s] ", callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid()));
checkRoleEscalation(callingAccount, userAccount);
s_logger.debug(String.format("Calling account [%s, %s] is allowed to perform this operation for user account [%s, %s] ", callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid()));
}
}
/** /**
* Updates the password in the user POJO if needed. If no password is provided, then the password is not updated. * Updates the password in the user POJO if needed. If no password is provided, then the password is not updated.
* The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the current password. * The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the current password.
@ -1760,6 +1860,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
checkAccess(caller, AccessType.OperateEntry, true, account); checkAccess(caller, AccessType.OperateEntry, true, account);
verifyCallerPrivilegeForUserOrAccountOperations(user);
boolean success = doSetUserStatus(userId, State.DISABLED); boolean success = doSetUserStatus(userId, State.DISABLED);
if (success) { if (success) {
@ -1801,6 +1902,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
checkAccess(caller, AccessType.OperateEntry, true, account); checkAccess(caller, AccessType.OperateEntry, true, account);
verifyCallerPrivilegeForUserOrAccountOperations(user);
boolean success = Transaction.execute(new TransactionCallback<Boolean>() { boolean success = Transaction.execute(new TransactionCallback<Boolean>() {
@Override @Override
@ -1853,6 +1955,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
checkAccess(caller, AccessType.OperateEntry, true, account); checkAccess(caller, AccessType.OperateEntry, true, account);
verifyCallerPrivilegeForUserOrAccountOperations(user);
// make sure the account is enabled too // make sure the account is enabled too
// if the user is either locked already or disabled already, don't change state...only lock currently enabled // if the user is either locked already or disabled already, don't change state...only lock currently enabled
@ -1909,6 +2012,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
checkIfAccountManagesProjects(accountId); checkIfAccountManagesProjects(accountId);
verifyCallerPrivilegeForUserOrAccountOperations(account);
CallContext.current().putContextParameter(Account.class, account.getUuid()); CallContext.current().putContextParameter(Account.class, account.getUuid());
@ -1971,6 +2075,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
// Check if user performing the action is allowed to modify this account // Check if user performing the action is allowed to modify this account
Account caller = getCurrentCallingAccount(); Account caller = getCurrentCallingAccount();
checkAccess(caller, AccessType.OperateEntry, true, account); checkAccess(caller, AccessType.OperateEntry, true, account);
verifyCallerPrivilegeForUserOrAccountOperations(account);
boolean success = enableAccount(account.getId()); boolean success = enableAccount(account.getId());
if (success) { if (success) {
@ -2004,6 +2109,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
checkAccess(caller, AccessType.OperateEntry, true, account); checkAccess(caller, AccessType.OperateEntry, true, account);
verifyCallerPrivilegeForUserOrAccountOperations(account);
if (lockAccount(account.getId())) { if (lockAccount(account.getId())) {
CallContext.current().putContextParameter(Account.class, account.getUuid()); CallContext.current().putContextParameter(Account.class, account.getUuid());
@ -2034,6 +2140,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
checkAccess(caller, AccessType.OperateEntry, true, account); checkAccess(caller, AccessType.OperateEntry, true, account);
verifyCallerPrivilegeForUserOrAccountOperations(account);
if (disableAccount(account.getId())) { if (disableAccount(account.getId())) {
CallContext.current().putContextParameter(Account.class, account.getUuid()); CallContext.current().putContextParameter(Account.class, account.getUuid());
@ -2079,6 +2186,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
// Check if user performing the action is allowed to modify this account // Check if user performing the action is allowed to modify this account
Account caller = getCurrentCallingAccount(); Account caller = getCurrentCallingAccount();
checkAccess(caller, _domainMgr.getDomain(account.getDomainId())); checkAccess(caller, _domainMgr.getDomain(account.getDomainId()));
verifyCallerPrivilegeForUserOrAccountOperations(account);
if(newAccountName != null) { if(newAccountName != null) {
@ -2160,6 +2268,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
// don't allow to delete the user from the account of type Project // don't allow to delete the user from the account of type Project
checkAccountAndAccess(user, account); checkAccountAndAccess(user, account);
verifyCallerPrivilegeForUserOrAccountOperations(user);
return Transaction.execute((TransactionCallback<Boolean>) status -> deleteAndCleanupUser(user)); return Transaction.execute((TransactionCallback<Boolean>) status -> deleteAndCleanupUser(user));
} }
@ -2178,6 +2288,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
UserVO user = getValidUserVO(id); UserVO user = getValidUserVO(id);
Account oldAccount = _accountDao.findById(user.getAccountId()); Account oldAccount = _accountDao.findById(user.getAccountId());
checkAccountAndAccess(user, oldAccount); checkAccountAndAccess(user, oldAccount);
verifyCallerPrivilegeForUserOrAccountOperations(user);
long domainId = oldAccount.getDomainId(); long domainId = oldAccount.getDomainId();
long newAccountId = getNewAccountId(domainId, cmd.getAccountName(), cmd.getAccountId()); long newAccountId = getNewAccountId(domainId, cmd.getAccountName(), cmd.getAccountId());
@ -2515,6 +2626,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
} }
if (!Account.Type.PROJECT.equals(accountType)) {
AccountVO newAccount = new AccountVO(accountName, domainId, networkDomain, accountType, roleId, uuid);
verifyCallerPrivilegeForUserOrAccountOperations(newAccount);
}
// Create the account // Create the account
return Transaction.execute(new TransactionCallback<AccountVO>() { return Transaction.execute(new TransactionCallback<AccountVO>() {
@Override @Override
@ -2863,8 +2979,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
final ControlledEntity account = getAccount(getUserAccountById(userId).getAccountId()); //Extracting the Account from the userID of the requested user. final ControlledEntity account = getAccount(getUserAccountById(userId).getAccountId()); //Extracting the Account from the userID of the requested user.
User caller = CallContext.current().getCallingUser(); User caller = CallContext.current().getCallingUser();
preventRootDomainAdminAccessToRootAdminKeys(caller, account);
checkAccess(caller, account); checkAccess(caller, account);
verifyCallerPrivilegeForUserOrAccountOperations(user);
Map<String, String> keys = new HashMap<String, String>(); Map<String, String> keys = new HashMap<String, String>();
keys.put("apikey", user.getApiKey()); keys.put("apikey", user.getApiKey());
@ -2920,8 +3036,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
} }
Account account = _accountDao.findById(user.getAccountId()); Account account = _accountDao.findById(user.getAccountId());
preventRootDomainAdminAccessToRootAdminKeys(user, account);
checkAccess(caller, null, true, account); checkAccess(caller, null, true, account);
verifyCallerPrivilegeForUserOrAccountOperations(user);
// don't allow updating system user // don't allow updating system user
if (user.getId() == User.UID_SYSTEM) { if (user.getId() == User.UID_SYSTEM) {
@ -3377,7 +3493,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
public ConfigKey<?>[] getConfigKeys() { public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {UseSecretKeyInResponse, enableUserTwoFactorAuthentication, return new ConfigKey<?>[] {UseSecretKeyInResponse, enableUserTwoFactorAuthentication,
userTwoFactorAuthenticationDefaultProvider, mandateUserTwoFactorAuthentication, userTwoFactorAuthenticationIssuer, userTwoFactorAuthenticationDefaultProvider, mandateUserTwoFactorAuthentication, userTwoFactorAuthenticationIssuer,
userAllowMultipleAccounts}; userAllowMultipleAccounts, listOfRoleTypesAllowedForOperationsOfSameRoleType, allowOperationsOnUsersInSameAccount};
} }
public List<UserTwoFactorAuthenticator> getUserTwoFactorAuthenticationProviders() { public List<UserTwoFactorAuthenticator> getUserTwoFactorAuthenticationProviders() {

View File

@ -16,12 +16,16 @@
// under the License. // under the License.
package com.cloud.configuration; package com.cloud.configuration;
import static org.junit.Assert.assertThrows;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.InvalidParameterValueException;
import com.cloud.storage.StorageManager; import com.cloud.storage.StorageManager;
import com.cloud.user.AccountManagerImpl;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.net.NetUtils; import com.cloud.utils.net.NetUtils;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.config.ConfigDepot; import org.apache.cloudstack.framework.config.ConfigDepot;
import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.ConfigKey;
import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.DataCenterDao;
@ -97,6 +101,7 @@ public class ConfigurationManagerImplTest {
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
configurationManagerImplSpy._configDepot = configDepot; configurationManagerImplSpy._configDepot = configDepot;
configurationManagerImplSpy.populateConfigKeysAllowedOnlyForDefaultAdmin();
} }
@Test @Test
@ -474,4 +479,46 @@ public class ConfigurationManagerImplTest {
Assert.assertThrows(InvalidParameterValueException.class, () -> configurationManagerImplSpy.checkIfDomainIsChildDomain(diskOfferingMock, accountMock, userMock, filteredDomainIds)); Assert.assertThrows(InvalidParameterValueException.class, () -> configurationManagerImplSpy.checkIfDomainIsChildDomain(diskOfferingMock, accountMock, userMock, filteredDomainIds));
} }
@Test
public void testValidateConfigurationAllowedOnlyForDefaultAdmin_withAdminUser_shouldNotThrowException() {
CallContext callContext = mock(CallContext.class);
when(callContext.getCallingUserId()).thenReturn(User.UID_ADMIN);
try (MockedStatic<CallContext> ignored = Mockito.mockStatic(CallContext.class)) {
when(CallContext.current()).thenReturn(callContext);
configurationManagerImplSpy.validateConfigurationAllowedOnlyForDefaultAdmin(AccountManagerImpl.listOfRoleTypesAllowedForOperationsOfSameRoleType.key(), "Admin");
}
}
@Test
public void testValidateConfigurationAllowedOnlyForDefaultAdmin_withNonAdminUser_shouldThrowException() {
CallContext callContext = mock(CallContext.class);
when(callContext.getCallingUserId()).thenReturn(123L);
try (MockedStatic<CallContext> ignored = Mockito.mockStatic(CallContext.class)) {
when(CallContext.current()).thenReturn(callContext);
assertThrows(CloudRuntimeException.class, () ->
configurationManagerImplSpy.validateConfigurationAllowedOnlyForDefaultAdmin(AccountManagerImpl.allowOperationsOnUsersInSameAccount.key(), "Admin")
);
}
}
@Test
public void testValidateConfigurationAllowedOnlyForDefaultAdmin_withNonRestrictedKey_shouldNotThrowException() {
CallContext callContext = mock(CallContext.class);
try (MockedStatic<CallContext> ignored = Mockito.mockStatic(CallContext.class)) {
when(CallContext.current()).thenReturn(callContext);
configurationManagerImplSpy.validateConfigurationAllowedOnlyForDefaultAdmin("some.other.config.key", "Admin");
}
}
@Test(expected = CloudRuntimeException.class)
public void testValidateConfigurationAllowedOnlyForDefaultAdmin_withValidConfigNameAndInvalidValue_shouldThrowException() {
CallContext callContext = mock(CallContext.class);
try (MockedStatic<CallContext> mockedCallContext = Mockito.mockStatic(CallContext.class)) {
mockedCallContext.when(CallContext::current).thenReturn(callContext);
when(callContext.getCallingUserId()).thenReturn(User.UID_ADMIN);
String invalidValue = "Admin, SuperUser";
configurationManagerImplSpy.validateConfigurationAllowedOnlyForDefaultAdmin(AccountManagerImpl.listOfRoleTypesAllowedForOperationsOfSameRoleType.key(), invalidValue);
}
}
} }

View File

@ -108,6 +108,9 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
@Mock @Mock
ConfigKey<Boolean> enableUserTwoFactorAuthenticationMock; ConfigKey<Boolean> enableUserTwoFactorAuthenticationMock;
@Mock
ConfigKey<Boolean> allowOperationsOnUsersInSameAccountMock;
@Mock @Mock
RoleService roleService; RoleService roleService;
@ -115,6 +118,9 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
public void setUp() throws Exception { public void setUp() throws Exception {
enableUserTwoFactorAuthenticationMock = Mockito.mock(ConfigKey.class); enableUserTwoFactorAuthenticationMock = Mockito.mock(ConfigKey.class);
accountManagerImpl.enableUserTwoFactorAuthentication = enableUserTwoFactorAuthenticationMock; accountManagerImpl.enableUserTwoFactorAuthentication = enableUserTwoFactorAuthenticationMock;
allowOperationsOnUsersInSameAccountMock = Mockito.mock(ConfigKey.class);
accountManagerImpl.allowOperationsOnUsersInSameAccount = allowOperationsOnUsersInSameAccountMock;
} }
@Before @Before
@ -125,6 +131,8 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
Mockito.doReturn(accountMockId).when(userVoMock).getAccountId(); Mockito.doReturn(accountMockId).when(userVoMock).getAccountId();
Mockito.doReturn(userVoIdMock).when(userVoMock).getId(); Mockito.doReturn(userVoIdMock).when(userVoMock).getId();
Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(accountMock, accountMock);
} }
@Test @Test
@ -166,6 +174,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class))).thenReturn(true); Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class))).thenReturn(true);
Mockito.when(_vmSnapshotDao.listByAccountId(Mockito.anyLong())).thenReturn(new ArrayList<VMSnapshotVO>()); Mockito.when(_vmSnapshotDao.listByAccountId(Mockito.anyLong())).thenReturn(new ArrayList<VMSnapshotVO>());
Mockito.when(_autoscaleMgr.deleteAutoScaleVmGroupsByAccount(42l)).thenReturn(true); Mockito.when(_autoscaleMgr.deleteAutoScaleVmGroupsByAccount(42l)).thenReturn(true);
Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations(Mockito.any(Account.class));
List<SSHKeyPairVO> sshkeyList = new ArrayList<SSHKeyPairVO>(); List<SSHKeyPairVO> sshkeyList = new ArrayList<SSHKeyPairVO>();
SSHKeyPairVO sshkey = new SSHKeyPairVO(); SSHKeyPairVO sshkey = new SSHKeyPairVO();
@ -193,6 +202,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
Mockito.when(_vmMgr.expunge(Mockito.any(UserVmVO.class))).thenReturn(false); Mockito.when(_vmMgr.expunge(Mockito.any(UserVmVO.class))).thenReturn(false);
Mockito.lenient().when(_domainMgr.getDomain(Mockito.anyLong())).thenReturn(domain); Mockito.lenient().when(_domainMgr.getDomain(Mockito.anyLong())).thenReturn(domain);
Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class))).thenReturn(true); Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class))).thenReturn(true);
Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations(Mockito.any(Account.class));
Assert.assertTrue(accountManagerImpl.deleteUserAccount(42l)); Assert.assertTrue(accountManagerImpl.deleteUserAccount(42l));
// assert that this was NOT a clean delete // assert that this was NOT a clean delete
@ -261,7 +271,6 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
// Queried account - admin account // Queried account - admin account
AccountVO adminAccountMock = Mockito.mock(AccountVO.class); AccountVO adminAccountMock = Mockito.mock(AccountVO.class);
Mockito.when(adminAccountMock.getAccountId()).thenReturn(2L);
Mockito.when(_accountDao.findByIdIncludingRemoved(2L)).thenReturn(adminAccountMock); Mockito.when(_accountDao.findByIdIncludingRemoved(2L)).thenReturn(adminAccountMock);
Mockito.lenient().when(accountService.isRootAdmin(2L)).thenReturn(true); Mockito.lenient().when(accountService.isRootAdmin(2L)).thenReturn(true);
Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class),
@ -309,6 +318,12 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
@Test @Test
public void updateUserTestTimeZoneAndEmailNull() { public void updateUserTestTimeZoneAndEmailNull() {
Mockito.when(userVoMock.getAccountId()).thenReturn(10L);
Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(10L);
Mockito.when(accountMock.getAccountId()).thenReturn(10L);
Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(10L);
Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.User);
prepareMockAndExecuteUpdateUserTest(0); prepareMockAndExecuteUpdateUserTest(0);
} }
@ -316,6 +331,11 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
public void updateUserTestTimeZoneAndEmailNotNull() { public void updateUserTestTimeZoneAndEmailNotNull() {
Mockito.when(UpdateUserCmdMock.getEmail()).thenReturn("email"); Mockito.when(UpdateUserCmdMock.getEmail()).thenReturn("email");
Mockito.when(UpdateUserCmdMock.getTimezone()).thenReturn("timezone"); Mockito.when(UpdateUserCmdMock.getTimezone()).thenReturn("timezone");
Mockito.when(userVoMock.getAccountId()).thenReturn(10L);
Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(10L);
Mockito.when(accountMock.getAccountId()).thenReturn(10L);
Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(10L);
Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.User);
prepareMockAndExecuteUpdateUserTest(1); prepareMockAndExecuteUpdateUserTest(1);
} }
@ -333,14 +353,17 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
Mockito.doReturn(true).when(userDaoMock).update(Mockito.anyLong(), Mockito.eq(userVoMock)); Mockito.doReturn(true).when(userDaoMock).update(Mockito.anyLong(), Mockito.eq(userVoMock));
Mockito.doReturn(Mockito.mock(UserAccountVO.class)).when(userAccountDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(Mockito.mock(UserAccountVO.class)).when(userAccountDaoMock).findById(Mockito.anyLong());
Mockito.doNothing().when(accountManagerImpl).checkAccess(nullable(User.class), nullable(Account.class));
accountManagerImpl.updateUser(UpdateUserCmdMock); accountManagerImpl.updateUser(UpdateUserCmdMock);
Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(accountMock, accountMock);
InOrder inOrder = Mockito.inOrder(userVoMock, accountManagerImpl, userDaoMock, userAccountDaoMock); InOrder inOrder = Mockito.inOrder(userVoMock, accountManagerImpl, userDaoMock, userAccountDaoMock);
inOrder.verify(accountManagerImpl).retrieveAndValidateUser(UpdateUserCmdMock); inOrder.verify(accountManagerImpl).retrieveAndValidateUser(UpdateUserCmdMock);
inOrder.verify(accountManagerImpl).validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
inOrder.verify(accountManagerImpl).retrieveAndValidateAccount(userVoMock); inOrder.verify(accountManagerImpl).retrieveAndValidateAccount(userVoMock);
inOrder.verify(accountManagerImpl).validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
inOrder.verify(accountManagerImpl).validateAndUpdateFirstNameIfNeeded(UpdateUserCmdMock, userVoMock); inOrder.verify(accountManagerImpl).validateAndUpdateFirstNameIfNeeded(UpdateUserCmdMock, userVoMock);
inOrder.verify(accountManagerImpl).validateAndUpdateLastNameIfNeeded(UpdateUserCmdMock, userVoMock); inOrder.verify(accountManagerImpl).validateAndUpdateLastNameIfNeeded(UpdateUserCmdMock, userVoMock);
@ -1203,8 +1226,8 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
accountManagerImpl.validateRoleChange(account, newRole, caller); accountManagerImpl.validateRoleChange(account, newRole, caller);
} }
@Test @Test
public void checkIfAccountManagesProjectsTestNotThrowExceptionWhenTheAccountIsNotAProjectAdministrator() { public void checkIfAccountManagesProjectsTestNotThrowExceptionWhenTheAccountIsNotAProjectAdministrator() {
long accountId = 1L; long accountId = 1L;
List<Long> managedProjectIds = new ArrayList<>(); List<Long> managedProjectIds = new ArrayList<>();
@ -1341,4 +1364,74 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
accountManagerImpl.assertUserNotAlreadyInDomain(existingUser, originalAccount); accountManagerImpl.assertUserNotAlreadyInDomain(existingUser, originalAccount);
} }
@Test
public void testCheckCallerRoleTypeAllowedToUpdateUserSameAccount() {
Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(accountMock);
Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.DomainAdmin);
accountManagerImpl.checkCallerRoleTypeAllowedForUserOrAccountOperations(accountMock, userVoMock);
}
@Test(expected = PermissionDeniedException.class)
public void testCheckCallerRoleTypeAllowedToUpdateUserLowerAccountRoleType() {
Account callingAccount = Mockito.mock(Account.class);
Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(2L);
Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(2L);
Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount);
Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(callingAccount))).thenReturn(RoleType.DomainAdmin);
Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.Admin);
accountManagerImpl.checkCallerRoleTypeAllowedForUserOrAccountOperations(accountMock, userVoMock);
}
@Test
public void testcheckCallerApiPermissionsForUserOperationsRootAdminSameCaller() {
Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(accountMock);
Mockito.when(accountMock.getId()).thenReturn(2L);
Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(2L);
accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock);
}
@Test(expected = PermissionDeniedException.class)
public void testcheckCallerApiPermissionsForUserOperationsRootAdminDifferentAccount() {
Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount);
Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L);
Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L);
Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L);
Mockito.when(accountMock.getAccountId()).thenReturn(2L);
Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(2L);
accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock);
}
@Test
public void testcheckCallerApiPermissionsForUserOperationsAllowedApis() {
Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount);
Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L);
Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L);
Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L);
Mockito.when(accountMock.getAccountId()).thenReturn(2L);
Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(2L);
Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(callingAccount, accountMock);
accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock);
}
@Test(expected = PermissionDeniedException.class)
public void testcheckCallerApiPermissionsForUserOperationsNotAllowedApis() {
Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount);
Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L);
Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L);
Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L);
Mockito.when(accountMock.getAccountId()).thenReturn(2L);
Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(2L);
Mockito.lenient().doThrow(PermissionDeniedException.class).when(accountManagerImpl).checkRoleEscalation(callingAccount, accountMock);
accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock);
}
} }

View File

@ -193,6 +193,7 @@ public class AccountManagerImplVolumeDeleteEventTest extends AccountManagetImplT
public void destroyedVMRootVolumeUsageEvent() public void destroyedVMRootVolumeUsageEvent()
throws SecurityException, IllegalArgumentException, ReflectiveOperationException, AgentUnavailableException, ConcurrentOperationException, CloudException { throws SecurityException, IllegalArgumentException, ReflectiveOperationException, AgentUnavailableException, ConcurrentOperationException, CloudException {
Mockito.lenient().doReturn(vm).when(_vmMgr).destroyVm(nullable(Long.class), nullable(Boolean.class)); Mockito.lenient().doReturn(vm).when(_vmMgr).destroyVm(nullable(Long.class), nullable(Boolean.class));
Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations(Mockito.any(Account.class));
List<UsageEventVO> emittedEvents = deleteUserAccountRootVolumeUsageEvents(true); List<UsageEventVO> emittedEvents = deleteUserAccountRootVolumeUsageEvents(true);
Assert.assertEquals(0, emittedEvents.size()); Assert.assertEquals(0, emittedEvents.size());
} }
@ -204,6 +205,7 @@ public class AccountManagerImplVolumeDeleteEventTest extends AccountManagetImplT
throws SecurityException, IllegalArgumentException, ReflectiveOperationException, AgentUnavailableException, ConcurrentOperationException, CloudException { throws SecurityException, IllegalArgumentException, ReflectiveOperationException, AgentUnavailableException, ConcurrentOperationException, CloudException {
Mockito.doNothing().when(vmStatsDaoMock).removeAllByVmId(Mockito.anyLong()); Mockito.doNothing().when(vmStatsDaoMock).removeAllByVmId(Mockito.anyLong());
Mockito.lenient().when(_vmMgr.destroyVm(nullable(Long.class), nullable(Boolean.class))).thenReturn(vm); Mockito.lenient().when(_vmMgr.destroyVm(nullable(Long.class), nullable(Boolean.class))).thenReturn(vm);
Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations(Mockito.any(Account.class));
List<UsageEventVO> emittedEvents = deleteUserAccountRootVolumeUsageEvents(false); List<UsageEventVO> emittedEvents = deleteUserAccountRootVolumeUsageEvents(false);
UsageEventVO event = emittedEvents.get(0); UsageEventVO event = emittedEvents.get(0);
Assert.assertEquals(EventTypes.EVENT_VOLUME_DELETE, event.getType()); Assert.assertEquals(EventTypes.EVENT_VOLUME_DELETE, event.getType());

View File

@ -488,4 +488,8 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco
public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) { public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
return null; return null;
} }
@Override
public void verifyCallerPrivilegeForUserOrAccountOperations(Account userAccount) {
}
} }