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
The following commit(s) were added to refs/heads/4.19 by this push:
new 796bd4f72cc Clean up network permissions on account deletion (#10176)
796bd4f72cc is described below
commit 796bd4f72cc021b91441b6c34a9d8f7425c53374
Author: Bernardo De Marco Gonçalves <[email protected]>
AuthorDate: Wed Jan 15 13:38:20 2025 -0300
Clean up network permissions on account deletion (#10176)
---
.../network/dao/NetworkPermissionDao.java | 7 ++++++
.../network/dao/NetworkPermissionDaoImpl.java | 15 +++++++++++
.../java/com/cloud/user/AccountManagerImpl.java | 29 ++++++++++++++--------
.../com/cloud/user/AccountManagerImplTest.java | 18 ++++++++++++++
.../com/cloud/user/AccountManagetImplTestBase.java | 3 +++
5 files changed, 61 insertions(+), 11 deletions(-)
diff --git
a/engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDao.java
b/engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDao.java
index 1c8d1cf48ff..e8b6322baee 100644
---
a/engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDao.java
+++
b/engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDao.java
@@ -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
*
diff --git
a/engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java
b/engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java
index ffc62b15dc2..950daa11083 100644
---
a/engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java
+++
b/engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java
@@ -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();
diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
index 2d7ebf595fd..1e727036d56 100644
--- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
@@ -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 ));
diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
index ed0a123d4a3..0e8e1df0f3a 100644
--- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
+++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
@@ -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);
+ }
}
diff --git
a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java
b/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java
index 7f9fa488471..2fa18221a94 100644
--- a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java
+++ b/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java
@@ -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
