Merge lp:~cjwatson/launchpad/add-webhook-secret into lp:launchpad

[Colin Watson]

Allow setting a secret when creating a new webhook.

Allow setting a secret when creating a new webhook.

For now, this field is disabled when editing an existing webhook. It will need some custom handling to, at minimum, not reveal the existing secret, and probably also do some clever JavaScript thing to obscure the entered secret except when you're editing the value. Secrets for existing webhooks can still be edited using the API though.

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'lib/lp/services/webhooks/browser.py'
--- lib/lp/services/webhooks/browser.py	2015-10-05 11:11:47 +0000
+++ lib/lp/services/webhooks/browser.py	2015-10-26 12:21:56 +0000
@@ -110,8 +110,8 @@
 
 
 class WebhookEditSchema(Interface):
-    # XXX wgrant 2015-08-04: Need custom widget for secret.
-    use_template(IWebhook, include=['delivery_url', 'event_types', 'active'])
+    use_template(
+        IWebhook, include=['delivery_url', 'event_types', 'active', 'secret'])
 
 
 class WebhookAddView(LaunchpadFormView):
@@ -140,7 +140,8 @@
     def new_action(self, action, data):
         webhook = self.context.newWebhook(
             registrant=self.user, delivery_url=data['delivery_url'],
-            event_types=data['event_types'], active=data['active'])
+            event_types=data['event_types'], active=data['active'],
+            secret=data['secret'])
         self.next_url = canonical_url(webhook)
 
 
@@ -149,6 +150,8 @@
     label = "Manage webhook"
 
     schema = WebhookEditSchema
+    # XXX wgrant 2015-08-04: Need custom widget for secret.
+    field_names = ['delivery_url', 'event_types', 'active']
     custom_widget('event_types', LabeledMultiCheckBoxWidget)
 
     def initialize(self):

=== modified file 'lib/lp/services/webhooks/tests/test_browser.py'
--- lib/lp/services/webhooks/tests/test_browser.py	2015-10-19 10:48:28 +0000
+++ lib/lp/services/webhooks/tests/test_browser.py	2015-10-26 12:21:56 +0000
@@ -188,6 +188,7 @@
                 "field.delivery_url": "http://example.com/test",
                 "field.active": "on", "field.event_types-empty-marker": "1",
                 "field.event_types": self.event_type,
+                "field.secret": "secret code",
                 "field.actions.new": "Add webhook"})
         self.assertEqual([], view.errors)
         hook = self.target.webhooks.one()
@@ -198,7 +199,8 @@
                 registrant=self.owner,
                 delivery_url="http://example.com/test",
                 active=True,
-                event_types=[self.event_type]))
+                event_types=[self.event_type],
+                secret="secret code"))
 
     def test_rejects_bad_scheme(self):
         transaction.commit()
@@ -212,6 +214,21 @@
             ['delivery_url'], [error.field_name for error in view.errors])
         self.assertIs(None, self.target.webhooks.one())
 
+    def test_no_secret(self):
+        # If the secret field is left empty, the secret is set to None
+        # rather than to the empty string.
+        view = self.makeView(
+            "+new-webhook", method="POST",
+            form={
+                "field.delivery_url": "http://example.com/test",
+                "field.active": "on", "field.event_types-empty-marker": "1",
+                "field.event_types": self.event_type,
+                "field.secret": "",
+                "field.actions.new": "Add webhook"})
+        self.assertEqual([], view.errors)
+        hook = self.target.webhooks.one()
+        self.assertIsNone(hook.secret)
+
     def test_event_types(self):
         # Only event types that are valid for the target are offered.
         browser = self.getUserBrowser(