Add additional validation for route's position field (#831)

* Add test for out of range route's order value

* Add additional validation for route's position when manual ordering is applied

* Remove print()

* Remove print

engine/apps/public_api/serializers/routes.py

@@ -163,15 +163,16 @@ class ChannelFilterSerializer(BaseChannelFilterSerializer):
     def create(self, validated_data):
         validated_data = self._correct_validated_data(validated_data)
         manual_order = validated_data.pop("manual_order")
-        if not manual_order:
+        if manual_order:
+            self._validate_manual_order(validated_data.get("order", None))
+            instance = super().create(validated_data)
+        else:
             order = validated_data.pop("order", None)
             alert_receive_channel_id = validated_data.get("alert_receive_channel")
             # validate 'order' value before creation
             self._validate_order(order, {"alert_receive_channel_id": alert_receive_channel_id, "is_default": False})
             instance = super().create(validated_data)
             self._change_position(order, instance)
-        else:
-            instance = super().create(validated_data)
 
         return instance
 
@@ -206,10 +207,13 @@ class ChannelFilterUpdateSerializer(ChannelFilterSerializer):
         validated_data = self._correct_validated_data(validated_data)
 
         manual_order = validated_data.pop("manual_order")
-        if not manual_order:
+        if manual_order:
+            self._validate_manual_order(validated_data.get("order", None))
+        else:
             order = validated_data.pop("order", None)
             self._validate_order(
-                order, {"alert_receive_channel_id": instance.alert_receive_channel_id, "is_default": False}
+                order,
+                {"alert_receive_channel_id": instance.alert_receive_channel_id, "is_default": instance.is_default},
             )
             self._change_position(order, instance)
 

engine/apps/public_api/tests/test_routes.py

@@ -381,3 +381,33 @@ def test_update_route_with_messaging_backend(
     assert new_channel_filter.notify_in_slack == data_to_update["slack"]["enabled"]
     assert new_channel_filter.notify_in_telegram == data_to_update["telegram"]["enabled"]
     assert new_channel_filter.notification_backends == {TestOnlyBackend.backend_id: {"channel": None, "enabled": True}}
+
+
+@pytest.mark.django_db
+def test_update_route_with_manual_ordering(
+    make_organization_and_user_with_token,
+    make_alert_receive_channel,
+    make_channel_filter,
+):
+    organization, _, token = make_organization_and_user_with_token()
+    alert_receive_channel = make_alert_receive_channel(organization)
+    channel_filter = make_channel_filter(
+        alert_receive_channel,
+        is_default=False,
+    )
+
+    client = APIClient()
+
+    url = reverse("api-public:routes-detail", kwargs={"pk": channel_filter.public_primary_key})
+
+    # Test negative value. Note, that for "manual_order"=False, -1 is valud option (It will move route to the bottom)
+    data_to_update = {"position": -1, "manual_order": True}
+
+    response = client.put(url, format="json", HTTP_AUTHORIZATION=token, data=data_to_update)
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+    # Test value bigger then PositiveIntegerField can hold
+    data_to_update = {"position": 9223372036854775807, "manual_order": True}
+
+    response = client.put(url, format="json", HTTP_AUTHORIZATION=token, data=data_to_update)
+    assert response.status_code == status.HTTP_400_BAD_REQUEST

engine/common/api_helpers/mixins.py

@@ -159,6 +159,18 @@ class OrderedModelSerializerMixin:
             if order > max_order:
                 raise BadRequest(detail="Invalid value for position field")
 
+    def _validate_manual_order(self, order):
+        """
+        For manual ordering validate just that order is valid PositiveIntegrer.
+        User of manual ordering is responsible for correct ordering.
+        However, manual ordering not intended for use somewhere, except terraform provider.
+        """
+
+        # https://docs.djangoproject.com/en/4.1/ref/models/fields/#positiveintegerfield
+        MAX_POSITIVE_INTEGER = 2147483647
+        if order is not None and order < 0 or order > MAX_POSITIVE_INTEGER:
+            raise BadRequest(detail="Invalid value for position field")
+
 
 class PublicPrimaryKeyMixin:
     def get_object(self):