From d73a37726d92ed6581d6b83cdcb6d1c7462c2600 Mon Sep 17 00:00:00 2001 From: "Hemanth V. Alluri" Date: Sun, 18 Aug 2019 16:26:21 +0530 Subject: [PATCH] bots: Allow incoming webhook bots to be configured via /bots. Without disturbing the flow of the existing code for configuring embedded bots too much, we now use the config_options feature to allow incoming webhook type bot to be configured via. the "/bots" endpoint of the API. --- .../api/incoming-webhooks-walkthrough.md | 23 +++++ zerver/lib/users.py | 25 ++++- zerver/tests/test_bots.py | 97 ++++++++++++++++++- zerver/views/users.py | 12 ++- 4 files changed, 148 insertions(+), 9 deletions(-) diff --git a/templates/zerver/api/incoming-webhooks-walkthrough.md b/templates/zerver/api/incoming-webhooks-walkthrough.md index 4b2a454094..6a17e5d210 100644 --- a/templates/zerver/api/incoming-webhooks-walkthrough.md +++ b/templates/zerver/api/incoming-webhooks-walkthrough.md @@ -204,6 +204,29 @@ At this point, if you're following along and/or writing your own Hello World webhook, you have written enough code to test your integration. There are three tools which you can use to test your webhook - 2 command line tools and a GUI. +### Webhooks requiring custom configuration + +In rare cases, it's necessary for an incoming webhook to require +additional user configuration beyond what is specified in the post +URL. The typical use case for this is APIs like the Stripe API that +require clients to do a callback to get details beyond an opaque +object ID that one would want to include in a Zulip notification. + +These configuration options are declared as follows: + +``` + WebhookIntegration('helloworld', ['misc'], display_name='Hello World', + config_options=[('HelloWorld API Key', 'hw_api_key', check_string)]) +``` + +`config_options` is a list describing the parameters the user should +configure: + 1. A user-facing string describing the field to display to users. + 2. The field name you'll use to access this from your `view.py` function. + 3. A Validator, used to verify the input is valid. + +Common validators are available in `zerver/lib/validators.py`. + ## Step 4: Manually testing the webhook For either one of the command line tools, first, you'll need to get an API key diff --git a/zerver/lib/users.py b/zerver/lib/users.py index 24c11d288b..34a59f07b6 100644 --- a/zerver/lib/users.py +++ b/zerver/lib/users.py @@ -49,7 +49,30 @@ def check_short_name(short_name_raw: str) -> str: def check_valid_bot_config(bot_type: int, service_name: str, config_data: Dict[str, str]) -> None: - if bot_type == UserProfile.EMBEDDED_BOT: + if bot_type == UserProfile.INCOMING_WEBHOOK_BOT: + from zerver.lib.integrations import WEBHOOK_INTEGRATIONS + config_options = None + for integration in WEBHOOK_INTEGRATIONS: + if integration.name == service_name: + # key: validator + config_options = {c[1]: c[2] for c in integration.config_options} + break + if not config_options: + raise JsonableError(_("Invalid integration '%s'.") % (service_name,)) + + missing_keys = set(config_options.keys()) - set(config_data.keys()) + if missing_keys: + raise JsonableError(_("Missing configuration parameters: %s") % ( + missing_keys,)) + + for key, validator in config_options.items(): + value = config_data[key] + error = validator(key, value) + if error: + raise JsonableError(_("Invalid {} value {} ({})").format( + key, value, error)) + + elif bot_type == UserProfile.EMBEDDED_BOT: try: from zerver.lib.bot_lib import get_bot_handler bot_handler = get_bot_handler(service_name) diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index d3e02b3f26..9678666c71 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -4,10 +4,10 @@ import ujson from django.core import mail from mock import patch, MagicMock -from typing import Any, Dict, List, Mapping +from typing import Any, Dict, List, Mapping, Optional from zerver.lib.actions import do_change_stream_invite_only, do_deactivate_user -from zerver.lib.bot_config import get_bot_config +from zerver.lib.bot_config import get_bot_config, ConfigError from zerver.models import get_realm, get_stream, \ Realm, UserProfile, get_user, get_bot_services, Service, \ is_cross_realm_bot_email @@ -18,11 +18,22 @@ from zerver.lib.test_helpers import ( queries_captured, tornado_redirected_to_list, ) -from zerver.lib.integrations import EMBEDDED_BOTS +from zerver.lib.integrations import EMBEDDED_BOTS, WebhookIntegration from zerver.lib.bot_lib import get_bot_handler - from zulip_bots.custom_exceptions import ConfigValidationError + +# A test validator +def _check_string(var_name: str, val: object) -> Optional[str]: + if str(val).startswith("_"): + return ('%s starts with a "_" and is hence invalid.') % (var_name,) + return None + +stripe_sample_config_options = [ + WebhookIntegration('stripe', ['financial'], display_name='Stripe', + config_options=[("Stripe API Key", "stripe_api_key", _check_string)]) +] + class BotTest(ZulipTestCase, UploadSerializeMixin): def get_bot_user(self, email: str) -> UserProfile: realm = get_realm("zulip") @@ -1441,3 +1452,81 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): with self.settings(CROSS_REALM_BOT_EMAILS={"random-bot@zulip.com"}): self.assertTrue(is_cross_realm_bot_email("random-bot@zulip.com")) self.assertFalse(is_cross_realm_bot_email("notification-bot@zulip.com")) + + @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) + def test_create_incoming_webhook_bot_with_service_name_and_with_keys(self) -> None: + self.login(self.example_email('hamlet')) + bot_metadata = { + "full_name": "My Stripe Bot", + "short_name": "my-stripe", + "bot_type": UserProfile.INCOMING_WEBHOOK_BOT, + "service_name": "stripe", + "config_data": ujson.dumps({"stripe_api_key": "sample-api-key"}) + } + self.create_bot(**bot_metadata) + new_bot = UserProfile.objects.get(full_name="My Stripe Bot") + config_data = get_bot_config(new_bot) + self.assertEqual(config_data, {"integration_id": "stripe", + "stripe_api_key": "sample-api-key"}) + + @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) + def test_create_incoming_webhook_bot_with_service_name_incorrect_keys(self) -> None: + self.login(self.example_email('hamlet')) + bot_metadata = { + "full_name": "My Stripe Bot", + "short_name": "my-stripe", + "bot_type": UserProfile.INCOMING_WEBHOOK_BOT, + "service_name": "stripe", + "config_data": ujson.dumps({"stripe_api_key": "_invalid_key"}) + } + response = self.client_post("/json/bots", bot_metadata) + self.assertEqual(response.status_code, 400) + expected_error_message = 'Invalid stripe_api_key value _invalid_key (stripe_api_key starts with a "_" and is hence invalid.)' + self.assertEqual(ujson.loads(response.content.decode('utf-8'))["msg"], expected_error_message) + with self.assertRaises(UserProfile.DoesNotExist): + UserProfile.objects.get(full_name="My Stripe Bot") + + @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) + def test_create_incoming_webhook_bot_with_service_name_without_keys(self) -> None: + self.login(self.example_email('hamlet')) + bot_metadata = { + "full_name": "My Stripe Bot", + "short_name": "my-stripe", + "bot_type": UserProfile.INCOMING_WEBHOOK_BOT, + "service_name": "stripe", + } + response = self.client_post("/json/bots", bot_metadata) + self.assertEqual(response.status_code, 400) + expected_error_message = "Missing configuration parameters: {'stripe_api_key'}" + self.assertEqual(ujson.loads(response.content.decode('utf-8'))["msg"], expected_error_message) + with self.assertRaises(UserProfile.DoesNotExist): + UserProfile.objects.get(full_name="My Stripe Bot") + + @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) + def test_create_incoming_webhook_bot_without_service_name(self) -> None: + self.login(self.example_email('hamlet')) + bot_metadata = { + "full_name": "My Stripe Bot", + "short_name": "my-stripe", + "bot_type": UserProfile.INCOMING_WEBHOOK_BOT, + } + self.create_bot(**bot_metadata) + new_bot = UserProfile.objects.get(full_name="My Stripe Bot") + with self.assertRaises(ConfigError): + get_bot_config(new_bot) + + @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) + def test_create_incoming_webhook_bot_with_incorrect_service_name(self) -> None: + self.login(self.example_email('hamlet')) + bot_metadata = { + "full_name": "My Stripe Bot", + "short_name": "my-stripe", + "bot_type": UserProfile.INCOMING_WEBHOOK_BOT, + "service_name": "stripes", + } + response = self.client_post("/json/bots", bot_metadata) + self.assertEqual(response.status_code, 400) + expected_error_message = "Invalid integration 'stripes'." + self.assertEqual(ujson.loads(response.content.decode('utf-8'))["msg"], expected_error_message) + with self.assertRaises(UserProfile.DoesNotExist): + UserProfile.objects.get(full_name="My Stripe Bot") diff --git a/zerver/views/users.py b/zerver/views/users.py index 1a0f38c6e7..46a3d14b90 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -1,7 +1,6 @@ from typing import Union, Optional, Dict, Any, List import ujson - from django.http import HttpRequest, HttpResponse from django.utils.translation import ugettext as _ @@ -273,7 +272,8 @@ def add_bot_backend( default_all_public_streams: Optional[bool]=REQ(validator=check_bool, default=None) ) -> HttpResponse: short_name = check_short_name(short_name_raw) - service_name = service_name or short_name + if bot_type != UserProfile.INCOMING_WEBHOOK_BOT: + service_name = service_name or short_name short_name += "-bot" full_name = check_full_name(full_name_raw) email = '%s@%s' % (short_name, user_profile.realm.get_bot_domain()) @@ -320,7 +320,7 @@ def add_bot_backend( (default_events_register_stream, ignored_rec, ignored_sub) = access_stream_by_name( user_profile, default_events_register_stream_name) - if bot_type == UserProfile.EMBEDDED_BOT: + if bot_type in (UserProfile.INCOMING_WEBHOOK_BOT, UserProfile.EMBEDDED_BOT) and service_name: check_valid_bot_config(bot_type, service_name, config_data) bot_profile = do_create_user(email=email, password='', @@ -337,13 +337,17 @@ def add_bot_backend( upload_avatar_image(user_file, user_profile, bot_profile) if bot_type in (UserProfile.OUTGOING_WEBHOOK_BOT, UserProfile.EMBEDDED_BOT): + assert(isinstance(service_name, str)) add_service(name=service_name, user_profile=bot_profile, base_url=payload_url, interface=interface_type, token=generate_api_key()) - if bot_type == UserProfile.EMBEDDED_BOT: + if bot_type == UserProfile.INCOMING_WEBHOOK_BOT and service_name: + set_bot_config(bot_profile, "integration_id", service_name) + + if bot_type in (UserProfile.INCOMING_WEBHOOK_BOT, UserProfile.EMBEDDED_BOT): for key, value in config_data.items(): set_bot_config(bot_profile, key, value)