Fix deleteAccount API to prevent deletion of the caller (#8743)

* Fix API deleteAccount deleting caller's account

* Remove extra line

* Add unit tests and change message

* apply suggestion

Co-authored-by: Bernardo De Marco Gonçalves <bernardomg2004@gmail.com>

* remove extra parentheses

Co-authored-by: Bernardo De Marco Gonçalves <bernardomg2004@gmail.com>

* Update server/src/test/java/com/cloud/user/AccountManagerImplTest.java

Co-authored-by: Bernardo De Marco Gonçalves <bernardomg2004@gmail.com>

---------

Co-authored-by: Lucas Martins <lucas.martins@scclouds.com.br>
Co-authored-by: Bernardo De Marco Gonçalves <bernardomg2004@gmail.com>
This commit is contained in:
Lucas Martins 2024-08-27 16:13:30 -03:00 committed by GitHub
parent ede39d8edc
commit 70131be8c2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 45 additions and 6 deletions

View File

@ -89,12 +89,11 @@ public class DeleteAccountCmd extends BaseAsyncCmd {
CallContext.current().setEventDetails("Account ID: " + (account != null ? account.getUuid() : getId())); // Account not found is already handled by service
boolean result = _regionService.deleteUserAccount(this);
if (result) {
SuccessResponse response = new SuccessResponse(getCommandName());
setResponseObject(response);
} else {
if (!result) {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete user account and all corresponding users");
}
SuccessResponse response = new SuccessResponse(getCommandName());
setResponseObject(response);
}
@Override

View File

@ -1842,7 +1842,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
// If the user is a System user, return an error. We do not allow this
AccountVO account = _accountDao.findById(accountId);
if (! isDeleteNeeded(account, accountId, caller)) {
if (caller.getId() == accountId) {
Domain domain = _domainDao.findById(account.getDomainId());
throw new InvalidParameterValueException(String.format("Deletion of your own account is not allowed. To delete account %s (ID: %s, Domain: %s), " +
"request to another user with permissions to perform the operation.",
account.getAccountName(), account.getUuid(), domain.getUuid()));
}
if (!isDeleteNeeded(account, accountId, caller)) {
return true;
}
@ -1862,7 +1869,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
return deleteAccount(account, callerUserId, caller);
}
private boolean isDeleteNeeded(AccountVO account, long accountId, Account caller) {
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 ));
return false;

View File

@ -206,6 +206,39 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
Mockito.verify(_accountDao, Mockito.atLeastOnce()).markForCleanup(Mockito.eq(42l));
}
@Test (expected = InvalidParameterValueException.class)
public void deleteUserAccountTestIfAccountIdIsEqualToCallerIdShouldThrowException() {
try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
CallContext callContextMock = Mockito.mock(CallContext.class);
callContextMocked.when(CallContext::current).thenReturn(callContextMock);
long accountId = 1L;
Mockito.doReturn(accountVoMock).when(callContextMock).getCallingAccount();
Mockito.doReturn(accountVoMock).when(_accountDao).findById(Mockito.anyLong());
Mockito.doReturn(domainVoMock).when(_domainDao).findById(Mockito.anyLong());
Mockito.doReturn(1L).when(accountVoMock).getId();
accountManagerImpl.deleteUserAccount(accountId);
}
}
@Test
public void deleteUserAccountTestIfAccountIdIsNotEqualToCallerAccountIdShouldNotThrowException() {
try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
CallContext callContextMock = Mockito.mock(CallContext.class);
callContextMocked.when(CallContext::current).thenReturn(callContextMock);
long accountId = 1L;
Mockito.doReturn(accountVoMock).when(callContextMock).getCallingAccount();
Mockito.doReturn(accountVoMock).when(_accountDao).findById(Mockito.anyLong());
Mockito.doReturn(2L).when(accountVoMock).getId();
Mockito.doReturn(true).when(accountManagerImpl).isDeleteNeeded(Mockito.any(), Mockito.anyLong(), Mockito.any());
Mockito.doReturn(new ArrayList<Long>()).when(_projectAccountDao).listAdministratedProjectIds(Mockito.anyLong());
accountManagerImpl.deleteUserAccount(accountId);
}
}
@Test (expected = InvalidParameterValueException.class)
public void deleteUserTestIfUserIdIsEqualToCallerIdShouldThrowException() {
try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {