Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ public interface NetworkPermissionDao extends GenericDao<NetworkPermissionVO, Lo
*/
void removeAllPermissions(long networkId);

/**
* Removes all network permissions associated with a given account.
*
* @param accountId The ID of the account from which all network permissions will be removed.
*/
void removeAccountPermissions(long accountId);

/**
* Find a Network permission by networkId, accountName, and domainId
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class NetworkPermissionDaoImpl extends GenericDaoBase<NetworkPermissionVO

private SearchBuilder<NetworkPermissionVO> NetworkAndAccountSearch;
private SearchBuilder<NetworkPermissionVO> NetworkIdSearch;
private SearchBuilder<NetworkPermissionVO> accountSearch;
private GenericSearchBuilder<NetworkPermissionVO, Long> FindNetworkIdsByAccount;

protected NetworkPermissionDaoImpl() {
Expand All @@ -47,6 +48,10 @@ protected NetworkPermissionDaoImpl() {
NetworkIdSearch.and("networkId", NetworkIdSearch.entity().getNetworkId(), SearchCriteria.Op.EQ);
NetworkIdSearch.done();

accountSearch = createSearchBuilder();
accountSearch.and("accountId", accountSearch.entity().getAccountId(), SearchCriteria.Op.EQ);
accountSearch.done();

FindNetworkIdsByAccount = createSearchBuilder(Long.class);
FindNetworkIdsByAccount.select(null, SearchCriteria.Func.DISTINCT, FindNetworkIdsByAccount.entity().getNetworkId());
FindNetworkIdsByAccount.and("account", FindNetworkIdsByAccount.entity().getAccountId(), SearchCriteria.Op.IN);
Expand All @@ -71,6 +76,16 @@ public void removeAllPermissions(long networkId) {
expunge(sc);
}

@Override
public void removeAccountPermissions(long accountId) {
SearchCriteria<NetworkPermissionVO> sc = accountSearch.create();
sc.setParameters("accountId", accountId);
int networkPermissionRemoved = expunge(sc);
if (networkPermissionRemoved > 0) {
s_logger.debug(String.format("Removed [%s] network permission(s) for the account with Id [%s]", networkPermissionRemoved, accountId));
}
}

@Override
public NetworkPermissionVO findByNetworkAndAccount(long networkId, long accountId) {
SearchCriteria<NetworkPermissionVO> sc = NetworkAndAccountSearch.create();
Expand Down
29 changes: 18 additions & 11 deletions server/src/main/java/com/cloud/user/AccountManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.apache.cloudstack.framework.messagebus.MessageBus;
import org.apache.cloudstack.framework.messagebus.PublishScope;
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
import org.apache.cloudstack.network.dao.NetworkPermissionDao;
import org.apache.cloudstack.region.gslb.GlobalLoadBalancerRuleDao;
import org.apache.cloudstack.resourcedetail.UserDetailVO;
import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao;
Expand Down Expand Up @@ -298,6 +299,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
private SSHKeyPairDao _sshKeyPairDao;
@Inject
private UserDataDao userDataDao;
@Inject
private NetworkPermissionDao networkPermissionDao;

private List<QuerySelector> _querySelectors;

Expand Down Expand Up @@ -870,6 +873,9 @@ protected boolean cleanupAccount(AccountVO account, long callerUserId, Account c
// delete the account from project accounts
_projectAccountDao.removeAccountFromProjects(accountId);

// Delete account's network permissions
networkPermissionDao.removeAccountPermissions(accountId);

if (account.getType() != Account.Type.PROJECT) {
// delete the account from group
_messageBus.publish(_name, MESSAGE_REMOVE_ACCOUNT_EVENT, PublishScope.LOCAL, accountId);
Expand Down Expand Up @@ -1857,26 +1863,27 @@ public boolean deleteUserAccount(long accountId) {
// If the user is a System user, return an error. We do not allow this
AccountVO account = _accountDao.findById(accountId);

if (! isDeleteNeeded(account, accountId, caller)) {
if (!isDeleteNeeded(account, accountId, caller)) {
return true;
}

// Account that manages project(s) can't be removed
List<Long> managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId);
if (!managedProjectIds.isEmpty()) {
StringBuilder projectIds = new StringBuilder();
for (Long projectId : managedProjectIds) {
projectIds.append(projectId).append(", ");
}

throw new InvalidParameterValueException("The account id=" + accountId + " manages project(s) with ids " + projectIds + "and can't be removed");
}
checkIfAccountManagesProjects(accountId);

CallContext.current().putContextParameter(Account.class, account.getUuid());

return deleteAccount(account, callerUserId, caller);
}

protected void checkIfAccountManagesProjects(long accountId) {
List<Long> managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId);
if (!CollectionUtils.isEmpty(managedProjectIds)) {
throw new InvalidParameterValueException(String.format(
"Unable to delete account [%s], because it manages the following project(s): %s. Please, remove the account from these projects or demote it to a regular project role first.",
accountId, managedProjectIds
));
}
}

private boolean isDeleteNeeded(AccountVO account, long accountId, Account caller) {
if (account == null) {
s_logger.info(String.format("The account, identified by id %d, doesn't exist", accountId ));
Expand Down
18 changes: 18 additions & 0 deletions server/src/test/java/com/cloud/user/AccountManagerImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1200,4 +1200,22 @@ public void testValidateRoleAdminCannotEscalateAdminFromNonRootDomain() {
Mockito.when(roleService.findRole(2L)).thenReturn(callerRole);
accountManagerImpl.validateRoleChange(account, newRole, caller);
}

@Test
public void checkIfAccountManagesProjectsTestNotThrowExceptionWhenTheAccountIsNotAProjectAdministrator() {
long accountId = 1L;
List<Long> managedProjectIds = new ArrayList<>();

Mockito.when(_projectAccountDao.listAdministratedProjectIds(accountId)).thenReturn(managedProjectIds);
accountManagerImpl.checkIfAccountManagesProjects(accountId);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIfAccountManagesProjectsTestThrowExceptionWhenTheAccountIsAProjectAdministrator() {
long accountId = 1L;
List<Long> managedProjectIds = List.of(1L);

Mockito.when(_projectAccountDao.listAdministratedProjectIds(accountId)).thenReturn(managedProjectIds);
accountManagerImpl.checkIfAccountManagesProjects(accountId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import org.apache.cloudstack.engine.service.api.OrchestrationService;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.messagebus.MessageBus;
import org.apache.cloudstack.network.dao.NetworkPermissionDao;
import org.apache.cloudstack.region.gslb.GlobalLoadBalancerRuleDao;
import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao;
import org.junit.After;
Expand Down Expand Up @@ -195,6 +196,8 @@ public class AccountManagetImplTestBase {
SSHKeyPairDao _sshKeyPairDao;
@Mock
UserDataDao userDataDao;
@Mock
NetworkPermissionDao networkPermissionDaoMock;

@Spy
@InjectMocks
Expand Down