From 8985a1fc7c2bd18ce69a0ef3404f1688aeef99dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Thu, 14 May 2026 18:32:10 -0300 Subject: [PATCH] add access validation for the deleteUserData, linkUserDataToTemplate and resetUserDataForVirtualMachine APIs --- .../user/userdata/DeleteUserDataCmd.java | 17 +++++----- .../user/userdata/DeleteUserDataCmdTest.java | 31 +++++++++---------- .../cloud/template/TemplateManagerImpl.java | 11 ++++++- .../java/com/cloud/vm/UserVmManagerImpl.java | 10 +++++- .../template/TemplateManagerImplTest.java | 10 ++++++ .../com/cloud/vm/UserVmManagerImplTest.java | 1 - 6 files changed, 51 insertions(+), 29 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmd.java index f6d29e5dc40..220043b26b5 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmd.java @@ -17,6 +17,8 @@ package org.apache.cloudstack.api.command.user.userdata; import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.acl.SecurityChecker; +import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -27,7 +29,6 @@ import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.api.response.ProjectResponse; import org.apache.cloudstack.api.response.SuccessResponse; import org.apache.cloudstack.api.response.UserDataResponse; -import org.apache.cloudstack.context.CallContext; import com.cloud.user.Account; import com.cloud.user.UserData; @@ -43,6 +44,7 @@ public class DeleteUserDataCmd extends BaseCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// + @ACL(accessType = SecurityChecker.AccessType.OperateEntry) @Parameter(name = ApiConstants.ID, type = CommandType.UUID, required = true, entityType = UserDataResponse.class, description = "The ID of the Userdata") private Long id; @@ -97,18 +99,13 @@ public class DeleteUserDataCmd extends BaseCmd { @Override public long getEntityOwnerId() { - Account account = CallContext.current().getCallingAccount(); - if ((account == null || _accountService.isAdmin(account.getId())) && (domainId != null && accountName != null)) { - Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId); - if (userAccount != null) { - return userAccount.getId(); + if (id != null) { + UserData userData = _entityMgr.findById(UserData.class, id); + if (userData != null) { + return userData.getAccountId(); } } - if (account != null) { - return account.getId(); - } - return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked } } diff --git a/api/src/test/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmdTest.java index 255c9f4ca12..9639211fddf 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmdTest.java @@ -17,11 +17,11 @@ package org.apache.cloudstack.api.command.user.userdata; import com.cloud.server.ManagementService; -import com.cloud.user.Account; import com.cloud.user.AccountService; +import com.cloud.user.UserData; +import com.cloud.utils.db.EntityManager; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.SuccessResponse; -import org.apache.cloudstack.context.CallContext; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -29,7 +29,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.mockito.junit.MockitoJUnitRunner; @@ -46,6 +45,12 @@ public class DeleteUserDataCmdTest { @Mock ManagementService _mgr; + @Mock + private EntityManager entityManagerMock; + + @Mock + private UserData userDataMock; + private static final long DOMAIN_ID = 5L; private static final long PROJECT_ID = 10L; private static final String ACCOUNT_NAME = "user"; @@ -84,19 +89,13 @@ public class DeleteUserDataCmdTest { } @Test - public void validateArgsCmd() { - try (MockedStatic callContextMocked = Mockito.mockStatic(CallContext.class)) { - CallContext callContextMock = Mockito.mock(CallContext.class); - callContextMocked.when(CallContext::current).thenReturn(callContextMock); - Account accountMock = Mockito.mock(Account.class); - Mockito.when(callContextMock.getCallingAccount()).thenReturn(accountMock); - Mockito.when(accountMock.getId()).thenReturn(2L); - Mockito.doReturn(false).when(_accountService).isAdmin(2L); + public void getEntityOwnerIdTestReturnUserDataOwnerWhenUserDataIdIsProvided() { + long userDataId = 1L; + long userDataOwnerId = 2L; + ReflectionTestUtils.setField(cmd, "id", userDataId); + Mockito.when(entityManagerMock.findById(UserData.class, userDataId)).thenReturn(userDataMock); + Mockito.when(userDataMock.getAccountId()).thenReturn(userDataOwnerId); - ReflectionTestUtils.setField(cmd, "id", 1L); - - Assert.assertEquals(1L, (long) cmd.getId()); - Assert.assertEquals(2L, cmd.getEntityOwnerId()); - } + Assert.assertEquals(userDataOwnerId, cmd.getEntityOwnerId()); } } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index ee0b2ff1321..d88bbd09bc2 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -36,6 +36,7 @@ import javax.naming.ConfigurationException; import com.cloud.cpu.CPU; import com.cloud.resourcelimit.CheckedReservation; +import com.cloud.user.dao.UserDataDao; import com.cloud.utils.UriUtils; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.ApiConstants; @@ -329,6 +330,9 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, @Inject private ReservationDao reservationDao; + @Inject + private UserDataDao userDataDao; + private TemplateAdapter getAdapter(HypervisorType type) { TemplateAdapter adapter = null; if (type == HypervisorType.BareMetal) { @@ -2532,12 +2536,17 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, _accountMgr.checkAccess(caller, AccessType.OperateEntry, true, template); - template.setUserDataId(userDataId); if (userDataId != null) { + UserData userData = userDataDao.findById(userDataId); + if (userData == null) { + throw new InvalidParameterValueException("Unable to find user data with the specified ID."); + } + _accountMgr.checkAccess(caller, null, false, userData); template.setUserDataLinkPolicy(overridePolicy); } else { template.setUserDataLinkPolicy(null); } + template.setUserDataId(userDataId); _tmpltDao.update(template.getId(), template); return _tmpltDao.findById(template.getId()); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 7e58cd01050..1754041a314 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -965,8 +965,16 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir throw new InvalidParameterValueException(String.format("VM %s should be stopped to do UserData reset", userVm)); } - String userData = cmd.getUserData(); Long userDataId = cmd.getUserdataId(); + if (userDataId != null) { + UserData userData = userDataDao.findById(userDataId); + if (userData == null) { + throw new InvalidParameterValueException("Unable to find user data with the specified ID."); + } + _accountMgr.checkAccess(caller, null, false, userData); + } + + String userData = cmd.getUserData(); String userDataDetails = null; if (MapUtils.isNotEmpty(cmd.getUserdataDetails())) { userDataDetails = cmd.getUserdataDetails().toString(); diff --git a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java index 353a4393700..513c3a72c65 100755 --- a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java +++ b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java @@ -50,7 +50,9 @@ import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; import com.cloud.user.User; import com.cloud.user.UserData; +import com.cloud.user.UserDataVO; import com.cloud.user.UserVO; +import com.cloud.user.dao.UserDataDao; import com.cloud.utils.UriUtils; import com.cloud.utils.concurrency.NamedThreadFactory; import com.cloud.utils.exception.CloudRuntimeException; @@ -180,6 +182,12 @@ public class TemplateManagerImplTest extends TestCase { @Mock HeuristicRuleHelper heuristicRuleHelperMock; + @Mock + private UserDataDao userDataDaoMock; + + @Mock + private UserDataVO userDataMock; + public class CustomThreadPoolExecutor extends ThreadPoolExecutor { AtomicInteger ai = new AtomicInteger(0); public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, @@ -484,6 +492,8 @@ public class TemplateManagerImplTest extends TestCase { VMTemplateVO template = Mockito.mock(VMTemplateVO.class); when(vmTemplateDao.findById(anyLong())).thenReturn(template); + when(userDataDaoMock.findById(anyLong())).thenReturn(userDataMock); + VirtualMachineTemplate resultTemplate = templateManager.linkUserDataToTemplate(cmd); Assert.assertEquals(template, resultTemplate); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index b21cd53d743..3616b85ce3e 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -949,7 +949,6 @@ public class UserVmManagerImplTest { when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Stopped); - when(cmd.getUserData()).thenReturn("testUserdata"); when(cmd.getUserdataId()).thenReturn(1L); try {