centralcloud/oncall-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Allow non-JSON custom webhook templates, provided they render as valid JSON (#639)

Browse source 
* Allow non-JSON custom webhook templates, provided they render as valid JSON

Previously, both the provided template _and_ the rendered template had
to be valid JSON in order for validation to pass. This was unnecessarily
restrictive: really, only the rendered template needs to be valid JSON.
It also disallowed using templates such as:

    {
      "labels": {{ alert_payload.labels | tojson }}
    }

even though this would be valid JSON after rendering.

This commit relaxes the validation of custom webhook templates so that
they don't need to be valid JSON, provided that the rendered template
_is_ valid JSON. This is checked using a dummy dictionary of render
params, which use a constant string for the `alert_group_id` field
and a `defaultdict(dict)` for the `alert_payload` field. This should
permit templates like the one above, but still deny templates such as

    {
      "labels": {{ alert_payload.labels }}
    }

which would otherwise fail later if `labels` is not valid JSON.

This should resolve #638.

* Use defaultdict(str) instead of defaultdict(lambda: "")

* Add missing comment to resolution note validation code

* Update validation of resolution notes in public API, too

* Add extra test cases for JSON webhook templates endpoint of public API
This commit is contained in:
Ben Sully 2022-10-19 12:32:21 +01:00 • committed by GitHub
parent 8e48c441e8
commit e36757b293
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 288 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
engine/apps/api/serializers/custom_button.py
View file

@ -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):

101
engine/apps/api/tests/test_custom_button.py
View file

@ -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

31
engine/apps/public_api/serializers/action.py
View file

@ -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):

137
engine/apps/public_api/tests/test_custom_actions.py
View file

@ -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(