Validation for rule node configuration; improve validation error message

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

@@ -597,7 +597,7 @@ public abstract class AbstractNotifyEntityTest extends AbstractWebTest {
     }
 
     protected String msgErrorFieldLength(String fieldName) {
-        return "length of " + fieldName + " must be equal or less than 255";
+        return fieldName + " length must be equal or less than 255";
     }
 
     protected String msgErrorNoFound(String entityClassName, String assetIdStr) {

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

@@ -26,6 +26,8 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Primary;
 import org.springframework.test.context.ContextConfiguration;
+import org.thingsboard.rule.engine.action.TbCreateAlarmNode;
+import org.thingsboard.rule.engine.action.TbCreateAlarmNodeConfiguration;
 import org.thingsboard.server.common.data.StringUtils;
 import org.thingsboard.server.common.data.Tenant;
 import org.thingsboard.server.common.data.User;
@@ -35,7 +37,9 @@ import org.thingsboard.server.common.data.id.RuleChainId;
 import org.thingsboard.server.common.data.page.PageData;
 import org.thingsboard.server.common.data.page.PageLink;
 import org.thingsboard.server.common.data.rule.RuleChain;
+import org.thingsboard.server.common.data.rule.RuleChainMetaData;
 import org.thingsboard.server.common.data.rule.RuleChainType;
+import org.thingsboard.server.common.data.rule.RuleNode;
 import org.thingsboard.server.common.data.security.Authority;
 import org.thingsboard.server.dao.exception.DataValidationException;
 import org.thingsboard.server.dao.rule.RuleChainDao;
@@ -44,6 +48,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
@@ -254,9 +259,35 @@ public abstract class BaseRuleChainControllerTest extends AbstractControllerTest
         testEntityDaoWithRelationsTransactionalException(ruleChainDao, savedTenant.getId(), ruleChainId, "/api/ruleChain/" + ruleChainId);
     }
 
+    @Test
+    public void givenRuleNodeWithInvalidConfiguration_thenReturnError() throws Exception {
+        RuleChain ruleChain = createRuleChain("Rule chain with invalid nodes");
+        RuleChainMetaData ruleChainMetaData = new RuleChainMetaData();
+        ruleChainMetaData.setRuleChainId(ruleChain.getId());
+
+        RuleNode createAlarmNode = new RuleNode();
+        createAlarmNode.setName("Create alarm");
+        createAlarmNode.setType(TbCreateAlarmNode.class.getName());
+        TbCreateAlarmNodeConfiguration invalidCreateAlarmNodeConfiguration = new TbCreateAlarmNodeConfiguration();
+        invalidCreateAlarmNodeConfiguration.setSeverity("<script/>");
+        invalidCreateAlarmNodeConfiguration.setAlarmType("<script/>");
+        createAlarmNode.setConfiguration(mapper.valueToTree(invalidCreateAlarmNodeConfiguration));
+
+        List<RuleNode> ruleNodes = new ArrayList<>();
+        ruleNodes.add(createAlarmNode);
+        ruleChainMetaData.setFirstNodeIndex(0);
+        ruleChainMetaData.setNodes(ruleNodes);
+
+        String error = getErrorMessage(doPost("/api/ruleChain/metadata", ruleChainMetaData)
+                .andExpect(status().isBadRequest()));
+        assertThat(error).contains("severity is malformed");
+        assertThat(error).contains("alarmType is malformed");
+    }
+
     private RuleChain createRuleChain(String name) {
         RuleChain ruleChain = new RuleChain();
         ruleChain.setName(name);
         return doPost("/api/ruleChain", ruleChain, RuleChain.class);
     }
+
 }

common/data/src/main/java/org/thingsboard/server/common/data/alarm/Alarm.java

@@ -31,6 +31,7 @@ import org.thingsboard.server.common.data.id.CustomerId;
 import org.thingsboard.server.common.data.id.EntityId;
 import org.thingsboard.server.common.data.id.TenantId;
 import org.thingsboard.server.common.data.validation.Length;
+import org.thingsboard.server.common.data.validation.NoXss;
 
 import java.util.List;
 
@@ -49,6 +50,7 @@ public class Alarm extends BaseData<AlarmId> implements HasName, HasTenantId, Ha
     @ApiModelProperty(position = 4, value = "JSON object with Customer Id", accessMode = ApiModelProperty.AccessMode.READ_ONLY)
     private CustomerId customerId;
 
+    @NoXss
     @ApiModelProperty(position = 6, required = true, value = "representing type of the Alarm", example = "High Temperature Alarm")
     @Length(fieldName = "type")
     private String type;

common/data/src/main/java/org/thingsboard/server/common/data/rule/RuleChainMetaData.java

@@ -39,7 +39,6 @@ public class RuleChainMetaData {
     @ApiModelProperty(position = 2, required = true, value = "Index of the first rule node in the 'nodes' list")
     private Integer firstNodeIndex;
 
-    @Valid
     @ApiModelProperty(position = 3, required = true, value = "List of rule node JSON objects")
     private List<RuleNode> nodes;
 

common/data/src/main/java/org/thingsboard/server/common/data/util/ReflectionUtils.java

@@ -0,0 +1,31 @@
+/**
+ * Copyright © 2016-2022 The Thingsboard Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.thingsboard.server.common.data.util;
+
+import java.lang.annotation.Annotation;
+
+@SuppressWarnings("unchecked")
+public class ReflectionUtils {
+
+    private ReflectionUtils() {}
+
+    public static <T> T getAnnotationProperty(String targetType, String annotationType, String property) throws Exception {
+        Class<Annotation> annotationClass = (Class<Annotation>) Class.forName(annotationType);
+        Annotation annotation = Class.forName(targetType).getAnnotation(annotationClass);
+        return (T) annotationClass.getDeclaredMethod(property).invoke(annotation);
+    }
+
+}

common/data/src/main/java/org/thingsboard/server/common/data/validation/Length.java

@@ -26,7 +26,7 @@ import java.lang.annotation.Target;
 @Target(ElementType.FIELD)
 @Constraint(validatedBy = {})
 public @interface Length {
-    String message() default "length of {fieldName} must be equal or less than {max}";
+    String message() default "length must be equal or less than {max}";
 
     String fieldName();
 

common/data/src/main/java/org/thingsboard/server/common/data/validation/NoXss.java

@@ -26,7 +26,7 @@ import java.lang.annotation.Target;
 @Target(ElementType.FIELD)
 @Constraint(validatedBy = {})
 public @interface NoXss {
-    String message() default "field value is malformed";
+    String message() default "is malformed";
 
     Class<?>[] groups() default {};
 

dao/src/main/java/org/thingsboard/server/dao/audit/AuditLogServiceImpl.java

@@ -389,7 +389,7 @@ public class AuditLogServiceImpl implements AuditLogService {
         try {
             auditLogValidator.validate(auditLogEntry, AuditLog::getTenantId);
         } catch (Exception e) {
-            if (StringUtils.contains(e.getMessage(), "value is malformed")) {
+            if (StringUtils.contains(e.getMessage(), "is malformed")) {
                 auditLogEntry.setEntityName("MALFORMED");
             } else {
                 return Futures.immediateFailedFuture(e);

dao/src/main/java/org/thingsboard/server/dao/rule/BaseRuleChainService.java

@@ -51,22 +51,20 @@ import org.thingsboard.server.common.data.rule.RuleNode;
 import org.thingsboard.server.common.data.rule.RuleNodeUpdateResult;
 import org.thingsboard.server.dao.entity.AbstractEntityService;
 import org.thingsboard.server.dao.exception.DataValidationException;
-import org.thingsboard.server.dao.service.ConstraintValidator;
 import org.thingsboard.server.dao.service.DataValidator;
 import org.thingsboard.server.dao.service.PaginatedRemover;
 import org.thingsboard.server.dao.service.Validator;
+import org.thingsboard.server.dao.service.validator.RuleChainDataValidator;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.thingsboard.server.common.data.DataConstants.TENANT;
@@ -137,12 +135,7 @@ public class BaseRuleChainService extends AbstractEntityService implements RuleC
         if (ruleChain == null) {
             return RuleChainUpdateResult.failed();
         }
-        ConstraintValidator.validateFields(ruleChainMetaData);
+        RuleChainDataValidator.validateMetaData(ruleChainMetaData);
-        List<RuleNodeUpdateResult> updatedRuleNodes = new ArrayList<>();
-
-        if (CollectionUtils.isNotEmpty(ruleChainMetaData.getConnections())) {
-            validateCircles(ruleChainMetaData.getConnections());
-        }
 
         List<RuleNode> nodes = ruleChainMetaData.getNodes();
         List<RuleNode> toAddOrUpdate = new ArrayList<>();
@@ -160,6 +153,7 @@ public class BaseRuleChainService extends AbstractEntityService implements RuleC
             }
         }
 
+        List<RuleNodeUpdateResult> updatedRuleNodes = new ArrayList<>();
         List<RuleNode> existingRuleNodes = getRuleChainNodes(tenantId, ruleChainMetaData.getRuleChainId());
         for (RuleNode existingNode : existingRuleNodes) {
             deleteEntityRelations(tenantId, existingNode.getId());
@@ -249,31 +243,6 @@ public class BaseRuleChainService extends AbstractEntityService implements RuleC
         return RuleChainUpdateResult.successful(updatedRuleNodes);
     }
 
-    private void validateCircles(List<NodeConnectionInfo> connectionInfos) {
-        Map<Integer, Set<Integer>> connectionsMap = new HashMap<>();
-        for (NodeConnectionInfo nodeConnection : connectionInfos) {
-            if (nodeConnection.getFromIndex() == nodeConnection.getToIndex()) {
-                throw new DataValidationException("Can't create the relation to yourself.");
-            }
-            connectionsMap
-                    .computeIfAbsent(nodeConnection.getFromIndex(), from -> new HashSet<>())
-                    .add(nodeConnection.getToIndex());
-        }
-        connectionsMap.keySet().forEach(key -> validateCircles(key, connectionsMap.get(key), connectionsMap));
-    }
-
-    private void validateCircles(int from, Set<Integer> toList, Map<Integer, Set<Integer>> connectionsMap) {
-        if (toList == null) {
-            return;
-        }
-        for (Integer to : toList) {
-            if (from == to) {
-                throw new DataValidationException("Can't create circling relations in rule chain.");
-            }
-            validateCircles(from, connectionsMap.get(to), connectionsMap);
-        }
-    }
-
     @Override
     public RuleChainMetaData loadRuleChainMetaData(TenantId tenantId, RuleChainId ruleChainId) {
         Validator.validateId(ruleChainId, "Incorrect rule chain id.");

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

@@ -15,6 +15,7 @@
  */
 package org.thingsboard.server.dao.service;
 
+import com.google.common.collect.Iterators;
 import lombok.extern.slf4j.Slf4j;
 import org.hibernate.validator.HibernateValidator;
 import org.hibernate.validator.HibernateValidatorConfiguration;
@@ -23,11 +24,10 @@ import org.thingsboard.server.common.data.validation.Length;
 import org.thingsboard.server.common.data.validation.NoXss;
 import org.thingsboard.server.dao.exception.DataValidationException;
 
-import javax.validation.ConstraintViolation;
+import javax.validation.Path;
 import javax.validation.Validation;
 import javax.validation.Validator;
 import java.util.List;
-import java.util.Set;
 import java.util.stream.Collectors;
 
 @Slf4j
@@ -40,14 +40,25 @@ public class ConstraintValidator {
     }
 
     public static void validateFields(Object data) {
-        Set<ConstraintViolation<Object>> constraintsViolations = fieldsValidator.validate(data);
+        validateFields(data, "Validation error: ");
-        List<String> validationErrors = constraintsViolations.stream()
+    }
-                .map(ConstraintViolation::getMessage)
+
+    public static void validateFields(Object data, String errorPrefix) {
+        List<String> constraintsViolations = getConstraintsViolations(data);
+        if (!constraintsViolations.isEmpty()) {
+            throw new DataValidationException(errorPrefix + String.join(", ", constraintsViolations));
+        }
+    }
+
+    public static List<String> getConstraintsViolations(Object data) {
+        return fieldsValidator.validate(data).stream()
+                .map(constraintViolation -> {
+                    Path propertyPath = constraintViolation.getPropertyPath();
+                    String property = Iterators.getLast(propertyPath.iterator()).toString();
+                    return property + " " + constraintViolation.getMessage();
+                })
                 .distinct()
                 .collect(Collectors.toList());
-        if (!validationErrors.isEmpty()) {
-            throw new DataValidationException("Validation error: " + String.join(", ", validationErrors));
-        }
     }
 
     private static void initializeValidators() {
@@ -60,4 +71,5 @@ public class ConstraintValidator {
 
         fieldsValidator = validatorConfiguration.buildValidatorFactory().getValidator();
     }
+
 }

dao/src/main/java/org/thingsboard/server/dao/service/validator/RuleChainDataValidator.java

@@ -15,23 +15,39 @@
  */
 package org.thingsboard.server.dao.service.validator;
 
+import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.stereotype.Component;
+import org.thingsboard.common.util.JacksonUtil;
 import org.thingsboard.server.common.data.EntityType;
 import org.thingsboard.server.common.data.StringUtils;
 import org.thingsboard.server.common.data.id.TenantId;
+import org.thingsboard.server.common.data.rule.NodeConnectionInfo;
 import org.thingsboard.server.common.data.rule.RuleChain;
+import org.thingsboard.server.common.data.rule.RuleChainMetaData;
 import org.thingsboard.server.common.data.rule.RuleChainType;
+import org.thingsboard.server.common.data.rule.RuleNode;
 import org.thingsboard.server.common.data.tenant.profile.DefaultTenantProfileConfiguration;
+import org.thingsboard.server.common.data.util.ReflectionUtils;
 import org.thingsboard.server.dao.exception.DataValidationException;
 import org.thingsboard.server.dao.rule.RuleChainDao;
 import org.thingsboard.server.dao.rule.RuleChainService;
+import org.thingsboard.server.dao.service.ConstraintValidator;
 import org.thingsboard.server.dao.service.DataValidator;
 import org.thingsboard.server.dao.tenant.TbTenantProfileCache;
 import org.thingsboard.server.dao.tenant.TenantService;
 
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 @Component
+@Slf4j
 public class RuleChainDataValidator extends DataValidator<RuleChain> {
 
     @Autowired
@@ -83,4 +99,53 @@ public class RuleChainDataValidator extends DataValidator<RuleChain> {
             }
         }
     }
+
+    public static void validateMetaData(RuleChainMetaData ruleChainMetaData) {
+        ConstraintValidator.validateFields(ruleChainMetaData);
+        ruleChainMetaData.getNodes().forEach(RuleChainDataValidator::validateRuleNode);
+        if (CollectionUtils.isNotEmpty(ruleChainMetaData.getConnections())) {
+            validateCircles(ruleChainMetaData.getConnections());
+        }
+    }
+
+    public static void validateRuleNode(RuleNode ruleNode) {
+        String errorPrefix = "'" + ruleNode.getName() + "' node configuration is invalid: ";
+        ConstraintValidator.validateFields(ruleNode, errorPrefix);
+        Object nodeConfig;
+        try {
+            Class<Object> nodeConfigType = ReflectionUtils.getAnnotationProperty(ruleNode.getType(),
+                    "org.thingsboard.rule.engine.api.RuleNode", "configClazz");
+            nodeConfig = JacksonUtil.treeToValue(ruleNode.getConfiguration(), nodeConfigType);
+        } catch (Exception e) {
+            log.warn("Failed to validate node configuration: {}", ExceptionUtils.getRootCauseMessage(e));
+            return;
+        }
+        ConstraintValidator.validateFields(nodeConfig, errorPrefix);
+    }
+
+    private static void validateCircles(List<NodeConnectionInfo> connectionInfos) {
+        Map<Integer, Set<Integer>> connectionsMap = new HashMap<>();
+        for (NodeConnectionInfo nodeConnection : connectionInfos) {
+            if (nodeConnection.getFromIndex() == nodeConnection.getToIndex()) {
+                throw new DataValidationException("Can't create the relation to yourself.");
+            }
+            connectionsMap
+                    .computeIfAbsent(nodeConnection.getFromIndex(), from -> new HashSet<>())
+                    .add(nodeConnection.getToIndex());
+        }
+        connectionsMap.keySet().forEach(key -> validateCircles(key, connectionsMap.get(key), connectionsMap));
+    }
+
+    private static void validateCircles(int from, Set<Integer> toList, Map<Integer, Set<Integer>> connectionsMap) {
+        if (toList == null) {
+            return;
+        }
+        for (Integer to : toList) {
+            if (from == to) {
+                throw new DataValidationException("Can't create circling relations in rule chain.");
+            }
+            validateCircles(from, connectionsMap.get(to), connectionsMap);
+        }
+    }
+
 }

dao/src/test/java/org/thingsboard/server/dao/service/BaseOtaPackageServiceTest.java

@@ -674,7 +674,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         firmwareInfo.setTenantId(tenantId);
 
         thrown.expect(DataValidationException.class);
-        thrown.expectMessage("length of title must be equal or less than 255");
+        thrown.expectMessage("title length must be equal or less than 255");
 
         otaPackageService.saveOtaPackageInfo(firmwareInfo, true);
     }
@@ -689,7 +689,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         firmwareInfo.setTitle(TITLE);
 
         firmwareInfo.setVersion(StringUtils.random(257));
-        thrown.expectMessage("length of version must be equal or less than 255");
+        thrown.expectMessage("version length must be equal or less than 255");
 
         otaPackageService.saveOtaPackageInfo(firmwareInfo, true);
     }

dao/src/test/java/org/thingsboard/server/dao/service/NoXssValidatorTest.java

@@ -43,7 +43,7 @@ public class NoXssValidatorTest {
 
         assertThatThrownBy(() -> {
             ConstraintValidator.validateFields(invalidAsset);
-        }).hasMessageContaining("field value is malformed");
+        }).hasMessageContaining("is malformed");
     }
 
     @Test
@@ -55,7 +55,7 @@ public class NoXssValidatorTest {
 
         assertThatThrownBy(() -> {
             ConstraintValidator.validateFields(invalidAsset);
-        }).hasMessageContaining("field value is malformed");
+        }).hasMessageContaining("is malformed");
     }
 
 }

rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/action/TbAbstractAlarmNodeConfiguration.java

@@ -17,6 +17,7 @@ package org.thingsboard.rule.engine.action;
 
 import lombok.Data;
 import org.thingsboard.server.common.data.script.ScriptLanguage;
+import org.thingsboard.server.common.data.validation.NoXss;
 
 @Data
 public abstract class TbAbstractAlarmNodeConfiguration {
@@ -46,6 +47,7 @@ public abstract class TbAbstractAlarmNodeConfiguration {
             "return details;";
 
 
+    @NoXss
     private String alarmType;
     private ScriptLanguage scriptLang;
     private String alarmDetailsBuildJs;

rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/action/TbCreateAlarmNodeConfiguration.java

@@ -19,6 +19,7 @@ import lombok.Data;
 import org.thingsboard.rule.engine.api.NodeConfiguration;
 import org.thingsboard.server.common.data.alarm.AlarmSeverity;
 import org.thingsboard.server.common.data.script.ScriptLanguage;
+import org.thingsboard.server.common.data.validation.NoXss;
 
 import java.util.Collections;
 import java.util.List;
@@ -26,6 +27,7 @@ import java.util.List;
 @Data
 public class TbCreateAlarmNodeConfiguration extends TbAbstractAlarmNodeConfiguration implements NodeConfiguration<TbCreateAlarmNodeConfiguration> {
 
+    @NoXss
     private String severity;
     private boolean propagate;
     private boolean propagateToOwner;