Merge branch 'fix-advance-search' into 'master'

Fix issue with advanced search to pick the correct enrollment

Closes product-iots#14 and product-iots#13

See merge request entgra/carbon-device-mgt!36

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/Constants.java

@@ -25,4 +25,7 @@ public class Constants {
     public static final String PROP_AND = "PROP_AND";
     public static final String PROP_OR = "PROP_OR";
     public static final String LOCATION = "LOCATION";
+
+    public static final String ANY_DEVICE_PERMISSION = "/device-mgt/devices/any-device";
+    public static final String UI_EXECUTE = "ui.execute";
 }

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/impl/ProcessorImpl.java

@@ -25,8 +25,6 @@ import org.wso2.carbon.device.mgt.common.Device;
 import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
 import org.wso2.carbon.device.mgt.common.DeviceManagementConstants;
 import org.wso2.carbon.device.mgt.common.EnrolmentInfo;
-import org.wso2.carbon.device.mgt.common.authorization.DeviceAccessAuthorizationException;
-import org.wso2.carbon.device.mgt.common.authorization.DeviceAccessAuthorizationService;
 import org.wso2.carbon.device.mgt.common.device.details.DeviceInfo;
 import org.wso2.carbon.device.mgt.common.device.details.DeviceLocation;
 import org.wso2.carbon.device.mgt.common.search.SearchContext;
@@ -34,11 +32,21 @@ import org.wso2.carbon.device.mgt.core.dao.ApplicationDAO;
 import org.wso2.carbon.device.mgt.core.dao.DeviceManagementDAOException;
 import org.wso2.carbon.device.mgt.core.dao.DeviceManagementDAOFactory;
 import org.wso2.carbon.device.mgt.core.dao.util.DeviceManagementDAOUtil;
-import org.wso2.carbon.device.mgt.core.internal.DeviceManagementDataHolder;
+import org.wso2.carbon.device.mgt.core.search.mgt.Constants;
-import org.wso2.carbon.device.mgt.core.search.mgt.*;
+import org.wso2.carbon.device.mgt.core.search.mgt.InvalidOperatorException;
+import org.wso2.carbon.device.mgt.core.search.mgt.Processor;
+import org.wso2.carbon.device.mgt.core.search.mgt.QueryBuilder;
+import org.wso2.carbon.device.mgt.core.search.mgt.QueryHolder;
+import org.wso2.carbon.device.mgt.core.search.mgt.ResultSetAggregator;
+import org.wso2.carbon.device.mgt.core.search.mgt.SearchMgtException;
+import org.wso2.carbon.device.mgt.core.search.mgt.ValueType;
 import org.wso2.carbon.device.mgt.core.search.mgt.dao.SearchDAOException;
 
-import java.sql.*;
+import java.sql.Array;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -47,17 +55,9 @@ import java.util.Map;
 public class ProcessorImpl implements Processor {
     private ApplicationDAO applicationDAO;
     private static final Log log = LogFactory.getLog(ProcessorImpl.class);
-    private DeviceAccessAuthorizationService deviceAccessAuthorizationService;
 
     public ProcessorImpl() {
         applicationDAO = DeviceManagementDAOFactory.getApplicationDAO();
-        deviceAccessAuthorizationService = DeviceManagementDataHolder.getInstance()
-                .getDeviceAccessAuthorizationService();
-        if (deviceAccessAuthorizationService == null) {
-            String msg = "DeviceAccessAuthorization service has not initialized.";
-            log.error(msg);
-            throw new IllegalStateException(msg);
-        }
     }
 
     @Override
@@ -115,35 +115,10 @@ public class ProcessorImpl implements Processor {
         devices.put(Constants.LOCATION, locationDevices);
 
         List<Device> finalDevices = aggregator.aggregate(devices);
-        finalDevices = authorizedDevices(finalDevices);
         this.setApplicationListOfDevices(finalDevices);
         return finalDevices;
     }
 
-    /**
-     * To get the authorized devices for a particular user
-     *
-     * @param devices Devices that satisfy search results
-     * @return Devices that satisfy search results and authorized to be viewed by particular user
-     */
-    private List<Device> authorizedDevices(List<Device> devices) throws SearchMgtException {
-        List<Device> filteredList = new ArrayList<>();
-        try {
-            for (Device device : devices) {
-                DeviceIdentifier deviceIdentifier = new DeviceIdentifier(device.getDeviceIdentifier(),
-                        device.getType());
-                if (deviceAccessAuthorizationService != null && deviceAccessAuthorizationService
-                        .isUserAuthorized(deviceIdentifier)) {
-                    filteredList.add(device);
-                }
-            }
-            return filteredList;
-        } catch (DeviceAccessAuthorizationException e) {
-            log.error("Error getting authorized search results for logged in user");
-            throw new SearchMgtException(e);
-        }
-    }
-
     @Override
     public List<Device> getUpdatedDevices(long epochTime) throws SearchMgtException {
 
@@ -256,7 +231,6 @@ public class ProcessorImpl implements Processor {
         PreparedStatement stmt = null;
         ResultSet rs = null;
         List<Device> devices = new ArrayList<>();
-        Map<Integer, Integer> devs = new HashMap<>();
         try {
             conn = this.getConnection();
             stmt = conn.prepareStatement(queryHolder.getQuery());
@@ -281,7 +255,6 @@ 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"));
@@ -290,6 +263,7 @@ public class ProcessorImpl implements Processor {
                 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")));
@@ -332,8 +306,6 @@ public class ProcessorImpl implements Processor {
                 deviceInfo.setLocation(deviceLocation);
                 device.setDeviceInfo(deviceInfo);
                 devices.add(device);
-                    devs.put(device.getId(), device.getId());
-                }
             }
         } catch (SQLException e) {
             throw new SearchDAOException("Error occurred while aquiring the device details.", e);

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" +

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,17 @@ import java.util.List;
 import java.util.Map;
 
 public class ResultSetAggregatorImpl implements ResultSetAggregator {
+    private static Log log = LogFactory.getLog(ResultSetAggregatorImpl.class);
 
     @Override
     public List<Device> aggregate(Map<String, List<Device>> devices) {
-
         Map<Integer, Device> generalQueryMap = this.convertToMap(devices.get(Constants.GENERAL));
         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> locationMap = this.convertToMap(devices.get(Constants.LOCATION));
         Map<Integer, Device> finalMap = new HashMap<>();
         List<Device> finalResult = new ArrayList<>();
+        List<Device> ownDevices = new ArrayList<>();
 
         if (andMap.isEmpty()) {
             finalMap = generalQueryMap;
@@ -70,8 +78,24 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator {
             }
         }
 
+        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<Integer, Device> convertToMap(List<Device> devices) {
         if (devices == null) {
@@ -79,7 +103,7 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator {
         }
         Map<Integer, Device> deviceWrapperMap = new HashMap<>();
         for (Device device : devices) {
-            deviceWrapperMap.put(device.getId(), device);
+            deviceWrapperMap.put(device.getEnrolmentInfo().getId(), device);
         }
         return deviceWrapperMap;
     }
@@ -92,4 +116,20 @@ 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(Constants.ANY_DEVICE_PERMISSION), 
+                        Constants.UI_EXECUTE);
+    }
+
 }

components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/search/ProcessorImplTest.java

@@ -17,14 +17,11 @@
  */
 package org.wso2.carbon.device.mgt.core.search;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 import org.wso2.carbon.device.mgt.common.Device;
 import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
-import org.wso2.carbon.device.mgt.common.authorization.DeviceAccessAuthorizationService;
 import org.wso2.carbon.device.mgt.common.search.Condition;
 import org.wso2.carbon.device.mgt.common.search.SearchContext;
 import org.wso2.carbon.device.mgt.core.TestDeviceManagementService;
@@ -41,7 +38,6 @@ import org.wso2.carbon.device.mgt.core.service.DeviceManagementProviderService;
 import org.wso2.carbon.device.mgt.core.service.DeviceManagementProviderServiceImpl;
 import org.wso2.carbon.utils.multitenancy.MultitenantConstants;
 
-import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -50,16 +46,12 @@ import java.util.List;
  */
 public class ProcessorImplTest extends BaseDeviceManagementTest {
 
-    private DeviceAccessAuthorizationService deviceAccessAuthorizationService;
-    private static final Log log = LogFactory.getLog(SearchManagementServiceTest.class);
     private static List<DeviceIdentifier> deviceIdentifiers = new ArrayList<>();
     private static final String DEVICE_ID_PREFIX = "SEARCH-DEVICE-ID-";
     private static final String DEVICE_TYPE = "SEARCH_TYPE";
 
     @BeforeClass
     public void init() throws Exception {
-        deviceAccessAuthorizationService = DeviceManagementDataHolder.getInstance()
-                .getDeviceAccessAuthorizationService();
         for (int i = 0; i < 5; i++) {
             deviceIdentifiers.add(new DeviceIdentifier(DEVICE_ID_PREFIX + i, DEVICE_TYPE));
         }
@@ -81,27 +73,59 @@ public class ProcessorImplTest extends BaseDeviceManagementTest {
         }
     }
 
-    @Test(description = "Test the Search Processor")
+    @Test (description = "Search for device with and condition")
-    public void testWithNoDeviceAccessAuthorization() throws NoSuchFieldException, IllegalAccessException,
+    public void testSearchDevicesWIthAndCondition() throws SearchMgtException {
-            SearchMgtException {
         SearchContext context = new SearchContext();
         List<Condition> conditions = new ArrayList<>();
-        Condition cond = new Condition();
+
-        cond.setKey("batteryLevel");
+        Condition condition = new Condition();
-        cond.setOperator("=");
+        condition.setKey("IMEI");
-        cond.setValue("40");
+        condition.setOperator("=");
-        cond.setState(Condition.State.AND);
+        condition.setValue("e6f236ac82537a8e");
-        conditions.add(cond);
+        condition.setState(Condition.State.AND);
+        conditions.add(condition);
+
+        context.setConditions(conditions);
+        ProcessorImpl processor = new ProcessorImpl();
+        List<Device> devices = processor.execute(context);
+        Assert.assertEquals(5, devices.size(), "There should be exactly 5 devices with matching search criteria");
+    }
+
+    @Test (description = "Search for device with or condition")
+    public void testSearchDevicesWIthORCondition() throws SearchMgtException {
+        SearchContext context = new SearchContext();
+        List<Condition> conditions = new ArrayList<>();
+
+        Condition condition = new Condition();
+        condition.setKey("IMSI");
+        condition.setOperator("=");
+        condition.setValue("432659632123654845");
+        condition.setState(Condition.State.OR);
+        conditions.add(condition);
+
         context.setConditions(conditions);
         ProcessorImpl processor = new ProcessorImpl();
-        Field deviceAccessAuthorizationServiceField = ProcessorImpl.class.getDeclaredField
+        List<Device> devices = processor.execute(context);
-                ("deviceAccessAuthorizationService");
+        Assert.assertEquals(5, devices.size(), "There should be exactly 5 devices with matching search criteria");
-        deviceAccessAuthorizationServiceField.setAccessible(true);
-        deviceAccessAuthorizationServiceField.set(processor, null);
-        List<Device> searchedDevices = processor.execute(context);
-        Assert.assertEquals(0, searchedDevices.size());
     }
 
+    @Test (description = "Search for device with wrong condition")
+    public void testSearchDevicesWIthWrongCondition() throws SearchMgtException {
+        SearchContext context = new SearchContext();
+        List<Condition> conditions = new ArrayList<>();
+
+        Condition condition = new Condition();
+        condition.setKey("IMSI");
+        condition.setOperator("=");
+        condition.setValue("43265963212378466");
+        condition.setState(Condition.State.OR);
+        conditions.add(condition);
+
+        context.setConditions(conditions);
+        ProcessorImpl processor = new ProcessorImpl();
+        List<Device> devices = processor.execute(context);
+        Assert.assertEquals(0, devices.size(), "There should be no devices with matching search criteria");
+    }
 
     @Test(description = "Test for invalid state")
     public void testInvalidState() throws SearchMgtException {
@@ -141,16 +165,4 @@ public class ProcessorImplTest extends BaseDeviceManagementTest {
             }
         }
     }
-
-    @Test(description = "Test when Device Access Authorization is null", expectedExceptions = {IllegalStateException
-            .class}, dependsOnMethods = {"testWithNoDeviceAccessAuthorization", "testInvalidState"})
-    public void testProcessorInitializationError() throws ClassNotFoundException, NoSuchMethodException,
-            NoSuchFieldException, IllegalAccessException, SearchMgtException {
-        DeviceManagementDataHolder deviceManagementDataHolder = DeviceManagementDataHolder.getInstance();
-        Field field = DeviceManagementDataHolder.class.getDeclaredField("deviceAccessAuthorizationService");
-        field.setAccessible(true);
-        field.set(deviceManagementDataHolder, null);
-        ProcessorImpl processor = new ProcessorImpl();
-        processor.execute(null);
-    }
 }