diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index d4c23e8d62be..2e0884cbce24 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -2112,14 +2112,30 @@ public boolean deleteUserAccount(long accountId) { protected void checkIfAccountManagesProjects(long accountId) { List managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId); - if (!CollectionUtils.isEmpty(managedProjectIds)) { - throw new InvalidParameterValueException(String.format( + + if (CollectionUtils.isEmpty(managedProjectIds)) { + return; + } + + List activeManagedProjects = new ArrayList<>(); + + for (Long projectId : managedProjectIds) { + ProjectVO project = _projectDao.findById(projectId); + if (project != null && project.getRemoved() == null) { + activeManagedProjects.add(projectId); + } + } + + if (!activeManagedProjects.isEmpty()) { + 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 + accountId, activeManagedProjects )); } } + protected boolean isDeleteNeeded(AccountVO account, long accountId, Account caller) { if (account == null) { 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 2aeb43469d19..44baf0cf4ac4 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -26,6 +26,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Date; + import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.Role; @@ -45,6 +47,7 @@ import org.apache.cloudstack.resourcedetail.UserDetailVO; import org.apache.cloudstack.webhook.WebhookHelper; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -54,8 +57,10 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.InjectMocks; import org.springframework.beans.factory.NoSuchBeanDefinitionException; + import com.cloud.acl.DomainChecker; import com.cloud.api.auth.SetupUserTwoFactorAuthenticationCmd; import com.cloud.domain.Domain; @@ -75,10 +80,16 @@ import com.cloud.vm.UserVmVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.projects.ProjectVO; +import com.cloud.projects.dao.ProjectAccountDao; +import com.cloud.projects.dao.ProjectDao; @RunWith(MockitoJUnitRunner.class) public class AccountManagerImplTest extends AccountManagetImplTestBase { + @InjectMocks + private AccountManagerImpl accountManagerImpl; + @Mock private UserVmManagerImpl _vmMgr; @Mock @@ -117,9 +128,16 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Mock private ProjectAccountVO projectAccountVO; + @Mock private Project project; + @Mock + private ProjectAccountDao _projectAccountDao; + + @Mock + private ProjectDao _projectDao; + @Mock PasswordPolicyImpl passwordPolicyMock; @@ -131,25 +149,29 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Mock RoleService roleService; + @BeforeClass + public static void initClass() { + } + + @Before public void setUp() throws Exception { - enableUserTwoFactorAuthenticationMock = Mockito.mock(ConfigKey.class); - accountManagerImpl.enableUserTwoFactorAuthentication = enableUserTwoFactorAuthenticationMock; - allowOperationsOnUsersInSameAccountMock = Mockito.mock(ConfigKey.class); + accountManagerImpl.enableUserTwoFactorAuthentication = enableUserTwoFactorAuthenticationMock; accountManagerImpl.allowOperationsOnUsersInSameAccount = allowOperationsOnUsersInSameAccountMock; - } - @Before - public void beforeTest() { Mockito.doReturn(accountMockId).when(accountMock).getId(); - Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); - Mockito.doReturn(accountMockId).when(userVoMock).getAccountId(); - Mockito.doReturn(userVoIdMock).when(userVoMock).getId(); - Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(accountMock, accountMock); + User callingUserMock = Mockito.mock(User.class); + CallContext.register(callingUserMock, accountMock); + + } + + @After + public void tearDown() { + CallContext.unregister(); } @Test @@ -795,7 +817,6 @@ public void valiateUserPasswordAndUpdateIfNeededTestBlankPassword() { @Test(expected = InvalidParameterValueException.class) public void valiateUserPasswordAndUpdateIfNeededTestNoAdminAndNoCurrentPasswordProvided() { - Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(false).when(accountManagerImpl).isDomainAdmin(accountMockId); Mockito.lenient().doReturn(true).when(accountManagerImpl).isResourceDomainAdmin(accountMockId); @@ -809,7 +830,6 @@ public void valiateUserPasswordAndUpdateIfNeededTestNoAdminAndNoCurrentPasswordP @Test(expected = CloudRuntimeException.class) public void valiateUserPasswordAndUpdateIfNeededTestNoUserAuthenticatorsConfigured() { - Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(false).when(accountManagerImpl).isDomainAdmin(accountMockId); @@ -824,7 +844,6 @@ public void valiateUserPasswordAndUpdateIfNeededTestNoUserAuthenticatorsConfigur @Test public void validateUserPasswordAndUpdateIfNeededTestRootAdminUpdatingUserPassword() { - Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(false).when(accountManagerImpl).isDomainAdmin(accountMockId); @@ -846,7 +865,6 @@ public void validateUserPasswordAndUpdateIfNeededTestRootAdminUpdatingUserPasswo @Test public void validateUserPasswordAndUpdateIfNeededTestDomainAdminUpdatingUserPassword() { - Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(true).when(accountManagerImpl).isDomainAdmin(accountMockId); @@ -868,7 +886,6 @@ public void validateUserPasswordAndUpdateIfNeededTestDomainAdminUpdatingUserPass @Test public void validateUserPasswordAndUpdateIfNeededTestUserUpdatingHisPassword() { - Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(false).when(accountManagerImpl).isDomainAdmin(accountMockId); @@ -1522,9 +1539,7 @@ public void testAssertUserNotAlreadyInDomain_UserExistsInDiffDomain() { @Test public void testCheckCallerRoleTypeAllowedToUpdateUserSameAccount() { - Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(accountMock); Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.DomainAdmin); - accountManagerImpl.checkCallerRoleTypeAllowedForUserOrAccountOperations(accountMock, userVoMock); } @@ -1533,7 +1548,6 @@ public void testCheckCallerRoleTypeAllowedToUpdateUserLowerAccountRoleType() { Account callingAccount = Mockito.mock(Account.class); Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(2L); Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(2L); - Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount); Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(callingAccount))).thenReturn(RoleType.DomainAdmin); Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.Admin); accountManagerImpl.checkCallerRoleTypeAllowedForUserOrAccountOperations(accountMock, userVoMock); @@ -1541,7 +1555,6 @@ public void testCheckCallerRoleTypeAllowedToUpdateUserLowerAccountRoleType() { @Test public void testcheckCallerApiPermissionsForUserOperationsRootAdminSameCaller() { - Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(accountMock); Mockito.when(accountMock.getId()).thenReturn(2L); Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(2L); accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); @@ -1549,7 +1562,6 @@ public void testcheckCallerApiPermissionsForUserOperationsRootAdminSameCaller() @Test(expected = PermissionDeniedException.class) public void testcheckCallerApiPermissionsForUserOperationsRootAdminDifferentAccount() { - Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount); Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L); Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L); Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L); @@ -1562,7 +1574,6 @@ public void testcheckCallerApiPermissionsForUserOperationsRootAdminDifferentAcco @Test public void testcheckCallerApiPermissionsForUserOperationsAllowedApis() { - Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount); Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L); Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L); Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L); @@ -1577,7 +1588,6 @@ public void testcheckCallerApiPermissionsForUserOperationsAllowedApis() { @Test(expected = PermissionDeniedException.class) public void testcheckCallerApiPermissionsForUserOperationsNotAllowedApis() { - Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount); Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L); Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L); Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L); @@ -1589,4 +1599,23 @@ public void testcheckCallerApiPermissionsForUserOperationsNotAllowedApis() { accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); } + + @Test + public void testCheckIfAccountManagesOnlyDeletedProjectsDoesNotThrow() { + long accountId = 42L; + long projectId = 100L; + + Mockito.when(_projectAccountDao.listAdministratedProjectIds(accountId)) + .thenReturn(List.of(projectId)); + + ProjectVO deletedProject = Mockito.mock(ProjectVO.class); + Mockito.when(deletedProject.getRemoved()).thenReturn(new Date()); + + Mockito.when(_projectDao.findById(projectId)) + .thenReturn(deletedProject); + + accountManagerImpl.checkIfAccountManagesProjects(accountId); + + } + }