This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new 0d6cae6 volume: fix volume metrics view from returning sensitive
info to end user (#3222)
0d6cae6 is described below
commit 0d6cae6339a3eddff9d116064b36ca6b88e4a3c5
Author: Dingane Hlaluku <[email protected]>
AuthorDate: Wed Jun 19 13:34:26 2019 +0200
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.
---
.../cloudstack/api/ListVolumesMetricsCmd.java | 23 ++++++++------
.../cloudstack/metrics/MetricsServiceImpl.java | 10 +++++-
.../java/com/cloud/api/query/QueryManagerImpl.java | 16 ++--------
.../com/cloud/api/query/ViewResponseHelper.java | 37 +++++++++++-----------
.../com/cloud/api/query/dao/VolumeJoinDaoImpl.java | 4 ++-
ui/scripts/metrics.js | 11 ++++++-
ui/scripts/ui/widgets/listView.js | 4 ++-
7 files changed, 60 insertions(+), 45 deletions(-)
diff --git
a/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesMetricsCmd.java
b/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesMetricsCmd.java
index 54ac922..df0adec 100644
---
a/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesMetricsCmd.java
+++
b/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesMetricsCmd.java
@@ -17,19 +17,24 @@
package org.apache.cloudstack.api;
+import java.util.List;
+
+import javax.inject.Inject;
+
import org.apache.cloudstack.acl.RoleType;
-import org.apache.cloudstack.api.command.admin.volume.ListVolumesCmdByAdmin;
+import org.apache.cloudstack.api.command.user.volume.ListVolumesCmd;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.metrics.MetricsService;
import org.apache.cloudstack.response.VolumeMetricsResponse;
-import javax.inject.Inject;
-import java.util.List;
-
-@APICommand(name = ListVolumesMetricsCmd.APINAME, description = "Lists volume
metrics", responseObject = VolumeMetricsResponse.class,
- requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
responseView = ResponseObject.ResponseView.Full,
- since = "4.9.3", authorized = {RoleType.Admin,
RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
-public class ListVolumesMetricsCmd extends ListVolumesCmdByAdmin {
+@APICommand(name = ListVolumesMetricsCmd.APINAME,
+ description = "Lists volume metrics",
+ responseObject = VolumeMetricsResponse.class,
+ requestHasSensitiveInfo = false,
+ responseHasSensitiveInfo = false,
+ since = "4.9.3",
+ authorized = {RoleType.Admin, RoleType.ResourceAdmin,
RoleType.DomainAdmin, RoleType.User})
+public class ListVolumesMetricsCmd extends ListVolumesCmd {
public static final String APINAME = "listVolumesMetrics";
@Inject
@@ -41,7 +46,7 @@ public class ListVolumesMetricsCmd extends
ListVolumesCmdByAdmin {
}
@Override
- public void execute() {
+ public void execute() {
final List<VolumeMetricsResponse> metricsResponses =
metricsService.listVolumeMetrics(_queryService.searchForVolumes(this).getResponses());
ListResponse<VolumeMetricsResponse> response = new ListResponse<>();
response.setResponses(metricsResponses, metricsResponses.size());
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 37f1f55..029d909 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
@@ -38,6 +38,8 @@ import com.cloud.host.dao.HostDao;
import com.cloud.org.Cluster;
import com.cloud.org.Grouping;
import com.cloud.org.Managed;
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
import com.cloud.utils.component.ComponentLifecycleBase;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
@@ -58,6 +60,7 @@ import org.apache.cloudstack.api.response.StoragePoolResponse;
import org.apache.cloudstack.api.response.UserVmResponse;
import org.apache.cloudstack.api.response.VolumeResponse;
import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.response.ClusterMetricsResponse;
import org.apache.cloudstack.response.HostMetricsResponse;
import org.apache.cloudstack.response.InfrastructureResponse;
@@ -97,6 +100,8 @@ public class MetricsServiceImpl extends
ComponentLifecycleBase implements Metric
@Inject
private CapacityDao capacityDao;
@Inject
+ private AccountManager accountMgr;
+ @Inject
private ManagementServerHostDao managementServerHostDao;
protected MetricsServiceImpl() {
@@ -158,7 +163,10 @@ public class MetricsServiceImpl extends
ComponentLifecycleBase implements Metric
}
metricsResponse.setDiskSizeGB(volumeResponse.getSize());
- metricsResponse.setStorageType(volumeResponse.getStorageType(),
volumeResponse.getVolumeType());
+ Account account = CallContext.current().getCallingAccount();
+ if (accountMgr.isAdmin(account.getAccountId())) {
+
metricsResponse.setStorageType(volumeResponse.getStorageType(),
volumeResponse.getVolumeType());
+ }
metricsResponses.add(metricsResponse);
}
return metricsResponses;
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 2cf72e2..9f67194 100644
--- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
+++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
@@ -50,7 +50,6 @@ import
org.apache.cloudstack.api.command.admin.storage.ListStorageTagsCmd;
import
org.apache.cloudstack.api.command.admin.template.ListTemplatesCmdByAdmin;
import org.apache.cloudstack.api.command.admin.user.ListUsersCmd;
import org.apache.cloudstack.api.command.admin.vm.ListVMsCmdByAdmin;
-import org.apache.cloudstack.api.command.admin.volume.ListVolumesCmdByAdmin;
import org.apache.cloudstack.api.command.admin.zone.ListZonesCmdByAdmin;
import org.apache.cloudstack.api.command.user.account.ListAccountsCmd;
import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd;
@@ -168,7 +167,6 @@ import com.cloud.exception.PermissionDeniedException;
import com.cloud.ha.HighAvailabilityManager;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.network.dao.NetworkDetailsDao;
import com.cloud.network.security.SecurityGroupVMMapVO;
import com.cloud.network.security.dao.SecurityGroupVMMapDao;
import com.cloud.org.Grouping;
@@ -208,7 +206,6 @@ import com.cloud.utils.DateUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.StringUtils;
import com.cloud.utils.Ternary;
-import com.cloud.utils.db.EntityManager;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.JoinBuilder;
import com.cloud.utils.db.SearchBuilder;
@@ -221,7 +218,6 @@ import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.dao.DomainRouterDao;
import com.cloud.vm.dao.UserVmDao;
-import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.dao.VMInstanceDao;
@Component
@@ -334,9 +330,6 @@ public class QueryManagerImpl extends
MutualExclusiveIdsManagerBase implements Q
private DomainRouterDao _routerDao;
@Inject
- private UserVmDetailsDao _userVmDetailDao;
-
- @Inject
private HighAvailabilityManager _haMgr;
@Inject
@@ -369,18 +362,12 @@ public class QueryManagerImpl extends
MutualExclusiveIdsManagerBase implements Q
private AffinityGroupDomainMapDao _affinityGroupDomainMapDao;
@Inject
- private NetworkDetailsDao _networkDetailsDao;
-
- @Inject
private ResourceTagDao _resourceTagDao;
@Inject
private DataStoreManager dataStoreManager;
@Inject
- private EntityManager _entityMgr;
-
- @Inject
ManagementServerHostDao managementServerHostDao;
/*
@@ -1665,7 +1652,8 @@ public class QueryManagerImpl extends
MutualExclusiveIdsManagerBase implements Q
ListResponse<VolumeResponse> response = new
ListResponse<VolumeResponse>();
ResponseView respView = ResponseView.Restricted;
- if (cmd instanceof ListVolumesCmdByAdmin) {
+ Account account = CallContext.current().getCallingAccount();
+ if (_accountMgr.isAdmin(account.getAccountId())) {
respView = ResponseView.Full;
}
diff --git a/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
b/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
index 949bc17..3ac9c8f 100644
--- a/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
+++ b/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
@@ -284,29 +284,30 @@ public class ViewResponseHelper {
}
vrDataList.put(vr.getId(), vrData);
- if (view == ResponseView.Full) {
- VolumeStats vs = null;
- if (vr.getFormat() == ImageFormat.QCOW2) {
- vs = ApiDBUtils.getVolumeStatistics(vrData.getId());
- }
- else if (vr.getFormat() == ImageFormat.VHD){
- vs = ApiDBUtils.getVolumeStatistics(vrData.getPath());
- }
- else if (vr.getFormat() == ImageFormat.OVA){
- if (vrData.getChainInfo() != null) {
- vs =
ApiDBUtils.getVolumeStatistics(vrData.getChainInfo());
- }
+ VolumeStats vs = null;
+ if (vr.getFormat() == ImageFormat.QCOW2) {
+ vs = ApiDBUtils.getVolumeStatistics(vrData.getId());
+ }
+ else if (vr.getFormat() == ImageFormat.VHD){
+ vs = ApiDBUtils.getVolumeStatistics(vrData.getPath());
+ }
+ else if (vr.getFormat() == ImageFormat.OVA){
+ if (vrData.getChainInfo() != null) {
+ vs = ApiDBUtils.getVolumeStatistics(vrData.getChainInfo());
}
- if (vs != null){
- long vsz = vs.getVirtualSize();
- long psz = vs.getPhysicalSize() ;
- double util = (double)psz/vsz;
+ }
+
+ if (vs != null){
+ long vsz = vs.getVirtualSize();
+ long psz = vs.getPhysicalSize() ;
+ double util = (double)psz/vsz;
+ vrData.setUtilization(df.format(util));
+
+ if (view == ResponseView.Full) {
vrData.setVirtualsize(vsz);
vrData.setPhysicalsize(psz);
- vrData.setUtilization(df.format(util));
}
}
-
}
return new ArrayList<VolumeResponse>(vrDataList.values());
}
diff --git
a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java
b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java
index 6ed9be9..61e9752 100644
--- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java
+++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java
@@ -185,7 +185,9 @@ public class VolumeJoinDaoImpl extends
GenericDaoBaseWithTagInformation<VolumeJo
volResponse.setDiskOfferingDisplayText(volume.getDiskOfferingDisplayText());
}
- volResponse.setStorageType(volume.isUseLocalStorage() ?
ServiceOffering.StorageType.local.toString() :
ServiceOffering.StorageType.shared.toString());
+ if (view == ResponseView.Full) {
+ volResponse.setStorageType(volume.isUseLocalStorage() ?
ServiceOffering.StorageType.local.toString() :
ServiceOffering.StorageType.shared.toString());
+ }
volResponse.setBytesReadRate(volume.getBytesReadRate());
volResponse.setBytesWriteRate(volume.getBytesReadRate());
volResponse.setIopsReadRate(volume.getIopsWriteRate());
diff --git a/ui/scripts/metrics.js b/ui/scripts/metrics.js
index aa67b03..c3c68e1 100644
--- a/ui/scripts/metrics.js
+++ b/ui/scripts/metrics.js
@@ -553,6 +553,15 @@
cloudStack.sections.metrics.volumes = {
title: 'label.metrics',
listView: {
+ preFilter: function(args) {
+ var hiddenFields = [];
+ if (!isAdmin()) {
+ hiddenFields.push('physicalsize');
+ hiddenFields.push('storage');
+ hiddenFields.push('storagetype');
+ }
+ return hiddenFields;
+ },
id: 'volumes',
fields: {
name: {
@@ -598,7 +607,7 @@
},
storage: {
label: 'label.metrics.storagepool'
- },
+ }
},
dataProvider: function(args) {
var data = {listAll: true};
diff --git a/ui/scripts/ui/widgets/listView.js
b/ui/scripts/ui/widgets/listView.js
index 7162b93..25bdc06 100644
--- a/ui/scripts/ui/widgets/listView.js
+++ b/ui/scripts/ui/widgets/listView.js
@@ -862,8 +862,10 @@
if (groupableColumns) {
$tr.addClass('groupable-header-columns').addClass('groupable-header');
$.each(fields, function(key) {
- if ($.inArray(key, hiddenFields) != -1)
+ if ($.inArray(key, hiddenFields) != -1) {
return true;
+ }
+
var field = this;
if (field.columns) {
var colspan = Object.keys(field.columns).length;
