From 001649e5fd4d28b3cc7c43dbc196b286f5939e16 Mon Sep 17 00:00:00 2001 From: Madawa Soysa Date: Thu, 29 Nov 2018 23:27:50 +1100 Subject: [PATCH] Fix issue with advanced search to pick the correct enrollment This commit addresses following two issues, 1. Advanced search feature returns the oldest enrollment instance This issue occurs when the same device is enrolled for multiple users. The cause of the issue is filtering the devices by device identifier which is a unique property for a device. This commit fixes the issue by filtering the devices from enrollment id and owner instead of the device identifier. 2. All devices do not return for privileged users when searched through advanced-search The cause of the issue is not considering the user permissions when filtering the devices. This commit fixes the issue by validating the permissions of the logged in user before filtering. Fixes entgra/product-iots#13 Fixes entgra/product-iots#14 --- .../core/search/mgt/impl/ProcessorImpl.java | 105 +++++++++--------- .../search/mgt/impl/QueryBuilderImpl.java | 4 +- .../mgt/impl/ResultSetAggregatorImpl.java | 47 +++++++- 3 files changed, 97 insertions(+), 59 deletions(-) diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ProcessorImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ProcessorImpl.java index e77f700fa8..0d65c3018b 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ProcessorImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ProcessorImpl.java @@ -256,7 +256,6 @@ public class ProcessorImpl implements Processor { PreparedStatement stmt = null; ResultSet rs = null; List devices = new ArrayList<>(); - Map devs = new HashMap<>(); try { conn = this.getConnection(); stmt = conn.prepareStatement(queryHolder.getQuery()); @@ -281,59 +280,57 @@ public class ProcessorImpl implements Processor { rs = stmt.executeQuery(); while (rs.next()) { - if (!devs.containsKey(rs.getInt("ID"))) { - Device device = new Device(); - device.setId(rs.getInt("ID")); - device.setDescription(rs.getString("DESCRIPTION")); - device.setName(rs.getString("NAME")); - device.setType(rs.getString("DEVICE_TYPE_NAME")); - device.setDeviceIdentifier(rs.getString("DEVICE_IDENTIFICATION")); - - EnrolmentInfo enrolmentInfo = new EnrolmentInfo(); - enrolmentInfo.setStatus(EnrolmentInfo.Status.valueOf(rs.getString("DE_STATUS"))); - enrolmentInfo.setOwner(rs.getString("OWNER")); - enrolmentInfo.setOwnership(EnrolmentInfo.OwnerShip.valueOf(rs.getString("OWNERSHIP"))); - device.setEnrolmentInfo(enrolmentInfo); - - DeviceIdentifier identifier = new DeviceIdentifier(); - identifier.setType(rs.getString("DEVICE_TYPE_NAME")); - identifier.setId(rs.getString("DEVICE_IDENTIFICATION")); - - DeviceInfo deviceInfo = new DeviceInfo(); - deviceInfo.setAvailableRAMMemory(rs.getDouble("AVAILABLE_RAM_MEMORY")); - deviceInfo.setBatteryLevel(rs.getDouble("BATTERY_LEVEL")); - deviceInfo.setConnectionType(rs.getString("CONNECTION_TYPE")); - deviceInfo.setCpuUsage(rs.getDouble("CPU_USAGE")); - deviceInfo.setDeviceModel(rs.getString("DEVICE_MODEL")); - deviceInfo.setExternalAvailableMemory(rs.getDouble("EXTERNAL_AVAILABLE_MEMORY")); - deviceInfo.setExternalTotalMemory(rs.getDouble("EXTERNAL_TOTAL_MEMORY")); - deviceInfo.setInternalAvailableMemory(rs.getDouble("INTERNAL_AVAILABLE_MEMORY")); - deviceInfo.setInternalTotalMemory(rs.getDouble("EXTERNAL_TOTAL_MEMORY")); - deviceInfo.setOsVersion(rs.getString("OS_VERSION")); - deviceInfo.setOsBuildDate(rs.getString("OS_BUILD_DATE")); - deviceInfo.setPluggedIn(rs.getBoolean("PLUGGED_IN")); - deviceInfo.setSsid(rs.getString("SSID")); - deviceInfo.setTotalRAMMemory(rs.getDouble("TOTAL_RAM_MEMORY")); - deviceInfo.setVendor(rs.getString("VENDOR")); - deviceInfo.setUpdatedTime(new java.util.Date(rs.getLong("UPDATE_TIMESTAMP"))); - - DeviceLocation deviceLocation = new DeviceLocation(); - deviceLocation.setLatitude(rs.getDouble("LATITUDE")); - deviceLocation.setLongitude(rs.getDouble("LONGITUDE")); - deviceLocation.setStreet1(rs.getString("STREET1")); - deviceLocation.setStreet2(rs.getString("STREET2")); - deviceLocation.setCity(rs.getString("CITY")); - deviceLocation.setState(rs.getString("STATE")); - deviceLocation.setZip(rs.getString("ZIP")); - deviceLocation.setCountry(rs.getString("COUNTRY")); - deviceLocation.setDeviceId(rs.getInt("ID")); - deviceLocation.setUpdatedTime(new java.util.Date(rs.getLong("DL_UPDATED_TIMESTAMP"))); - - deviceInfo.setLocation(deviceLocation); - device.setDeviceInfo(deviceInfo); - devices.add(device); - devs.put(device.getId(), device.getId()); - } + Device device = new Device(); + device.setId(rs.getInt("ID")); + device.setDescription(rs.getString("DESCRIPTION")); + device.setName(rs.getString("NAME")); + device.setType(rs.getString("DEVICE_TYPE_NAME")); + device.setDeviceIdentifier(rs.getString("DEVICE_IDENTIFICATION")); + + EnrolmentInfo enrolmentInfo = new EnrolmentInfo(); + enrolmentInfo.setId(rs.getInt("ENROLLMENT_ID")); + enrolmentInfo.setStatus(EnrolmentInfo.Status.valueOf(rs.getString("DE_STATUS"))); + enrolmentInfo.setOwner(rs.getString("OWNER")); + enrolmentInfo.setOwnership(EnrolmentInfo.OwnerShip.valueOf(rs.getString("OWNERSHIP"))); + device.setEnrolmentInfo(enrolmentInfo); + + DeviceIdentifier identifier = new DeviceIdentifier(); + identifier.setType(rs.getString("DEVICE_TYPE_NAME")); + identifier.setId(rs.getString("DEVICE_IDENTIFICATION")); + + DeviceInfo deviceInfo = new DeviceInfo(); + deviceInfo.setAvailableRAMMemory(rs.getDouble("AVAILABLE_RAM_MEMORY")); + deviceInfo.setBatteryLevel(rs.getDouble("BATTERY_LEVEL")); + deviceInfo.setConnectionType(rs.getString("CONNECTION_TYPE")); + deviceInfo.setCpuUsage(rs.getDouble("CPU_USAGE")); + deviceInfo.setDeviceModel(rs.getString("DEVICE_MODEL")); + deviceInfo.setExternalAvailableMemory(rs.getDouble("EXTERNAL_AVAILABLE_MEMORY")); + deviceInfo.setExternalTotalMemory(rs.getDouble("EXTERNAL_TOTAL_MEMORY")); + deviceInfo.setInternalAvailableMemory(rs.getDouble("INTERNAL_AVAILABLE_MEMORY")); + deviceInfo.setInternalTotalMemory(rs.getDouble("EXTERNAL_TOTAL_MEMORY")); + deviceInfo.setOsVersion(rs.getString("OS_VERSION")); + deviceInfo.setOsBuildDate(rs.getString("OS_BUILD_DATE")); + deviceInfo.setPluggedIn(rs.getBoolean("PLUGGED_IN")); + deviceInfo.setSsid(rs.getString("SSID")); + deviceInfo.setTotalRAMMemory(rs.getDouble("TOTAL_RAM_MEMORY")); + deviceInfo.setVendor(rs.getString("VENDOR")); + deviceInfo.setUpdatedTime(new java.util.Date(rs.getLong("UPDATE_TIMESTAMP"))); + + DeviceLocation deviceLocation = new DeviceLocation(); + deviceLocation.setLatitude(rs.getDouble("LATITUDE")); + deviceLocation.setLongitude(rs.getDouble("LONGITUDE")); + deviceLocation.setStreet1(rs.getString("STREET1")); + deviceLocation.setStreet2(rs.getString("STREET2")); + deviceLocation.setCity(rs.getString("CITY")); + deviceLocation.setState(rs.getString("STATE")); + deviceLocation.setZip(rs.getString("ZIP")); + deviceLocation.setCountry(rs.getString("COUNTRY")); + deviceLocation.setDeviceId(rs.getInt("ID")); + deviceLocation.setUpdatedTime(new java.util.Date(rs.getLong("DL_UPDATED_TIMESTAMP"))); + + deviceInfo.setLocation(deviceLocation); + device.setDeviceInfo(deviceInfo); + devices.add(device); } } catch (SQLException e) { throw new SearchDAOException("Error occurred while aquiring the device details.", e); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/QueryBuilderImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/QueryBuilderImpl.java index 65b6af46fa..27e6457a2f 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/QueryBuilderImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/QueryBuilderImpl.java @@ -331,7 +331,7 @@ public class QueryBuilderImpl implements QueryBuilder { "DD.EXTERNAL_TOTAL_MEMORY, DD.EXTERNAL_AVAILABLE_MEMORY, DD.CONNECTION_TYPE, \n" + "DD.SSID, DD.CPU_USAGE, DD.TOTAL_RAM_MEMORY, DD.AVAILABLE_RAM_MEMORY, \n" + "DD.PLUGGED_IN, DD.UPDATE_TIMESTAMP, DL.LATITUDE, DL.LONGITUDE, DL.STREET1, DL.STREET2, DL.CITY, DL.ZIP, \n" + - "DL.STATE, DL.COUNTRY, DL.UPDATE_TIMESTAMP AS DL_UPDATED_TIMESTAMP, DE.OWNER, DE.OWNERSHIP, DE.STATUS " + + "DL.STATE, DL.COUNTRY, DL.UPDATE_TIMESTAMP AS DL_UPDATED_TIMESTAMP, DE.ID AS ENROLLMENT_ID, DE.OWNER, DE.OWNERSHIP, DE.STATUS " + "AS DE_STATUS FROM DM_DEVICE_DETAIL DD INNER JOIN DM_DEVICE D ON D.ID=DD.DEVICE_ID\n" + "LEFT JOIN DM_DEVICE_LOCATION DL ON DL.DEVICE_ID=D.ID \n" + "INNER JOIN DM_DEVICE_TYPE DT ON DT.ID=D.DEVICE_TYPE_ID\n" + @@ -359,7 +359,7 @@ public class QueryBuilderImpl implements QueryBuilder { "DD.SSID, DD.CPU_USAGE, DD.TOTAL_RAM_MEMORY, DD.AVAILABLE_RAM_MEMORY, \n" + "DD.PLUGGED_IN, DD.UPDATE_TIMESTAMP, DL.LATITUDE, DL.LONGITUDE, DL.STREET1, DL.STREET2, DL.CITY, DL.ZIP, \n" + "DL.STATE, DL.COUNTRY, DL.UPDATE_TIMESTAMP AS DL_UPDATED_TIMESTAMP, DI.KEY_FIELD, DI.VALUE_FIELD, \n" + - "DE.OWNER, DE.OWNERSHIP, DE.STATUS AS DE_STATUS " + + "DE.ID ENROLLMENT_ID, DE.OWNER, DE.OWNERSHIP, DE.STATUS AS DE_STATUS " + "FROM DM_DEVICE_DETAIL DD INNER JOIN DM_DEVICE D ON D.ID=DD.DEVICE_ID\n" + "LEFT JOIN DM_DEVICE_LOCATION DL ON DL.DEVICE_ID=D.ID \n" + "INNER JOIN DM_DEVICE_TYPE DT ON DT.ID=D.DEVICE_TYPE_ID\n" + diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ResultSetAggregatorImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ResultSetAggregatorImpl.java index acc565a9af..df8e1a8757 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ResultSetAggregatorImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ResultSetAggregatorImpl.java @@ -19,9 +19,16 @@ package org.wso2.carbon.device.mgt.core.search.mgt.impl; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.wso2.carbon.context.PrivilegedCarbonContext; import org.wso2.carbon.device.mgt.common.Device; +import org.wso2.carbon.device.mgt.core.internal.DeviceManagementDataHolder; +import org.wso2.carbon.device.mgt.core.permission.mgt.PermissionUtils; import org.wso2.carbon.device.mgt.core.search.mgt.Constants; import org.wso2.carbon.device.mgt.core.search.mgt.ResultSetAggregator; +import org.wso2.carbon.user.api.UserRealm; +import org.wso2.carbon.user.api.UserStoreException; import java.util.ArrayList; import java.util.HashMap; @@ -29,16 +36,19 @@ import java.util.List; import java.util.Map; public class ResultSetAggregatorImpl implements ResultSetAggregator { + private static Log log = LogFactory.getLog(ResultSetAggregatorImpl.class); + private final static String ANY_DEVICE_PERMISSION = "/device-mgt/devices/any-device"; + private static final String UI_EXECUTE = "ui.execute"; @Override public List aggregate(Map> devices) { - Map generalQueryMap = this.convertToMap(devices.get(Constants.GENERAL)); Map andMap = this.convertToMap(devices.get(Constants.PROP_AND)); Map orMap = this.convertToMap(devices.get(Constants.PROP_OR)); Map locationMap = this.convertToMap(devices.get(Constants.LOCATION)); Map finalMap = new HashMap<>(); List finalResult = new ArrayList<>(); + List ownDevices = new ArrayList<>(); if (andMap.isEmpty()) { finalMap = generalQueryMap; @@ -70,7 +80,23 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator { } } - return finalResult; + String username = PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername(); + + try { + if (isPermittedToViewAnyDevice(username)) { + return finalResult; + } + } catch (UserStoreException e) { + log.error("Unable to check permissions of the user: " + username, e); + } + + for (Device device: finalResult) { + if (username.equals(device.getEnrolmentInfo().getOwner())) { + ownDevices.add(device); + } + } + + return ownDevices; } private Map convertToMap(List devices) { @@ -79,7 +105,7 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator { } Map deviceWrapperMap = new HashMap<>(); for (Device device : devices) { - deviceWrapperMap.put(device.getId(), device); + deviceWrapperMap.put(device.getEnrolmentInfo().getId(), device); } return deviceWrapperMap; } @@ -92,4 +118,19 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator { return list; } + /** + * Checks if the user has permissions to view all devices. + * + * @param username username + * @return {@code true} if user is permitted + * @throws UserStoreException If unable to check user permissions + */ + private boolean isPermittedToViewAnyDevice(String username) throws UserStoreException { + int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId(true); + UserRealm userRealm = DeviceManagementDataHolder.getInstance().getRealmService().getTenantUserRealm(tenantId); + return userRealm != null && userRealm.getAuthorizationManager() != null && + userRealm.getAuthorizationManager().isUserAuthorized(username, + PermissionUtils.getAbsolutePermissionPath(ANY_DEVICE_PERMISSION), UI_EXECUTE); + } + }