mirror of
https://github.com/apache/cloudstack.git
synced 2025-10-26 08:42:29 +01:00
Clean up network permissions on account deletion (#10176)
This commit is contained in:
parent
e278bcd08a
commit
796bd4f72c
@ -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
|
||||
*
|
||||
|
||||
@ -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() {
|
||||
@ -47,6 +48,10 @@ public class NetworkPermissionDaoImpl extends GenericDaoBase<NetworkPermissionVO
|
||||
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);
|
||||
@ -71,6 +76,16 @@ public class NetworkPermissionDaoImpl extends GenericDaoBase<NetworkPermissionVO
|
||||
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();
|
||||
|
||||
@ -74,6 +74,7 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
|
||||
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;
|
||||
@ -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;
|
||||
|
||||
@ -870,6 +873,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
|
||||
// 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);
|
||||
@ -1857,26 +1863,27 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
|
||||
// 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 ));
|
||||
|
||||
@ -1200,4 +1200,22 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@ -65,6 +65,7 @@ import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationSe
|
||||
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;
|
||||
@ -195,6 +196,8 @@ public class AccountManagetImplTestBase {
|
||||
SSHKeyPairDao _sshKeyPairDao;
|
||||
@Mock
|
||||
UserDataDao userDataDao;
|
||||
@Mock
|
||||
NetworkPermissionDao networkPermissionDaoMock;
|
||||
|
||||
@Spy
|
||||
@InjectMocks
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user