diff --git a/api/src/main/java/com/cloud/projects/ProjectService.java b/api/src/main/java/com/cloud/projects/ProjectService.java index d11e9ae0446d..17413de320c3 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.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 aab20f108f9e..36e1cccca5e9 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 @@ 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 ApiCommandResourceType getApiResourceType() { } @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 b947b61c8f02..47dbb34dd629 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 @@ 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 4710a815f978..0e2f32d5f8b5 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 long getProjectAccountId() { 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 887939311b24..09f19b046c84 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 Long getForAccountId() { 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 f4b2f6460020..cbc300aa56cf 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 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 { @@ -47,9 +48,11 @@ public interface ProjectAccountDao extends GenericDao { void removeAccountFromProjects(long accountId); - void removeUserFromProjects(long userId); - boolean canUserModifyProject(long projectId, long accountId, long userId); List listUsersOrAccountsByRole(long id); + + List 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 b6eb6d44cea8..4de64eef4ca5 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 @@ import java.util.List; +import com.cloud.user.User; import org.springframework.stereotype.Component; import com.cloud.projects.ProjectAccount; @@ -192,17 +193,6 @@ public void removeAccountFromProjects(long accountId) { } } - @Override - public void removeUserFromProjects(long userId) { - SearchCriteria 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 sc = AllFieldsSearch.create(); @@ -222,4 +212,23 @@ public List listUsersOrAccountsByRole(long id) { sc.setParameters("projectRoleId", id); return listBy(sc); } + + @Override + public List listBy(Long projectId, Long accountId, Long userId) { + SearchCriteria 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 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 976d53998e2e..aba2a881e967 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 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 { @@ -43,4 +44,9 @@ public interface ProjectInvitationDao extends GenericDao listInvitationsToExpire(long timeOut); + int removeBy(Long projectId, Long accountId, Long userId); + + List 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 d30b1c9f1f10..17e841967fb0 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 @@ 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 List listInvitationsToExpire(long timeOut) { return listBy(sc); } + @Override + public int removeBy(Long projectId, Long accountId, Long userId) { + SearchCriteria sc = prepareAllFieldsSearchCriteria(projectId, accountId, userId); + return remove(sc); + } + + @Override + public List listBy(Long projectId, Long accountId, Long userId) { + SearchCriteria sc = prepareAllFieldsSearchCriteria(projectId, accountId, userId); + return listBy(sc); + } + + @Override + public void move(User oldUser, User newUser) { + List 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 prepareAllFieldsSearchCriteria(Long projectId, Long accountId, Long userId) { + SearchCriteria 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 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 e2b066af7800..f96c885c5bb2 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`); +DELETE FROM `cloud`.`project_invitations` WHERE `user_id` IN (SELECT `id` FROM `cloud`.`user` WHERE `removed`); diff --git a/server/src/main/java/com/cloud/projects/ProjectManager.java b/server/src/main/java/com/cloud/projects/ProjectManager.java index 5f58205208be..17a443befaec 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 @@ 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 92af441d06b9..9ba5402443ef 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 boolean addUserToProject(Long projectId, String username, String email, L } } + /** + * 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 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 boolean deleteUserFromProject(long projectId, long userId) { //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) 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() { - @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 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 eca1a571dd88..e3840e297254 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.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 @@ void buildACLViewSearchCriteria(SearchCriteria s * 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 @@ void buildACLViewSearchCriteria(SearchCriteria s ConfigKey UseSecretKeyInResponse = new ConfigKey("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 db9c1d1dafde..e4d835f74b11 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.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 boolean deleteUser(DeleteUserCmd deleteUserCmd) { 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) 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 boolean moveUser(MoveUserCmd cmd) { } @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 boolean moveUser(long id, Long domainId, Account newAccount) { 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) 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 fedd66d94401..4e7eaad9f2c0 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 @@ 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 3085f655943f..a49ba4085cb2 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 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 boolean deleteUser(DeleteUserCmd cmd) { * {@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 982395637e35..f0db79f7cee7 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 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 boolean deleteUser(DeleteUserCmd cmd) { * {@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 0abcf9591d44..36a2e78d461b 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.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 long getInvitationTimeout() { 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 b9b568facc28..ca6c2193fbd5 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.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.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 updateProjects; @Before @@ -128,4 +156,158 @@ public void testDeleteWebhooksForAccountNoBean() { 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 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 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 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 61cdde697dd0..5953323cb99e 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 void testCreateUserSuccess() { ); Assert.assertNotNull(userResultVO); } + + @Test + public void deleteAndCleanupUserTestUserCleanup() { + long userId = userVoMock.getId(); + Mockito.doNothing().when(accountManagerImpl).removeUserApiKeys(userId); + Mockito.doNothing().when(_projectMgr).cleanupProjectsForUser(null, userVoMock); + + accountManagerImpl.deleteAndCleanupUser(userVoMock); + + Mockito.verify(accountManagerImpl).removeUserApiKeys(userId); + Mockito.verify(_projectMgr).cleanupProjectsForUser(null, userVoMock); + } }