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>
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java
index 36e22ac..a90fc4a 100644
--- a/api/src/main/java/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java
@@ -89,12 +89,11 @@
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
diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
index 0552739..07d06fb 100644
--- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
@@ -1842,7 +1842,14 @@
// 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 @@
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;
diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
index 9d78009..db4fbed 100644
--- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
+++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
@@ -207,6 +207,39 @@
}
@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)) {
DeleteUserCmd cmd = Mockito.mock(DeleteUserCmd.class);
