From 001649e5fd4d28b3cc7c43dbc196b286f5939e16 Mon Sep 17 00:00:00 2001 From: Madawa Soysa Date: Thu, 29 Nov 2018 23:27:50 +1100 Subject: [PATCH 1/3] 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); + } + } From eea040f391807de805c79b753636619aebefd40b Mon Sep 17 00:00:00 2001 From: Madawa Soysa Date: Sun, 9 Dec 2018 14:01:01 +1100 Subject: [PATCH 2/3] Remove redundant authorization check and fix the test --- .../core/search/mgt/impl/ProcessorImpl.java | 51 +++--------- .../mgt/core/search/ProcessorImplTest.java | 82 +++++++++++-------- 2 files changed, 60 insertions(+), 73 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 0d65c3018b..5510247082 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 @@ -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.*; +import org.wso2.carbon.device.mgt.core.search.mgt.Constants; +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 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 authorizedDevices(List devices) throws SearchMgtException { - List 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 getUpdatedDevices(long epochTime) throws SearchMgtException { diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/search/ProcessorImplTest.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/search/ProcessorImplTest.java index 92f3eac37d..221ed02461 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/search/ProcessorImplTest.java +++ b/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 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") - public void testWithNoDeviceAccessAuthorization() throws NoSuchFieldException, IllegalAccessException, - SearchMgtException { + @Test (description = "Search for device with and condition") + public void testSearchDevicesWIthAndCondition() throws SearchMgtException { SearchContext context = new SearchContext(); List conditions = new ArrayList<>(); - Condition cond = new Condition(); - cond.setKey("batteryLevel"); - cond.setOperator("="); - cond.setValue("40"); - cond.setState(Condition.State.AND); - conditions.add(cond); + + Condition condition = new Condition(); + condition.setKey("IMEI"); + condition.setOperator("="); + condition.setValue("e6f236ac82537a8e"); + condition.setState(Condition.State.AND); + conditions.add(condition); + + context.setConditions(conditions); + ProcessorImpl processor = new ProcessorImpl(); + List 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 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 - ("deviceAccessAuthorizationService"); - deviceAccessAuthorizationServiceField.setAccessible(true); - deviceAccessAuthorizationServiceField.set(processor, null); - List searchedDevices = processor.execute(context); - Assert.assertEquals(0, searchedDevices.size()); + List 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 wrong condition") + public void testSearchDevicesWIthWrongCondition() throws SearchMgtException { + SearchContext context = new SearchContext(); + List 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 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); - } } From 6f80d1993d2eef9bbd6cc7dd97b01ec8dad9b1ef Mon Sep 17 00:00:00 2001 From: Madawa Soysa Date: Thu, 13 Dec 2018 20:21:59 +1100 Subject: [PATCH 3/3] Move private constants to a public constant class --- .../wso2/carbon/device/mgt/core/search/mgt/Constants.java | 3 +++ .../mgt/core/search/mgt/impl/ResultSetAggregatorImpl.java | 5 ++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/Constants.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/Constants.java index 0ba5bb7d10..a944aec330 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/search/mgt/Constants.java +++ b/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"; } 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 df8e1a8757..cdb6322ca6 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 @@ -37,8 +37,6 @@ 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) { @@ -130,7 +128,8 @@ public class ResultSetAggregatorImpl implements ResultSetAggregator { UserRealm userRealm = DeviceManagementDataHolder.getInstance().getRealmService().getTenantUserRealm(tenantId); return userRealm != null && userRealm.getAuthorizationManager() != null && userRealm.getAuthorizationManager().isUserAuthorized(username, - PermissionUtils.getAbsolutePermissionPath(ANY_DEVICE_PERMISSION), UI_EXECUTE); + PermissionUtils.getAbsolutePermissionPath(Constants.ANY_DEVICE_PERMISSION), + Constants.UI_EXECUTE); } }