Skip to content

Commit 0d6cae6

Browse files
dhlalukuyadvr
dhlaluku
authored and
yadvr
committed
volume: fix volume metrics view from returning sensitive info to end user (#3222)
Problem: The listVolumeMetrics API response does not honor the volume detail visibility restrictions set for normal users and returns sensitive information which should only be visible to the root admin. Root Cause: The listVolumeMetrics API response extends the ListVolumesByAdmin API internally and this results in a full display view response that is only meant for the root admin. Solution: This has been fixed by rectifying the API response to not show ‘physical size’, 'storage type', and ‘storage pool’ information. The UI has also been fixed to hide these columns for normal users.
1 parent 0e87040 commit 0d6cae6

7 files changed

Lines changed: 60 additions & 45 deletions

File tree

‎plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesMetricsCmd.java‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,24 @@
1717

1818
package org.apache.cloudstack.api;
1919

20+
import java.util.List;
21+
22+
import javax.inject.Inject;
23+
2024
import org.apache.cloudstack.acl.RoleType;
21-
import org.apache.cloudstack.api.command.admin.volume.ListVolumesCmdByAdmin;
25+
import org.apache.cloudstack.api.command.user.volume.ListVolumesCmd;
2226
import org.apache.cloudstack.api.response.ListResponse;
2327
import org.apache.cloudstack.metrics.MetricsService;
2428
import org.apache.cloudstack.response.VolumeMetricsResponse;
2529

26-
import javax.inject.Inject;
27-
import java.util.List;
28-
29-
@APICommand(name = ListVolumesMetricsCmd.APINAME, description = "Lists volume metrics", responseObject = VolumeMetricsResponse.class,
30-
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, responseView = ResponseObject.ResponseView.Full,
31-
since = "4.9.3", authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
32-
public class ListVolumesMetricsCmd extends ListVolumesCmdByAdmin {
30+
@APICommand(name = ListVolumesMetricsCmd.APINAME,
31+
description = "Lists volume metrics",
32+
responseObject = VolumeMetricsResponse.class,
33+
requestHasSensitiveInfo = false,
34+
responseHasSensitiveInfo = false,
35+
since = "4.9.3",
36+
authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
37+
public class ListVolumesMetricsCmd extends ListVolumesCmd {
3338
public static final String APINAME = "listVolumesMetrics";
3439

3540
@Inject
@@ -41,7 +46,7 @@ public String getCommandName() {
4146
}
4247

4348
@Override
44-
public void execute() {
49+
public void execute() {
4550
final List<VolumeMetricsResponse> metricsResponses = metricsService.listVolumeMetrics(_queryService.searchForVolumes(this).getResponses());
4651
ListResponse<VolumeMetricsResponse> response = new ListResponse<>();
4752
response.setResponses(metricsResponses, metricsResponses.size());

‎plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
import com.cloud.org.Cluster;
3939
import com.cloud.org.Grouping;
4040
import com.cloud.org.Managed;
41+
import com.cloud.user.Account;
42+
import com.cloud.user.AccountManager;
4143
import com.cloud.utils.component.ComponentLifecycleBase;
4244
import com.cloud.vm.VMInstanceVO;
4345
import com.cloud.vm.VirtualMachine;
@@ -58,6 +60,7 @@
5860
import org.apache.cloudstack.api.response.UserVmResponse;
5961
import org.apache.cloudstack.api.response.VolumeResponse;
6062
import org.apache.cloudstack.api.response.ZoneResponse;
63+
import org.apache.cloudstack.context.CallContext;
6164
import org.apache.cloudstack.response.ClusterMetricsResponse;
6265
import org.apache.cloudstack.response.HostMetricsResponse;
6366
import org.apache.cloudstack.response.InfrastructureResponse;
@@ -97,6 +100,8 @@ public class MetricsServiceImpl extends ComponentLifecycleBase implements Metric
97100
@Inject
98101
private CapacityDao capacityDao;
99102
@Inject
103+
private AccountManager accountMgr;
104+
@Inject
100105
private ManagementServerHostDao managementServerHostDao;
101106

102107
protected MetricsServiceImpl() {
@@ -158,7 +163,10 @@ public List<VolumeMetricsResponse> listVolumeMetrics(List<VolumeResponse> volume
158163
}
159164

160165
metricsResponse.setDiskSizeGB(volumeResponse.getSize());
161-
metricsResponse.setStorageType(volumeResponse.getStorageType(), volumeResponse.getVolumeType());
166+
Account account = CallContext.current().getCallingAccount();
167+
if (accountMgr.isAdmin(account.getAccountId())) {
168+
metricsResponse.setStorageType(volumeResponse.getStorageType(), volumeResponse.getVolumeType());
169+
}
162170
metricsResponses.add(metricsResponse);
163171
}
164172
return metricsResponses;

‎server/src/main/java/com/cloud/api/query/QueryManagerImpl.java‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import org.apache.cloudstack.api.command.admin.template.ListTemplatesCmdByAdmin;
5151
import org.apache.cloudstack.api.command.admin.user.ListUsersCmd;
5252
import org.apache.cloudstack.api.command.admin.vm.ListVMsCmdByAdmin;
53-
import org.apache.cloudstack.api.command.admin.volume.ListVolumesCmdByAdmin;
5453
import org.apache.cloudstack.api.command.admin.zone.ListZonesCmdByAdmin;
5554
import org.apache.cloudstack.api.command.user.account.ListAccountsCmd;
5655
import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd;
@@ -168,7 +167,6 @@
168167
import com.cloud.ha.HighAvailabilityManager;
169168
import com.cloud.hypervisor.Hypervisor;
170169
import com.cloud.hypervisor.Hypervisor.HypervisorType;
171-
import com.cloud.network.dao.NetworkDetailsDao;
172170
import com.cloud.network.security.SecurityGroupVMMapVO;
173171
import com.cloud.network.security.dao.SecurityGroupVMMapDao;
174172
import com.cloud.org.Grouping;
@@ -208,7 +206,6 @@
208206
import com.cloud.utils.Pair;
209207
import com.cloud.utils.StringUtils;
210208
import com.cloud.utils.Ternary;
211-
import com.cloud.utils.db.EntityManager;
212209
import com.cloud.utils.db.Filter;
213210
import com.cloud.utils.db.JoinBuilder;
214211
import com.cloud.utils.db.SearchBuilder;
@@ -221,7 +218,6 @@
221218
import com.cloud.vm.VirtualMachine;
222219
import com.cloud.vm.dao.DomainRouterDao;
223220
import com.cloud.vm.dao.UserVmDao;
224-
import com.cloud.vm.dao.UserVmDetailsDao;
225221
import com.cloud.vm.dao.VMInstanceDao;
226222

227223
@Component
@@ -333,9 +329,6 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
333329
@Inject
334330
private DomainRouterDao _routerDao;
335331

336-
@Inject
337-
private UserVmDetailsDao _userVmDetailDao;
338-
339332
@Inject
340333
private HighAvailabilityManager _haMgr;
341334

@@ -368,18 +361,12 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
368361
@Inject
369362
private AffinityGroupDomainMapDao _affinityGroupDomainMapDao;
370363

371-
@Inject
372-
private NetworkDetailsDao _networkDetailsDao;
373-
374364
@Inject
375365
private ResourceTagDao _resourceTagDao;
376366

377367
@Inject
378368
private DataStoreManager dataStoreManager;
379369

380-
@Inject
381-
private EntityManager _entityMgr;
382-
383370
@Inject
384371
ManagementServerHostDao managementServerHostDao;
385372

@@ -1665,7 +1652,8 @@ public ListResponse<VolumeResponse> searchForVolumes(ListVolumesCmd cmd) {
16651652
ListResponse<VolumeResponse> response = new ListResponse<VolumeResponse>();
16661653

16671654
ResponseView respView = ResponseView.Restricted;
1668-
if (cmd instanceof ListVolumesCmdByAdmin) {
1655+
Account account = CallContext.current().getCallingAccount();
1656+
if (_accountMgr.isAdmin(account.getAccountId())) {
16691657
respView = ResponseView.Full;
16701658
}
16711659

‎server/src/main/java/com/cloud/api/query/ViewResponseHelper.java‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -284,29 +284,30 @@ public static List<VolumeResponse> createVolumeResponse(ResponseView view, Volum
284284
}
285285
vrDataList.put(vr.getId(), vrData);
286286

287-
if (view == ResponseView.Full) {
288-
VolumeStats vs = null;
289-
if (vr.getFormat() == ImageFormat.QCOW2) {
290-
vs = ApiDBUtils.getVolumeStatistics(vrData.getId());
291-
}
292-
else if (vr.getFormat() == ImageFormat.VHD){
293-
vs = ApiDBUtils.getVolumeStatistics(vrData.getPath());
294-
}
295-
else if (vr.getFormat() == ImageFormat.OVA){
296-
if (vrData.getChainInfo() != null) {
297-
vs = ApiDBUtils.getVolumeStatistics(vrData.getChainInfo());
298-
}
287+
VolumeStats vs = null;
288+
if (vr.getFormat() == ImageFormat.QCOW2) {
289+
vs = ApiDBUtils.getVolumeStatistics(vrData.getId());
290+
}
291+
else if (vr.getFormat() == ImageFormat.VHD){
292+
vs = ApiDBUtils.getVolumeStatistics(vrData.getPath());
293+
}
294+
else if (vr.getFormat() == ImageFormat.OVA){
295+
if (vrData.getChainInfo() != null) {
296+
vs = ApiDBUtils.getVolumeStatistics(vrData.getChainInfo());
299297
}
300-
if (vs != null){
301-
long vsz = vs.getVirtualSize();
302-
long psz = vs.getPhysicalSize() ;
303-
double util = (double)psz/vsz;
298+
}
299+
300+
if (vs != null){
301+
long vsz = vs.getVirtualSize();
302+
long psz = vs.getPhysicalSize() ;
303+
double util = (double)psz/vsz;
304+
vrData.setUtilization(df.format(util));
305+
306+
if (view == ResponseView.Full) {
304307
vrData.setVirtualsize(vsz);
305308
vrData.setPhysicalsize(psz);
306-
vrData.setUtilization(df.format(util));
307309
}
308310
}
309-
310311
}
311312
return new ArrayList<VolumeResponse>(vrDataList.values());
312313
}

‎server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,9 @@ public VolumeResponse newVolumeResponse(ResponseView view, VolumeJoinVO volume)
185185
volResponse.setDiskOfferingDisplayText(volume.getDiskOfferingDisplayText());
186186
}
187187

188-
volResponse.setStorageType(volume.isUseLocalStorage() ? ServiceOffering.StorageType.local.toString() : ServiceOffering.StorageType.shared.toString());
188+
if (view == ResponseView.Full) {
189+
volResponse.setStorageType(volume.isUseLocalStorage() ? ServiceOffering.StorageType.local.toString() : ServiceOffering.StorageType.shared.toString());
190+
}
189191
volResponse.setBytesReadRate(volume.getBytesReadRate());
190192
volResponse.setBytesWriteRate(volume.getBytesReadRate());
191193
volResponse.setIopsReadRate(volume.getIopsWriteRate());

‎ui/scripts/metrics.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,15 @@
553553
cloudStack.sections.metrics.volumes = {
554554
title: 'label.metrics',
555555
listView: {
556+
preFilter: function(args) {
557+
var hiddenFields = [];
558+
if (!isAdmin()) {
559+
hiddenFields.push('physicalsize');
560+
hiddenFields.push('storage');
561+
hiddenFields.push('storagetype');
562+
}
563+
return hiddenFields;
564+
},
556565
id: 'volumes',
557566
fields: {
558567
name: {
@@ -598,7 +607,7 @@
598607
},
599608
storage: {
600609
label: 'label.metrics.storagepool'
601-
},
610+
}
602611
},
603612
dataProvider: function(args) {
604613
var data = {listAll: true};

‎ui/scripts/ui/widgets/listView.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -862,8 +862,10 @@
862862
if (groupableColumns) {
863863
$tr.addClass('groupable-header-columns').addClass('groupable-header');
864864
$.each(fields, function(key) {
865-
if ($.inArray(key, hiddenFields) != -1)
865+
if ($.inArray(key, hiddenFields) != -1) {
866866
return true;
867+
}
868+
867869
var field = this;
868870
if (field.columns) {
869871
var colspan = Object.keys(field.columns).length;

0 commit comments

Comments
 (0)