From 34e165c1001c0b3a53a1e1d19adb4e15b3712446 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Fri, 16 Mar 2018 15:37:32 -0700 Subject: [PATCH] webhooks: Fix passing client string to authenticated webhook API views. This fixes a regression in 93678e89cd29da5b8106e278fa776f90f6fed33f and a4979410f983e7aaa10f4833a9c302110bea685c, where the webhooks using authenticated_rest_api_view were migrated to a new model that didn't include setting a custom Client string for the webhook. When restoring these webhooks' client strings, we also fix places where the client string was not capitalized the same was as the product's name. --- zerver/decorator.py | 19 +++++++++++++++---- zerver/lib/rest.py | 2 +- zerver/webhooks/beanstalk/view.py | 2 +- zerver/webhooks/bitbucket/view.py | 2 +- zerver/webhooks/deskdotcom/view.py | 2 +- zerver/webhooks/freshdesk/view.py | 2 +- zerver/webhooks/zendesk/view.py | 2 +- 7 files changed, 21 insertions(+), 10 deletions(-) diff --git a/zerver/decorator.py b/zerver/decorator.py index 362344ba2a..4c30fbb8ed 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -294,8 +294,13 @@ body: message = message.strip(' ') webhook_logger.exception(message) +def full_webhook_client_name(raw_client_name: Optional[str]=None) -> Optional[str]: + if raw_client_name is None: + return None + return "Zulip{}Webhook".format(raw_client_name) + # Use this for webhook views that don't get an email passed in. -def api_key_only_webhook_view(client_name: Text) -> Callable[[ViewFuncT], ViewFuncT]: +def api_key_only_webhook_view(webhook_client_name: Text) -> Callable[[ViewFuncT], ViewFuncT]: # TODO The typing here could be improved by using the Extended Callable types: # https://mypy.readthedocs.io/en/latest/kinds_of_types.html#extended-callable-types def _wrapped_view_func(view_func: ViewFuncT) -> ViewFuncT: @@ -305,7 +310,7 @@ def api_key_only_webhook_view(client_name: Text) -> Callable[[ViewFuncT], ViewFu def _wrapped_func_arguments(request: HttpRequest, api_key: Text=REQ(), *args: Any, **kwargs: Any) -> HttpResponse: user_profile = validate_api_key(request, None, api_key, is_webhook=True, - client_name="Zulip{}Webhook".format(client_name)) + client_name=full_webhook_client_name(webhook_client_name)) if settings.RATE_LIMITING: rate_limit_user(request, user_profile, domain='all') @@ -468,7 +473,11 @@ def authenticated_api_view(is_webhook: bool=False) -> Callable[[ViewFuncT], View # A more REST-y authentication decorator, using, in particular, HTTP Basic # authentication. -def authenticated_rest_api_view(is_webhook: bool=False) -> Callable[[ViewFuncT], ViewFuncT]: +# +# If webhook_client_name is specific, the request is a webhook view +# with that string as the basis for the client string. +def authenticated_rest_api_view(*, webhook_client_name: str=None, + is_webhook: bool=False) -> Callable[[ViewFuncT], ViewFuncT]: def _wrapped_view_func(view_func: ViewFuncT) -> ViewFuncT: @csrf_exempt @wraps(view_func) @@ -490,7 +499,9 @@ def authenticated_rest_api_view(is_webhook: bool=False) -> Callable[[ViewFuncT], # Now we try to do authentication or die try: # profile is a Union[UserProfile, RemoteZulipServer] - profile = validate_api_key(request, role, api_key, is_webhook) + profile = validate_api_key(request, role, api_key, + is_webhook=is_webhook or webhook_client_name is not None, + client_name=full_webhook_client_name(webhook_client_name)) except JsonableError as e: return json_unauthorized(e.msg) # Apply rate limiting diff --git a/zerver/lib/rest.py b/zerver/lib/rest.py index 05f7293770..6e0c5aa1b1 100644 --- a/zerver/lib/rest.py +++ b/zerver/lib/rest.py @@ -99,7 +99,7 @@ def rest_dispatch(request: HttpRequest, **kwargs: Any) -> HttpResponse: view_kwargs = {} if 'allow_incoming_webhooks' in view_flags: view_kwargs['is_webhook'] = True - target_function = authenticated_rest_api_view(**view_kwargs)(target_function) + target_function = authenticated_rest_api_view(**view_kwargs)(target_function) # type: ignore # likely mypy bug # Pick a way to tell user they're not authed based on how the request was made else: # If this looks like a request from a top-level page in a diff --git a/zerver/webhooks/beanstalk/view.py b/zerver/webhooks/beanstalk/view.py index e1559378fd..d66160c047 100644 --- a/zerver/webhooks/beanstalk/view.py +++ b/zerver/webhooks/beanstalk/view.py @@ -37,7 +37,7 @@ def beanstalk_decoder(view_func: ViewFuncT) -> ViewFuncT: return _wrapped_view_func # type: ignore # https://github.com/python/mypy/issues/1927 @beanstalk_decoder -@authenticated_rest_api_view(is_webhook=True) +@authenticated_rest_api_view(webhook_client_name="Beanstalk") @has_request_variables def api_beanstalk_webhook(request: HttpRequest, user_profile: UserProfile, payload: Dict[str, Any]=REQ(validator=check_dict([])), diff --git a/zerver/webhooks/bitbucket/view.py b/zerver/webhooks/bitbucket/view.py index ee650dc020..7e68c33d5b 100644 --- a/zerver/webhooks/bitbucket/view.py +++ b/zerver/webhooks/bitbucket/view.py @@ -11,7 +11,7 @@ from zerver.lib.webhooks.git import SUBJECT_WITH_BRANCH_TEMPLATE, \ get_push_commits_event_message from zerver.models import UserProfile, get_client -@authenticated_rest_api_view(is_webhook=True) +@authenticated_rest_api_view(webhook_client_name="Bitbucket") @has_request_variables def api_bitbucket_webhook(request: HttpRequest, user_profile: UserProfile, payload: Mapping[Text, Any]=REQ(validator=check_dict([])), diff --git a/zerver/webhooks/deskdotcom/view.py b/zerver/webhooks/deskdotcom/view.py index 2d7c0e4ba7..ec2e76f9bc 100644 --- a/zerver/webhooks/deskdotcom/view.py +++ b/zerver/webhooks/deskdotcom/view.py @@ -14,7 +14,7 @@ from zerver.models import UserProfile, get_client # There's no raw JSON for us to work from. Thus, it makes sense to just write # a template Zulip message within Desk.com and have the webhook extract that # from the "data" param and post it, which this does. -@authenticated_rest_api_view(is_webhook=True) +@authenticated_rest_api_view(webhook_client_name="Desk") @has_request_variables def api_deskdotcom_webhook(request: HttpRequest, user_profile: UserProfile, data: Text=REQ()) -> HttpResponse: diff --git a/zerver/webhooks/freshdesk/view.py b/zerver/webhooks/freshdesk/view.py index 3c600873bb..7ad2589d46 100644 --- a/zerver/webhooks/freshdesk/view.py +++ b/zerver/webhooks/freshdesk/view.py @@ -103,7 +103,7 @@ def format_freshdesk_ticket_creation_message(ticket: TicketDict) -> str: return content -@authenticated_rest_api_view(is_webhook=True) +@authenticated_rest_api_view(webhook_client_name="Freshdesk") @has_request_variables def api_freshdesk_webhook(request: HttpRequest, user_profile: UserProfile, payload: Dict[str, Any]=REQ(argument_type='body')) -> HttpResponse: diff --git a/zerver/webhooks/zendesk/view.py b/zerver/webhooks/zendesk/view.py index bde642d402..2bac060f1a 100644 --- a/zerver/webhooks/zendesk/view.py +++ b/zerver/webhooks/zendesk/view.py @@ -14,7 +14,7 @@ def truncate(string: Text, length: int) -> Text: string = string[:length-3] + '...' return string -@authenticated_rest_api_view(is_webhook=True) +@authenticated_rest_api_view(webhook_client_name="Zendesk") @has_request_variables def api_zendesk_webhook(request: HttpRequest, user_profile: UserProfile, ticket_title: str=REQ(), ticket_id: str=REQ(),