This is an automated email from the ASF dual-hosted git repository.
winterhazel pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 64178ead02d Add missing project cleanups while deleting/moving user
(#13817)
64178ead02d is described below
commit 64178ead02d18ec565ede449754bfad98799e857
Author: Fabricio Duarte <[email protected]>
AuthorDate: Tue Aug 18 08:39:29 2026 -0300
Add missing project cleanups while deleting/moving user (#13817)
Co-authored-by: Bernardo De Marco Gonçalves <[email protected]>
---
.../java/com/cloud/projects/ProjectService.java | 2 +
.../api/command/admin/user/MoveUserCmd.java | 3 +-
.../apache/cloudstack/region/RegionService.java | 3 +-
.../java/com/cloud/projects/ProjectAccountVO.java | 4 +
.../com/cloud/projects/ProjectInvitationVO.java | 4 +
.../com/cloud/projects/dao/ProjectAccountDao.java | 7 +-
.../cloud/projects/dao/ProjectAccountDaoImpl.java | 31 ++--
.../cloud/projects/dao/ProjectInvitationDao.java | 6 +
.../projects/dao/ProjectInvitationDaoImpl.java | 35 ++++
.../META-INF/db/schema-42210to42300-cleanup.sql | 4 +
.../apache/cloudstack/ldap/LdapAuthenticator.java | 8 +-
.../java/com/cloud/projects/ProjectManager.java | 3 +
.../com/cloud/projects/ProjectManagerImpl.java | 90 ++++++----
.../main/java/com/cloud/user/AccountManager.java | 5 +-
.../java/com/cloud/user/AccountManagerImpl.java | 52 ++++--
.../apache/cloudstack/region/RegionManager.java | 3 +-
.../cloudstack/region/RegionManagerImpl.java | 3 +-
.../cloudstack/region/RegionServiceImpl.java | 3 +-
.../com/cloud/projects/MockProjectManagerImpl.java | 12 ++
.../com/cloud/projects/ProjectManagerImplTest.java | 182 +++++++++++++++++++++
.../com/cloud/user/AccountManagerImplTest.java | 12 ++
21 files changed, 400 insertions(+), 72 deletions(-)
diff --git a/api/src/main/java/com/cloud/projects/ProjectService.java
b/api/src/main/java/com/cloud/projects/ProjectService.java
index d11e9ae0446..17413de320c 100644
--- a/api/src/main/java/com/cloud/projects/ProjectService.java
+++ b/api/src/main/java/com/cloud/projects/ProjectService.java
@@ -23,6 +23,7 @@ import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.projects.ProjectAccount.Role;
import com.cloud.user.Account;
+import com.cloud.user.User;
public interface ProjectService {
/**
@@ -102,4 +103,5 @@ public interface ProjectService {
boolean addUserToProject(Long projectId, String username, String email,
Long projectRoleId, Role projectRole) throws ResourceAllocationException;
+ void moveProjectAssociationsToUser(User oldUser, User newUser) throws
ResourceAllocationException;
}
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/admin/user/MoveUserCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/admin/user/MoveUserCmd.java
index aab20f108f9..36e1cccca5e 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/admin/user/MoveUserCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/admin/user/MoveUserCmd.java
@@ -18,6 +18,7 @@ package org.apache.cloudstack.api.command.admin.user;
import javax.inject.Inject;
+import com.cloud.exception.ResourceAllocationException;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiCommandResourceType;
@@ -112,7 +113,7 @@ public class MoveUserCmd extends BaseCmd {
}
@Override
- public void execute() {
+ public void execute() throws ResourceAllocationException {
Preconditions.checkNotNull(getId(),"I have to have an user to move!");
Preconditions.checkState(ObjectUtils.anyNotNull(getAccountId(),getAccountName()),"provide
either an account name or an account id!");
diff --git a/api/src/main/java/org/apache/cloudstack/region/RegionService.java
b/api/src/main/java/org/apache/cloudstack/region/RegionService.java
index b947b61c8f0..47dbb34dd62 100644
--- a/api/src/main/java/org/apache/cloudstack/region/RegionService.java
+++ b/api/src/main/java/org/apache/cloudstack/region/RegionService.java
@@ -18,6 +18,7 @@ package org.apache.cloudstack.region;
import java.util.List;
+import com.cloud.exception.ResourceAllocationException;
import org.apache.cloudstack.api.command.admin.account.DeleteAccountCmd;
import org.apache.cloudstack.api.command.admin.account.DisableAccountCmd;
import org.apache.cloudstack.api.command.admin.account.EnableAccountCmd;
@@ -116,7 +117,7 @@ public interface RegionService {
* @param moveUserCmd
* @return true if delete was successful, false otherwise
*/
- boolean moveUser(MoveUserCmd moveUserCmd);
+ boolean moveUser(MoveUserCmd moveUserCmd) throws
ResourceAllocationException;
/**
* update an existing domain
diff --git
a/engine/schema/src/main/java/com/cloud/projects/ProjectAccountVO.java
b/engine/schema/src/main/java/com/cloud/projects/ProjectAccountVO.java
index 4710a815f97..0e2f32d5f8b 100644
--- a/engine/schema/src/main/java/com/cloud/projects/ProjectAccountVO.java
+++ b/engine/schema/src/main/java/com/cloud/projects/ProjectAccountVO.java
@@ -110,6 +110,10 @@ public class ProjectAccountVO implements ProjectAccount,
InternalIdentity {
return projectAccountId;
}
+ public void setAccountId(long accountId) {
+ this.accountId = accountId;
+ }
+
public void setProjectRoleId(Long projectRoleId) {
this.projectRoleId = projectRoleId;
}
diff --git
a/engine/schema/src/main/java/com/cloud/projects/ProjectInvitationVO.java
b/engine/schema/src/main/java/com/cloud/projects/ProjectInvitationVO.java
index 887939311b2..09f19b046c8 100644
--- a/engine/schema/src/main/java/com/cloud/projects/ProjectInvitationVO.java
+++ b/engine/schema/src/main/java/com/cloud/projects/ProjectInvitationVO.java
@@ -102,6 +102,10 @@ public class ProjectInvitationVO implements
ProjectInvitation {
return forAccountId;
}
+ public void setForAccountId(Long forAccountId) {
+ this.forAccountId = forAccountId;
+ }
+
@Override
public String getToken() {
return token;
diff --git
a/engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDao.java
b/engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDao.java
index f4b2f646002..cbc300aa56c 100644
--- a/engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDao.java
+++ b/engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDao.java
@@ -20,6 +20,7 @@ import java.util.List;
import com.cloud.projects.ProjectAccount;
import com.cloud.projects.ProjectAccountVO;
+import com.cloud.user.User;
import com.cloud.utils.db.GenericDao;
public interface ProjectAccountDao extends GenericDao<ProjectAccountVO, Long> {
@@ -47,9 +48,11 @@ public interface ProjectAccountDao extends
GenericDao<ProjectAccountVO, Long> {
void removeAccountFromProjects(long accountId);
- void removeUserFromProjects(long userId);
-
boolean canUserModifyProject(long projectId, long accountId, long userId);
List<ProjectAccountVO> listUsersOrAccountsByRole(long id);
+
+ List<ProjectAccountVO> listBy(Long projectId, Long accountId, Long userId);
+
+ void move(User oldUser, User newUser);
}
diff --git
a/engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDaoImpl.java
b/engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDaoImpl.java
index b6eb6d44cea..4de64eef4ca 100644
---
a/engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDaoImpl.java
+++
b/engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDaoImpl.java
@@ -18,6 +18,7 @@ package com.cloud.projects.dao;
import java.util.List;
+import com.cloud.user.User;
import org.springframework.stereotype.Component;
import com.cloud.projects.ProjectAccount;
@@ -192,17 +193,6 @@ public class ProjectAccountDaoImpl extends
GenericDaoBase<ProjectAccountVO, Long
}
}
- @Override
- public void removeUserFromProjects(long userId) {
- SearchCriteria<ProjectAccountVO> sc = AllFieldsSearch.create();
- sc.setParameters("userId", userId);
-
- int removedCount = remove(sc);
- if (removedCount > 0) {
- logger.debug(String.format("Removed user [%s] from %s
project(s).", userId, removedCount));
- }
- }
-
@Override
public boolean canUserModifyProject(long projectId, long accountId, long
userId) {
SearchCriteria<ProjectAccountVO> sc = AllFieldsSearch.create();
@@ -222,4 +212,23 @@ public class ProjectAccountDaoImpl extends
GenericDaoBase<ProjectAccountVO, Long
sc.setParameters("projectRoleId", id);
return listBy(sc);
}
+
+ @Override
+ public List<ProjectAccountVO> listBy(Long projectId, Long accountId, Long
userId) {
+ SearchCriteria<ProjectAccountVO> sc = AllFieldsSearch.create();
+ sc.setParametersIfNotNull("projectId", projectId);
+ sc.setParametersIfNotNull("userId", userId);
+ sc.setParametersIfNotNull("accountId", accountId);
+ return listBy(sc);
+ }
+
+ @Override
+ public void move(User oldUser, User newUser) {
+ List<ProjectAccountVO> projectAccounts = listBy(null,
oldUser.getAccountId(), oldUser.getId());
+ for (ProjectAccountVO projectAccount : projectAccounts) {
+ projectAccount.setAccountId(newUser.getAccountId());
+ projectAccount.setUserId(newUser.getId());
+ update(projectAccount.getId(), projectAccount);
+ }
+ }
}
diff --git
a/engine/schema/src/main/java/com/cloud/projects/dao/ProjectInvitationDao.java
b/engine/schema/src/main/java/com/cloud/projects/dao/ProjectInvitationDao.java
index 976d53998e2..aba2a881e96 100644
---
a/engine/schema/src/main/java/com/cloud/projects/dao/ProjectInvitationDao.java
+++
b/engine/schema/src/main/java/com/cloud/projects/dao/ProjectInvitationDao.java
@@ -20,6 +20,7 @@ import java.util.List;
import com.cloud.projects.ProjectInvitation.State;
import com.cloud.projects.ProjectInvitationVO;
+import com.cloud.user.User;
import com.cloud.utils.db.GenericDao;
public interface ProjectInvitationDao extends GenericDao<ProjectInvitationVO,
Long> {
@@ -43,4 +44,9 @@ public interface ProjectInvitationDao extends
GenericDao<ProjectInvitationVO, Lo
List<ProjectInvitationVO> listInvitationsToExpire(long timeOut);
+ int removeBy(Long projectId, Long accountId, Long userId);
+
+ List<ProjectInvitationVO> listBy(Long projectId, Long accountId, Long
userId);
+
+ void move(User oldUser, User newUser);
}
diff --git
a/engine/schema/src/main/java/com/cloud/projects/dao/ProjectInvitationDaoImpl.java
b/engine/schema/src/main/java/com/cloud/projects/dao/ProjectInvitationDaoImpl.java
index d30b1c9f1f1..17e841967fb 100644
---
a/engine/schema/src/main/java/com/cloud/projects/dao/ProjectInvitationDaoImpl.java
+++
b/engine/schema/src/main/java/com/cloud/projects/dao/ProjectInvitationDaoImpl.java
@@ -19,6 +19,7 @@ package com.cloud.projects.dao;
import java.sql.Date;
import java.util.List;
+import com.cloud.user.User;
import org.springframework.stereotype.Component;
import com.cloud.projects.ProjectInvitation.State;
@@ -124,6 +125,40 @@ public class ProjectInvitationDaoImpl extends
GenericDaoBase<ProjectInvitationVO
return listBy(sc);
}
+ @Override
+ public int removeBy(Long projectId, Long accountId, Long userId) {
+ SearchCriteria<ProjectInvitationVO> sc =
prepareAllFieldsSearchCriteria(projectId, accountId, userId);
+ return remove(sc);
+ }
+
+ @Override
+ public List<ProjectInvitationVO> listBy(Long projectId, Long accountId,
Long userId) {
+ SearchCriteria<ProjectInvitationVO> sc =
prepareAllFieldsSearchCriteria(projectId, accountId, userId);
+ return listBy(sc);
+ }
+
+ @Override
+ public void move(User oldUser, User newUser) {
+ List<ProjectInvitationVO> projectInvitations = listBy(null,
oldUser.getAccountId(), oldUser.getId());
+ for (ProjectInvitationVO projectInvitation : projectInvitations) {
+ projectInvitation.setForAccountId(newUser.getAccountId());
+ projectInvitation.setForUserId(newUser.getId());
+ update(projectInvitation.getId(), projectInvitation);
+ }
+ }
+
+ private SearchCriteria<ProjectInvitationVO>
prepareAllFieldsSearchCriteria(Long projectId, Long accountId, Long userId) {
+ SearchCriteria<ProjectInvitationVO> sc = AllFieldsSearch.create();
+
+ sc.setParametersIfNotNull("userId", userId);
+ sc.setParametersIfNotNull("accountId", accountId);
+ if (projectId != null && projectId != -1) {
+ sc.setParameters("projectId", projectId);
+ }
+
+ return sc;
+ }
+
@Override
public boolean isActive(long id, long timeout) {
SearchCriteria<ProjectInvitationVO> sc = InactiveSearch.create();
diff --git
a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300-cleanup.sql
b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300-cleanup.sql
index e2b066af780..13b73fe9648 100644
---
a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300-cleanup.sql
+++
b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300-cleanup.sql
@@ -18,3 +18,7 @@
--;
-- Schema upgrade cleanup from 4.22.1.0 to 4.23.0.0
--;
+
+-- Delete stale project association entries for users that were removed
+DELETE FROM `cloud`.`project_account` WHERE `user_id` IN (SELECT `id` FROM
`cloud`.`user` WHERE `removed` IS NOT NULL);
+DELETE FROM `cloud`.`project_invitations` WHERE `user_id` IN (SELECT `id` FROM
`cloud`.`user` WHERE `removed` IS NOT NULL);
diff --git
a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java
b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java
index 09519c5641c..464514f58ce 100644
---
a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java
+++
b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java
@@ -23,6 +23,7 @@ import java.util.UUID;
import javax.inject.Inject;
+import com.cloud.exception.ResourceAllocationException;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.auth.UserAuthenticator;
import org.apache.commons.collections.CollectionUtils;
@@ -170,7 +171,12 @@ public class LdapAuthenticator extends AdapterBase
implements UserAuthenticator
if (mappedAccount == null ||
mappedAccount.getRemoved() != null) {
throw new CloudRuntimeException("Mapped account
for users does not exist. Please contact your administrator.");
}
- _accountManager.moveUser(userAccount.getId(),
userAccount.getDomainId(), mappedAccount);
+ try {
+ _accountManager.moveUser(userAccount.getId(),
userAccount.getDomainId(), mappedAccount);
+ } catch (ResourceAllocationException e) {
+ throw new
CloudRuntimeException(String.format("Failed to move User [%s] to mapped Account
[%s] due to insufficient Project limits.",
+ userAccount.getUsername(),
mappedAccount.getAccountName()), e);
+ }
}
// else { the user hasn't changed in ldap, the ldap group
stayed the same, hurray, pass, fun thou self a lot of fun }
}
diff --git a/server/src/main/java/com/cloud/projects/ProjectManager.java
b/server/src/main/java/com/cloud/projects/ProjectManager.java
index 5f58205208b..17a443befae 100644
--- a/server/src/main/java/com/cloud/projects/ProjectManager.java
+++ b/server/src/main/java/com/cloud/projects/ProjectManager.java
@@ -19,6 +19,7 @@ package com.cloud.projects;
import java.util.List;
import com.cloud.user.Account;
+import com.cloud.user.User;
import org.apache.cloudstack.framework.config.ConfigKey;
public interface ProjectManager extends ProjectService {
@@ -47,6 +48,8 @@ public interface ProjectManager extends ProjectService {
long getInvitationTimeout();
+ boolean cleanupProjectsForUser(Project project, User user);
+
public static final String MESSAGE_CREATE_TUNGSTEN_PROJECT_EVENT =
"Message.CreateTungstenProject.Event";
public static final String MESSAGE_DELETE_TUNGSTEN_PROJECT_EVENT =
"Message.DeleteTungstenProject.Event";
diff --git a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
index 92af441d06b..9ba5402443e 100644
--- a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
+++ b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
@@ -617,6 +617,37 @@ public class ProjectManagerImpl extends ManagerBase
implements ProjectManager, C
}
}
+ /**
+ * Transfers all project associations and project invitations from one
user to another.
+ *
+ * @param oldUser the user whose project associations are being transferred
+ * @param newUser the user to whom the project associations are being
transferred
+ * @throws ResourceAllocationException if there is an issue with
allocating the required project resources to the new user
+ */
+ @Override
+ public void moveProjectAssociationsToUser(User oldUser, User newUser)
throws ResourceAllocationException {
+ _projectInvitationDao.move(oldUser, newUser);
+
+ List<ProjectAccountVO> projectAccounts =
_projectAccountDao.listBy(null, oldUser.getAccountId(), oldUser.getId());
+ if (projectAccounts.isEmpty()) {
+ return;
+ }
+
+ Account oldAccount = _accountDao.findById(oldUser.getAccountId());
+ Account newAccount = _accountDao.findById(newUser.getAccountId());
+ long requiredProjectsAmount = oldAccount.getId() != newAccount.getId()
+ ? projectAccounts.stream().filter(pa -> pa.getAccountRole() ==
ProjectAccount.Role.Admin).count()
+ : 0L;
+
+ try (CheckedReservation projectReservation = new
CheckedReservation(newAccount, ResourceType.project, null, null,
requiredProjectsAmount, reservationDao, _resourceLimitMgr)) {
+ _projectAccountDao.move(oldUser, newUser);
+ if (requiredProjectsAmount > 0) {
+ _resourceLimitMgr.incrementResourceCount(newAccount.getId(),
ResourceType.project, requiredProjectsAmount);
+ _resourceLimitMgr.decrementResourceCount(oldAccount.getId(),
ResourceType.project, requiredProjectsAmount);
+ }
+ }
+ }
+
@Override
public Project findByNameAndDomainId(String name, long domainId) {
return _projectDao.findByNameAndDomain(name, domainId);
@@ -1033,50 +1064,41 @@ public class ProjectManagerImpl extends ManagerBase
implements ProjectManager, C
//verify permissions
_accountMgr.checkAccess(caller, AccessType.ModifyProject, true,
_accountMgr.getAccount(project.getProjectAccountId()));
- //Check if the user exists in the project
- ProjectAccount projectUser =
_projectAccountDao.findByProjectIdUserId(projectId, user.getAccountId(),
user.getId());
- if (projectUser == null) {
- deletePendingInvite(projectId, user);
+ boolean success = cleanupProjectsForUser(project, user);
+ if (!success) {
InvalidParameterValueException ex = new
InvalidParameterValueException("User " + user.getUsername() + " is not assigned
to the project with specified id");
- // Use the projectVO object and not the projectAccount object to
inject the projectId.
ex.addProxyObject(project.getUuid(), "projectId");
throw ex;
}
- return deleteUserFromProject(projectId, user);
+ return true;
}
- private void deletePendingInvite(Long projectId, User user) {
- ProjectInvitation invite =
_projectInvitationDao.findByUserIdProjectId(user.getId(), user.getAccountId(),
projectId);
- if (invite != null) {
- boolean success = _projectInvitationDao.remove(invite.getId());
- if (success){
- logger.info("Successfully deleted invite pending for the user
: {}", user);
- } else {
- logger.info("Failed to delete project invite for user: {}",
user);
- }
- }
- }
+ /**
+ * Cleans up project associations and invitations for a specified user in
a given project.
+ *
+ * @param project the project from which the user is being cleaned up; if
null, cleanup applies to all projects associated with the user
+ * @param user the user whose project associations and invitations are
being cleaned up
+ * @return true if any project accounts associated with the user were
removed, false otherwise
+ */
+ @Override
+ public boolean cleanupProjectsForUser(Project project, User user) {
+ return Transaction.execute((TransactionCallback<Boolean>) status -> {
+ Long projectId = project != null ? project.getId() : null;
+ long userId = user.getId();
+ long accountId = user.getAccountId();
- @DB
- private boolean deleteUserFromProject(Long projectId, User user) {
- return Transaction.execute(new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(TransactionStatus status) {
- boolean success = true;
- ProjectAccountVO projectAccount =
_projectAccountDao.findByProjectIdUserId(projectId, user.getAccountId(),
user.getId());
- success = _projectAccountDao.remove(projectAccount.getId());
+ _projectInvitationDao.removeBy(projectId, accountId, userId);
+
+ List<ProjectAccountVO> projectAccounts =
_projectAccountDao.listBy(projectId, accountId, userId);
+ for (ProjectAccountVO projectAccount : projectAccounts) {
+ _projectAccountDao.remove(projectAccount.getId());
if (projectAccount.getAccountRole() == Role.Admin) {
-
_resourceLimitMgr.decrementResourceCount(user.getAccountId(),
ResourceType.project);
+ _resourceLimitMgr.decrementResourceCount(accountId,
ResourceType.project);
}
- if (success) {
- logger.debug("Removed user {} from project. Removing any
invite sent to the user", user);
- ProjectInvitation invite =
_projectInvitationDao.findByUserIdProjectId(user.getId(), user.getAccountId(),
projectId);
- if (invite != null) {
- success = success &&
_projectInvitationDao.remove(invite.getId());
- }
- }
- return success;
+ logger.debug("Removed user [{}] from project [{}].", user,
projectAccount.getProjectId());
}
+
+ return !projectAccounts.isEmpty();
});
}
diff --git a/server/src/main/java/com/cloud/user/AccountManager.java
b/server/src/main/java/com/cloud/user/AccountManager.java
index eca1a571dd8..e3840e29725 100644
--- a/server/src/main/java/com/cloud/user/AccountManager.java
+++ b/server/src/main/java/com/cloud/user/AccountManager.java
@@ -20,6 +20,7 @@ import java.net.InetAddress;
import java.util.List;
import java.util.Map;
+import com.cloud.exception.ResourceAllocationException;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.apikeypair.ApiKeyPair;
import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd;
@@ -148,7 +149,7 @@ public interface AccountManager extends AccountService,
Configurable {
* moves a user to another account within the same domain
* @return true if the user was successfully moved
*/
- boolean moveUser(MoveUserCmd moveUserCmd);
+ boolean moveUser(MoveUserCmd moveUserCmd) throws
ResourceAllocationException;
@Override
UserAccount updateUser(UpdateUserCmd cmd);
@@ -190,7 +191,7 @@ public interface AccountManager extends AccountService,
Configurable {
ConfigKey<Boolean> UseSecretKeyInResponse = new
ConfigKey<Boolean>("Advanced", Boolean.class, "use.secret.key.in.response",
"false",
"This parameter allows the users to enable or disable of showing
secret key as a part of response for various APIs. By default it is set to
false.", true);
- boolean moveUser(long id, Long domainId, Account newAccount);
+ boolean moveUser(long id, Long domainId, Account newAccount) throws
ResourceAllocationException;
UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final Long
domainId, final Long userAccountId);
diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
index db9c1d1dafd..e4d835f74b1 100644
--- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
@@ -45,10 +45,13 @@ import javax.crypto.spec.SecretKeySpec;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.projects.dao.ProjectInvitationDao;
import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.SSHKeyPairDao;
import com.cloud.user.dao.UserAccountDao;
import com.cloud.user.dao.UserDao;
+import com.cloud.utils.db.TransactionCallbackWithException;
import org.apache.cloudstack.acl.APIChecker;
import org.apache.cloudstack.acl.ApiKeyPairManagerImpl;
import org.apache.cloudstack.acl.ApiKeyPairPermissionVO;
@@ -315,6 +318,8 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
@Inject
private ProjectAccountDao _projectAccountDao;
@Inject
+ private ProjectInvitationDao projectInvitationDao;
+ @Inject
private IPAddressDao _ipAddressDao;
@Inject
private HostDao hostDao;
@@ -2521,14 +2526,29 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
checkAccountAndAccess(user, account);
verifyCallerPrivilegeForUserOrAccountOperations(user);
- removeUserApiKeys(id);
+ return deleteAndCleanupUser(user);
+ }
+
+ /**
+ * Removes the specified user and performs cleanup operations associated
with the user.
+ *
+ * @param user the user to be deleted and cleaned up
+ * @return true if the user was successfully marked as removed, false
otherwise
+ */
+ protected boolean deleteAndCleanupUser(User user) {
+ return Transaction.execute((TransactionCallback<Boolean>) status -> {
+ long userId = user.getId();
+
+ removeUserApiKeys(userId);
+ _projectMgr.cleanupProjectsForUser(null, user);
- return _userDao.remove(id);
+ return _userDao.remove(userId);
+ });
}
@Override
@ActionEvent(eventType = EventTypes.EVENT_USER_MOVE, eventDescription =
"moving User to a new account")
- public boolean moveUser(MoveUserCmd cmd) {
+ public boolean moveUser(MoveUserCmd cmd) throws
ResourceAllocationException {
final Long id = cmd.getId();
UserVO user = getValidUserVO(id);
Account oldAccount = _accountDao.findById(user.getAccountId());
@@ -2542,7 +2562,7 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
}
@Override
- public boolean moveUser(long id, Long domainId, Account newAccount) {
+ public boolean moveUser(long id, Long domainId, Account newAccount) throws
ResourceAllocationException {
UserVO user = getValidUserVO(id);
Account oldAccount = _accountDao.findById(user.getAccountId());
checkAccountAndAccess(user, oldAccount);
@@ -2550,24 +2570,22 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
return moveUser(user, newAccount.getId());
}
- private boolean moveUser(UserVO user, long newAccountId) {
+ private boolean moveUser(UserVO user, long newAccountId) throws
ResourceAllocationException {
if (newAccountId == user.getAccountId()) {
// could do a not silent fail but the objective of the user is
reached
return true; // no need to create a new user object for this user
}
- return Transaction.execute(new TransactionCallback<>() {
- @Override
- public Boolean doInTransaction(TransactionStatus status) {
- UserVO newUser = new UserVO(user);
- user.setExternalEntity(user.getUuid());
- user.setUuid(UUID.randomUUID().toString());
- _userDao.update(user.getId(), user);
- newUser.setAccountId(newAccountId);
- boolean success = _userDao.remove(user.getId());
- UserVO persisted = _userDao.persist(newUser);
- return success &&
persisted.getUuid().equals(user.getExternalEntity());
- }
+ return Transaction.execute((TransactionCallbackWithException<Boolean,
ResourceAllocationException>) status -> {
+ UserVO newUser = new UserVO(user);
+ user.setExternalEntity(user.getUuid());
+ user.setUuid(UUID.randomUUID().toString());
+ _userDao.update(user.getId(), user);
+ newUser.setAccountId(newAccountId);
+ UserVO persisted = _userDao.persist(newUser);
+ _projectMgr.moveProjectAssociationsToUser(user, persisted);
+ boolean success = _userDao.remove(user.getId());
+ return success &&
persisted.getUuid().equals(user.getExternalEntity());
});
}
diff --git
a/server/src/main/java/org/apache/cloudstack/region/RegionManager.java
b/server/src/main/java/org/apache/cloudstack/region/RegionManager.java
index fedd66d9440..4e7eaad9f2c 100644
--- a/server/src/main/java/org/apache/cloudstack/region/RegionManager.java
+++ b/server/src/main/java/org/apache/cloudstack/region/RegionManager.java
@@ -18,6 +18,7 @@ package org.apache.cloudstack.region;
import java.util.List;
+import com.cloud.exception.ResourceAllocationException;
import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd;
import org.apache.cloudstack.api.command.admin.domain.UpdateDomainCmd;
import org.apache.cloudstack.api.command.admin.user.DeleteUserCmd;
@@ -128,7 +129,7 @@ public interface RegionManager {
* @param moveUserCmd
* @return
*/
- boolean moveUser(MoveUserCmd moveUserCmd);
+ boolean moveUser(MoveUserCmd moveUserCmd) throws
ResourceAllocationException;
/**
* update an existing domain
diff --git
a/server/src/main/java/org/apache/cloudstack/region/RegionManagerImpl.java
b/server/src/main/java/org/apache/cloudstack/region/RegionManagerImpl.java
index 3085f655943..a49ba4085cb 100644
--- a/server/src/main/java/org/apache/cloudstack/region/RegionManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/region/RegionManagerImpl.java
@@ -24,6 +24,7 @@ import java.util.Properties;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import com.cloud.exception.ResourceAllocationException;
import org.apache.cloudstack.api.command.admin.user.MoveUserCmd;
import org.springframework.stereotype.Component;
@@ -227,7 +228,7 @@ public class RegionManagerImpl extends ManagerBase
implements RegionManager, Man
* {@inheritDoc}
*/
@Override
- public boolean moveUser(MoveUserCmd cmd) {
+ public boolean moveUser(MoveUserCmd cmd) throws
ResourceAllocationException {
return _accountMgr.moveUser(cmd);
}
diff --git
a/server/src/main/java/org/apache/cloudstack/region/RegionServiceImpl.java
b/server/src/main/java/org/apache/cloudstack/region/RegionServiceImpl.java
index 982395637e3..f0db79f7cee 100644
--- a/server/src/main/java/org/apache/cloudstack/region/RegionServiceImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/region/RegionServiceImpl.java
@@ -22,6 +22,7 @@ import java.util.Map;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import com.cloud.exception.ResourceAllocationException;
import org.springframework.stereotype.Component;
import org.apache.cloudstack.api.command.admin.account.DeleteAccountCmd;
@@ -154,7 +155,7 @@ public class RegionServiceImpl extends ManagerBase
implements RegionService, Man
* {@inheritDoc}
*/
@Override
- public boolean moveUser(MoveUserCmd cmd) {
+ public boolean moveUser(MoveUserCmd cmd) throws
ResourceAllocationException {
return _regionMgr.moveUser(cmd);
}
diff --git
a/server/src/test/java/com/cloud/projects/MockProjectManagerImpl.java
b/server/src/test/java/com/cloud/projects/MockProjectManagerImpl.java
index 0abcf9591d4..36a2e78d461 100644
--- a/server/src/test/java/com/cloud/projects/MockProjectManagerImpl.java
+++ b/server/src/test/java/com/cloud/projects/MockProjectManagerImpl.java
@@ -21,6 +21,7 @@ import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.projects.ProjectAccount.Role;
import com.cloud.user.Account;
+import com.cloud.user.User;
import com.cloud.utils.component.ManagerBase;
import javax.naming.ConfigurationException;
@@ -214,6 +215,17 @@ public class MockProjectManagerImpl extends ManagerBase
implements ProjectManage
return 0;
}
+ @Override
+ public boolean cleanupProjectsForUser(Project project, User user) {
+ // TODO Auto-generated method stub
+ return false;
+ }
+
+ @Override
+ public void moveProjectAssociationsToUser(User oldUser, User newUser)
throws ResourceAllocationException {
+ // TODO Auto-generated method stub
+ }
+
@Override
public Project findByProjectAccountIdIncludingRemoved(long
projectAccountId) {
return null;
diff --git
a/server/src/test/java/com/cloud/projects/ProjectManagerImplTest.java
b/server/src/test/java/com/cloud/projects/ProjectManagerImplTest.java
index b9b568facc2..ca6c2193fbd 100644
--- a/server/src/test/java/com/cloud/projects/ProjectManagerImplTest.java
+++ b/server/src/test/java/com/cloud/projects/ProjectManagerImplTest.java
@@ -17,9 +17,11 @@
package com.cloud.projects;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import org.apache.cloudstack.acl.ControlledEntity;
+import org.apache.cloudstack.reservation.dao.ReservationDao;
import org.apache.cloudstack.webhook.WebhookHelper;
import org.apache.commons.collections.CollectionUtils;
import org.junit.Assert;
@@ -28,6 +30,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
+import org.mockito.MockedConstruction;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.Spy;
@@ -35,7 +38,17 @@ import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.stubbing.Answer;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
+import com.cloud.configuration.Resource.ResourceType;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.projects.ProjectAccount.Role;
+import com.cloud.projects.dao.ProjectAccountDao;
import com.cloud.projects.dao.ProjectDao;
+import com.cloud.projects.dao.ProjectInvitationDao;
+import com.cloud.resourcelimit.CheckedReservation;
+import com.cloud.user.AccountVO;
+import com.cloud.user.ResourceLimitService;
+import com.cloud.user.User;
+import com.cloud.user.dao.AccountDao;
import com.cloud.utils.component.ComponentContext;
@@ -49,6 +62,21 @@ public class ProjectManagerImplTest {
@Mock
ProjectDao projectDao;
+ @Mock
+ ProjectInvitationDao projectInvitationDao;
+
+ @Mock
+ ProjectAccountDao projectAccountDao;
+
+ @Mock
+ AccountDao accountDao;
+
+ @Mock
+ ResourceLimitService resourceLimitMgr;
+
+ @Mock
+ ReservationDao reservationDao;
+
List<ProjectVO> updateProjects;
@Before
@@ -128,4 +156,158 @@ public class ProjectManagerImplTest {
Assert.assertTrue(CollectionUtils.isEmpty(result));
}
}
+
+ @Test
+ public void cleanupProjectsForUserTestNoAssociationsReturnsFalse() {
+ Project project = Mockito.mock(Project.class);
+ Mockito.when(project.getId()).thenReturn(100L);
+ User user = mockUser(1L, 10L);
+ Mockito.when(projectAccountDao.listBy(Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong()))
+ .thenReturn(Collections.emptyList());
+
+ boolean result = projectManager.cleanupProjectsForUser(project, user);
+
+ Assert.assertFalse(result);
+ Mockito.verify(projectInvitationDao).removeBy(100L, 10L, 1L);
+ Mockito.verify(projectAccountDao,
Mockito.never()).remove(Mockito.anyLong());
+ Mockito.verify(resourceLimitMgr,
Mockito.never()).decrementResourceCount(Mockito.anyLong(),
Mockito.any(ResourceType.class));
+ }
+
+ @Test
+ public void cleanupProjectsForUserTestRemovesAdminAndRegularAssociations()
{
+ Project project = Mockito.mock(Project.class);
+ Mockito.when(project.getId()).thenReturn(100L);
+ User user = mockUser(1L, 10L);
+ ProjectAccountVO admin = mockProjectAccount(1L, Role.Admin);
+ ProjectAccountVO regular = mockProjectAccount(2L, Role.Regular);
+ Mockito.when(projectAccountDao.listBy(Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong()))
+ .thenReturn(List.of(admin, regular));
+
+ boolean result = projectManager.cleanupProjectsForUser(project, user);
+
+ Assert.assertTrue(result);
+ Mockito.verify(projectInvitationDao).removeBy(100L, 10L, 1L);
+ Mockito.verify(projectAccountDao).remove(1L);
+ Mockito.verify(projectAccountDao).remove(2L);
+ Mockito.verify(resourceLimitMgr).decrementResourceCount(10L,
ResourceType.project);
+ }
+
+ @Test
+ public void cleanupProjectsForUserTestNullProject() {
+ User user = mockUser(1L, 10L);
+ ProjectAccountVO admin = mockProjectAccount(1L, Role.Admin);
+ Mockito.when(projectAccountDao.listBy(Mockito.isNull(),
Mockito.eq(10L), Mockito.eq(1L)))
+ .thenReturn(List.of(admin));
+
+ boolean result = projectManager.cleanupProjectsForUser(null, user);
+
+ Assert.assertTrue(result);
+ Mockito.verify(projectInvitationDao).removeBy(Mockito.isNull(),
Mockito.eq(10L), Mockito.eq(1L));
+ Mockito.verify(projectAccountDao).remove(1L);
+ Mockito.verify(resourceLimitMgr).decrementResourceCount(10L,
ResourceType.project);
+ }
+
+ @Test
+ public void moveProjectAssociationsToUserTestNoProjectAccounts() throws
ResourceAllocationException {
+ User oldUser = mockUser(1L, 10L);
+ User newUser = mockUser(2L, 20L);
+ Mockito.when(projectAccountDao.listBy(Mockito.isNull(),
Mockito.eq(10L), Mockito.eq(1L)))
+ .thenReturn(Collections.emptyList());
+
+ projectManager.moveProjectAssociationsToUser(oldUser, newUser);
+
+ Mockito.verify(projectInvitationDao).move(oldUser, newUser);
+ Mockito.verify(projectAccountDao, Mockito.never()).move(Mockito.any(),
Mockito.any());
+ Mockito.verifyNoInteractions(accountDao);
+ Mockito.verifyNoInteractions(resourceLimitMgr);
+ }
+
+ @Test
+ public void moveProjectAssociationsToUserTestSameAccount() throws
ResourceAllocationException {
+ User oldUser = mockUser(1L, 10L);
+ User newUser = mockUser(2L, 10L);
+ ProjectAccountVO regular = mockProjectAccount(1L, Role.Regular);
+ Mockito.when(projectAccountDao.listBy(Mockito.isNull(),
Mockito.eq(10L), Mockito.eq(1L)))
+ .thenReturn(List.of(regular));
+ AccountVO oldAccount = mockAccount(10L);
+ AccountVO newAccount = mockAccount(10L);
+
Mockito.when(accountDao.findById(10L)).thenReturn(oldAccount).thenReturn(newAccount);
+
+ try (MockedConstruction<CheckedReservation> ignored =
Mockito.mockConstruction(CheckedReservation.class)) {
+ projectManager.moveProjectAssociationsToUser(oldUser, newUser);
+ }
+
+ Mockito.verify(projectInvitationDao).move(oldUser, newUser);
+ Mockito.verify(projectAccountDao).move(oldUser, newUser);
+ Mockito.verify(resourceLimitMgr, Mockito.never())
+ .incrementResourceCount(Mockito.anyLong(),
Mockito.any(ResourceType.class), Mockito.anyLong());
+ Mockito.verify(resourceLimitMgr, Mockito.never())
+ .decrementResourceCount(Mockito.anyLong(),
Mockito.any(ResourceType.class), Mockito.anyLong());
+ }
+
+ @Test
+ public void
moveProjectAssociationsToUserTestDifferentAccountsWithAdminRole() throws
ResourceAllocationException {
+ User oldUser = mockUser(1L, 10L);
+ User newUser = mockUser(2L, 20L);
+ ProjectAccountVO admin = mockProjectAccount(1L, Role.Admin);
+ Mockito.when(projectAccountDao.listBy(Mockito.isNull(),
Mockito.eq(10L), Mockito.eq(1L)))
+ .thenReturn(List.of(admin));
+ AccountVO oldAccount = mockAccount(10L);
+ AccountVO newAccount = mockAccount(20L);
+ Mockito.when(accountDao.findById(10L)).thenReturn(oldAccount);
+ Mockito.when(accountDao.findById(20L)).thenReturn(newAccount);
+
+ try (MockedConstruction<CheckedReservation> ignored =
Mockito.mockConstruction(CheckedReservation.class)) {
+ projectManager.moveProjectAssociationsToUser(oldUser, newUser);
+ }
+
+ Mockito.verify(projectInvitationDao).move(oldUser, newUser);
+ Mockito.verify(projectAccountDao).move(oldUser, newUser);
+ Mockito.verify(resourceLimitMgr).incrementResourceCount(20L,
ResourceType.project, 1L);
+ Mockito.verify(resourceLimitMgr).decrementResourceCount(10L,
ResourceType.project, 1L);
+ }
+
+ @Test
+ public void
moveProjectAssociationsToUserTestDifferentAccountsWithoutAdminRole() throws
ResourceAllocationException {
+ User oldUser = mockUser(1L, 10L);
+ User newUser = mockUser(2L, 20L);
+ ProjectAccountVO regular = mockProjectAccount(1L, Role.Regular);
+ Mockito.when(projectAccountDao.listBy(Mockito.isNull(),
Mockito.eq(10L), Mockito.eq(1L)))
+ .thenReturn(List.of(regular));
+ AccountVO oldAccount = mockAccount(10L);
+ AccountVO newAccount = mockAccount(20L);
+ Mockito.when(accountDao.findById(10L)).thenReturn(oldAccount);
+ Mockito.when(accountDao.findById(20L)).thenReturn(newAccount);
+
+ try (MockedConstruction<CheckedReservation> ignored =
Mockito.mockConstruction(CheckedReservation.class)) {
+ projectManager.moveProjectAssociationsToUser(oldUser, newUser);
+ }
+
+ Mockito.verify(projectInvitationDao).move(oldUser, newUser);
+ Mockito.verify(projectAccountDao).move(oldUser, newUser);
+ Mockito.verify(resourceLimitMgr, Mockito.never())
+ .incrementResourceCount(Mockito.anyLong(),
Mockito.any(ResourceType.class), Mockito.anyLong());
+ Mockito.verify(resourceLimitMgr, Mockito.never())
+ .decrementResourceCount(Mockito.anyLong(),
Mockito.any(ResourceType.class), Mockito.anyLong());
+ }
+
+ private User mockUser(long id, long accountId) {
+ User user = Mockito.mock(User.class);
+ Mockito.when(user.getId()).thenReturn(id);
+ Mockito.when(user.getAccountId()).thenReturn(accountId);
+ return user;
+ }
+
+ private AccountVO mockAccount(long id) {
+ AccountVO account = Mockito.mock(AccountVO.class);
+ Mockito.when(account.getId()).thenReturn(id);
+ return account;
+ }
+
+ private ProjectAccountVO mockProjectAccount(long id, Role role) {
+ ProjectAccountVO projectAccount = Mockito.mock(ProjectAccountVO.class);
+ Mockito.when(projectAccount.getId()).thenReturn(id);
+ Mockito.when(projectAccount.getAccountRole()).thenReturn(role);
+ return projectAccount;
+ }
}
diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
index 61cdde697dd..379ae76d119 100644
--- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
+++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
@@ -2131,4 +2131,16 @@ public class AccountManagerImplTest extends
AccountManagentImplTestBase {
);
Assert.assertNotNull(userResultVO);
}
+
+ @Test
+ public void deleteAndCleanupUserTestUserCleanup() {
+ long userId = userVoMock.getId();
+ Mockito.doNothing().when(accountManagerImpl).removeUserApiKeys(userId);
+ Mockito.doReturn(true).when(_projectMgr).cleanupProjectsForUser(null,
userVoMock);
+
+ accountManagerImpl.deleteAndCleanupUser(userVoMock);
+
+ Mockito.verify(accountManagerImpl).removeUserApiKeys(userId);
+ Mockito.verify(_projectMgr).cleanupProjectsForUser(null, userVoMock);
+ }
}
