Merge pull request #7768 from ViacheslavKlimov/fix/pagelink-validation

[3.4.3] Additional pageLink validation

application/src/main/java/org/thingsboard/server/controller/BaseController.java

@@ -120,6 +120,7 @@ import org.thingsboard.server.dao.queue.QueueService;
 import org.thingsboard.server.dao.relation.RelationService;
 import org.thingsboard.server.dao.rpc.RpcService;
 import org.thingsboard.server.dao.rule.RuleChainService;
+import org.thingsboard.server.dao.service.Validator;
 import org.thingsboard.server.dao.tenant.TbTenantProfileCache;
 import org.thingsboard.server.dao.tenant.TenantProfileService;
 import org.thingsboard.server.dao.tenant.TenantService;
@@ -131,7 +132,6 @@ import org.thingsboard.server.queue.discovery.PartitionService;
 import org.thingsboard.server.queue.provider.TbQueueProducerProvider;
 import org.thingsboard.server.queue.util.TbCoreComponent;
 import org.thingsboard.server.service.component.ComponentDiscoveryService;
-import org.thingsboard.server.service.edge.EdgeNotificationService;
 import org.thingsboard.server.service.edge.rpc.EdgeRpcService;
 import org.thingsboard.server.service.entitiy.TbNotificationEntityService;
 import org.thingsboard.server.service.ota.OtaPackageStateService;
@@ -417,9 +417,12 @@ public abstract class BaseController {
     }
 
     PageLink createPageLink(int pageSize, int page, String textSearch, String sortProperty, String sortOrder) throws ThingsboardException {
-        if (!StringUtils.isEmpty(sortProperty)) {
+        if (StringUtils.isNotEmpty(sortProperty)) {
+            if (!Validator.isValidProperty(sortProperty)) {
+                throw new IllegalArgumentException("Invalid sort property");
+            }
             SortOrder.Direction direction = SortOrder.Direction.ASC;
-            if (!StringUtils.isEmpty(sortOrder)) {
+            if (StringUtils.isNotEmpty(sortOrder)) {
                 try {
                     direction = SortOrder.Direction.valueOf(sortOrder.toUpperCase());
                 } catch (IllegalArgumentException e) {

application/src/test/java/org/thingsboard/server/controller/BaseEntityQueryControllerTest.java

@@ -22,6 +22,7 @@ import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.springframework.test.web.servlet.ResultActions;
 import org.thingsboard.common.util.JacksonUtil;
 import org.thingsboard.server.common.data.DataConstants;
 import org.thingsboard.server.common.data.Device;
@@ -55,6 +56,7 @@ import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 public abstract class BaseEntityQueryControllerTest extends AbstractControllerTest {
@@ -381,4 +383,24 @@ public abstract class BaseEntityQueryControllerTest extends AbstractControllerTe
             Assert.assertEquals("1" + i, alarmActiveTime);
         }
     }
+
+    @Test
+    public void givenInvalidEntityDataPageLink_thenReturnError() throws Exception {
+        DeviceTypeFilter filter = new DeviceTypeFilter();
+        filter.setDeviceType("default");
+        filter.setDeviceNameFilter("");
+
+        String invalidSortProperty = "created(Time)";
+        EntityDataSortOrder sortOrder = new EntityDataSortOrder(
+                new EntityKey(EntityKeyType.ENTITY_FIELD, invalidSortProperty), EntityDataSortOrder.Direction.ASC
+        );
+        EntityDataPageLink pageLink = new EntityDataPageLink(10, 0, null, sortOrder);
+        List<EntityKey> entityFields = Collections.singletonList(new EntityKey(EntityKeyType.ENTITY_FIELD, "name"));
+        List<EntityKey> latestValues = Collections.singletonList(new EntityKey(EntityKeyType.ATTRIBUTE, "temperature"));
+        EntityDataQuery query = new EntityDataQuery(filter, pageLink, entityFields, latestValues, null);
+
+        ResultActions result = doPost("/api/entitiesQuery/find", query).andExpect(status().isBadRequest());
+        assertThat(getErrorMessage(result)).contains("Invalid").contains("sort property");
+    }
+
 }

application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java

@@ -28,6 +28,7 @@ import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Primary;
 import org.springframework.http.HttpHeaders;
 import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.web.servlet.ResultActions;
 import org.thingsboard.server.common.data.Customer;
 import org.thingsboard.server.common.data.StringUtils;
 import org.thingsboard.server.common.data.Tenant;
@@ -729,6 +730,17 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest {
         testEntityDaoWithRelationsTransactionalException(userDao, tenantId, userId, "/api/user/" + userId);
     }
 
+    @Test
+    public void givenInvalidPageLink_thenReturnError() throws Exception {
+        loginTenantAdmin();
+
+        String invalidSortProperty = "abc(abc)";
+
+        ResultActions result = doGet("/api/users?page={page}&pageSize={pageSize}&sortProperty={sortProperty}", 0, 100, invalidSortProperty)
+                .andExpect(status().isBadRequest());
+        assertThat(getErrorMessage(result)).containsIgnoringCase("invalid sort property");
+    }
+
     private User createUser() throws Exception {
         loginSysAdmin();
         String email = "tenant2@thingsboard.org";

common/util/src/main/java/org/thingsboard/common/util/RegexUtils.java

@@ -33,4 +33,8 @@ public class RegexUtils {
         });
     }
 
+    public static boolean matches(String input, Pattern pattern) {
+        return pattern.matcher(input).matches();
+    }
+
 }

dao/src/main/java/org/thingsboard/server/dao/alarm/BaseAlarmService.java

@@ -65,6 +65,7 @@ import java.util.concurrent.Executors;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static org.thingsboard.server.dao.service.Validator.validateEntityDataPageLink;
 import static org.thingsboard.server.dao.service.Validator.validateId;
 
 @Service
@@ -139,6 +140,7 @@ public class BaseAlarmService extends AbstractEntityService implements AlarmServ
     public PageData<AlarmData> findAlarmDataByQueryForEntities(TenantId tenantId,
                                                                AlarmDataQuery query, Collection<EntityId> orderedEntityIds) {
         validateId(tenantId, INCORRECT_TENANT_ID + tenantId);
+        validateEntityDataPageLink(query.getPageLink());
         return alarmDao.findAlarmDataByQueryForEntities(tenantId, query, orderedEntityIds);
     }
 

dao/src/main/java/org/thingsboard/server/dao/entity/BaseEntityService.java

@@ -20,6 +20,7 @@ import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import org.springframework.util.CollectionUtils;
@@ -59,6 +60,7 @@ import org.thingsboard.server.dao.tenant.TenantService;
 import org.thingsboard.server.dao.user.UserService;
 
 import static org.thingsboard.server.dao.model.ModelConstants.NULL_UUID;
+import static org.thingsboard.server.dao.service.Validator.validateEntityDataPageLink;
 import static org.thingsboard.server.dao.service.Validator.validateId;
 
 /**
@@ -238,16 +240,6 @@ public class BaseEntityService extends AbstractEntityService implements EntitySe
         validateEntityDataPageLink(query.getPageLink());
     }
 
-    private static void validateEntityDataPageLink(EntityDataPageLink pageLink) {
-        if (pageLink == null) {
-            throw new IncorrectParameterException("Entity Data Page link must be specified.");
-        } else if (pageLink.getPageSize() < 1) {
-            throw new IncorrectParameterException("Incorrect entity data page link page size '" + pageLink.getPageSize() + "'. Page size must be greater than zero.");
-        } else if (pageLink.getPage() < 0) {
-            throw new IncorrectParameterException("Incorrect entity data page link page '" + pageLink.getPage() + "'. Page must be positive integer.");
-        }
-    }
-
     private static void validateRelationQuery(RelationsQueryFilter queryFilter) {
         if (queryFilter.isMultiRoot() && queryFilter.getMultiRootEntitiesType() ==null){
             throw new IncorrectParameterException("Multi-root relation query filter should contain 'multiRootEntitiesType'");

dao/src/main/java/org/thingsboard/server/dao/service/Validator.java

@@ -15,16 +15,24 @@
  */
 package org.thingsboard.server.dao.service;
 
+import org.apache.commons.lang3.StringUtils;
+import org.thingsboard.common.util.RegexUtils;
 import org.thingsboard.server.common.data.id.EntityId;
 import org.thingsboard.server.common.data.id.UUIDBased;
 import org.thingsboard.server.common.data.page.PageLink;
+import org.thingsboard.server.common.data.query.EntityDataPageLink;
+import org.thingsboard.server.common.data.query.EntityKey;
+import org.thingsboard.server.common.data.query.EntityKeyType;
 import org.thingsboard.server.dao.exception.IncorrectParameterException;
 
 import java.util.List;
 import java.util.UUID;
+import java.util.regex.Pattern;
 
 public class Validator {
 
+    public static final Pattern PROPERTY_PATTERN = Pattern.compile("^[\\p{L}0-9_-]+$"); // Unicode letters, numbers, '_' and '-' allowed
+
     /**
      * This method validate <code>EntityId</code> entity id. If entity id is invalid than throw
      * <code>IncorrectParameterException</code> exception
@@ -122,7 +130,31 @@ public class Validator {
             throw new IncorrectParameterException("Incorrect page link page size '"+pageLink.getPageSize()+"'. Page size must be greater than zero.");
         } else if (pageLink.getPage() < 0) {
             throw new IncorrectParameterException("Incorrect page link page '"+pageLink.getPage()+"'. Page must be positive integer.");
+        } else if (pageLink.getSortOrder() != null) {
+            if (!isValidProperty(pageLink.getSortOrder().getProperty())) {
+                throw new IncorrectParameterException("Invalid page link sort property");
+            }
         }
     }
 
+    public static void validateEntityDataPageLink(EntityDataPageLink pageLink) {
+        if (pageLink == null) {
+            throw new IncorrectParameterException("Entity Data Page link must be specified.");
+        } else if (pageLink.getPageSize() < 1) {
+            throw new IncorrectParameterException("Incorrect entity data page link page size '" + pageLink.getPageSize() + "'. Page size must be greater than zero.");
+        } else if (pageLink.getPage() < 0) {
+            throw new IncorrectParameterException("Incorrect entity data page link page '" + pageLink.getPage() + "'. Page must be positive integer.");
+        } else if (pageLink.getSortOrder() != null && pageLink.getSortOrder().getKey() != null) {
+            EntityKey sortKey = pageLink.getSortOrder().getKey();
+            if ((sortKey.getType() == EntityKeyType.ENTITY_FIELD || sortKey.getType() == EntityKeyType.ALARM_FIELD)
+                    && !isValidProperty(sortKey.getKey())) {
+                throw new IncorrectParameterException("Invalid entity data page link sort property");
+            }
+        }
+    }
+
+    public static boolean isValidProperty(String key) {
+        return StringUtils.isEmpty(key) || RegexUtils.matches(key, PROPERTY_PATTERN);
+    }
+
 }