Validate webhook's payload

.coveragerc

@@ -18,3 +18,4 @@ exclude_lines =
     raise NotImplementedError
     if __name__ == .__main__.:
     if getattr(settings, 'DEBUG'):
+    if six.PY2:

docs/webhooks.rst

@@ -149,6 +149,16 @@ It might be necessary to re-establish a webhook if you are noticing problems.
 To resync a webhook from Read the Docs, visit the integration detail page and
 follow the directions for re-syncing your repository webhook.
 
+Payload validation
+------------------
+
+If your project was imported through a connected account,
+we create a secret for every integration that offers a way to verify that a webhook request is legitimate.
+Currently, `GitHub <https://developer.github.com/webhooks/securing/>`__ and `GitLab <https://docs.gitlab.com/ee/user/project/integrations/webhooks.html#secret-token>`__
+offer a way to check this.
+
+When :ref:`resyncing the webhook <webhooks:Resyncing webhooks>`, the secret is changed too.
+
 Troubleshooting
 ---------------
 

readthedocs/integrations/migrations/0004_add_integration_secret.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-12-10 21:28
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('integrations', '0003_add_missing_model_change_migrations'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='integration',
+            name='secret',
+            field=models.CharField(blank=True, default=None, help_text='Secret used to validate the payload of the webhook', max_length=255, null=True),
+        ),
+    ]

readthedocs/integrations/migrations/0005_change_default_integration_secret.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-12-10 22:08
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import readthedocs.integrations.utils
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('integrations', '0004_add_integration_secret'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='integration',
+            name='secret',
+            field=models.CharField(blank=True, default=readthedocs.integrations.utils.get_secret, help_text='Secret used to validate the payload of the webhook', max_length=255, null=True),
+        ),
+    ]

readthedocs/integrations/models.py

@@ -23,7 +23,7 @@
 from readthedocs.core.fields import default_token
 from readthedocs.projects.models import Project
 
-from .utils import normalize_request_payload
+from .utils import get_secret, normalize_request_payload
 
 
 class HttpExchangeManager(models.Manager):
@@ -257,12 +257,23 @@ class Integration(models.Model):
         'HttpExchange',
         related_query_name='integrations',
     )
+    secret = models.CharField(
+        help_text=_('Secret used to validate the payload of the webhook'),
+        max_length=255,
+        blank=True,
+        null=True,
+        default=get_secret,
+    )
 
     objects = IntegrationQuerySet.as_manager()
 
     # Integration attributes
     has_sync = False
 
+    def recreate_secret(self):
+        self.secret = get_secret()
+        self.save(update_fields=['secret'])
+
     def __str__(self):
         return (
             _('{0} for {1}')

readthedocs/integrations/utils.py

@@ -1,7 +1,7 @@
-# -*- coding: utf-8 -*-
-
 """Integration utility functions."""
 
+import os
+
 
 def normalize_request_payload(request):
     """
@@ -23,3 +23,13 @@ def normalize_request_payload(request):
         except AttributeError:
             pass
     return request_payload
+
+
+def get_secret(size=64):
+    """
+    Get a random string of `size` bytes.
+
+    :param size: Number of bytes
+    """
+    secret = os.urandom(size)
+    return secret.hex()

readthedocs/oauth/services/github.py

@@ -12,6 +12,7 @@
 
 from readthedocs.builds import utils as build_utils
 from readthedocs.integrations.models import Integration
+from readthedocs.integrations.utils import get_secret
 from readthedocs.restapi.client import api
 
 from ..models import RemoteOrganization, RemoteRepository
@@ -178,6 +179,7 @@ def get_webhook_data(self, project, integration):
                         },
                     ),
                 ),
+                'secret': integration.secret,
                 'content_type': 'json',
             },
             'events': ['push', 'pull_request', 'create', 'delete'],
@@ -262,6 +264,7 @@ def update_webhook(self, project, integration):
         :rtype: (Bool, Response)
         """
         session = self.get_session()
+        integration.recreate_secret()
         data = self.get_webhook_data(project, integration)
         url = integration.provider_data.get('url')
         resp = None

readthedocs/oauth/services/gitlab.py

@@ -11,6 +11,7 @@
 
 from readthedocs.builds.utils import get_gitlab_username_repo
 from readthedocs.integrations.models import Integration
+from readthedocs.integrations.utils import get_secret
 from readthedocs.projects.models import Project
 
 from ..models import RemoteOrganization, RemoteRepository
@@ -252,6 +253,7 @@ def get_webhook_data(self, repo_id, project, integration):
                     },
                 ),
             ),
+            'token': integration.secret,
 
             # Optional
             'issues_events': False,
@@ -275,7 +277,6 @@ def setup_webhook(self, project):
             project=project,
             integration_type=Integration.GITLAB_WEBHOOK,
         )
-
         repo_id = self._get_repo_id(project)
         if repo_id is None:
             return (False, None)
@@ -333,6 +334,7 @@ def update_webhook(self, project, integration):
         if repo_id is None:
             return (False, None)
 
+        integration.recreate_secret()
         data = self.get_webhook_data(repo_id, project, integration)
         hook_id = integration.provider_data.get('id')
         resp = None

readthedocs/projects/forms.py

@@ -762,7 +762,7 @@ class IntegrationForm(forms.ModelForm):
 
     class Meta:
         model = Integration
-        exclude = ['provider_data', 'exchanges']  # pylint: disable=modelform-uses-exclude
+        exclude = ['provider_data', 'exchanges', 'secret']  # pylint: disable=modelform-uses-exclude
 
     def __init__(self, *args, **kwargs):
         self.project = kwargs.pop('project', None)
@@ -775,6 +775,9 @@ def clean_project(self):
 
     def save(self, commit=True):
         self.instance = Integration.objects.subclass(self.instance)
+        # We don't set the secret on the integration
+        # when it's created via the form.
+        self.instance.secret = None
         return super().save(commit)