This is an automated email from the ASF dual-hosted git repository.
joao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 70131be8c22 Fix `deleteAccount` API to prevent deletion of the caller
(#8743)
70131be8c22 is described below
commit 70131be8c22b7c22c3cd97d5b74390d280797d41
Author: Lucas Martins <[email protected]>
AuthorDate: Tue Aug 27 16:13:30 2024 -0300
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 <[email protected]>
* remove extra parentheses
Co-authored-by: Bernardo De Marco Gonçalves <[email protected]>
* Update server/src/test/java/com/cloud/user/AccountManagerImplTest.java
Co-authored-by: Bernardo De Marco Gonçalves <[email protected]>
---------
Co-authored-by: Lucas Martins <[email protected]>
Co-authored-by: Bernardo De Marco Gonçalves <[email protected]>
---
.../command/admin/account/DeleteAccountCmd.java | 7 ++---
.../java/com/cloud/user/AccountManagerImpl.java | 11 ++++++--
.../com/cloud/user/AccountManagerImplTest.java | 33 ++++++++++++++++++++++
3 files changed, 45 insertions(+), 6 deletions(-)
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 36e22acff91..a90fc4aebe9 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 @@ 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
diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
index 0552739a05e..07d06fbd2f7 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 @@ 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;
diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
index 9d780096abf..db4fbed5320 100644
--- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
+++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
@@ -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)) {
