From eea040f391807de805c79b753636619aebefd40b Mon Sep 17 00:00:00 2001 From: Madawa Soysa Date: Sun, 9 Dec 2018 14:01:01 +1100 Subject: [PATCH] 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); - } }