This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit a0080a04fe66b1b1cf1f67bb674a079d7cab857a
Author: nvazquez <nicovazque...@gmail.com>
AuthorDate: Sun May 11 22:48:41 2025 -0300
Adding privilege checks on user and account operations
Co-authored-by: Harikrishna <harikrishna.patn...@gmail.com>
---
api/src/main/java/com/cloud/user/Account.java | 1 +
.../contrail/management/MockAccountManager.java | 4 +
.../configuration/ConfigurationManagerImpl.java | 37 ++++++
.../resourcelimit/ResourceLimitManagerImpl.java | 6 +
.../main/java/com/cloud/user/AccountManager.java | 1 +
.../java/com/cloud/user/AccountManagerImpl.java | 128 ++++++++++++++++++++-
.../ConfigurationManagerImplTest.java | 47 ++++++++
.../com/cloud/user/AccountManagerImplTest.java | 101 +++++++++++++++-
.../AccountManagerImplVolumeDeleteEventTest.java | 2 +
.../com/cloud/user/MockAccountManagerImpl.java | 4 +
10 files changed, 321 insertions(+), 10 deletions(-)
diff --git a/api/src/main/java/com/cloud/user/Account.java
b/api/src/main/java/com/cloud/user/Account.java
index bb9838f137a..2b815531116 100644
--- a/api/src/main/java/com/cloud/user/Account.java
+++ b/api/src/main/java/com/cloud/user/Account.java
@@ -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_ADMIN = 2;
public String getAccountName();
diff --git
a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
index a63c5b68e57..353b062b90b 100644
---
a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
+++
b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
@@ -521,4 +521,8 @@ public class MockAccountManager extends ManagerBase
implements AccountManager {
public UserAccount
clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
return null;
}
+
+ @Override
+ public void verifyCallerPrivilegeForUserOrAccountOperations(Account
userAccount) {
+ }
}
diff --git
a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
index 11d478ccf62..4c03122e463 100644
--- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -47,11 +47,13 @@ import javax.naming.ConfigurationException;
import com.cloud.hypervisor.HypervisorGuru;
+import com.cloud.user.AccountManagerImpl;
import com.cloud.utils.crypt.DBEncryptionUtil;
import com.cloud.host.HostTagVO;
import com.cloud.storage.StoragePoolTagVO;
import com.cloud.storage.VolumeApiServiceImpl;
import com.googlecode.ipv6.IPv6Address;
+import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.affinity.AffinityGroup;
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 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> configKeysAllowedOnlyForDefaultAdmin = new
HashSet<String>();
private Set<String> weightBasedParametersForValidation = new
HashSet<String>();
private Set<String> overprovisioningFactorsForValidation = new
HashSet<String>();
@@ -533,6 +536,7 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
populateConfigValuesForValidationSet();
weightBasedParametersForValidation();
overProvisioningFactorsForValidation();
+ populateConfigKeysAllowedOnlyForDefaultAdmin();
initMessageBusListener();
return true;
}
@@ -596,6 +600,11 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
overprovisioningFactorsForValidation.add(CapacityManager.StorageOverprovisioningFactor.key());
}
+ protected void populateConfigKeysAllowedOnlyForDefaultAdmin() {
+
configKeysAllowedOnlyForDefaultAdmin.add(AccountManagerImpl.listOfRoleTypesAllowedForOperationsOfSameRoleType.key());
+
configKeysAllowedOnlyForDefaultAdmin.add(AccountManagerImpl.allowOperationsOnUsersInSameAccount.key());
+ }
+
private void initMessageBusListener() {
messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, new
MessageSubscriber() {
@Override
@@ -1183,6 +1192,7 @@ public class ConfigurationManagerImpl extends ManagerBase
implements Configurati
s_logger.error("Missing configuration variable " + name + " in
configuration table");
return "Invalid configuration variable.";
}
+ validateConfigurationAllowedOnlyForDefaultAdmin(name, value);
final String configScope = cfg.getScope();
if (scope != null) {
@@ -1347,6 +1357,33 @@ public class ConfigurationManagerImpl extends
ManagerBase implements Configurati
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).
*/
diff --git
a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
index 959a0dc3bb2..0e9fc0f80ef 100644
--- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
+++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
@@ -784,6 +784,7 @@ public class ResourceLimitManagerImpl extends ManagerBase
implements ResourceLim
} else {
_accountMgr.checkAccess(caller, null, true, account);
}
+
_accountMgr.verifyCallerPrivilegeForUserOrAccountOperations(account);
ownerType = ResourceOwnerType.Account;
ownerId = accountId;
@@ -853,6 +854,11 @@ public class ResourceLimitManagerImpl extends ManagerBase
implements ResourceLim
throw new InvalidParameterValueException("Please specify a valid
domain ID.");
}
_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) {
resourceTypes.add(resourceType);
diff --git a/server/src/main/java/com/cloud/user/AccountManager.java
b/server/src/main/java/com/cloud/user/AccountManager.java
index 1d7611d5b54..98939cb7deb 100644
--- a/server/src/main/java/com/cloud/user/AccountManager.java
+++ b/server/src/main/java/com/cloud/user/AccountManager.java
@@ -200,4 +200,5 @@ public interface AccountManager extends AccountService,
Configurable {
UserAccount
clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user);
+ void verifyCallerPrivilegeForUserOrAccountOperations(Account userAccount);
}
diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
index 180f4e49521..cd07303a82d 100644
--- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
@@ -20,6 +20,7 @@ import java.net.InetAddress;
import java.net.URLEncoder;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
@@ -386,6 +387,22 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
true,
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() {
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
*/
- private void checkRoleEscalation(Account caller, Account requested) {
+ protected void checkRoleEscalation(Account caller, Account requested) {
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]",
caller.getAccountName(),
@@ -1471,7 +1488,7 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
// users can't exist in same account
assertUserNotAlreadyInAccount(duplicatedUser, account);
}
-
+ verifyCallerPrivilegeForUserOrAccountOperations(account);
UserVO user = null;
user = createUser(account.getId(), userName, password, firstName,
lastName, email, timeZone, userUUID, source);
return user;
@@ -1488,10 +1505,14 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription =
"Updating User")
public UserAccount updateUser(UpdateUserCmd 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());
validateAndUpdateApiAndSecretKeyIfNeeded(updateUserCmd, user);
- Account account = retrieveAndValidateAccount(user);
validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
@@ -1514,6 +1535,85 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
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.
* 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);
+ verifyCallerPrivilegeForUserOrAccountOperations(user);
boolean success = doSetUserStatus(userId, State.DISABLED);
if (success) {
@@ -1801,6 +1902,7 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
}
checkAccess(caller, AccessType.OperateEntry, true, account);
+ verifyCallerPrivilegeForUserOrAccountOperations(user);
boolean success = Transaction.execute(new
TransactionCallback<Boolean>() {
@Override
@@ -1853,6 +1955,7 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
}
checkAccess(caller, AccessType.OperateEntry, true, account);
+ verifyCallerPrivilegeForUserOrAccountOperations(user);
// 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
@@ -1909,6 +2012,7 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
}
checkIfAccountManagesProjects(accountId);
+ verifyCallerPrivilegeForUserOrAccountOperations(account);
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
Account caller = getCurrentCallingAccount();
checkAccess(caller, AccessType.OperateEntry, true, account);
+ verifyCallerPrivilegeForUserOrAccountOperations(account);
boolean success = enableAccount(account.getId());
if (success) {
@@ -2004,6 +2109,7 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
}
checkAccess(caller, AccessType.OperateEntry, true, account);
+ verifyCallerPrivilegeForUserOrAccountOperations(account);
if (lockAccount(account.getId())) {
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);
+ verifyCallerPrivilegeForUserOrAccountOperations(account);
if (disableAccount(account.getId())) {
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
Account caller = getCurrentCallingAccount();
checkAccess(caller, _domainMgr.getDomain(account.getDomainId()));
+ verifyCallerPrivilegeForUserOrAccountOperations(account);
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
checkAccountAndAccess(user, account);
+ verifyCallerPrivilegeForUserOrAccountOperations(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);
Account oldAccount = _accountDao.findById(user.getAccountId());
checkAccountAndAccess(user, oldAccount);
+ verifyCallerPrivilegeForUserOrAccountOperations(user);
long domainId = oldAccount.getDomainId();
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
return Transaction.execute(new TransactionCallback<AccountVO>() {
@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.
User caller = CallContext.current().getCallingUser();
- preventRootDomainAdminAccessToRootAdminKeys(caller, account);
checkAccess(caller, account);
+ verifyCallerPrivilegeForUserOrAccountOperations(user);
Map<String, String> keys = new HashMap<String, String>();
keys.put("apikey", user.getApiKey());
@@ -2920,8 +3036,8 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
}
Account account = _accountDao.findById(user.getAccountId());
- preventRootDomainAdminAccessToRootAdminKeys(user, account);
checkAccess(caller, null, true, account);
+ verifyCallerPrivilegeForUserOrAccountOperations(user);
// don't allow updating system user
if (user.getId() == User.UID_SYSTEM) {
@@ -3377,7 +3493,7 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {UseSecretKeyInResponse,
enableUserTwoFactorAuthentication,
userTwoFactorAuthenticationDefaultProvider,
mandateUserTwoFactorAuthentication, userTwoFactorAuthenticationIssuer,
- userAllowMultipleAccounts};
+ userAllowMultipleAccounts,
listOfRoleTypesAllowedForOperationsOfSameRoleType,
allowOperationsOnUsersInSameAccount};
}
public List<UserTwoFactorAuthenticator>
getUserTwoFactorAuthenticationProviders() {
diff --git
a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java
b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java
index 8f4dbbe675d..f4541cd49f3 100644
---
a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java
+++
b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java
@@ -16,12 +16,16 @@
// under the License.
package com.cloud.configuration;
+import static org.junit.Assert.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.storage.StorageManager;
+import com.cloud.user.AccountManagerImpl;
+import com.cloud.utils.exception.CloudRuntimeException;
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.ConfigKey;
import com.cloud.dc.dao.DataCenterDao;
@@ -97,6 +101,7 @@ public class ConfigurationManagerImplTest {
@Before
public void setUp() throws Exception {
configurationManagerImplSpy._configDepot = configDepot;
+
configurationManagerImplSpy.populateConfigKeysAllowedOnlyForDefaultAdmin();
}
@Test
@@ -474,4 +479,46 @@ public class ConfigurationManagerImplTest {
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);
+ }
+ }
}
diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
index f1cf0008676..8472a85cc30 100644
--- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
+++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
@@ -108,6 +108,9 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
@Mock
ConfigKey<Boolean> enableUserTwoFactorAuthenticationMock;
+
+ @Mock
+ ConfigKey<Boolean> allowOperationsOnUsersInSameAccountMock;
@Mock
RoleService roleService;
@@ -115,6 +118,9 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
public void setUp() throws Exception {
enableUserTwoFactorAuthenticationMock = Mockito.mock(ConfigKey.class);
accountManagerImpl.enableUserTwoFactorAuthentication =
enableUserTwoFactorAuthenticationMock;
+
+ allowOperationsOnUsersInSameAccountMock =
Mockito.mock(ConfigKey.class);
+ accountManagerImpl.allowOperationsOnUsersInSameAccount =
allowOperationsOnUsersInSameAccountMock;
}
@Before
@@ -125,6 +131,8 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
Mockito.doReturn(accountMockId).when(userVoMock).getAccountId();
Mockito.doReturn(userVoIdMock).when(userVoMock).getId();
+
+
Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(accountMock,
accountMock);
}
@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.when(_vmSnapshotDao.listByAccountId(Mockito.anyLong())).thenReturn(new
ArrayList<VMSnapshotVO>());
Mockito.when(_autoscaleMgr.deleteAutoScaleVmGroupsByAccount(42l)).thenReturn(true);
+
Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations(Mockito.any(Account.class));
List<SSHKeyPairVO> sshkeyList = new ArrayList<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.lenient().when(_domainMgr.getDomain(Mockito.anyLong())).thenReturn(domain);
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 that this was NOT a clean delete
@@ -261,7 +271,6 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
// Queried account - admin account
AccountVO adminAccountMock = Mockito.mock(AccountVO.class);
- Mockito.when(adminAccountMock.getAccountId()).thenReturn(2L);
Mockito.when(_accountDao.findByIdIncludingRemoved(2L)).thenReturn(adminAccountMock);
Mockito.lenient().when(accountService.isRootAdmin(2L)).thenReturn(true);
Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class),
@@ -309,6 +318,12 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
@Test
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);
}
@@ -316,6 +331,11 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
public void updateUserTestTimeZoneAndEmailNotNull() {
Mockito.when(UpdateUserCmdMock.getEmail()).thenReturn("email");
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);
}
@@ -333,14 +353,17 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
Mockito.doReturn(true).when(userDaoMock).update(Mockito.anyLong(),
Mockito.eq(userVoMock));
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);
+
Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(accountMock,
accountMock);
+
InOrder inOrder = Mockito.inOrder(userVoMock, accountManagerImpl,
userDaoMock, userAccountDaoMock);
inOrder.verify(accountManagerImpl).retrieveAndValidateUser(UpdateUserCmdMock);
-
inOrder.verify(accountManagerImpl).validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock,
userVoMock);
inOrder.verify(accountManagerImpl).retrieveAndValidateAccount(userVoMock);
+
inOrder.verify(accountManagerImpl).validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock,
userVoMock);
inOrder.verify(accountManagerImpl).validateAndUpdateFirstNameIfNeeded(UpdateUserCmdMock,
userVoMock);
inOrder.verify(accountManagerImpl).validateAndUpdateLastNameIfNeeded(UpdateUserCmdMock,
userVoMock);
@@ -1203,8 +1226,8 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
accountManagerImpl.validateRoleChange(account, newRole, caller);
}
- @Test
- public void
checkIfAccountManagesProjectsTestNotThrowExceptionWhenTheAccountIsNotAProjectAdministrator()
{
+ @Test
+ public void
checkIfAccountManagesProjectsTestNotThrowExceptionWhenTheAccountIsNotAProjectAdministrator()
{
long accountId = 1L;
List<Long> managedProjectIds = new ArrayList<>();
@@ -1341,4 +1364,74 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
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);
+ }
}
diff --git
a/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
b/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
index 21474a53ee7..31a12c0432c 100644
---
a/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
+++
b/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
@@ -193,6 +193,7 @@ public class AccountManagerImplVolumeDeleteEventTest
extends AccountManagetImplT
public void destroyedVMRootVolumeUsageEvent()
throws SecurityException, IllegalArgumentException,
ReflectiveOperationException, AgentUnavailableException,
ConcurrentOperationException, CloudException {
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);
Assert.assertEquals(0, emittedEvents.size());
}
@@ -204,6 +205,7 @@ public class AccountManagerImplVolumeDeleteEventTest
extends AccountManagetImplT
throws SecurityException, IllegalArgumentException,
ReflectiveOperationException, AgentUnavailableException,
ConcurrentOperationException, CloudException {
Mockito.doNothing().when(vmStatsDaoMock).removeAllByVmId(Mockito.anyLong());
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);
UsageEventVO event = emittedEvents.get(0);
Assert.assertEquals(EventTypes.EVENT_VOLUME_DELETE, event.getType());
diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
index 96b73cc63dd..414b98aacfd 100644
--- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
+++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
@@ -488,4 +488,8 @@ public class MockAccountManagerImpl extends ManagerBase
implements Manager, Acco
public UserAccount
clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
return null;
}
+
+ @Override
+ public void verifyCallerPrivilegeForUserOrAccountOperations(Account
userAccount) {
+ }
}
