diff --git a/engine/apps/api/serializers/custom_button.py b/engine/apps/api/serializers/custom_button.py index 11e184e7..0ece3b2e 100644 --- a/engine/apps/api/serializers/custom_button.py +++ b/engine/apps/api/serializers/custom_button.py @@ -1,4 +1,5 @@ import json +from collections import defaultdict from django.core.validators import URLValidator, ValidationError from jinja2 import Template, TemplateError @@ -51,15 +52,33 @@ class CustomButtonSerializer(serializers.ModelSerializer): return None try: - json.loads(data) - except ValueError: - raise serializers.ValidationError("Data has incorrect format") - - try: - Template(data) + template = Template(data) except TemplateError: raise serializers.ValidationError("Data has incorrect template") + try: + rendered = template.render( + { + # Validate that the template can be rendered with a JSON-ish alert payload. + # We don't know what the actual payload will be, so we use a defaultdict + # so that attribute access within a template will never fail + # (provided it's only one level deep - we won't accept templates that attempt + # to do nested attribute access). + # Every attribute access should return a string to ensure that users are + # correctly using `tojson` or wrapping fields in strings. + # If we instead used a `defaultdict(dict)` or `defaultdict(lambda: 1)` we + # would accidentally accept templates such as `{"name": {{ alert_payload.name }}}` + # which would then fail at the true render time due to the + # lack of explicit quotes around the template variable; this would render + # as `{"name": some_alert_name}` which is not valid JSON. + "alert_payload": defaultdict(str), + "alert_group_id": "abcd", + } + ) + json.loads(rendered) + except ValueError: + raise serializers.ValidationError("Data has incorrect format") + return data def validate_forward_whole_payload(self, data): diff --git a/engine/apps/api/tests/test_custom_button.py b/engine/apps/api/tests/test_custom_button.py index bc91fc60..d957b9a1 100644 --- a/engine/apps/api/tests/test_custom_button.py +++ b/engine/apps/api/tests/test_custom_button.py @@ -128,6 +128,91 @@ def test_create_valid_data_button(custom_button_internal_api_setup, make_user_au assert response.json() == expected_response +@pytest.mark.django_db +def test_create_valid_nested_data_button(custom_button_internal_api_setup, make_user_auth_headers): + user, token, custom_button = custom_button_internal_api_setup + client = APIClient() + url = reverse("api-internal:custom_button-list") + + data = { + "name": "amixr_button_with_valid_data", + "webhook": TEST_URL, + # Assert that nested field access still works as long as the variable + # is quoted, making it valid JSON. + # This ensures backwards compatibility from when templates were required + # to be JSON. + "data": '{"nested_item": "{{ alert_payload.foo.bar }}"}', + "team": None, + } + + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + # modify initial data by adding id and None for optional fields + custom_button = CustomButton.objects.get(public_primary_key=response.data["id"]) + expected_response = data | { + "id": custom_button.public_primary_key, + "user": None, + "password": None, + "authorization_header": None, + "forward_whole_payload": False, + } + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_response + + +@pytest.mark.django_db +def test_create_valid_data_after_render_button(custom_button_internal_api_setup, make_user_auth_headers): + user, token, custom_button = custom_button_internal_api_setup + client = APIClient() + url = reverse("api-internal:custom_button-list") + + data = { + "name": "amixr_button_with_valid_data", + "webhook": TEST_URL, + "data": '{"name": "{{ alert_payload.name }}", "labels": {{ alert_payload.labels | tojson }}}', + "team": None, + } + + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + # modify initial data by adding id and None for optional fields + custom_button = CustomButton.objects.get(public_primary_key=response.data["id"]) + expected_response = data | { + "id": custom_button.public_primary_key, + "user": None, + "password": None, + "authorization_header": None, + "forward_whole_payload": False, + } + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_response + + +@pytest.mark.django_db +def test_create_valid_data_after_render_use_all_data_button(custom_button_internal_api_setup, make_user_auth_headers): + user, token, custom_button = custom_button_internal_api_setup + client = APIClient() + url = reverse("api-internal:custom_button-list") + + data = { + "name": "amixr_button_with_valid_data", + "webhook": TEST_URL, + "data": "{{ alert_payload | tojson }}", + "team": None, + } + + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + # modify initial data by adding id and None for optional fields + custom_button = CustomButton.objects.get(public_primary_key=response.data["id"]) + expected_response = data | { + "id": custom_button.public_primary_key, + "user": None, + "password": None, + "authorization_header": None, + "forward_whole_payload": False, + } + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_response + + @pytest.mark.django_db def test_create_invalid_url_custom_button(custom_button_internal_api_setup, make_user_auth_headers): user, token, custom_button = custom_button_internal_api_setup @@ -157,6 +242,22 @@ def test_create_invalid_data_custom_button(custom_button_internal_api_setup, mak assert response.status_code == status.HTTP_400_BAD_REQUEST +@pytest.mark.django_db +def test_create_invalid_templated_data_custom_button(custom_button_internal_api_setup, make_user_auth_headers): + user, token, custom_button = custom_button_internal_api_setup + client = APIClient() + url = reverse("api-internal:custom_button-list") + + data = { + "name": "amixr_button_invalid_data", + "webhook": TEST_URL, + # This would need a `| tojson` or some double quotes around it to pass validation. + "data": "{{ alert_payload.name }}", + } + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db def test_update_custom_button(custom_button_internal_api_setup, make_user_auth_headers): user, token, custom_button = custom_button_internal_api_setup diff --git a/engine/apps/public_api/serializers/action.py b/engine/apps/public_api/serializers/action.py index f652bc7c..ab9e4740 100644 --- a/engine/apps/public_api/serializers/action.py +++ b/engine/apps/public_api/serializers/action.py @@ -1,4 +1,5 @@ import json +from collections import defaultdict from django.core.validators import URLValidator, ValidationError from jinja2 import Template, TemplateError @@ -55,15 +56,33 @@ class ActionCreateSerializer(serializers.ModelSerializer): return None try: - json.loads(data) - except ValueError: - raise serializers.ValidationError("Data has incorrect format") - - try: - Template(data) + template = Template(data) except TemplateError: raise serializers.ValidationError("Data has incorrect template") + try: + rendered = template.render( + { + # Validate that the template can be rendered with a JSON-ish alert payload. + # We don't know what the actual payload will be, so we use a defaultdict + # so that attribute access within a template will never fail + # (provided it's only one level deep - we won't accept templates that attempt + # to do nested attribute access). + # Every attribute access should return a string to ensure that users are + # correctly using `tojson` or wrapping fields in strings. + # If we instead used a `defaultdict(dict)` or `defaultdict(lambda: 1)` we + # would accidentally accept templates such as `{"name": {{ alert_payload.name }}}` + # which would then fail at the true render time due to the + # lack of explicit quotes around the template variable; this would render + # as `{"name": some_alert_name}` which is not valid JSON. + "alert_payload": defaultdict(str), + "alert_group_id": "abcd", + } + ) + json.loads(rendered) + except ValueError: + raise serializers.ValidationError("Data has incorrect format") + return data def validate_forward_whole_payload(self, data): diff --git a/engine/apps/public_api/tests/test_custom_actions.py b/engine/apps/public_api/tests/test_custom_actions.py index 9fb4ebb6..7c6a55e3 100644 --- a/engine/apps/public_api/tests/test_custom_actions.py +++ b/engine/apps/public_api/tests/test_custom_actions.py @@ -167,6 +167,120 @@ def test_create_custom_action(make_organization_and_user_with_token): assert response.data == expected_result +@pytest.mark.django_db +def test_create_custom_action_nested_data(make_organization_and_user_with_token): + + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + url = reverse("api-public:actions-list") + + data = { + "name": "Test outgoing webhook with nested data", + "url": "https://example.com", + # Assert that nested field access still works as long as the variable + # is quoted, making it valid JSON. + # This ensures backwards compatibility from when templates were required + # to be JSON. + "data": '{"nested_item": "{{ alert_payload.foo.bar }}"}', + } + + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + + custom_action = CustomButton.objects.get(public_primary_key=response.data["id"]) + + expected_result = { + "id": custom_action.public_primary_key, + "name": custom_action.name, + "team_id": None, + "url": custom_action.webhook, + "data": custom_action.data, + "user": custom_action.user, + "password": custom_action.password, + "authorization_header": custom_action.authorization_header, + "forward_whole_payload": custom_action.forward_whole_payload, + } + + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_result + + +@pytest.mark.django_db +def test_create_custom_action_valid_after_render(make_organization_and_user_with_token): + + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + url = reverse("api-public:actions-list") + + data = { + "name": "Test outgoing webhook with nested data", + "url": "https://example.com", + # Assert that nested field access still works as long as the variable + # is quoted, making it valid JSON. + # This ensures backwards compatibility from when templates were required + # to be JSON. + "data": '{"name": "{{ alert_payload.name }}", "labels": {{ alert_payload.labels | tojson }}}', + } + + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + + custom_action = CustomButton.objects.get(public_primary_key=response.data["id"]) + + expected_result = { + "id": custom_action.public_primary_key, + "name": custom_action.name, + "team_id": None, + "url": custom_action.webhook, + "data": custom_action.data, + "user": custom_action.user, + "password": custom_action.password, + "authorization_header": custom_action.authorization_header, + "forward_whole_payload": custom_action.forward_whole_payload, + } + + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_result + + +@pytest.mark.django_db +def test_create_custom_action_valid_after_render_use_all_data(make_organization_and_user_with_token): + + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + url = reverse("api-public:actions-list") + + data = { + "name": "Test outgoing webhook with nested data", + "url": "https://example.com", + # Assert that nested field access still works as long as the variable + # is quoted, making it valid JSON. + # This ensures backwards compatibility from when templates were required + # to be JSON. + "data": "{{ alert_payload | tojson }}", + } + + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + + custom_action = CustomButton.objects.get(public_primary_key=response.data["id"]) + + expected_result = { + "id": custom_action.public_primary_key, + "name": custom_action.name, + "team_id": None, + "url": custom_action.webhook, + "data": custom_action.data, + "user": custom_action.user, + "password": custom_action.password, + "authorization_header": custom_action.authorization_header, + "forward_whole_payload": custom_action.forward_whole_payload, + } + + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_result + + @pytest.mark.django_db def test_create_custom_action_invalid_data( make_organization_and_user_with_token, @@ -205,6 +319,29 @@ def test_create_custom_action_invalid_data( assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.data["name"][0] == "This field is required." + data = { + "name": "Test outgoing webhook", + "url": "https://example.com", + "data": "invalid_json", + } + + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data["data"][0] == "Data has incorrect format" + + data = { + "name": "Test outgoing webhook", + "url": "https://example.com", + # This would need a `| tojson` or some double quotes around it to pass validation. + "data": "{{ alert_payload.name }}", + } + + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data["data"][0] == "Data has incorrect format" + @pytest.mark.django_db def test_update_custom_action(