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.

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

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)

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")

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)