From 2af4e850b8b9f3c24e96c7cb204dc62faa7b0c05 Mon Sep 17 00:00:00 2001 From: Stefan Behl Date: Fri, 2 Jul 2021 09:25:12 +0200 Subject: [PATCH] Consider Target Fields when validating RSQL filters (#1121) * Removed unnecessary DB queries when editing/validating RSQL query in Target Filter Management (#1023) Added valid TargetFields to RSQL validation when editing Target Filter. Signed-off-by: Sergey Gerasimov * Corrected visit OrNode implementation. Changed isValid to receive FieldNameProvider as parameter Reduced code duplication by moving commonly used utility methods to AbstractFieldNameRSQLVisitor from ValidationRSQLVisitor abd JpqQueryRSQLVisitor Refactored and extended Unit Tests. Minor corrections and typos. Signed-off-by: Sergey Gerasimov * Added Maven entry for devolo 2020 copyright header. Signed-off-by: Sergey Gerasimov * Fix failing unit tests * # WARNING: head commit changed in the meantime Signed-off-by: Stefan Behl * Fix Sonar findings. Signed-off-by: Stefan Behl * Cleanup Signed-off-by: Stefan Behl * Fix PR review findings * Fix invalid queries in unit tests Signed-off-by: Stefan Behl * Added test case to create filter with invalid query via Mgmt REST API Signed-off-by: Stefan Behl Co-authored-by: Sergey Gerasimov --- .../jpa/JpaTargetFilterQueryManagement.java | 34 ++- .../rsql/AbstractFieldNameRSQLVisitor.java | 147 +++++++++++++ .../repository/jpa/rsql/RSQLUtility.java | 204 ++++++------------ .../jpa/rsql/RsqlParserValidationOracle.java | 8 +- .../rsql/RSQLTargetFieldValidationTest.java | 42 ++++ .../repository/jpa/rsql/RSQLUtilityTest.java | 73 ++++++- .../MgmtTargetFilterQueryResourceTest.java | 28 ++- pom.xml | 1 + 8 files changed, 366 insertions(+), 171 deletions(-) create mode 100644 hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/AbstractFieldNameRSQLVisitor.java create mode 100644 hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldValidationTest.java diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java index a246a11d0..91a9a247c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java @@ -27,6 +27,7 @@ import org.eclipse.hawkbit.repository.builder.TargetFilterQueryUpdate; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.exception.InvalidAutoAssignActionTypeException; import org.eclipse.hawkbit.repository.exception.InvalidAutoAssignDistributionSetException; +import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException; import org.eclipse.hawkbit.repository.jpa.builder.JpaTargetFilterQueryCreate; import org.eclipse.hawkbit.repository.jpa.configuration.Constants; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet; @@ -43,6 +44,8 @@ import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer; import org.eclipse.hawkbit.security.SystemSecurityContext; import org.eclipse.hawkbit.tenancy.TenantAware; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.dao.ConcurrencyFailureException; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; @@ -58,6 +61,8 @@ import org.springframework.validation.annotation.Validated; import com.google.common.collect.Lists; +import cz.jirutka.rsql.parser.RSQLParserException; + /** * JPA implementation of {@link TargetFilterQueryManagement}. * @@ -66,6 +71,8 @@ import com.google.common.collect.Lists; @Validated public class JpaTargetFilterQueryManagement implements TargetFilterQueryManagement { + private static final Logger LOGGER = LoggerFactory.getLogger(JpaTargetFilterQueryManagement.class); + private final TargetFilterQueryRepository targetFilterQueryRepository; private final TargetManagement targetManagement; @@ -102,12 +109,18 @@ public class JpaTargetFilterQueryManagement implements TargetFilterQueryManageme public TargetFilterQuery create(final TargetFilterQueryCreate c) { final JpaTargetFilterQueryCreate create = (JpaTargetFilterQueryCreate) c; - // enforce the 'max targets per auto assign' quota right here even if - // the result of the filter query can vary over time - if (create.getAutoAssignDistributionSetId().isPresent()) { - WeightValidationHelper.usingContext(systemSecurityContext, tenantConfigurationManagement).validate(create); - create.getQuery().ifPresent(this::assertMaxTargetsQuota); - } + create.getQuery().ifPresent(query -> { + // validate the RSQL query syntax + RSQLUtility.validateRsqlFor(query, TargetFields.class); + + // enforce the 'max targets per auto assign' quota right here even + // if the result of the filter query can vary over time + if (create.getAutoAssignDistributionSetId().isPresent()) { + WeightValidationHelper.usingContext(systemSecurityContext, tenantConfigurationManagement) + .validate(create); + assertMaxTargetsQuota(query); + } + }); return targetFilterQueryRepository.save(create.build()); } @@ -291,8 +304,13 @@ public class JpaTargetFilterQueryManagement implements TargetFilterQueryManageme @Override public boolean verifyTargetFilterQuerySyntax(final String query) { - RSQLUtility.parse(query, TargetFields.class, virtualPropertyReplacer, database); - return true; + try { + RSQLUtility.validateRsqlFor(query, TargetFields.class); + return true; + } catch (RSQLParserException | RSQLParameterUnsupportedFieldException e) { + LOGGER.debug("The RSQL query '" + query + "' is invalid.", e); + return false; + } } private void assertMaxTargetsQuota(final String query) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/AbstractFieldNameRSQLVisitor.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/AbstractFieldNameRSQLVisitor.java new file mode 100644 index 000000000..ece88ff41 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/AbstractFieldNameRSQLVisitor.java @@ -0,0 +1,147 @@ +/** + * Copyright (c) 2020 devolo AG and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository.jpa.rsql; + +import cz.jirutka.rsql.parser.ast.ComparisonNode; +import org.eclipse.hawkbit.repository.FieldNameProvider; +import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +public abstract class AbstractFieldNameRSQLVisitor & FieldNameProvider> { + + private static final Logger LOGGER = LoggerFactory.getLogger(AbstractFieldNameRSQLVisitor.class); + + private final Class fieldNameProvider; + + public AbstractFieldNameRSQLVisitor(final Class fieldNameProvider) { + this.fieldNameProvider = fieldNameProvider; + } + + protected A getFieldEnumByName(final ComparisonNode node) { + String enumName = node.getSelector(); + final String[] graph = getSubAttributesFrom(enumName); + if (graph.length != 0) { + enumName = graph[0]; + } + LOGGER.debug("get field identifier by name {} of enum type {}", enumName, fieldNameProvider); + try { + return Enum.valueOf(fieldNameProvider, enumName.toUpperCase()); + } catch (final IllegalArgumentException e) { + throw createRSQLParameterUnsupportedException(node, e); + } + } + + protected static String[] getSubAttributesFrom(final String property) { + return property.split("\\" + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR); + } + + protected String getAndValidatePropertyFieldName(final A propertyEnum, final ComparisonNode node) { + + final String[] graph = getSubAttributesFrom(node.getSelector()); + + validateMapParameter(propertyEnum, node, graph); + + // sub entity need minimum 1 dot + if (!propertyEnum.getSubEntityAttributes().isEmpty() && graph.length < 2) { + throw createRSQLParameterUnsupportedException(node); + } + + final StringBuilder fieldNameBuilder = new StringBuilder(propertyEnum.getFieldName()); + + for (int i = 1; i < graph.length; i++) { + + final String propertyField = graph[i]; + fieldNameBuilder.append(FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR).append(propertyField); + + // the key of map is not in the graph + if (propertyEnum.isMap() && graph.length == (i + 1)) { + continue; + } + + if (!propertyEnum.containsSubEntityAttribute(propertyField)) { + throw createRSQLParameterUnsupportedException(node); + } + } + + return fieldNameBuilder.toString(); + } + + protected void validateMapParameter(final A propertyEnum, final ComparisonNode node, final String[] graph) { + if (!propertyEnum.isMap()) { + return; + + } + + if (!propertyEnum.getSubEntityAttributes().isEmpty()) { + throw new UnsupportedOperationException( + "Currently subentity attributes for maps are not supported, alternatively you could use the key/value tuple, defined by SimpleImmutableEntry class"); + } + + // enum.key + final int minAttributeForMap = 2; + if (graph.length != minAttributeForMap) { + throw new RSQLParameterUnsupportedFieldException("The syntax of the given map search parameter field {" + + node.getSelector() + "} is wrong. Syntax is: fieldname.keyname", new Exception()); + } + } + + protected RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException( + final ComparisonNode node) { + return createRSQLParameterUnsupportedException(node, new Exception()); + } + + protected RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException(final ComparisonNode node, + final Exception rootException) { + return createRSQLParameterUnsupportedException(String.format( + "The given search parameter field {%s} does not exist, must be one of the following fields %s", + node.getSelector(), getExpectedFieldList()), rootException); + } + + protected RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException(final String message) { + return createRSQLParameterUnsupportedException(message, null); + } + + protected RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException(final String message, + final Exception rootException) { + return new RSQLParameterUnsupportedFieldException(message, rootException); + } + + // Exception squid:S2095 - see + // https://jira.sonarsource.com/browse/SONARJAVA-1478 + @SuppressWarnings({ "squid:S2095" }) + private List getExpectedFieldList() { + final List expectedFieldList = Arrays.stream(fieldNameProvider.getEnumConstants()) + .filter(enumField -> enumField.getSubEntityAttributes().isEmpty()).map(enumField -> { + final String enumFieldName = enumField.name().toLowerCase(); + + if (enumField.isMap()) { + return enumFieldName + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR + "keyName"; + } + + return enumFieldName; + }).collect(Collectors.toList()); + + final List expectedSubFieldList = Arrays.stream(fieldNameProvider.getEnumConstants()) + .filter(enumField -> !enumField.getSubEntityAttributes().isEmpty()).flatMap(enumField -> { + final List subEntity = enumField + .getSubEntityAttributes().stream().map(fieldName -> enumField.name().toLowerCase() + + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR + fieldName) + .collect(Collectors.toList()); + + return subEntity.stream(); + }).collect(Collectors.toList()); + expectedFieldList.addAll(expectedSubFieldList); + return expectedFieldList; + } +} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtility.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtility.java index 7e0e03dff..6b2fac015 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtility.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtility.java @@ -140,19 +140,27 @@ public final class RSQLUtility { } /** - * Validate the given rsql string regarding existence and correct syntax. - * + * Validates the RSQL string + * * @param rsql - * the rsql string to get validated - * + * RSQL string to validate + * @param fieldNameProvider + * + * @throws RSQLParserException + * if RSQL syntax is invalid + * @throws RSQLParameterUnsupportedFieldException + * if RSQL key is not allowed */ - public static void isValid(final String rsql) { - parseRsql(rsql); + public static & FieldNameProvider> void validateRsqlFor(final String rsql, + final Class fieldNameProvider) { + final RSQLVisitor visitor = new ValidationRSQLVisitor<>(fieldNameProvider); + final Node rootNode = parseRsql(rsql); + rootNode.accept(visitor); } private static Node parseRsql(final String rsql) { try { - LOGGER.debug("parsing rsql string {}", rsql); + LOGGER.debug("Parsing rsql string {}", rsql); final Set operators = RSQLOperators.defaultOperators(); return new RSQLParser(operators).parse(rsql); } catch (final IllegalArgumentException e) { @@ -162,6 +170,36 @@ public final class RSQLUtility { } } + private static final class ValidationRSQLVisitor & FieldNameProvider> + extends AbstractFieldNameRSQLVisitor implements RSQLVisitor { + + public ValidationRSQLVisitor(final Class fieldNameProvider) { + super(fieldNameProvider); + } + + @Override + public Void visit(final AndNode node, final String param) { + return visitNode(node, param); + } + + @Override + public Void visit(final OrNode node, final String param) { + return visitNode(node, param); + } + + @Override + public Void visit(final ComparisonNode node, final String param) { + final A fieldName = getFieldEnumByName(node); + getAndValidatePropertyFieldName(fieldName, node); + return null; + } + + private Void visitNode(final LogicalNode node, final String param) { + node.getChildren().forEach(child -> child.accept(this, param)); + return null; + } + } + private static final class RSQLSpecification & FieldNameProvider, T> implements Specification { private static final long serialVersionUID = 1L; @@ -184,7 +222,7 @@ public final class RSQLUtility { final Node rootNode = parseRsql(rsql); query.distinct(true); - final JpqQueryRSQLVisitor jpqQueryRSQLVisitor = new JpqQueryRSQLVisitor<>(root, cb, enumType, + final JpaQueryRSQLVisitor jpqQueryRSQLVisitor = new JpaQueryRSQLVisitor<>(root, cb, enumType, virtualPropertyReplacer, database, query); final List accept = rootNode., String> accept(jpqQueryRSQLVisitor); @@ -198,9 +236,7 @@ public final class RSQLUtility { /** * An implementation of the {@link RSQLVisitor} to visit the parsed tokens - * and build jpa where clauses. - * - * + * and build JPA where clauses. * * @param * the enum for providing the field name of the entity field to @@ -208,34 +244,35 @@ public final class RSQLUtility { * @param * the entity type referenced by the root */ - private static final class JpqQueryRSQLVisitor & FieldNameProvider, T> - implements RSQLVisitor, String> { + private static final class JpaQueryRSQLVisitor & FieldNameProvider, T> + extends AbstractFieldNameRSQLVisitor implements RSQLVisitor, String> { + + private static final Logger LOGGER = LoggerFactory.getLogger(JpaQueryRSQLVisitor.class); + + public static final Character LIKE_WILDCARD = '*'; private static final char ESCAPE_CHAR = '\\'; private static final List NO_JOINS_OPERATOR = Lists.newArrayList("!=", "=out="); - public static final Character LIKE_WILDCARD = '*'; + private final Map>> joinsInLevel = new HashMap<>(3); - private final Root root; private final CriteriaBuilder cb; private final CriteriaQuery query; - private final Class enumType; + private final Database database; + private final Root root; + private final SimpleTypeConverter simpleTypeConverter; private final VirtualPropertyReplacer virtualPropertyReplacer; + private int level; private boolean isOrLevel; - private final Map>> joinsInLevel = new HashMap<>(3); private boolean joinsNeeded; - private final SimpleTypeConverter simpleTypeConverter; - - private final Database database; - - private JpqQueryRSQLVisitor(final Root root, final CriteriaBuilder cb, final Class enumType, + private JpaQueryRSQLVisitor(final Root root, final CriteriaBuilder cb, final Class enumType, final VirtualPropertyReplacer virtualPropertyReplacer, final Database database, final CriteriaQuery query) { + super(enumType); this.root = root; this.cb = cb; this.query = query; - this.enumType = enumType; this.virtualPropertyReplacer = virtualPropertyReplacer; this.simpleTypeConverter = new SimpleTypeConverter(); this.database = database; @@ -297,64 +334,6 @@ public final class RSQLUtility { return Collections.singletonList(predicate); } - private String getAndValidatePropertyFieldName(final A propertyEnum, final ComparisonNode node) { - - final String[] graph = getSubAttributesFrom(node.getSelector()); - - validateMapParameter(propertyEnum, node, graph); - - // sub entity need minium 1 dot - if (!propertyEnum.getSubEntityAttributes().isEmpty() && graph.length < 2) { - throw createRSQLParameterUnsupportedException(node); - } - - final StringBuilder fieldNameBuilder = new StringBuilder(propertyEnum.getFieldName()); - - for (int i = 1; i < graph.length; i++) { - - final String propertyField = graph[i]; - fieldNameBuilder.append(FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR).append(propertyField); - - // the key of map is not in the graph - if (propertyEnum.isMap() && graph.length == (i + 1)) { - continue; - } - - if (!propertyEnum.containsSubEntityAttribute(propertyField)) { - throw createRSQLParameterUnsupportedException(node); - } - } - - return fieldNameBuilder.toString(); - } - - private void validateMapParameter(final A propertyEnum, final ComparisonNode node, final String[] graph) { - if (!propertyEnum.isMap()) { - return; - - } - - if (!propertyEnum.getSubEntityAttributes().isEmpty()) { - throw new UnsupportedOperationException( - "Currently subentity attributes for maps are not supported, alternatively you could use the key/value tuple, defined by SimpleImmutableEntry class"); - } - - // enum.key - final int minAttributeForMap = 2; - if (graph.length != minAttributeForMap) { - throw new RSQLParameterUnsupportedFieldException("The syntax of the given map search parameter field {" - + node.getSelector() + "} is wrong. Syntax is: fieldname.keyname", new Exception()); - } - } - - private RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException( - final ComparisonNode node) { - return new RSQLParameterUnsupportedFieldException( - "The given search parameter field {" + node.getSelector() - + "} does not exist, must be one of the following fields {" + getExpectedFieldList() + "}", - new Exception()); - } - /** * Resolves the Path for a field in the persistence layer and joins the * required models. This operation is part of a tree traversal through @@ -374,12 +353,14 @@ public final class RSQLUtility { * dot notated field path * @return the Path for a field */ + @SuppressWarnings("unchecked") private Path getFieldPath(final A enumField, final String finalProperty) { return (Path) getFieldPath(root, getSubAttributesFrom(finalProperty), enumField.isMap(), this::getJoinFieldPath).orElseThrow( - () -> new RSQLParameterUnsupportedFieldException("RSQL field path cannot be empty", null)); + () -> createRSQLParameterUnsupportedException("RSQL field path cannot be empty", null)); } + @SuppressWarnings("unchecked") private Path getJoinFieldPath(final Path fieldPath, final String fieldNameSplit) { if (fieldPath instanceof PluralJoin) { final Join join = (Join) fieldPath; @@ -416,17 +397,7 @@ public final class RSQLUtility { // https://jira.sonarsource.com/browse/SONARJAVA-1478 @SuppressWarnings({ "squid:S2095" }) public List visit(final ComparisonNode node, final String param) { - A fieldName = null; - try { - fieldName = getFieldEnumByName(node); - } catch (final IllegalArgumentException e) { - throw new RSQLParameterUnsupportedFieldException("The given search parameter field {" - + node.getSelector() + "} does not exist, must be one of the following fields {" - + Arrays.stream(enumType.getEnumConstants()).map(v -> v.name().toLowerCase()) - .collect(Collectors.toList()) - + "}", e); - - } + final A fieldName = getFieldEnumByName(node); final String finalProperty = getAndValidatePropertyFieldName(fieldName, node); final List values = node.getArguments(); @@ -446,44 +417,6 @@ public final class RSQLUtility { return !NO_JOINS_OPERATOR.contains(node.getOperator().getSymbol()); } - // Exception squid:S2095 - see - // https://jira.sonarsource.com/browse/SONARJAVA-1478 - @SuppressWarnings({ "squid:S2095" }) - private List getExpectedFieldList() { - final List expectedFieldList = Arrays.stream(enumType.getEnumConstants()) - .filter(enumField -> enumField.getSubEntityAttributes().isEmpty()).map(enumField -> { - final String enumFieldName = enumField.name().toLowerCase(); - - if (enumField.isMap()) { - return enumFieldName + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR + "keyName"; - } - - return enumFieldName; - }).collect(Collectors.toList()); - - final List expectedSubFieldList = Arrays.stream(enumType.getEnumConstants()) - .filter(enumField -> !enumField.getSubEntityAttributes().isEmpty()).flatMap(enumField -> { - final List subEntity = enumField.getSubEntityAttributes().stream() - .map(fieldName -> enumField.name().toLowerCase() - + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR + fieldName) - .collect(Collectors.toList()); - - return subEntity.stream(); - }).collect(Collectors.toList()); - expectedFieldList.addAll(expectedSubFieldList); - return expectedFieldList; - } - - private A getFieldEnumByName(final ComparisonNode node) { - String enumName = node.getSelector(); - final String[] graph = getSubAttributesFrom(enumName); - if (graph.length != 0) { - enumName = graph[0]; - } - LOGGER.debug("get fieldidentifier by name {} of enum type {}", enumName, enumType); - return Enum.valueOf(enumType, enumName.toUpperCase()); - } - private Object convertValueIfNecessary(final ComparisonNode node, final A fieldName, final String value, final Path fieldPath) { // in case the value of an rsql query e.g. type==application is an @@ -522,7 +455,7 @@ public final class RSQLUtility { private Object convertFieldConverterValue(final ComparisonNode node, final A fieldName, final String value) { final Object convertedValue = ((FieldValueConverter) fieldName).convertValue(fieldName, value); if (convertedValue == null) { - throw new RSQLParameterUnsupportedFieldException( + throw createRSQLParameterUnsupportedException( "field {" + node.getSelector() + "} must be one of the following values {" + Arrays.toString(((FieldValueConverter) fieldName).possibleValues(fieldName)) + "}", null); @@ -775,10 +708,6 @@ public final class RSQLUtility { return cb.not(cb.exists(subquery)); } - private static String[] getSubAttributesFrom(final String property) { - return property.split("\\" + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR); - } - private static boolean isSimpleField(final String[] split, final boolean isMapKeyField) { return split.length == 1 || (split.length == 2 && isMapKeyField); } @@ -788,8 +717,7 @@ public final class RSQLUtility { return pathOfString(innerFieldPath); } if (innerFieldPath instanceof MapJoin) { - // Currently we support only string key .So below cast - // is safe. + // Currently we support only string key. So below cast is safe. return (Expression) (((MapJoin) pathOfString(innerFieldPath)).value()); } final String valueFieldName = enumField.getSubEntityMapTuple().map(Entry::getValue) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RsqlParserValidationOracle.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RsqlParserValidationOracle.java index 0c43cfbf6..ca602cf93 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RsqlParserValidationOracle.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RsqlParserValidationOracle.java @@ -21,7 +21,6 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import org.eclipse.hawkbit.repository.TargetFields; -import org.eclipse.hawkbit.repository.TargetManagement; import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException; import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException; import org.eclipse.hawkbit.repository.jpa.rsql.ParseExceptionWrapper.TokenWrapper; @@ -33,8 +32,6 @@ import org.eclipse.hawkbit.repository.rsql.ValidationOracleContext; import org.eclipse.persistence.exceptions.ConversionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.data.domain.PageRequest; import org.springframework.orm.jpa.JpaSystemException; import org.springframework.util.CollectionUtils; @@ -61,9 +58,6 @@ public class RsqlParserValidationOracle implements RsqlValidationOracle { private static final Logger LOGGER = LoggerFactory.getLogger(RsqlParserValidationOracle.class); - @Autowired - private TargetManagement targetManagement; - @Override public ValidationOracleContext suggest(final String rsqlQuery, final int cursorPosition) { @@ -76,7 +70,7 @@ public class RsqlParserValidationOracle implements RsqlValidationOracle { context.setSyntaxErrorContext(errorContext); try { - targetManagement.findByRsql(PageRequest.of(0, 1), rsqlQuery); + RSQLUtility.validateRsqlFor(rsqlQuery, TargetFields.class); context.setSyntaxError(false); suggestionContext.getSuggestions().addAll(getLogicalOperatorSuggestion(rsqlQuery)); } catch (final RSQLParameterSyntaxException | RSQLParserException ex) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldValidationTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldValidationTest.java new file mode 100644 index 000000000..168bc7764 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldValidationTest.java @@ -0,0 +1,42 @@ +/** + * Copyright (c) 2020 devolo AG and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository.jpa.rsql; + +import org.eclipse.hawkbit.repository.TargetFields; +import org.junit.jupiter.api.Test; + +import io.qameta.allure.Description; +import io.qameta.allure.Feature; +import io.qameta.allure.Story; + +@Feature("Component Tests - Repository") +@Story("RSQL target field validation") +public class RSQLTargetFieldValidationTest { + @Test + @Description("Testing allowed RSQL keys based on TargetFields.class") + public void rsqlValidTargetFields() { + final String rsql1 = "ID == '0123' and NAME == abcd and DESCRIPTION == absd" + + " and CREATEDAT =lt= 0123 and LASTMODIFIEDAT =gt= 0123" + + " and CONTROLLERID == 0123 and UPDATESTATUS == PENDING" + + " and IPADDRESS == 0123 and LASTCONTROLLERREQUESTAT == 0123" + " and tag == beta"; + + RSQLUtility.validateRsqlFor(rsql1, TargetFields.class); + + final String rsql2 = "ASSIGNEDDS.name == abcd and ASSIGNEDDS.version == 0123" + + " and INSTALLEDDS.name == abcd and INSTALLEDDS.version == 0123"; + RSQLUtility.validateRsqlFor(rsql2, TargetFields.class); + + final String rsql3 = "ATTRIBUTE.subkey1 == test and ATTRIBUTE.subkey2 == test" + + " and METADATA.metakey1 == abcd and METADATA.metavalue2 == asdfg"; + RSQLUtility.validateRsqlFor(rsql3, TargetFields.class); + + final String rsql4 = "CREATEDAT =lt= ${NOW_TS} and LASTMODIFIEDAT =ge= ${OVERDUE_TS}"; + RSQLUtility.validateRsqlFor(rsql4, TargetFields.class); + } +} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java index c7d0e4cdc..d228aa1f3 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java @@ -8,6 +8,7 @@ */ package org.eclipse.hawkbit.repository.jpa.rsql; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -19,6 +20,9 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.Arrays; +import java.util.List; + import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; import javax.persistence.criteria.Expression; @@ -99,6 +103,46 @@ public class RSQLUtilityTest { private static final TenantConfigurationValue TEST_POLLING_OVERDUE_TIME_INTERVAL = TenantConfigurationValue . builder().value("00:07:37").build(); + @Test + @Description("Testing throwing exception in case of not allowed RSQL key") + public void rsqlUnsupportedFieldExceptionTest() { + final String rsql1 = "wrongfield == abcd"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql1, TestFieldEnum.class)); + + final String rsql2 = "wrongfield == abcd or TESTFIELD_WITH_SUB_ENTITIES.subentity11 == 0123"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql2, TestFieldEnum.class)); + } + + @Test + @Description("Testing exception in case of not allowed subkey") + public void rsqlUnsupportedSubkeyThrowException() { + final String rsql1 = "TESTFIELD_WITH_SUB_ENTITIES.unsupported == abcd and TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql1, TestFieldEnum.class)); + + final String rsql2 = "TESTFIELD_WITH_SUB_ENTITIES.unsupported == abcd or TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql2, TestFieldEnum.class)); + + final String rsql3 = "TESTFIELD == abcd or TESTFIELD_WITH_SUB_ENTITIES.unsupported == 0123"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql3, TestFieldEnum.class)); + } + + @Test + @Description("Testing valid RSQL keys based on TestFieldEnum.class") + public void rsqlFieldValidation() { + final String rsql1 = "TESTFIELD_WITH_SUB_ENTITIES.subentity11 == abcd and TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123"; + final String rsql2 = "TESTFIELD_WITH_SUB_ENTITIES.subentity11 == abcd or TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123"; + final String rsql3 = "TESTFIELD_WITH_SUB_ENTITIES.subentity11 == abcd and TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123 and TESTFIELD == any"; + + RSQLUtility.validateRsqlFor(rsql1, TestFieldEnum.class); + RSQLUtility.validateRsqlFor(rsql2, TestFieldEnum.class); + RSQLUtility.validateRsqlFor(rsql3, TestFieldEnum.class); + } + @Test public void wrongRsqlSyntaxThrowSyntaxException() { final String wrongRSQL = "name==abc;d"; @@ -421,23 +465,32 @@ public class RSQLUtilityTest { } private enum TestFieldEnum implements FieldNameProvider { - TESTFIELD; + TESTFIELD("testfield"), TESTFIELD_WITH_SUB_ENTITIES("testfieldWithSubEntities", "subentity11", "subentity22"); + + private final String fieldName; + private List subEntityAttributes; + + TestFieldEnum(final String fieldName) { + this(fieldName, new String[0]); + } + + TestFieldEnum(final String fieldName, final String... subEntityAttributes) { + this.fieldName = fieldName; + this.subEntityAttributes = (Arrays.asList(subEntityAttributes)); + } - /* - * (non-Javadoc) - * - * @see - * org.eclipse.hawkbit.server.rest.resource.model.FieldNameProvider# - * getFieldName() - */ @Override public String getFieldName() { - return "testfield"; + return this.fieldName; + } + + @Override + public List getSubEntityAttributes() { + return subEntityAttributes; } } private enum TestValueEnum { BUMLUX; } - } diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetFilterQueryResourceTest.java b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetFilterQueryResourceTest.java index 83a559a80..f97e7c0f1 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetFilterQueryResourceTest.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetFilterQueryResourceTest.java @@ -85,7 +85,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte @Description("Ensures that deletion is executed if permitted.") public void deleteTargetFilterQueryReturnsOK() throws Exception { final String filterName = "filter_01"; - final TargetFilterQuery filterQuery = createSingleTargetFilterQuery(filterName, "name=test_01"); + final TargetFilterQuery filterQuery = createSingleTargetFilterQuery(filterName, "name==test_01"); mvc.perform(delete(MgmtRestConstants.TARGET_FILTER_V1_REQUEST_MAPPING + "/" + filterQuery.getId())) .andExpect(status().isOk()); @@ -114,8 +114,8 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte @Description("Ensures that update request is reflected by repository.") public void updateTargetFilterQueryQuery() throws Exception { final String filterName = "filter_02"; - final String filterQuery = "name=test_02"; - final String filterQuery2 = "name=test_02_changed"; + final String filterQuery = "name==test_02"; + final String filterQuery2 = "name==test_02_changed"; final String body = new JSONObject().put("query", filterQuery2).toString(); // prepare @@ -137,7 +137,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte public void updateTargetFilterQueryName() throws Exception { final String filterName = "filter_03"; final String filterName2 = "filter_03_changed"; - final String filterQuery = "name=test_03"; + final String filterQuery = "name==test_03"; final String body = new JSONObject().put("name", filterName2).toString(); // prepare @@ -162,7 +162,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte final String idA = "a"; final String idB = "b"; final String idC = "c"; - final String testQuery = "name=test"; + final String testQuery = "name==test"; createSingleTargetFilterQuery(idA, testQuery); createSingleTargetFilterQuery(idB, testQuery); @@ -191,7 +191,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte final String idA = "a"; final String idB = "b"; final String idC = "c"; - final String testQuery = "name=test"; + final String testQuery = "name==test"; createSingleTargetFilterQuery(idA, testQuery); createSingleTargetFilterQuery(idB, testQuery); @@ -217,7 +217,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte final String idC = "c"; final String idD = "d"; final String idE = "e"; - final String testQuery = "name=test"; + final String testQuery = "name==test"; createSingleTargetFilterQuery("a", testQuery); createSingleTargetFilterQuery("b", testQuery); @@ -247,7 +247,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte @Description("Ensures that a single target filter query can be retrieved via its id.") public void getSingleTarget() throws Exception { // create first a target which can be retrieved by rest interface - final String knownQuery = "name=test01"; + final String knownQuery = "name==test01"; final String knownName = "someName"; final TargetFilterQuery tfq = createSingleTargetFilterQuery(knownName, knownQuery); final String hrefPrefix = "http://localhost" + MgmtRestConstants.TARGET_FILTER_V1_REQUEST_MAPPING + "/" @@ -294,6 +294,18 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte assertThat(exceptionInfo.getExceptionClass()).isEqualTo(MessageNotReadableException.class.getName()); assertThat(exceptionInfo.getErrorCode()).isEqualTo(SpServerError.SP_REST_BODY_NOT_READABLE.getKey()); } + + @Test + @Description("Ensures that the creation of a target filter query based on an invalid RSQL query results in a HTTP Bad Request error (400).") + public void createTargetFilterWithInvalidQuery() throws Exception { + final String invalidQuery = "name=abc"; + final String body = new JSONObject().put("query", invalidQuery).put("name", "invalidFilter").toString(); + + mvc.perform(post(MgmtRestConstants.TARGET_FILTER_V1_REQUEST_MAPPING).content(body) + .contentType(MediaType.APPLICATION_JSON)).andDo(print()).andExpect(status().isBadRequest()).andReturn(); + + assertThat(targetFilterQueryManagement.count()).isEqualTo(0); + } @Test @Description("Ensures that the assignment of an auto-assign distribution set results in a HTTP Forbidden error (403) " diff --git a/pom.xml b/pom.xml index d6f2ad870..68c92ad8e 100644 --- a/pom.xml +++ b/pom.xml @@ -332,6 +332,7 @@ licenses/LICENSE_HEADER_TEMPLATE_MICROSOFT_18.txt licenses/LICENSE_HEADER_TEMPLATE_MICROSOFT_20.txt licenses/LICENSE_HEADER_TEMPLATE_DEVOLO_19.txt + licenses/LICENSE_HEADER_TEMPLATE_DEVOLO_20.txt licenses/LICENSE_HEADER_TEMPLATE_KIWIGRID_19.txt licenses/LICENSE_HEADER_TEMPLATE_ENAPTER.txt