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
merge-requests/36/head
Madawa Soysa 6 years ago
parent 0dbe2b30d7
commit 001649e5fd

@ -256,7 +256,6 @@ public class ProcessorImpl implements Processor {
PreparedStatement stmt = null; PreparedStatement stmt = null;
ResultSet rs = null; ResultSet rs = null;
List<Device> devices = new ArrayList<>(); List<Device> devices = new ArrayList<>();
Map<Integer, Integer> devs = new HashMap<>();
try { try {
conn = this.getConnection(); conn = this.getConnection();
stmt = conn.prepareStatement(queryHolder.getQuery()); stmt = conn.prepareStatement(queryHolder.getQuery());
@ -281,7 +280,6 @@ public class ProcessorImpl implements Processor {
rs = stmt.executeQuery(); rs = stmt.executeQuery();
while (rs.next()) { while (rs.next()) {
if (!devs.containsKey(rs.getInt("ID"))) {
Device device = new Device(); Device device = new Device();
device.setId(rs.getInt("ID")); device.setId(rs.getInt("ID"));
device.setDescription(rs.getString("DESCRIPTION")); device.setDescription(rs.getString("DESCRIPTION"));
@ -290,6 +288,7 @@ public class ProcessorImpl implements Processor {
device.setDeviceIdentifier(rs.getString("DEVICE_IDENTIFICATION")); device.setDeviceIdentifier(rs.getString("DEVICE_IDENTIFICATION"));
EnrolmentInfo enrolmentInfo = new EnrolmentInfo(); EnrolmentInfo enrolmentInfo = new EnrolmentInfo();
enrolmentInfo.setId(rs.getInt("ENROLLMENT_ID"));
enrolmentInfo.setStatus(EnrolmentInfo.Status.valueOf(rs.getString("DE_STATUS"))); enrolmentInfo.setStatus(EnrolmentInfo.Status.valueOf(rs.getString("DE_STATUS")));
enrolmentInfo.setOwner(rs.getString("OWNER")); enrolmentInfo.setOwner(rs.getString("OWNER"));
enrolmentInfo.setOwnership(EnrolmentInfo.OwnerShip.valueOf(rs.getString("OWNERSHIP"))); enrolmentInfo.setOwnership(EnrolmentInfo.OwnerShip.valueOf(rs.getString("OWNERSHIP")));
@ -332,8 +331,6 @@ public class ProcessorImpl implements Processor {
deviceInfo.setLocation(deviceLocation); deviceInfo.setLocation(deviceLocation);
device.setDeviceInfo(deviceInfo); device.setDeviceInfo(deviceInfo);
devices.add(device); devices.add(device);
devs.put(device.getId(), device.getId());
}
} }
} catch (SQLException e) { } catch (SQLException e) {
throw new SearchDAOException("Error occurred while aquiring the device details.", e); throw new SearchDAOException("Error occurred while aquiring the device details.", e);

@ -331,7 +331,7 @@ public class QueryBuilderImpl implements QueryBuilder {
"DD.EXTERNAL_TOTAL_MEMORY, DD.EXTERNAL_AVAILABLE_MEMORY, DD.CONNECTION_TYPE, \n" + "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.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" + "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" + "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" + "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" + "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.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" + "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" + "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" + "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" + "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" + "INNER JOIN DM_DEVICE_TYPE DT ON DT.ID=D.DEVICE_TYPE_ID\n" +

@ -19,9 +19,16 @@
package org.wso2.carbon.device.mgt.core.search.mgt.impl; 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.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.Constants;
import org.wso2.carbon.device.mgt.core.search.mgt.ResultSetAggregator; 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.ArrayList;
import java.util.HashMap; import java.util.HashMap;
@ -29,16 +36,19 @@ import java.util.List;
import java.util.Map; import java.util.Map;
public class ResultSetAggregatorImpl implements ResultSetAggregator { 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 @Override
public List<Device> aggregate(Map<String, List<Device>> devices) { public List<Device> aggregate(Map<String, List<Device>> devices) {
Map<Integer, Device> generalQueryMap = this.convertToMap(devices.get(Constants.GENERAL)); Map<Integer, Device> generalQueryMap = this.convertToMap(devices.get(Constants.GENERAL));
Map<Integer, Device> andMap = this.convertToMap(devices.get(Constants.PROP_AND)); Map<Integer, Device> andMap = this.convertToMap(devices.get(Constants.PROP_AND));
Map<Integer, Device> orMap = this.convertToMap(devices.get(Constants.PROP_OR)); Map<Integer, Device> orMap = this.convertToMap(devices.get(Constants.PROP_OR));
Map<Integer, Device> locationMap = this.convertToMap(devices.get(Constants.LOCATION)); Map<Integer, Device> locationMap = this.convertToMap(devices.get(Constants.LOCATION));
Map<Integer, Device> finalMap = new HashMap<>(); Map<Integer, Device> finalMap = new HashMap<>();
List<Device> finalResult = new ArrayList<>(); List<Device> finalResult = new ArrayList<>();
List<Device> ownDevices = new ArrayList<>();
if (andMap.isEmpty()) { if (andMap.isEmpty()) {
finalMap = generalQueryMap; finalMap = generalQueryMap;
@ -70,8 +80,24 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator {
} }
} }
String username = PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername();
try {
if (isPermittedToViewAnyDevice(username)) {
return finalResult; 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<Integer, Device> convertToMap(List<Device> devices) { private Map<Integer, Device> convertToMap(List<Device> devices) {
if (devices == null) { if (devices == null) {
@ -79,7 +105,7 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator {
} }
Map<Integer, Device> deviceWrapperMap = new HashMap<>(); Map<Integer, Device> deviceWrapperMap = new HashMap<>();
for (Device device : devices) { for (Device device : devices) {
deviceWrapperMap.put(device.getId(), device); deviceWrapperMap.put(device.getEnrolmentInfo().getId(), device);
} }
return deviceWrapperMap; return deviceWrapperMap;
} }
@ -92,4 +118,19 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator {
return list; 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);
}
} }

Loading…
Cancel
Save