diff --git a/CHANGELOG.md b/CHANGELOG.md index fd9a6481..01d3be74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix Slack direct paging issue when there are more than 100 schedules by @vadimkerr ([#2594](https://github.com/grafana/oncall/pull/2594)) +- Fix webhooks unable to be copied if they contain password or authorization header ([#2608](https://github.com/grafana/oncall/pull/2608)) ## v1.3.15 (2023-07-19) diff --git a/engine/apps/api/serializers/webhook.py b/engine/apps/api/serializers/webhook.py index e12c0b94..d5945de7 100644 --- a/engine/apps/api/serializers/webhook.py +++ b/engine/apps/api/serializers/webhook.py @@ -77,10 +77,18 @@ class WebhookSerializer(serializers.ModelSerializer): return result def to_internal_value(self, data): + webhook = self.instance + + # If webhook is being copied instance won't exist to copy values from + if not webhook and "id" in data: + webhook = Webhook.objects.get( + public_primary_key=data["id"], organization=self.context["request"].auth.organization + ) + if data.get("password") == WEBHOOK_FIELD_PLACEHOLDER: - data["password"] = self.instance.password + data["password"] = webhook.password if data.get("authorization_header") == WEBHOOK_FIELD_PLACEHOLDER: - data["authorization_header"] = self.instance.authorization_header + data["authorization_header"] = webhook.authorization_header return super().to_internal_value(data) def _validate_template_field(self, template): diff --git a/engine/apps/api/tests/test_webhooks.py b/engine/apps/api/tests/test_webhooks.py index d3f55a8f..589630be 100644 --- a/engine/apps/api/tests/test_webhooks.py +++ b/engine/apps/api/tests/test_webhooks.py @@ -546,3 +546,113 @@ def test_webhook_preview_template( response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK assert response.data["preview"] == expected_result + + +@mock.patch("apps.api.views.webhooks.WebhooksView.check_webhooks_2_enabled") +@pytest.mark.django_db +def test_webhook_field_masking(mock_check_webhooks_2_enabled, webhook_internal_api_setup, make_user_auth_headers): + user, token, webhook = webhook_internal_api_setup + client = APIClient() + url = reverse("api-internal:webhooks-list") + + data = { + "name": "the_webhook", + "url": TEST_URL, + "trigger_type": str(Webhook.TRIGGER_ALERT_GROUP_CREATED), + "team": None, + "password": "secret_password", + "authorization_header": "auth 1234", + } + + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + webhook = Webhook.objects.get(public_primary_key=response.data["id"]) + + expected_response = data | { + "id": webhook.public_primary_key, + "data": None, + "username": None, + "password": WEBHOOK_FIELD_PLACEHOLDER, + "authorization_header": WEBHOOK_FIELD_PLACEHOLDER, + "forward_all": True, + "headers": None, + "http_method": "POST", + "integration_filter": None, + "is_webhook_enabled": True, + "is_legacy": False, + "last_response_log": { + "request_data": "", + "request_headers": "", + "timestamp": None, + "content": "", + "status_code": None, + "request_trigger": "", + "url": "", + "event_data": "", + }, + "trigger_template": None, + "trigger_type_name": "Alert Group Created", + } + + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_response + assert webhook.password == data["password"] + assert webhook.authorization_header == data["authorization_header"] + assert webhook.user == user + + +@mock.patch("apps.api.views.webhooks.WebhooksView.check_webhooks_2_enabled") +@pytest.mark.django_db +def test_webhook_copy(mock_check_webhooks_2_enabled, webhook_internal_api_setup, make_user_auth_headers): + user, token, webhook = webhook_internal_api_setup + client = APIClient() + url = reverse("api-internal:webhooks-list") + + data = { + "name": "the_webhook", + "url": TEST_URL, + "trigger_type": str(Webhook.TRIGGER_ALERT_GROUP_CREATED), + "team": None, + "password": "secret_password", + "authorization_header": "auth 1234", + } + response1 = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + get_url = reverse("api-internal:webhooks-detail", kwargs={"pk": response1.data["id"]}) + response2 = client.get(get_url, format="json", **make_user_auth_headers(user, token)) + to_copy = response2.json() + to_copy["name"] = "copied_webhook" + response3 = client.post(url, to_copy, format="json", **make_user_auth_headers(user, token)) + webhook = Webhook.objects.get(public_primary_key=response3.data["id"]) + + expected_response = data | { + "id": webhook.public_primary_key, + "name": to_copy["name"], + "data": None, + "username": None, + "password": WEBHOOK_FIELD_PLACEHOLDER, + "authorization_header": WEBHOOK_FIELD_PLACEHOLDER, + "forward_all": True, + "headers": None, + "http_method": "POST", + "integration_filter": None, + "is_webhook_enabled": True, + "is_legacy": False, + "last_response_log": { + "request_data": "", + "request_headers": "", + "timestamp": None, + "content": "", + "status_code": None, + "request_trigger": "", + "url": "", + "event_data": "", + }, + "trigger_template": None, + "trigger_type_name": "Alert Group Created", + } + + assert response3.status_code == status.HTTP_201_CREATED + assert response3.json() == expected_response + assert webhook.password == data["password"] + assert webhook.authorization_header == data["authorization_header"] + assert webhook.id != to_copy["id"] + assert webhook.user == user