Merge pull request #992 from sinthuja/origin-wso2-master

Adding more testcases and fixing an intermittent issue in testcase.

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/ActivityProviderServiceImpl.java

@@ -29,6 +29,7 @@ import org.wso2.carbon.device.mgt.jaxrs.beans.ErrorResponse;
 import org.wso2.carbon.device.mgt.jaxrs.service.api.ActivityInfoProviderService;
 import org.wso2.carbon.device.mgt.jaxrs.service.impl.util.RequestValidationUtil;
 import org.wso2.carbon.device.mgt.jaxrs.util.DeviceMgtAPIUtils;
+import org.wso2.carbon.user.api.UserStoreException;
 
 import javax.validation.constraints.Size;
 import javax.ws.rs.*;
@@ -54,6 +55,8 @@ public class ActivityProviderServiceImpl implements ActivityInfoProviderService
                                 @HeaderParam("If-Modified-Since") String ifModifiedSince) {
         Activity activity;
         DeviceManagementProviderService dmService;
+        Response response = validateAdminUser();
+        if (response == null) {
             try {
                 RequestValidationUtil.validateActivityId(id);
 
@@ -71,6 +74,9 @@ public class ActivityProviderServiceImpl implements ActivityInfoProviderService
                 return Response.serverError().entity(
                         new ErrorResponse.ErrorResponseBuilder().setMessage(msg).build()).build();
             }
+        } else {
+            return response;
+        }
     }
 
 
@@ -120,7 +126,6 @@ public class ActivityProviderServiceImpl implements ActivityInfoProviderService
         long sinceTimestamp;
         long timestamp = 0;
         boolean isIfModifiedSinceSet = false;
-        boolean isSinceSet = false;
         if (log.isDebugEnabled()) {
             log.debug("getActivities since: " + since + " , offset: " + offset + " ,limit: " + limit + " ," +
                     "ifModifiedSince: " + ifModifiedSince);
@@ -150,7 +155,6 @@ public class ActivityProviderServiceImpl implements ActivityInfoProviderService
                                 "Invalid date string is provided in 'since' filter").build()).build();
             }
             sinceTimestamp = sinceDate.getTime();
-            isSinceSet = true;
             timestamp = sinceTimestamp / 1000;
         }
 
@@ -162,7 +166,8 @@ public class ActivityProviderServiceImpl implements ActivityInfoProviderService
         if (log.isDebugEnabled()) {
             log.debug("getActivities final timestamp " + timestamp);
         }
-
+        Response response = validateAdminUser();
+        if (response == null) {
             List<Activity> activities;
             ActivityList activityList = new ActivityList();
             DeviceManagementProviderService dmService;
@@ -194,6 +199,24 @@ public class ActivityProviderServiceImpl implements ActivityInfoProviderService
                 return Response.serverError().entity(
                         new ErrorResponse.ErrorResponseBuilder().setMessage(msg).build()).build();
             }
+        } else {
+            return response;
+        }
     }
 
+    private Response validateAdminUser(){
+        try {
+            if (!DeviceMgtAPIUtils.isAdmin()) {
+                return Response.status(Response.Status.UNAUTHORIZED).entity("Unauthorized operation! Only admin role can perform " +
+                        "this operation.").build();
+            }
+            return null;
+        } catch (UserStoreException e) {
+            String msg
+                    = "Error occurred while validating the user have admin role!";
+            log.error(msg, e);
+            return Response.serverError().entity(
+                    new ErrorResponse.ErrorResponseBuilder().setMessage(msg).build()).build();
+        }
+    }
 }

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/util/DeviceMgtAPIUtils.java

@@ -702,4 +702,18 @@ public class DeviceMgtAPIUtils {
         SSLContext.setDefault(sslContext);
     }
 
+
+    public static boolean isAdmin() throws UserStoreException {
+        int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId(true);
+        UserRealm realmService = DeviceMgtAPIUtils.getRealmService().getTenantUserRealm(tenantId);
+        String adminRoleName = realmService.getRealmConfiguration().getAdminRoleName();
+        String userName = PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername();
+        String[] roles = realmService.getUserStoreManager().getRoleListOfUser(userName);
+        for (String role: roles){
+            if (role != null && role.equals(adminRoleName)){
+                return true;
+            }
+        }
+        return false;
+    }
 }

components/device-mgt/org.wso2.carbon.device.mgt.common/src/main/java/org/wso2/carbon/device/mgt/common/operation/mgt/OperationManager.java

@@ -94,8 +94,6 @@ public interface OperationManager {
     Activity getOperationByActivityIdAndDevice(String activity, DeviceIdentifier deviceId)
             throws OperationManagementException;
 
-    List<Activity> getActivitiesUpdatedAfter(long timestamp) throws OperationManagementException;
-
     List<Activity> getActivitiesUpdatedAfter(long timestamp, int limit, int offset) throws OperationManagementException;
 
     int getActivityCountUpdatedAfter(long timestamp) throws OperationManagementException;

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/OperationManagerImpl.java

@@ -741,7 +741,11 @@ public class OperationManagerImpl implements OperationManager {
         if (operationId == 0) {
             throw new IllegalArgumentException("Operation ID cannot be null or zero (0).");
         }
-
+        if (!isActionAuthorized(deviceId)) {
+            throw new OperationManagementException("User '" + getUser() + "' is not authorized to access the '" +
+                    deviceId.getType() + "' device, which carries the identifier '" +
+                    deviceId.getId() + "'");
+        }
         Device device = this.getDevice(deviceId);
         try {
             OperationManagementDAOFactory.openConnection();
@@ -756,21 +760,6 @@ public class OperationManagerImpl implements OperationManager {
         }
     }
 
-    @Override
-    public List<Activity> getActivitiesUpdatedAfter(long timestamp) throws OperationManagementException {
-        try {
-            OperationManagementDAOFactory.openConnection();
-            return operationDAO.getActivitiesUpdatedAfter(timestamp);
-        } catch (SQLException e) {
-            throw new OperationManagementException("Error occurred while opening a connection to the data source.", e);
-        } catch (OperationManagementDAOException e) {
-            throw new OperationManagementException("Error occurred while getting the activity list changed after a " +
-                    "given time.", e);
-        } finally {
-            OperationManagementDAOFactory.closeConnection();
-        }
-    }
-
     @Override
     public List<Activity> getActivitiesUpdatedAfter(long timestamp, int limit,
                                                     int offset) throws OperationManagementException {

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/DeviceManagementProviderService.java

@@ -566,8 +566,6 @@ public interface DeviceManagementProviderService {
 
     Activity getOperationByActivityIdAndDevice(String activity, DeviceIdentifier deviceId) throws OperationManagementException;
 
-    List<Activity> getActivitiesUpdatedAfter(long timestamp) throws OperationManagementException;
-
     List<Activity> getActivitiesUpdatedAfter(long timestamp, int limit, int offset) throws OperationManagementException;
 
     int getActivityCountUpdatedAfter(long timestamp) throws OperationManagementException;

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/DeviceManagementProviderServiceImpl.java

@@ -1458,11 +1458,6 @@ public class DeviceManagementProviderServiceImpl implements DeviceManagementProv
         return DeviceManagementDataHolder.getInstance().getOperationManager().getOperationByActivityIdAndDevice(activity, deviceId);
     }
 
-    @Override
-    public List<Activity> getActivitiesUpdatedAfter(long timestamp) throws OperationManagementException {
-        return DeviceManagementDataHolder.getInstance().getOperationManager().getActivitiesUpdatedAfter(timestamp);
-    }
-
     @Override
     public List<Activity> getActivitiesUpdatedAfter(long timestamp, int limit, int offset) throws OperationManagementException {
         limit = DeviceManagerUtil.validateActivityListPageSize(limit);

components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/operation/OperationManagementTests.java

@@ -63,7 +63,6 @@ import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
-import java.util.TimeZone;
 
 /**
  * This is the testcase which covers the methods from {@link OperationManager}
@@ -84,6 +83,7 @@ public class OperationManagementTests {
     private List<DeviceIdentifier> deviceIds = new ArrayList<>();
     private OperationManager operationMgtService;
     private Activity commandActivity;
+    private long commandActivityBeforeUpdatedTimestamp;
 
     @BeforeClass
     public void init() throws Exception {
@@ -149,15 +149,19 @@ public class OperationManagementTests {
     @Test
     public void addNonAdminUserDevicesCommandOperation() throws DeviceManagementException, OperationManagementException,
             InvalidDeviceException {
-        PrivilegedCarbonContext.startTenantFlow();
+        startTenantFlowAsNonAdmin();
-        PrivilegedCarbonContext.getThreadLocalCarbonContext().setTenantId(MultitenantConstants.SUPER_TENANT_ID, true);
-        PrivilegedCarbonContext.getThreadLocalCarbonContext().setUsername(NON_ADMIN_USER);
         Activity activity = this.operationMgtService.addOperation(getOperation(new CommandOperation(), Operation.Type.COMMAND, COMMAND_OPERATON_CODE),
                 deviceIds);
         PrivilegedCarbonContext.endTenantFlow();
         validateOperationResponse(activity, ActivityStatus.Status.UNAUTHORIZED);
     }
 
+    private void startTenantFlowAsNonAdmin() {
+        PrivilegedCarbonContext.startTenantFlow();
+        PrivilegedCarbonContext.getThreadLocalCarbonContext().setTenantId(MultitenantConstants.SUPER_TENANT_ID, true);
+        PrivilegedCarbonContext.getThreadLocalCarbonContext().setUsername(NON_ADMIN_USER);
+    }
+
     @Test(dependsOnMethods = "addCommandOperation")
     public void addPolicyOperation() throws DeviceManagementException, OperationManagementException, InvalidDeviceException {
         Activity activity = this.operationMgtService.addOperation(getOperation(new PolicyOperation(), Operation.Type.POLICY, POLICY_OPERATION_CODE),
@@ -203,6 +207,18 @@ public class OperationManagementTests {
         }
     }
 
+    @Test(dependsOnMethods = "addProfileOperation", expectedExceptions = OperationManagementException.class)
+    public void getOperationsAsNonAdmin() throws DeviceManagementException, OperationManagementException, InvalidDeviceException {
+        try {
+            startTenantFlowAsNonAdmin();
+            for (DeviceIdentifier deviceIdentifier : deviceIds) {
+                this.operationMgtService.getOperations(deviceIdentifier);
+            }
+        } finally {
+            PrivilegedCarbonContext.endTenantFlow();
+        }
+    }
+
     @Test(dependsOnMethods = "getOperations")
     public void getPendingOperations() throws DeviceManagementException, OperationManagementException, InvalidDeviceException {
         for (DeviceIdentifier deviceIdentifier : deviceIds) {
@@ -211,6 +227,18 @@ public class OperationManagementTests {
         }
     }
 
+    @Test(dependsOnMethods = "getOperations", expectedExceptions = OperationManagementException.class)
+    public void getPendingOperationsAsNonAdmin() throws DeviceManagementException, OperationManagementException, InvalidDeviceException {
+        try {
+            startTenantFlowAsNonAdmin();
+            for (DeviceIdentifier deviceIdentifier : deviceIds) {
+                this.operationMgtService.getPendingOperations(deviceIdentifier);
+            }
+        } finally {
+            PrivilegedCarbonContext.endTenantFlow();
+        }
+    }
+
     @Test(dependsOnMethods = "getPendingOperations")
     public void getPaginatedRequestAsAdmin() throws OperationManagementException {
         PrivilegedCarbonContext.startTenantFlow();
@@ -228,11 +256,10 @@ public class OperationManagementTests {
         PrivilegedCarbonContext.endTenantFlow();
     }
 
-    @Test(dependsOnMethods = "getPendingOperations")
+    @Test(dependsOnMethods = "getPendingOperations", expectedExceptions = OperationManagementException.class)
     public void getPaginatedRequestAsNonAdmin() throws OperationManagementException {
-        PrivilegedCarbonContext.startTenantFlow();
+        try {
-        PrivilegedCarbonContext.getThreadLocalCarbonContext().setTenantId(MultitenantConstants.SUPER_TENANT_ID, true);
+            startTenantFlowAsNonAdmin();
-        PrivilegedCarbonContext.getThreadLocalCarbonContext().setUsername(NON_ADMIN_USER);
             PaginationRequest request = new PaginationRequest(1, 2);
             request.setDeviceType(DEVICE_TYPE);
             request.setOwner(ADMIN_USER);
@@ -243,14 +270,18 @@ public class OperationManagementTests {
                     if (ex.getMessage() == null) {
                         Assert.assertTrue(ex.getMessage().contains("User '" + NON_ADMIN_USER + "' is not authorized"));
                     }
+                    throw ex;
                 }
             }
+        } finally {
             PrivilegedCarbonContext.endTenantFlow();
         }
+    }
 
     @Test(dependsOnMethods = "getPaginatedRequestAsAdmin")
     public void updateOperation() throws OperationManagementException {
         //This is required to introduce a delay for the update operation of the device.
+        this.commandActivityBeforeUpdatedTimestamp = System.currentTimeMillis();
         try {
             Thread.sleep(2000);
         } catch (InterruptedException ignored) {
@@ -266,6 +297,30 @@ public class OperationManagementTests {
         Assert.assertEquals(pendingOperations.size(), 3);
     }
 
+    @Test(dependsOnMethods = "updateOperation", expectedExceptions = OperationManagementException.class)
+    public void updateOperationAsNonAdmin() throws OperationManagementException {
+        //This is required to introduce a delay for the update operation of the device.
+        try {
+            Thread.sleep(2000);
+        } catch (InterruptedException ignored) {
+        }
+        try {
+            DeviceIdentifier deviceIdentifier = this.deviceIds.get(0);
+            List operations = this.operationMgtService.getPendingOperations(deviceIdentifier);
+            Assert.assertTrue(operations != null && operations.size() == 3);
+            startTenantFlowAsNonAdmin();
+            Operation operation = (Operation) operations.get(0);
+            operation.setStatus(Operation.Status.COMPLETED);
+            operation.setOperationResponse("The operation is successfully completed, and updated by non admin!");
+            this.operationMgtService.updateOperation(deviceIdentifier, operation);
+            List pendingOperations = this.operationMgtService.getPendingOperations(deviceIdentifier);
+            Assert.assertEquals(pendingOperations.size(), 3);
+        } finally {
+            PrivilegedCarbonContext.endTenantFlow();
+        }
+
+    }
+
     @Test(dependsOnMethods = "updateOperation")
     public void getNextPendingOperation() throws OperationManagementException {
         DeviceIdentifier deviceIdentifier = this.deviceIds.get(0);
@@ -284,6 +339,20 @@ public class OperationManagementTests {
         Assert.assertTrue(operation.getType().equals(Operation.Type.COMMAND));
     }
 
+    @Test(dependsOnMethods = "getNextPendingOperation", expectedExceptions = OperationManagementException.class)
+    public void getOperationByDeviceAndOperationIdNonAdmin() throws OperationManagementException {
+        startTenantFlowAsNonAdmin();
+        try {
+            DeviceIdentifier deviceIdentifier = this.deviceIds.get(0);
+            String operationId = this.commandActivity.getActivityId().
+                    replace(DeviceManagementConstants.OperationAttributes.ACTIVITY, "");
+            this.operationMgtService.getOperationByDeviceAndOperationId(deviceIdentifier,
+                    Integer.parseInt(operationId));
+        } finally {
+            PrivilegedCarbonContext.endTenantFlow();
+        }
+    }
+
     @Test(dependsOnMethods = "getOperationByDeviceAndOperationId")
     public void getOperationsByDeviceAndStatus() throws OperationManagementException, DeviceManagementException {
         DeviceIdentifier deviceIdentifier = this.deviceIds.get(0);
@@ -291,6 +360,17 @@ public class OperationManagementTests {
         Assert.assertEquals(operation.size(), 3);
     }
 
+    @Test(dependsOnMethods = "getOperationByDeviceAndOperationId", expectedExceptions = OperationManagementException.class)
+    public void getOperationsByDeviceAndStatusByNonAdmin() throws OperationManagementException, DeviceManagementException {
+        startTenantFlowAsNonAdmin();
+        try {
+            DeviceIdentifier deviceIdentifier = this.deviceIds.get(0);
+            this.operationMgtService.getOperationsByDeviceAndStatus(deviceIdentifier, Operation.Status.PENDING);
+        } finally {
+            PrivilegedCarbonContext.endTenantFlow();
+        }
+    }
+
     @Test(dependsOnMethods = "getOperationsByDeviceAndStatus")
     public void getOperation() throws OperationManagementException, DeviceManagementException {
         String operationId = this.commandActivity.getActivityId().
@@ -319,31 +399,21 @@ public class OperationManagementTests {
         Assert.assertEquals(activity.getActivityStatus().get(0).getStatus(), ActivityStatus.Status.COMPLETED);
     }
 
-    @Test(dependsOnMethods = "updateOperation")
+    @Test(dependsOnMethods = "getOperationActivity", expectedExceptions = OperationManagementException.class)
-    public void getOperationUpdatedAfterWithLimitAndOffet() throws OperationManagementException, ParseException {
+    public void getOperationByActivityIdAndDeviceAsNonAdmin() throws OperationManagementException {
-        String timestamp = this.commandActivity.getCreatedTimeStamp();
+        startTenantFlowAsNonAdmin();
-        SimpleDateFormat dateFormat = new SimpleDateFormat("EEE MMM dd hh:mm:ss Z yyyy");
+        try {
-        dateFormat.setTimeZone(TimeZone.getTimeZone("IST"));
+            this.operationMgtService.
-        Date date = dateFormat.parse(timestamp);
+                    getOperationByActivityIdAndDevice(this.commandActivity.getActivityId(), this.deviceIds.get(0));
-        List<Activity> operations = this.operationMgtService.getActivitiesUpdatedAfter(date.getTime() / 1000, 10, 0);
+        } finally {
-        Assert.assertTrue(operations != null && operations.size() == 1,
+            PrivilegedCarbonContext.endTenantFlow();
-                "The operations updated after the created should be 1");
+        }
-        Activity operation = operations.get(0);
-        Assert.assertTrue(operation.getActivityStatus() != null && operation.getActivityStatus().size() == 1,
-                "The operation should be having the activity status of atleast one device");
-        Assert.assertEquals(operation.getActivityStatus().get(0).getDeviceIdentifier().getId(),
-                deviceIds.get(0).getId());
-        Assert.assertEquals(operation.getActivityStatus().get(0).getDeviceIdentifier().getType(),
-                deviceIds.get(0).getType());
     }
 
     @Test(dependsOnMethods = "updateOperation")
-    public void getOperationUpdatedAfter() throws OperationManagementException, ParseException {
+    public void getOperationUpdatedAfterWithLimitAndOffset() throws OperationManagementException, ParseException {
-        String timestamp = this.commandActivity.getCreatedTimeStamp();
+        List<Activity> operations = this.operationMgtService.getActivitiesUpdatedAfter
-        SimpleDateFormat dateFormat = new SimpleDateFormat("EEE MMM dd hh:mm:ss Z yyyy");
+                (this.commandActivityBeforeUpdatedTimestamp / 1000, 10, 0);
-        dateFormat.setTimeZone(TimeZone.getTimeZone("IST"));
-        Date date = dateFormat.parse(timestamp);
-        List<Activity> operations = this.operationMgtService.getActivitiesUpdatedAfter(date.getTime() / 1000);
         Assert.assertTrue(operations != null && operations.size() == 1,
                 "The operations updated after the created should be 1");
         Activity operation = operations.get(0);
@@ -355,13 +425,10 @@ public class OperationManagementTests {
                 deviceIds.get(0).getType());
     }
 
-    @Test(dependsOnMethods = "getOperationUpdatedAfter")
+    @Test(dependsOnMethods = "getOperationUpdatedAfterWithLimitAndOffset")
     public void getActivityCountUpdatedAfter() throws OperationManagementException, ParseException {
-        String timestamp = this.commandActivity.getCreatedTimeStamp();
+        int activityCount = this.operationMgtService.getActivityCountUpdatedAfter
-        SimpleDateFormat dateFormat = new SimpleDateFormat("EEE MMM dd hh:mm:ss Z yyyy");
+                (this.commandActivityBeforeUpdatedTimestamp / 1000);
-        dateFormat.setTimeZone(TimeZone.getTimeZone("IST"));
-        Date date = dateFormat.parse(timestamp);
-        int activityCount = this.operationMgtService.getActivityCountUpdatedAfter(date.getTime() / 1000);
         Assert.assertTrue(activityCount == 1,
                 "The activities updated after the created should be 1");
     }