This is an automated email from the ASF dual-hosted git repository. weizhou pushed a commit to branch 4.20 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit c8d44d92a76efe6dd197128dab8faf5f0f8ac85b Author: Abhishek Kumar <[email protected]> AuthorDate: Wed Sep 17 02:34:31 2025 +0530 api,server: fix entity access Added access check for: - createNetworkACL - listNetworkACLs - listResourceDetails - listVirtualMachinesUsageHistory - listVolumesUsageHistory Signed-off-by: Abhishek Kumar <[email protected]> --- .../java/com/cloud/server/ResourceManagerUtil.java | 1 + .../cloudstack/metrics/MetricsServiceImpl.java | 76 +++++++++- .../cloudstack/metrics/MetricsServiceImplTest.java | 163 +++++++++++++++++---- .../java/com/cloud/api/query/QueryManagerImpl.java | 2 +- .../cloud/network/vpc/NetworkACLServiceImpl.java | 3 + .../com/cloud/tags/ResourceManagerUtilImpl.java | 11 ++ 6 files changed, 221 insertions(+), 35 deletions(-) diff --git a/api/src/main/java/com/cloud/server/ResourceManagerUtil.java b/api/src/main/java/com/cloud/server/ResourceManagerUtil.java index 9a3b51a70d5..f5081cbe307 100644 --- a/api/src/main/java/com/cloud/server/ResourceManagerUtil.java +++ b/api/src/main/java/com/cloud/server/ResourceManagerUtil.java @@ -18,6 +18,7 @@ package com.cloud.server; public interface ResourceManagerUtil { long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType); + long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType, boolean checkAccess); String getUuid(String resourceId, ResourceTag.ResourceObjectType resourceType); ResourceTag.ResourceObjectType getResourceType(String resourceTypeStr); void checkResourceAccessible(Long accountId, Long domainId, String exceptionMessage); diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java index 0ef094d3d4e..c305d9312ba 100644 --- a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java +++ b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java @@ -22,6 +22,7 @@ import static com.cloud.utils.NumbersUtil.toReadableSize; import java.lang.reflect.InvocationTargetException; import java.text.DecimalFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -32,8 +33,6 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.dc.ClusterVO; -import com.cloud.utils.Ternary; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ListClustersMetricsCmd; import org.apache.cloudstack.api.ListDbMetricsCmd; @@ -100,6 +99,7 @@ import com.cloud.capacity.CapacityManager; import com.cloud.capacity.dao.CapacityDao; import com.cloud.capacity.dao.CapacityDaoImpl; import com.cloud.cluster.dao.ManagementServerHostDao; +import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenter; import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; @@ -112,6 +112,7 @@ import com.cloud.host.Status; import com.cloud.host.dao.HostDao; import com.cloud.network.router.VirtualRouter; import com.cloud.org.Cluster; +import com.cloud.projects.Project; import com.cloud.server.DbStatsCollection; import com.cloud.server.ManagementServerHostStats; import com.cloud.server.StatsCollector; @@ -124,6 +125,7 @@ import com.cloud.usage.dao.UsageJobDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; import com.cloud.utils.db.DbProperties; import com.cloud.utils.db.DbUtil; import com.cloud.utils.db.Filter; @@ -184,6 +186,10 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements private static Gson gson = new Gson(); + private final List<Account.Type> AccountTypesWithRecursiveUsageAccess = Arrays.asList( + Account.Type.ADMIN, Account.Type.DOMAIN_ADMIN, Account.Type.READ_ONLY_ADMIN + ); + protected MetricsServiceImpl() { super(); } @@ -245,17 +251,30 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements * @return the list of VMs. */ protected Pair<List<UserVmVO>, Integer> searchForUserVmsInternal(ListVMsUsageHistoryCmd cmd) { + final Long id = cmd.getId(); + Account caller = CallContext.current().getCallingAccount(); + List<Long> permittedAccounts = new ArrayList<>(); + boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType()); + Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, recursive, null); + accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false); + Long domainId = domainIdRecursiveListProject.first(); + Boolean isRecursive = domainIdRecursiveListProject.second(); + Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + Filter searchFilter = new Filter(UserVmVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); String name = cmd.getName(); String keyword = cmd.getKeyword(); SearchBuilder<UserVmVO> sb = userVmDao.createSearchBuilder(); + sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); // select distinct + accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN); sb.and("displayName", sb.entity().getDisplayName(), SearchCriteria.Op.LIKE); sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); SearchCriteria<UserVmVO> sc = sb.create(); + accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); if (CollectionUtils.isNotEmpty(ids)) { sc.setParameters("idIN", ids.toArray()); } @@ -269,7 +288,14 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements sc.addAnd("displayName", SearchCriteria.Op.SC, ssc); } - return userVmDao.searchAndCount(sc, searchFilter); + Pair<List<UserVmVO>, Integer> uniqueVmPair = userVmDao.searchAndCount(sc, searchFilter); + Integer count = uniqueVmPair.second(); + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + List<Long> vmIds = uniqueVmPair.first().stream().map(UserVmVO::getId).collect(Collectors.toList()); + List<UserVmVO> vms = userVmDao.listByIds(vmIds); + return new Pair<>(vms, count); } /** @@ -331,17 +357,49 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements * @return the list of VMs. */ protected Pair<List<VolumeVO>, Integer> searchForVolumesInternal(ListVolumesUsageHistoryCmd cmd) { + final Long id = cmd.getId(); + Account caller = CallContext.current().getCallingAccount(); + List<Long> permittedAccounts = new ArrayList<>(); + boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType()); + Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, recursive, null); + accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false); + Long domainId = domainIdRecursiveListProject.first(); + Boolean isRecursive = domainIdRecursiveListProject.second(); + Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + Filter searchFilter = new Filter(VolumeVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); String name = cmd.getName(); String keyword = cmd.getKeyword(); SearchBuilder<VolumeVO> sb = volumeDao.createSearchBuilder(); + sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); // select distinct + accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN); sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); + boolean shouldListSystemVmVolumes = accountMgr.isRootAdmin(CallContext.current().getCallingAccountId()); + List<Long> vmIds = new ArrayList<>(); + if (!shouldListSystemVmVolumes) { + SearchBuilder<UserVmVO> vmSearch = userVmDao.createSearchBuilder(); + vmSearch.select(null, SearchCriteria.Func.DISTINCT, vmSearch.entity().getId()); + accountMgr.buildACLSearchBuilder(vmSearch, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + SearchCriteria<UserVmVO> vmSc = vmSearch.create(); + accountMgr.buildACLSearchCriteria(vmSc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + List<UserVmVO> vms = userVmDao.search(vmSc, null); + vmIds = vms.stream().map(UserVmVO::getId).collect(Collectors.toList()); + if (vmIds.isEmpty()) { + sb.and("instanceIdNull", sb.entity().getInstanceId(), SearchCriteria.Op.NULL); + } else { + sb.and().op("instanceIdNull", sb.entity().getInstanceId(), SearchCriteria.Op.NULL); + sb.or("instanceIds", sb.entity().getInstanceId(), SearchCriteria.Op.IN); + sb.cp(); + } + } + SearchCriteria<VolumeVO> sc = sb.create(); + accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); if (CollectionUtils.isNotEmpty(ids)) { sc.setParameters("idIN", ids.toArray()); } @@ -354,8 +412,18 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements ssc.addOr("state", SearchCriteria.Op.EQ, keyword); sc.addAnd("name", SearchCriteria.Op.SC, ssc); } + if (!shouldListSystemVmVolumes && CollectionUtils.isNotEmpty(vmIds)) { + sc.setParameters("instanceIds", vmIds.toArray()); + } - return volumeDao.searchAndCount(sc, searchFilter); + Pair<List<VolumeVO>, Integer> uniqueVolumePair = volumeDao.searchAndCount(sc, searchFilter); + Integer count = uniqueVolumePair.second(); + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + List<Long> volumeIds = uniqueVolumePair.first().stream().map(VolumeVO::getId).collect(Collectors.toList()); + List<VolumeVO> volumes = volumeDao.listByIds(volumeIds); + return new Pair<>(volumes, count); } /** diff --git a/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java b/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java index b37be68b3e4..9184d744410 100644 --- a/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java +++ b/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java @@ -19,13 +19,16 @@ package org.apache.cloudstack.metrics; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.cloudstack.api.ListVMsUsageHistoryCmd; +import org.apache.cloudstack.api.ListVolumesUsageHistoryCmd; import org.apache.cloudstack.api.response.ListResponse; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.response.VmMetricsStatsResponse; import org.apache.commons.lang3.time.DateUtils; import org.junit.Assert; @@ -35,12 +38,18 @@ import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; import com.cloud.utils.Pair; +import com.cloud.utils.db.Filter; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.vm.UserVmVO; @@ -75,6 +84,12 @@ public class MetricsServiceImplTest { @Mock VmStatsDao vmStatsDaoMock; + @Mock + AccountManager accountManager; + + @Mock + VolumeDao volumeDao; + @Captor ArgumentCaptor<String> stringCaptor1, stringCaptor2; @@ -95,12 +110,26 @@ public class MetricsServiceImplTest { @Mock VmStatsVO vmStatsVOMock; + @Mock + Account mockAccount; + @Mock + ListVolumesUsageHistoryCmd listVolumesUsageHistoryCmdMock; + @Mock + VolumeVO volumeVOMock; + @Mock + SearchBuilder<VolumeVO> volumeSearchBuilderMock; + @Mock + SearchCriteria<VolumeVO> volumeSearchCriteriaMock; + @Mock + Filter filterMock; + private void prepareSearchCriteriaWhenUseSetParameters() { Mockito.doNothing().when(scMock).setParameters(Mockito.anyString(), Mockito.any()); } private void preparesearchForUserVmsInternalTest() { + Mockito.when(mockAccount.getType()).thenReturn(Account.Type.NORMAL); expectedVmListAndCounter = new Pair<>(Arrays.asList(userVmVOMock), 1); Mockito.doReturn(1L).when(listVMsUsageHistoryCmdMock).getStartIndex(); @@ -111,8 +140,10 @@ public class MetricsServiceImplTest { Mockito.doReturn(userVmVOMock).when(sbMock).entity(); Mockito.doReturn(scMock).when(sbMock).create(); - Mockito.doReturn(new Pair<List<UserVmVO>, Integer>(Arrays.asList(userVmVOMock), 1)) + Mockito.doReturn(expectedVmListAndCounter) .when(userVmDaoMock).searchAndCount(Mockito.any(), Mockito.any()); + Mockito.doReturn(expectedVmListAndCounter.first()) + .when(userVmDaoMock).listByIds(Mockito.anyList()); } @Test @@ -124,12 +155,17 @@ public class MetricsServiceImplTest { Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName(); Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword(); - Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - - Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); - Assert.assertEquals("idIN", stringCaptor1.getValue()); - Assert.assertEquals(fakeVmId1, objectArrayCaptor.getAllValues().get(0)[0]); - Assert.assertEquals(expectedVmListAndCounter, result); + try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); + + Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); + Assert.assertEquals("idIN", stringCaptor1.getValue()); + Assert.assertEquals(fakeVmId1, objectArrayCaptor.getAllValues().get(0)[0]); + Assert.assertEquals(expectedVmListAndCounter, result); + } } @Test @@ -141,13 +177,17 @@ public class MetricsServiceImplTest { Mockito.doReturn(expected).when(listVMsUsageHistoryCmdMock).getIds(); Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName(); Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword(); - - Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - - Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); - Assert.assertEquals("idIN", stringCaptor1.getValue()); - Assert.assertArrayEquals(expected.toArray(), objectArrayCaptor.getAllValues().get(0)); - Assert.assertEquals(expectedVmListAndCounter, result); + try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); + + Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); + Assert.assertEquals("idIN", stringCaptor1.getValue()); + Assert.assertArrayEquals(expected.toArray(), objectArrayCaptor.getAllValues().get(0)); + Assert.assertEquals(expectedVmListAndCounter, result); + } } @Test @@ -159,12 +199,17 @@ public class MetricsServiceImplTest { Mockito.doReturn("fakeName").when(listVMsUsageHistoryCmdMock).getName(); Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword(); - Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - - Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); - Assert.assertEquals("displayName", stringCaptor1.getValue()); - Assert.assertEquals("%fakeName%", objectArrayCaptor.getValue()[0]); - Assert.assertEquals(expectedVmListAndCounter, result); + try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); + + Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); + Assert.assertEquals("displayName", stringCaptor1.getValue()); + Assert.assertEquals("%fakeName%", objectArrayCaptor.getValue()[0]); + Assert.assertEquals(expectedVmListAndCounter, result); + } } @Test @@ -177,16 +222,21 @@ public class MetricsServiceImplTest { Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName(); Mockito.doReturn("fakeKeyword").when(listVMsUsageHistoryCmdMock).getKeyword(); - Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - - Mockito.verify(scMock, Mockito.times(2)).addOr(stringCaptor1.capture(), opCaptor.capture(), objectArrayCaptor.capture()); - List<String> conditions = stringCaptor1.getAllValues(); - List<Object[]> params = objectArrayCaptor.getAllValues(); - Assert.assertEquals("displayName", conditions.get(0)); - Assert.assertEquals("state", conditions.get(1)); - Assert.assertEquals("%fakeKeyword%", params.get(0)[0]); - Assert.assertEquals("fakeKeyword", params.get(1)[0]); - Assert.assertEquals(expectedVmListAndCounter, result); + try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); + + Mockito.verify(scMock, Mockito.times(2)).addOr(stringCaptor1.capture(), opCaptor.capture(), objectArrayCaptor.capture()); + List<String> conditions = stringCaptor1.getAllValues(); + List<Object[]> params = objectArrayCaptor.getAllValues(); + Assert.assertEquals("displayName", conditions.get(0)); + Assert.assertEquals("state", conditions.get(1)); + Assert.assertEquals("%fakeKeyword%", params.get(0)[0]); + Assert.assertEquals("fakeKeyword", params.get(1)[0]); + Assert.assertEquals(expectedVmListAndCounter, result); + } } @Test @@ -317,4 +367,57 @@ public class MetricsServiceImplTest { spy.createStatsResponse(Arrays.asList(vmStatsVOMock)); } + + @Test + public void searchForVolumesInternalWithValidParameters() { + Mockito.doReturn(null).when(listVolumesUsageHistoryCmdMock).getId(); + Mockito.doReturn(Arrays.asList(1L, 2L)).when(listVolumesUsageHistoryCmdMock).getIds(); + Mockito.doReturn("volumeName").when(listVolumesUsageHistoryCmdMock).getName(); + Mockito.doReturn("keyword").when(listVolumesUsageHistoryCmdMock).getKeyword(); + Mockito.doReturn(volumeSearchBuilderMock).when(volumeDao).createSearchBuilder(); + Mockito.doReturn(volumeVOMock).when(volumeSearchBuilderMock).entity(); + SearchBuilder vmSearchBuilderMock = Mockito.mock(SearchBuilder.class); + Mockito.doReturn(vmSearchBuilderMock).when(userVmDaoMock).createSearchBuilder(); + Mockito.doReturn(userVmVOMock).when(vmSearchBuilderMock).entity(); + Mockito.doReturn(volumeSearchCriteriaMock).when(volumeSearchBuilderMock).create(); + Mockito.doReturn(volumeSearchCriteriaMock).when(volumeDao).createSearchCriteria(); + Mockito.doReturn(new Pair<>(Arrays.asList(volumeVOMock), 1)).when(volumeDao).searchAndCount(Mockito.any(), Mockito.any()); + Mockito.doReturn(Arrays.asList(volumeVOMock)).when(volumeDao).listByIds(Mockito.anyList()); + + + try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair<List<VolumeVO>, Integer> result = spy.searchForVolumesInternal(listVolumesUsageHistoryCmdMock); + + Assert.assertNotNull(result); + Assert.assertEquals(1, result.second().intValue()); + Assert.assertEquals(volumeVOMock, result.first().get(0)); + } + } + + @Test + public void searchForVolumesInternalWithValidParametersNoItem() { + Mockito.doReturn(1L).when(listVolumesUsageHistoryCmdMock).getId(); + Mockito.doReturn(volumeSearchBuilderMock).when(volumeDao).createSearchBuilder(); + Mockito.doReturn(volumeVOMock).when(volumeSearchBuilderMock).entity(); + Mockito.doReturn(volumeSearchCriteriaMock).when(volumeSearchBuilderMock).create(); + Mockito.doReturn(new Pair<>(Collections.emptyList(), 0)).when(volumeDao).searchAndCount(Mockito.any(), Mockito.any()); + + + try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Mockito.when(callContextMock.getCallingAccountId()).thenReturn(1L); + Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true); + Mockito.when(mockAccount.getType()).thenReturn(Account.Type.ADMIN); + Pair<List<VolumeVO>, Integer> result = spy.searchForVolumesInternal(listVolumesUsageHistoryCmdMock); + + Assert.assertNotNull(result); + Assert.assertEquals(0, result.second().intValue()); + } + } + } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 6fb9ab515cb..6ed2fed9cb6 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -5335,7 +5335,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q //Validation - 1.3 if (resourceIdStr != null) { - resourceId = resourceManagerUtil.getResourceId(resourceIdStr, resourceType); + resourceId = resourceManagerUtil.getResourceId(resourceIdStr, resourceType, true); } List<? extends ResourceDetail> detailList = new ArrayList<>(); diff --git a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java index 0791ca7ecd3..f99ea6b9b8c 100644 --- a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java @@ -483,6 +483,8 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ throw new InvalidParameterValueException("Cannot create Network ACL Item. ACL Id or network Id is required"); } Network network = networkModel.getNetwork(createNetworkACLCmd.getNetworkId()); + Account caller = CallContext.current().getCallingAccount(); + _accountMgr.checkAccess(caller, null, true, network); if (network.getVpcId() == null) { throw new InvalidParameterValueException("Network: " + network.getUuid() + " does not belong to VPC"); } @@ -744,6 +746,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ if (networkId != null) { final Network network = _networkDao.findById(networkId); + _accountMgr.checkAccess(caller, null, true, network); aclId = network.getNetworkACLId(); if (aclId == null) { // No aclId associated with the network. diff --git a/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java b/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java index c02f4136863..82d69992cdf 100644 --- a/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java +++ b/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java @@ -22,6 +22,7 @@ import java.util.Objects; import javax.inject.Inject; +import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.api.Identity; import org.apache.cloudstack.api.InternalIdentity; import org.apache.cloudstack.context.CallContext; @@ -128,6 +129,11 @@ public class ResourceManagerUtilImpl implements ResourceManagerUtil { @Override public long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType) { + return getResourceId(resourceId, resourceType, false); + } + + @Override + public long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType, boolean checkAccess) { Class<?> clazz = s_typeMap.get(resourceType); Object entity = entityMgr.findByUuid(clazz, resourceId); if (entity != null) { @@ -138,6 +144,11 @@ public class ResourceManagerUtilImpl implements ResourceManagerUtil { } entity = entityMgr.findById(clazz, resourceId); if (entity != null) { + if (checkAccess && entity instanceof ControlledEntity) { + ControlledEntity controlledEntity = (ControlledEntity)entity; + Account caller = CallContext.current().getCallingAccount(); + accountMgr.checkAccess(caller, null, false, controlledEntity); + } return ((InternalIdentity)entity).getId(); } throw new InvalidParameterValueException("Unable to find resource by id " + resourceId + " and type " + resourceType);
