From 61b242d1945519594e5e81203a4823c14f6937cd Mon Sep 17 00:00:00 2001 From: Sarabraj Singh Date: Wed, 2 Nov 2022 10:28:18 -0400 Subject: [PATCH] initial commit of new machinery to handle redirects for webhook notifications (#13083) --- awx/main/notifications/webhook_backend.py | 74 +++++++++++++++---- .../tests/functional/test_notifications.py | 44 +++++++++++ .../tests/unit/notifications/test_webhook.py | 21 ++++-- 3 files changed, 118 insertions(+), 21 deletions(-) diff --git a/awx/main/notifications/webhook_backend.py b/awx/main/notifications/webhook_backend.py index 30518e0714..17ced60f9f 100644 --- a/awx/main/notifications/webhook_backend.py +++ b/awx/main/notifications/webhook_backend.py @@ -5,9 +5,6 @@ import json import logging import requests -from django.utils.encoding import smart_str -from django.utils.translation import gettext_lazy as _ - from awx.main.notifications.base import AWXBaseEmailBackend from awx.main.utils import get_awx_http_client_headers from awx.main.notifications.custom_notification_base import CustomNotificationBase @@ -17,6 +14,8 @@ logger = logging.getLogger('awx.main.notifications.webhook_backend') class WebhookBackend(AWXBaseEmailBackend, CustomNotificationBase): + MAX_RETRIES = 5 + init_parameters = { "url": {"label": "Target URL", "type": "string"}, "http_method": {"label": "HTTP Method", "type": "string", "default": "POST"}, @@ -64,20 +63,67 @@ class WebhookBackend(AWXBaseEmailBackend, CustomNotificationBase): if self.http_method.lower() not in ['put', 'post']: raise ValueError("HTTP method must be either 'POST' or 'PUT'.") chosen_method = getattr(requests, self.http_method.lower(), None) + for m in messages: + auth = None if self.username or self.password: auth = (self.username, self.password) - r = chosen_method( - "{}".format(m.recipients()[0]), - auth=auth, - data=json.dumps(m.body, ensure_ascii=False).encode('utf-8'), - headers=dict(list(get_awx_http_client_headers().items()) + list((self.headers or {}).items())), - verify=(not self.disable_ssl_verification), - ) - if r.status_code >= 400: - logger.error(smart_str(_("Error sending notification webhook: {}").format(r.status_code))) + + # the constructor for EmailMessage - https://docs.djangoproject.com/en/4.1/_modules/django/core/mail/message will turn an empty dictionary to an empty string + # sometimes an empty dict is intentional and we added this conditional to enforce that + if not m.body: + m.body = {} + + url = str(m.recipients()[0]) + data = json.dumps(m.body, ensure_ascii=False).encode('utf-8') + headers = {**(get_awx_http_client_headers()), **(self.headers or {})} + + err = None + + for retries in range(self.MAX_RETRIES): + + # Sometimes we hit redirect URLs. We must account for this. We still extract the redirect URL from the response headers and try again. Max retires == 5 + resp = chosen_method( + url=url, + auth=auth, + data=data, + headers=headers, + verify=(not self.disable_ssl_verification), + allow_redirects=False, # override default behaviour for redirects + ) + + # either success or error reached if this conditional fires + if resp.status_code not in [301, 307]: + break + + # we've hit a redirect. extract the redirect URL out of the first response header and try again + logger.warning( + f"Received a {resp.status_code} from {url}, trying to reach redirect url {resp.headers.get('Location', None)}; attempt #{retries+1}" + ) + + # take the first redirect URL in the response header and try that + url = resp.headers.get("Location", None) + + if url is None: + err = f"Webhook notification received redirect to a blank URL from {url}. Response headers={resp.headers}" + break + else: + # no break condition in the loop encountered; therefore we have hit the maximum number of retries + err = f"Webhook notification max number of retries [{self.MAX_RETRIES}] exceeded. Failed to send webhook notification to {url}" + + if resp.status_code >= 400: + err = f"Error sending webhook notification: {resp.status_code}" + + # log error message + if err: + logger.error(err) if not self.fail_silently: - raise Exception(smart_str(_("Error sending notification webhook: {}").format(r.status_code))) - sent_messages += 1 + raise Exception(err) + + # no errors were encountered therefore we successfully sent off the notification webhook + if resp.status_code in range(200, 299): + logger.debug(f"Notification webhook successfully sent to {url}. Received {resp.status_code}") + sent_messages += 1 + return sent_messages diff --git a/awx/main/tests/functional/test_notifications.py b/awx/main/tests/functional/test_notifications.py index ce3873c223..7396b77843 100644 --- a/awx/main/tests/functional/test_notifications.py +++ b/awx/main/tests/functional/test_notifications.py @@ -75,6 +75,7 @@ def test_encrypted_subfields(get, post, user, organization): url = reverse('api:notification_template_detail', kwargs={'pk': response.data['id']}) response = get(url, u) assert response.data['notification_configuration']['account_token'] == "$encrypted$" + with mock.patch.object(notification_template_actual.notification_class, "send_messages", assert_send): notification_template_actual.send("Test", {'body': "Test"}) @@ -175,3 +176,46 @@ def test_custom_environment_injection(post, user, organization): fake_send.side_effect = _send_side_effect template.send('subject', 'message') + + +def mock_post(*args, **kwargs): + class MockGoodResponse: + def __init__(self): + self.status_code = 200 + + class MockRedirectResponse: + def __init__(self): + self.status_code = 301 + self.headers = {"Location": "http://goodendpoint"} + + if kwargs['url'] == "http://goodendpoint": + return MockGoodResponse() + else: + return MockRedirectResponse() + + +@pytest.mark.django_db +@mock.patch('requests.post', side_effect=mock_post) +def test_webhook_notification_pointed_to_a_redirect_launch_endpoint(post, admin, organization): + + n1 = NotificationTemplate.objects.create( + name="test-webhook", + description="test webhook", + organization=organization, + notification_type="webhook", + notification_configuration=dict( + url="http://some.fake.url", + disable_ssl_verification=True, + http_method="POST", + headers={ + "Content-Type": "application/json", + }, + username=admin.username, + password=admin.password, + ), + messages={ + "success": {"message": "", "body": "{}"}, + }, + ) + + assert n1.send("", n1.messages.get("success").get("body")) == 1 diff --git a/awx/main/tests/unit/notifications/test_webhook.py b/awx/main/tests/unit/notifications/test_webhook.py index db4255fb35..b2c92c59ab 100644 --- a/awx/main/tests/unit/notifications/test_webhook.py +++ b/awx/main/tests/unit/notifications/test_webhook.py @@ -27,11 +27,12 @@ def test_send_messages_as_POST(): ] ) requests_mock.post.assert_called_once_with( - 'http://example.com', + url='http://example.com', auth=None, data=json.dumps({'text': 'test body'}, ensure_ascii=False).encode('utf-8'), headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -57,11 +58,12 @@ def test_send_messages_as_PUT(): ] ) requests_mock.put.assert_called_once_with( - 'http://example.com', + url='http://example.com', auth=None, data=json.dumps({'text': 'test body 2'}, ensure_ascii=False).encode('utf-8'), headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -87,11 +89,12 @@ def test_send_messages_with_username(): ] ) requests_mock.post.assert_called_once_with( - 'http://example.com', + url='http://example.com', auth=('userstring', None), data=json.dumps({'text': 'test body'}, ensure_ascii=False).encode('utf-8'), headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -117,11 +120,12 @@ def test_send_messages_with_password(): ] ) requests_mock.post.assert_called_once_with( - 'http://example.com', + url='http://example.com', auth=(None, 'passwordstring'), data=json.dumps({'text': 'test body'}, ensure_ascii=False).encode('utf-8'), headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -147,11 +151,12 @@ def test_send_messages_with_username_and_password(): ] ) requests_mock.post.assert_called_once_with( - 'http://example.com', + url='http://example.com', auth=('userstring', 'passwordstring'), data=json.dumps({'text': 'test body'}, ensure_ascii=False).encode('utf-8'), headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -177,11 +182,12 @@ def test_send_messages_with_no_verify_ssl(): ] ) requests_mock.post.assert_called_once_with( - 'http://example.com', + url='http://example.com', auth=None, data=json.dumps({'text': 'test body'}, ensure_ascii=False).encode('utf-8'), headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}, verify=False, + allow_redirects=False, ) assert sent_messages == 1 @@ -207,7 +213,7 @@ def test_send_messages_with_additional_headers(): ] ) requests_mock.post.assert_called_once_with( - 'http://example.com', + url='http://example.com', auth=None, data=json.dumps({'text': 'test body'}, ensure_ascii=False).encode('utf-8'), headers={ @@ -217,5 +223,6 @@ def test_send_messages_with_additional_headers(): 'X-Test-Header2': 'test-content-2', }, verify=True, + allow_redirects=False, ) assert sent_messages == 1