Skip to content

[4.20] Fix account deletion blocked by deleted project admin mappings #12615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SURYAS1306 wants to merge 13 commits into
base: 4.20
Choose a base branch
from
+70 −25
Open

[4.20] Fix account deletion blocked by deleted project admin mappings #12615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d57e1f8
Fix account deletion blocked by deleted project admin mappings
Feb 7, 2026
29a09c2
Fix indentation per review
Feb 9, 2026
c645292
Update server/src/main/java/com/cloud/user/AccountManagerImpl.java
SURYAS1306 Feb 9, 2026
235efbe
Fix test mocks to use injected _projectAccountDao and _projectDao
Feb 10, 2026
77cb72f
Fix missing imports for ProjectAccountDao, ProjectDao and InjectMocks
Feb 10, 2026
58ffd9d
Fix AccountManagerImplTest by spying AccountManagerImpl to allow stub…
Feb 10, 2026
769bbb0
Fix AccountManagerImplTest spy initialization order
Feb 11, 2026
7f3fd1f
Remove unused org.mockito.Spy import
Feb 11, 2026
15531e1
Remove manual Mockito.mock calls and rely on @Mock injection
Feb 12, 2026
f6613a8
test: fix Mockito NotMock errors by using proper @BeforeClass/@Before…
Feb 12, 2026
ebf966a
fix checkstyle issue in AccountManagerImplTest
Feb 13, 2026
3b8969a
test: add missing BeforeClass import
Feb 13, 2026
f2aec9c
test: stop mocking getCurrentCallingAccount and use CallContext
Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions server/src/main/java/com/cloud/user/AccountManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2112,14 +2112,30 @@ public boolean deleteUserAccount(long accountId) {

protected void checkIfAccountManagesProjects(long accountId) {
List<Long> managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId);
if (!CollectionUtils.isEmpty(managedProjectIds)) {
throw new InvalidParameterValueException(String.format(

if (CollectionUtils.isEmpty(managedProjectIds)) {
return;
}

List<Long> 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 ));
Expand Down
73 changes: 51 additions & 22 deletions server/src/test/java/com/cloud/user/AccountManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;

Expand All @@ -131,25 +149,29 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
@Mock
RoleService roleService;

@BeforeClass
Copy link
Contributor

@DaanHoogland DaanHoogland Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you forget to import it @SURYAS1306 ?

[INFO] -------------------------------------------------------------
Error:  /home/runner/work/cloudstack/cloudstack/server/src/test/java/com/cloud/user/AccountManagerImplTest.java:[151,5] error: cannot find symbol
  symbol:   class BeforeClass
  location: class AccountManagerImplTest

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
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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);
}

Expand All @@ -1533,23 +1548,20 @@ 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);
}

@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);
}

@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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);

}

}
Loading