From d8dee522b605ef485d35331afda23b9dc0e84941 Mon Sep 17 00:00:00 2001 From: Tomasz Kolek Date: Tue, 15 Nov 2016 17:20:22 +0100 Subject: [PATCH] Fix trello integration by adding handling HEAD confirmation request. Previously, we rejected the HEAD requests that the trello integration uses to check if the server accepts the integration. Add decorator for returning 200 status code if request is HEAD. Fixes: #2311. --- zerver/decorator.py | 12 ++++++++- zerver/tests/test_decorators.py | 32 +++++++++++++++++++++++- zerver/tests/webhooks/test_trello.py | 5 ++++ zerver/views/webhooks/trello/__init__.py | 2 ++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/zerver/decorator.py b/zerver/decorator.py index c302a8a0be..e6e6d55d74 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -7,7 +7,7 @@ from django.views.decorators.csrf import csrf_exempt from django.http import QueryDict, HttpResponseNotAllowed, HttpRequest from django.http.multipartparser import MultiPartParser from zerver.models import UserProfile, get_client, get_user_profile_by_email -from zerver.lib.response import json_error, json_unauthorized +from zerver.lib.response import json_error, json_unauthorized, json_success from django.shortcuts import resolve_url from django.utils.decorators import available_attrs from django.utils.timezone import now @@ -649,3 +649,13 @@ def uses_mandrill(func): kwargs['mail_client'] = get_mandrill_client() return func(*args, **kwargs) return wrapped_func # type: ignore # https://github.com/python/mypy/issues/1927 + +def return_success_on_head_request(view_func): + # type: (Callable) -> Callable + @wraps(view_func) + def _wrapped_view_func(request, *args, **kwargs): + # type: (HttpResponse, *Any, **Any) -> Callable + if request.method == 'HEAD': + return json_success() + return view_func(request, *args, **kwargs) + return _wrapped_view_func diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index cdf125ba2e..bf23238eda 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -17,6 +17,7 @@ from zerver.lib.test_classes import ( ZulipTestCase, WebhookTestCase, ) +from zerver.lib.response import json_response from zerver.lib.request import \ REQ, has_request_variables, RequestVariableMissingError, \ RequestVariableConversionError, JsonableError @@ -25,7 +26,8 @@ from zerver.decorator import ( authenticated_json_post_view, authenticated_json_view, authenticate_notify, get_client_name, internal_notify_view, is_local_addr, - rate_limit, validate_api_key, logged_in_and_active + rate_limit, validate_api_key, logged_in_and_active, + return_success_on_head_request ) from zerver.lib.validator import ( check_string, check_dict, check_bool, check_int, check_list @@ -872,3 +874,31 @@ class TestZulipLoginRequiredDecorator(ZulipTestCase): with mock.patch('zerver.decorator.get_subdomain', return_value='acme'): result = self.client_get('/accounts/accept_terms/') self.assertEqual(result.status_code, 302) + +class ReturnSuccessOnHeadRequestDecorator(ZulipTestCase): + def test_return_success_on_head_request_returns_200_if_request_method_is_head(self): + class HeadRequest(object): + method = 'HEAD' + + request = HeadRequest() + + @return_success_on_head_request + def test_function(request): + return json_response(msg=u'from_test_function') + + response = test_function(request) + self.assert_json_success(response) + self.assertNotEqual(ujson.loads(response.content).get('msg'), u'from_test_function') + + def test_return_success_on_head_request_returns_normal_response_if_request_method_is_not_head(self): + class HeadRequest(object): + method = 'POST' + + request = HeadRequest() + + @return_success_on_head_request + def test_function(request): + return json_response(msg=u'from_test_function') + + response = test_function(request) + self.assertEqual(ujson.loads(response.content).get('msg'), u'from_test_function') diff --git a/zerver/tests/webhooks/test_trello.py b/zerver/tests/webhooks/test_trello.py index a0ad544f76..31a825a6e9 100644 --- a/zerver/tests/webhooks/test_trello.py +++ b/zerver/tests/webhooks/test_trello.py @@ -6,6 +6,11 @@ class TrelloHookTests(WebhookTestCase): URL_TEMPLATE = u"/api/v1/external/trello?stream={stream}&api_key={api_key}" FIXTURE_DIR_NAME = 'trello' + def test_trello_confirmation_request(self): + # type: () -> None + response = self.client.head(self.build_webhook_url()) + self.assertEqual(response.status_code, 200, response) + def test_trello_webhook_when_card_was_moved_to_another_list(self): # type: () -> None expected_message = u"TomaszKolek moved [This is a card.](https://trello.com/c/r33ylX2Z) from Basics to Intermediate." diff --git a/zerver/views/webhooks/trello/__init__.py b/zerver/views/webhooks/trello/__init__.py index 59e57e2e43..11dc2b8c23 100644 --- a/zerver/views/webhooks/trello/__init__.py +++ b/zerver/views/webhooks/trello/__init__.py @@ -6,6 +6,7 @@ from typing import Mapping, Any, Tuple from django.utils.translation import ugettext as _ from django.http import HttpRequest, HttpResponse from zerver.lib.actions import check_send_message +from zerver.decorator import return_success_on_head_request from zerver.lib.response import json_success, json_error from zerver.models import UserProfile, Client from zerver.decorator import REQ, has_request_variables, api_key_only_webhook_view @@ -15,6 +16,7 @@ from .board_actions import SUPPORTED_BOARD_ACTIONS, process_board_action from .exceptions import UnsupportedAction @api_key_only_webhook_view('Trello') +@return_success_on_head_request @has_request_variables def api_trello_webhook(request, user_profile, client, payload=REQ(argument_type='body'), stream=REQ(default='trello')): # type: (HttpRequest, UserProfile, Client, Mapping[str, Any], text_type) -> HttpResponse