From 043700d228afc1687938a2edcbdd096f1230d984 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Wed, 16 Nov 2022 17:55:53 +0800 Subject: [PATCH] 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 | 14 +++++---- engine/apps/public_api/tests/test_routes.py | 30 ++++++++++++++++++++ engine/common/api_helpers/mixins.py | 12 ++++++++ 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/engine/apps/public_api/serializers/routes.py b/engine/apps/public_api/serializers/routes.py index 5779b98e..04fa1821 100644 --- a/engine/apps/public_api/serializers/routes.py +++ b/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) diff --git a/engine/apps/public_api/tests/test_routes.py b/engine/apps/public_api/tests/test_routes.py index 184286c5..2cb88d9c 100644 --- a/engine/apps/public_api/tests/test_routes.py +++ b/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 diff --git a/engine/common/api_helpers/mixins.py b/engine/common/api_helpers/mixins.py index 3af06bcc..7a2f84c0 100644 --- a/engine/common/api_helpers/mixins.py +++ b/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):