Add feature flip for build endpoints (#3205)

* Add feature flipping models to Projects, plus example feature flip

Also, use a new pattern for API instance objects

* Refactoring the make_api_* calls into the API models

* Add migrations for proxy models

* Add some admin features

* Bump setuptools version.

* Add feature flip for setuptools latest version

Also, add a convenience method on the Project model for feature flag dependent
values.

* Add feature flip for build endpoints

Returns 403 if the project doesn't have the build endpoint enabled.

* Rework feature relationship, add default true value based on date

This allows for features on deprecated code, where we can say the feature is
true historically.

* Fix some issues, more tests

* Add data migration adding feature to database

* Fix migration

* Fix tests

* Rename Feature.feature -> Feature.feature_id, other cleanup

* Drop setuptools pinning

* Update field name

* Fix migration as well

* Use semver for setuptools version

* Missed merge conflict

* Rename function, also go private function

Author: agjohnson

readthedocs/core/views/hooks.py

@@ -11,7 +11,7 @@
 from readthedocs.core.utils import trigger_build
 from readthedocs.builds.constants import LATEST
 from readthedocs.projects import constants
-from readthedocs.projects.models import Project
+from readthedocs.projects.models import Project, Feature
 from readthedocs.projects.tasks import update_imported_docs
 
 import logging
@@ -23,6 +23,10 @@ class NoProjectException(Exception):
     pass
 
 
+def _allow_deprecated_webhook(project):
+    return project.has_feature(Feature.ALLOW_DEPRECATED_WEBHOOKS)
+
+
 def _build_version(project, slug, already_built=()):
     """
     Where we actually trigger builds for a project and slug.
@@ -108,6 +112,13 @@ def _build_url(url, projects, branches):
     ret = ""
     all_built = {}
     all_not_building = {}
+
+    # This endpoint doesn't require authorization, we shouldn't allow builds to
+    # be triggered from this any longer. Deprecation plan is to selectively
+    # allow access to this endpoint for now.
+    if not any(_allow_deprecated_webhook(project) for project in projects):
+        return HttpResponse('This API endpoint is deprecated', status=403)
+
     for project in projects:
         (built, not_building) = build_branches(project, branches)
         if not built:
@@ -314,6 +325,11 @@ def generic_build(request, project_id_or_slug=None):
                 project_id_or_slug)
             return HttpResponseNotFound(
                 'Repo not found: %s' % project_id_or_slug)
+    # This endpoint doesn't require authorization, we shouldn't allow builds to
+    # be triggered from this any longer. Deprecation plan is to selectively
+    # allow access to this endpoint for now.
+    if not _allow_deprecated_webhook(project):
+        return HttpResponse('This API endpoint is deprecated', status=403)
     if request.method == 'POST':
         slug = request.POST.get('version_slug', project.default_version)
         log.info(

readthedocs/projects/migrations/0021_add-webhook-deprecation-feature.py

@@ -0,0 +1,34 @@
+# -*- coding: utf-8 -*-
+
+"""Add feature for allowing access to deprecated webhook endpoints"""
+
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+FEATURE_ID = 'allow_deprecated_webhooks'
+
+
+def forward_add_feature(apps, schema_editor):
+    Feature = apps.get_model('projects', 'Feature')
+    Feature.objects.create(
+        feature_id=FEATURE_ID,
+        default_true=True,
+    )
+
+
+def reverse_add_feature(apps, schema_editor):
+    Feature = apps.get_model('projects', 'Feature')
+    Feature.objects.filter(feature_id=FEATURE_ID).delete()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('projects', '0020_add-api-project-proxy'),
+    ]
+
+    operations = [
+        migrations.RunPython(forward_add_feature, reverse_add_feature)
+    ]

readthedocs/projects/models.py

@@ -1003,11 +1003,13 @@ def add_features(sender, **kwargs):
     # may be added by other packages
     USE_SPHINX_LATEST = 'use_sphinx_latest'
     USE_SETUPTOOLS_LATEST = 'use_setuptools_latest'
+    ALLOW_DEPRECATED_WEBHOOKS = 'allow_deprecated_webhooks'
     PIP_ALWAYS_UPGRADE = 'pip_always_upgrade'
 
     FEATURES = (
         (USE_SPHINX_LATEST, _('Use latest version of Sphinx')),
         (USE_SETUPTOOLS_LATEST, _('Use latest version of setuptools')),
+        (ALLOW_DEPRECATED_WEBHOOKS, _('Allow deprecated webhook views')),
         (PIP_ALWAYS_UPGRADE, _('Always run pip install --upgrade')),
     )
 

readthedocs/rtd_tests/tests/test_post_commit_hooks.py

@@ -9,7 +9,7 @@
 from future.backports.urllib.parse import urlencode
 
 from readthedocs.builds.models import Version
-from readthedocs.projects.models import Project
+from readthedocs.projects.models import Project, Feature
 
 
 log = logging.getLogger(__name__)
@@ -32,6 +32,11 @@ def _setup(self):
         self.mocks = [mock.patch('readthedocs.core.views.hooks.trigger_build')]
         self.patches = [m.start() for m in self.mocks]
 
+        self.feature = Feature.objects.get(feature_id=Feature.ALLOW_DEPRECATED_WEBHOOKS)
+        self.feature.projects.add(self.pip)
+        self.feature.projects.add(self.rtfd)
+        self.feature.projects.add(self.sphinx)
+
         self.client.login(username='eric', password='test')
 
 
@@ -141,6 +146,23 @@ def test_gitlab_post_commit_knows_default_branches(self):
         rtd.default_branch = old_default
         rtd.save()
 
+    def test_gitlab_webhook_is_deprecated(self):
+        # Project is created after feature, not included in historical allowance
+        url = 'https://github.com/rtfd/readthedocs-build'
+        payload = self.payload.copy()
+        payload['project']['http_url'] = url
+        project = get(
+            Project,
+            main_language_project=None,
+            repo=url,
+        )
+        r = self.client.post(
+            '/gitlab/',
+            data=json.dumps(payload),
+            content_type='application/json'
+        )
+        self.assertEqual(r.status_code, 403)
+
 
 class GitHubPostCommitTest(BasePostCommitTest):
     fixtures = ["eric"]
@@ -297,6 +319,23 @@ def test_github_post_commit_knows_default_branches(self):
         rtd.default_branch = old_default
         rtd.save()
 
+    def test_github_webhook_is_deprecated(self):
+        # Project is created after feature, not included in historical allowance
+        url = 'https://github.com/rtfd/readthedocs-build'
+        payload = self.payload.copy()
+        payload['repository']['url'] = url
+        project = get(
+            Project,
+            main_language_project=None,
+            repo=url,
+        )
+        r = self.client.post(
+            '/github/',
+            data=json.dumps(payload),
+            content_type='application/json'
+        )
+        self.assertEqual(r.status_code, 403)
+
 
 class CorePostCommitTest(BasePostCommitTest):
     fixtures = ["eric"]
@@ -320,6 +359,12 @@ def test_hook_state_tracking(self):
         # Need to re-query to get updated DB entry
         self.assertEqual(Project.objects.get(slug='read-the-docs').has_valid_webhook, True)
 
+    def test_generic_webhook_is_deprecated(self):
+        # Project is created after feature, not included in historical allowance
+        project = get(Project, main_language_project=None)
+        r = self.client.post('/build/%s' % project.pk, {'version_slug': 'master'})
+        self.assertEqual(r.status_code, 403)
+
 
 class BitBucketHookTests(BasePostCommitTest):
 
@@ -476,6 +521,7 @@ def test_bitbucket_default_branch(self):
             Project, repo='HTTPS://bitbucket.org/test/project', slug='test-project',
             default_branch='integration', repo_type='git',
         )
+        self.feature.projects.add(self.test_project)
 
         self.git_payload['commits'] = [{
             "branch": "integration",
@@ -487,3 +533,20 @@ def test_bitbucket_default_branch(self):
         r = self.client.post('/bitbucket/', data=json.dumps(self.git_payload),
                              content_type='application/json')
         self.assertContains(r, '(URL Build) Build Started: bitbucket.org/test/project [latest]')
+
+    def test_bitbucket_webhook_is_deprecated(self):
+        # Project is created after feature, not included in historical allowance
+        url = 'https://bitbucket.org/rtfd/readthedocs-build'
+        payload = self.git_payload.copy()
+        payload['repository']['absolute_url'] = '/rtfd/readthedocs-build'
+        project = get(
+            Project,
+            main_language_project=None,
+            repo=url,
+        )
+        r = self.client.post(
+            '/bitbucket/',
+            data=json.dumps(payload),
+            content_type='application/json'
+        )
+        self.assertEqual(r.status_code, 403)