From e41fc6af029adc6d0e70e8f24a2b227c3493659e Mon Sep 17 00:00:00 2001
From: Maciej Pichura <41297682+mpichura@users.noreply.github.com>
Date: Mon, 7 Nov 2022 15:31:19 +0100
Subject: [PATCH] ACS-3855: Add validation for rule condition comparator.
 (#1538)

---
 .../alfresco/rest/rules/CreateRulesTests.java | 22 +++++++++++++++++++
 .../RestRuleSimpleConditionModelMapper.java   | 13 ++++++++++-
 ...estRuleSimpleConditionModelMapperTest.java | 17 ++++++++++++++
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java
index 68605676b5..3cfc1902a0 100644
--- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java
+++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java
@@ -951,6 +951,28 @@ public class CreateRulesTests extends RestTest
         restClient.assertLastError().containsSummary("Category in condition is invalid");
     }
 
+    /**
+     * Check we get 400 error when condition comparator is invalid
+     */
+    @Test(groups = {TestGroup.REST_API, TestGroup.RULES})
+    public void createRuleWithConditions_invalidComparator()
+    {
+        STEP("Try to create a rule with invalid comparator in conditions.");
+        final String comparator = "greaterthan";
+        RestCompositeConditionDefinitionModel conditions = rulesUtils.createCompositeCondition(List.of(
+                rulesUtils.createCompositeCondition(!INVERTED, List.of(
+                        rulesUtils.createSimpleCondition("size", comparator, "500")
+                ))
+        ));
+        RestRuleModel ruleModel = rulesUtils.createRuleModelWithDefaultValues();
+        ruleModel.setConditions(conditions);
+
+        restClient.authenticateUser(user).withPrivateAPI().usingNode(ruleFolder).usingDefaultRuleSet().createSingleRule(ruleModel);
+
+        restClient.assertStatusCodeIs(BAD_REQUEST);
+        restClient.assertLastError().containsSummary("Comparator value for condition is invalid: " + comparator);
+    }
+
     private String getAddPermissionsBody(String username, String role)
     {
         JsonObject userPermission = Json.createObjectBuilder().add("permissions",
diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapper.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapper.java
index 8f16fcb15c..45ba6fbdff 100644
--- a/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapper.java
+++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapper.java
@@ -64,6 +64,7 @@ public class RestRuleSimpleConditionModelMapper implements RestModelMapper<Simpl
     static final String FIELD_NOT_NULL = "Field in condition must not be blank";
     static final String PARAMETER_NOT_NULL = "Parameter in condition must not be blank";
     static final String COMPARATOR_NOT_NULL = "Comparator in condition must not be blank";
+    static final String INVALID_COMPARATOR_VALUE = "Comparator value for condition is invalid: %s";
     private final NamespaceService namespaceService;
     private final Nodes nodes;
 
@@ -157,13 +158,23 @@ public class RestRuleSimpleConditionModelMapper implements RestModelMapper<Simpl
                     parameterValues.put(ComparePropertyValueEvaluator.PARAM_PROPERTY, QName.createQName(field, namespaceService));
                 }
                 checkStringNotBlank(restModel.getComparator(), COMPARATOR_NOT_NULL);
-                parameterValues.put(ComparePropertyValueEvaluator.PARAM_OPERATION, restModel.getComparator().toUpperCase());
+                parameterValues.put(ComparePropertyValueEvaluator.PARAM_OPERATION, getComparatorValue(restModel.getComparator()));
                 parameterValues.put(ComparePropertyValueEvaluator.PARAM_VALUE, parameter);
                 break;
         }
         return new ActionConditionImpl(UUID.randomUUID().toString(), conditionDefinitionId, parameterValues);
     }
 
+    private String getComparatorValue(String comparator)
+    {
+        try
+        {
+            return ComparePropertyValueOperation.valueOf(comparator.toUpperCase()).toString();
+        } catch (IllegalArgumentException e) {
+            throw new InvalidArgumentException(String.format(INVALID_COMPARATOR_VALUE, comparator));
+        }
+    }
+
     private void checkStringNotBlank(final String string, final String message) {
         if (Strings.isBlank(string))
         {
diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapperTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapperTest.java
index 6a502a1224..bbe7d00c94 100644
--- a/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapperTest.java
+++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleSimpleConditionModelMapperTest.java
@@ -28,6 +28,7 @@ package org.alfresco.rest.api.impl.mapper.rules;
 
 import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.COMPARATOR_NOT_NULL;
 import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.FIELD_NOT_NULL;
+import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.INVALID_COMPARATOR_VALUE;
 import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.PARAMETER_NOT_NULL;
 import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.PARAM_CATEGORY;
 import static org.alfresco.rest.api.impl.mapper.rules.RestRuleSimpleConditionModelMapper.PARAM_MIMETYPE;
@@ -366,6 +367,22 @@ public class RestRuleSimpleConditionModelMapperTest
                 .hasMessageContaining(COMPARATOR_NOT_NULL);
     }
 
+    @Test
+    public void testToServiceModel_invalidComparator()
+    {
+        final String comparator = "greaterthan";
+        final SimpleCondition simpleConditionNullComparator = SimpleCondition.builder()
+                .field("size")
+                .comparator(comparator)
+                .parameter("65000")
+                .create();
+
+        // when
+        assertThatThrownBy(() -> objectUnderTest.toServiceModel(simpleConditionNullComparator))
+                .isInstanceOf(InvalidArgumentException.class)
+                .hasMessageContaining(String.format(INVALID_COMPARATOR_VALUE, comparator));
+    }
+
     private static ActionCondition createActionCondition(final String actionDefinitionName)
     {
         return new ActionConditionImpl("fake-id", actionDefinitionName, createParameterValues());