github validate hook again

docs/topics/api/github.rst

@@ -0,0 +1,29 @@
+===============
+GitHub Webhooks
+===============
+
+.. note::
+    
+    This is an Experimental API. We aren't sure of the value of this API yet, so we'd like to see how widely it's used before committing to long term support.
+
+This API provides an endpoint that works with GitHub to provide add-on validation as a GitHub webhook. This end point is designed to be called specifically from GitHub and will only send API responses back to `api.github.com`.
+
+To set this up on a GitHub repository you will need to:
+
+* Go to `Settings > Webhooks & Services`
+* Add a new Webhook with Payload URL of `https://addons.mozilla.org/api/v3/github/`
+* Click `Update webhook`
+
+The validator will run when you create or alter a pull request.
+
+.. http:post:: /api/v3/github/
+
+    **Request:**
+
+    A `GitHub API webhook <https://developer.github.com/v3/repos/hooks/>`_ body. Currently only `pull_request` events are processed, all others are ignored.
+
+    **Response:**
+
+    :statuscode 201: request has been processed and a pending message sent back to GitHub.
+    :statuscode 200: request is not a `pull_request`, it's been accepted.
+    :statuscode 422: body is invalid.

docs/topics/api/index.rst

@@ -22,7 +22,7 @@ Staging or Development
     you're testing features that aren't available in production yet.
     Your production account is not linked to any of these APIs.
 
-Dive into the :ref:`overview section <api-overview>` and the 
+Dive into the :ref:`overview section <api-overview>` and the
 :ref:`authentication section <api-auth>` for an example of how to get started
 using the API.
 
@@ -42,3 +42,4 @@ using the API.
    reviews
    signing
    stats
+   github

settings.py

@@ -132,6 +132,10 @@ INBOUND_EMAIL_SECRET_KEY = 'totally-unsecure-secret-string'
 # Validation key we need to send in POST response.
 INBOUND_EMAIL_VALIDATION_KEY = 'totally-unsecure-validation-string'
 
+# For the Github webhook API.
+GITHUB_API_USER = ''
+GITHUB_API_TOKEN = ''
+
 # If you have settings you want to overload, put them in a local_settings.py.
 try:
     from local_settings import *  # noqa

src/olympia/api/urls.py

@@ -17,4 +17,5 @@ urlpatterns = patterns(
     url(r'^v3/', include('olympia.signing.urls')),
     url(r'^v3/statistics/', include('olympia.stats.api_urls')),
     url(r'^v3/activity/', include('olympia.activity.urls')),
+    url(r'^v3/github/', include('olympia.github.urls')),
 )

src/olympia/devhub/tests/test_views_validation.py

@@ -63,6 +63,16 @@ class TestUploadValidation(BaseUploadTest):
         upload = FileUpload.objects.get(uuid=uuid)
         assert upload.processed_validation['errors'] == 1
 
+    def test_login_required(self):
+        upload = FileUpload.objects.get(name='invalid-id-20101206.xpi')
+        upload.user_id = 999
+        upload.save()
+        url = reverse('devhub.upload_detail', args=[upload.uuid.hex])
+        assert self.client.head(url, follow=True).status_code == 200
+
+        self.client.logout()
+        assert self.client.head(url).status_code == 302
+
 
 class TestUploadErrors(BaseUploadTest):
     fixtures = ('base/addon_3615', 'base/users')

src/olympia/devhub/views.py

@@ -883,12 +883,13 @@ def upload_validation_context(request, upload, addon=None, url=None):
             'processed_by_addons_linter': processed_by_linter}
 
 
-@login_required
 def upload_detail(request, uuid, format='html'):
+    upload = get_object_or_404(FileUpload, uuid=uuid)
+    if upload.user_id and not request.user.is_authenticated():
+        return redirect_for_login(request)
+
     if format == 'json' or request.is_ajax():
         try:
-            # This is duplicated in the HTML code path.
-            upload = get_object_or_404(FileUpload, uuid=uuid)
             response = json_upload_detail(request, upload)
             statsd.incr('devhub.upload_detail.success')
             return response
@@ -898,9 +899,6 @@ def upload_detail(request, uuid, format='html'):
                 type(exc), exc))
             raise
 
-    # This is duplicated in the JSON code path.
-    upload = get_object_or_404(FileUpload, uuid=uuid)
-
     validate_url = reverse('devhub.standalone_upload_detail',
                            args=[upload.uuid.hex])
 

src/olympia/files/utils.py

@@ -514,6 +514,13 @@ class SafeUnzip(object):
     def close(self):
         self.zip_file.close()
 
+    @property
+    def filelist(self):
+        return self.zip_file.filelist
+
+    def read(self, filename):
+        return self.zip_file.read(filename)
+
 
 def extract_zip(source, remove=False, fatal=True):
     """Extracts the zip file. If remove is given, removes the source file."""

src/olympia/github/tasks.py

@@ -0,0 +1,54 @@
+import commonware
+import json
+
+from olympia.amo.celery import task
+from olympia.amo.helpers import absolutify
+from olympia.amo.urlresolvers import reverse
+
+from olympia.github.utils import GithubCallback, rezip_file
+from olympia.files.models import FileUpload
+from olympia.devhub.tasks import validate
+
+log = commonware.log.getLogger('z.github')
+
+
+@task
+def process_webhook(upload_pk, callbacks):
+    log.info('Processing webhook for: {}'.format(upload_pk))
+    upload = FileUpload.objects.get(pk=upload_pk)
+    github = GithubCallback(callbacks)
+    res = github.get()
+
+    upload.name = '{}-github-webhook.xpi'.format(upload.pk)
+    upload.path = rezip_file(res, upload.pk)
+    upload.save()
+
+    log.info('Validating: {}'.format(upload_pk))
+    validate(
+        upload,
+        listed=True,
+        subtask=process_results.si(upload_pk, callbacks)
+    )
+
+
+@task
+def process_results(upload_pk, callbacks):
+    log.info('Processing validation results for: {}'.format(upload_pk))
+    upload = FileUpload.objects.get(pk=upload_pk)
+    validation = json.loads(upload.validation) if upload.validation else {}
+    github = GithubCallback(callbacks)
+    url = absolutify(
+        reverse('devhub.upload_detail', args=[upload.uuid]))
+
+    if not validation:
+        log.error('Validation not written: {}'.format(upload_pk))
+        github.failure()
+        return
+
+    if validation.get('success'):
+        log.info('Notifying success for: {}'.format(upload_pk))
+        github.success(url)
+        return
+
+    log.info('Notifying errors for: {}'.format(upload_pk))
+    github.error(url)

src/olympia/github/tests/test_github.py

@@ -0,0 +1,140 @@
+import zipfile
+from copy import deepcopy
+
+import mock
+
+from django import forms
+from django.test.utils import override_settings
+
+from olympia.amo.tests import AMOPaths, TestCase
+from olympia.github.utils import GithubCallback, GithubRequest, rezip_file
+
+
+example_root = 'https://api.github.com/repos/org/repo'
+example_pull_request = {
+    'pull_request': {
+        'head': {
+            'repo': {
+                'archive_url': example_root + '/{archive_format}{/ref}',
+                'statuses_url': example_root + '/statuses/{sha}',
+                'pulls_url': example_root + '/pulls{/number}'
+            },
+            'sha': 'abc'
+        },
+        'number': 1,
+    },
+    'repository': {
+        'commits_url': example_root + '/commits{/sha}'
+    }
+}
+
+
+class TestGithub(TestCase):
+
+    def test_github(self):
+        form = GithubRequest(data=example_pull_request)
+        assert form.is_valid(), form.errors
+        assert (
+            form.cleaned_data['status_url'] ==
+            'https://api.github.com/repos/org/repo/statuses/abc')
+        assert (
+            form.cleaned_data['zip_url'] ==
+            'https://api.github.com/repos/org/repo/zipball/abc')
+
+    def test_invalid(self):
+        example = deepcopy(example_pull_request)
+        del example['pull_request']['head']
+        form = GithubRequest(data=example)
+        assert not form.is_valid()
+
+    def test_url_wrong(self):
+        example = deepcopy(example_pull_request)
+        example['pull_request']['head']['repo'] = 'http://a.m.o'
+        form = GithubRequest(data=example)
+        assert not form.is_valid()
+
+
+@override_settings(GITHUB_API_USER='key', GITHUB_API_TOKEN='token')
+class GithubBase(TestCase):
+
+    def setUp(self):
+        super(GithubBase, self).setUp()
+        patch = mock.patch('olympia.github.utils.requests', autospec=True)
+        self.addCleanup(patch.stop)
+        self.requests = patch.start()
+        self.data = {
+            'type': 'github',
+            'status_url': 'https://github/status',
+            'zip_url': 'https://github/zip',
+            'sha': 'some:sha'
+        }
+        self.github = GithubCallback(self.data)
+
+    def check_status(self, status, call=None, url=None, **kw):
+        url = url or self.data['status_url']
+        body = {'context': 'addons/linter'}
+        if status != 'comment':
+            body['state'] = status
+
+        body.update(**kw)
+        if not call:
+            call = self.requests.post.call_args_list
+            if len(call) != 1:
+                # If you don't specify a call to test, we'll get the last
+                # one off the stack, if there's more than one, that's a
+                # problem.
+                raise AssertionError('More than one call to requests.post')
+            call = call[0]
+        assert call == mock.call(url, json=body, auth=('key', 'token'))
+
+
+class TestCallback(GithubBase):
+
+    def test_create_not_github(self):
+        with self.assertRaises(ValueError):
+            GithubCallback({'type': 'bitbucket'})
+
+    def test_pending(self):
+        self.github.pending()
+        self.check_status('pending')
+
+    def test_success(self):
+        self.github.success('http://a.m.o/')
+        self.check_status('success', target_url='http://a.m.o/')
+
+    def test_error(self):
+        self.github.error('http://a.m.o/')
+        self.check_status(
+            'error', description=mock.ANY, target_url='http://a.m.o/')
+
+    def test_failure(self):
+        self.github.failure()
+        self.check_status(
+            'failure', description=mock.ANY)
+
+    def test_get(self):
+        self.github.get()
+        self.requests.get.assert_called_with(
+            'https://github/zip'
+        )
+
+
+class TestRezip(AMOPaths, TestCase):
+
+    def setUp(self):
+        self.response = mock.Mock()
+        self.response.content = open(self.xpi_path('github-repo')).read()
+
+    def test_rezip(self):
+        new_path = rezip_file(self.response, 1)
+        with open(new_path, 'r') as new_file:
+            new_zip = zipfile.ZipFile(new_file)
+            self.assertSetEqual(
+                set([f.filename for f in new_zip.filelist]),
+                set(['manifest.json', 'index.js'])
+            )
+
+    def test_badzip(self):
+        with self.settings(FILE_UNZIP_SIZE_LIMIT=5):
+            with self.assertRaises(forms.ValidationError):
+                rezip_file(self.response, 1)

src/olympia/github/tests/test_tasks.py

@@ -0,0 +1,60 @@
+import json
+
+import mock
+from django.test.utils import override_settings
+
+from olympia.amo.helpers import absolutify
+from olympia.amo.tests import AMOPaths
+from olympia.github.tasks import process_results, process_webhook
+from olympia.github.tests.test_github import GithubBase
+from olympia.amo.urlresolvers import reverse
+from olympia.files.models import FileUpload
+
+
+@override_settings(GITHUB_API_USER='key', GITHUB_API_TOKEN='token')
+class TestGithub(AMOPaths, GithubBase):
+
+    def get_url(self, upload_uuid):
+        return absolutify(
+            reverse('devhub.upload_detail', args=[upload_uuid]))
+
+    def test_good_results(self):
+        upload = FileUpload.objects.create(
+            validation=json.dumps({'success': True, 'errors': 0})
+        )
+        process_results(upload.pk, self.data)
+        self.check_status('success', target_url=self.get_url(upload.uuid))
+
+    def test_failed_results(self):
+        upload = FileUpload.objects.create()
+        process_results(upload.pk, self.data)
+        self.check_status('failure', description=mock.ANY)
+
+    def test_error_results(self):
+        upload = FileUpload.objects.create(
+            validation=json.dumps({
+                'errors': 1,
+                'messages': [{
+                    'description': ['foo'],
+                    'file': 'some/file',
+                    'line': 3,
+                    'type': 'error'
+                }]
+            })
+        )
+        process_results(upload.pk, self.data)
+        error = self.requests.post.call_args_list[0]
+        self.check_status(
+            'error',
+            call=error, description=mock.ANY,
+            target_url=self.get_url(upload.uuid))
+
+    def test_webhook(self):
+        upload = FileUpload.objects.create()
+
+        self.response = mock.Mock()
+        self.response.content = open(self.xpi_path('github-repo')).read()
+        self.requests.get.return_value = self.response
+
+        process_webhook(upload.pk, self.data)
+        self.check_status('success', target_url=self.get_url(upload.uuid))

src/olympia/github/tests/test_views.py

@@ -0,0 +1,49 @@
+import json
+import mock
+
+from olympia.amo.tests import AMOPaths, TestCase
+from olympia.files.models import FileUpload
+from olympia.amo.urlresolvers import reverse
+
+from olympia.github.tests.test_github import example_pull_request, GithubBase
+
+
+class TestGithubView(AMOPaths, GithubBase, TestCase):
+
+    def setUp(self):
+        super(TestGithubView, self).setUp()
+        self.url = reverse('github.validate')
+
+    def post(self, data, header=None):
+        return self.client.post(
+            self.url, data=json.dumps(data),
+            content_type='application/json',
+            HTTP_X_GITHUB_EVENT=header or 'pull_request'
+        )
+
+    def test_not_pull_request(self):
+        assert self.post({}, header='meh').status_code == 200
+
+    def test_bad_pull_request(self):
+        assert self.post({'pull_request': {}}).status_code == 422
+
+    def test_good(self):
+        self.response = mock.Mock()
+        self.response.content = open(self.xpi_path('github-repo')).read()
+        self.requests.get.return_value = self.response
+
+        self.post(example_pull_request)
+        pending, success = self.requests.post.call_args_list
+        self.check_status(
+            'pending',
+            call=pending,
+            url='https://api.github.com/repos/org/repo/statuses/abc'
+        )
+        self.check_status(
+            'success',
+            call=success,
+            url='https://api.github.com/repos/org/repo/statuses/abc',
+            target_url=mock.ANY
+        )
+
+        assert FileUpload.objects.get()

src/olympia/github/urls.py

@@ -0,0 +1,6 @@
+from django.conf.urls import url
+from olympia.github.views import GithubView
+
+urlpatterns = [
+    url(r'^validate/$', GithubView.as_view(), name='github.validate'),
+]

src/olympia/github/utils.py

@@ -0,0 +1,149 @@
+import os
+import uuid
+import zipfile
+
+import commonware.log
+import requests
+from django import forms
+from django.conf import settings
+from django.core.files.storage import default_storage as storage
+from django_statsd.clients import statsd
+
+from olympia.amo.helpers import user_media_path
+from olympia.files.utils import SafeUnzip
+
+log = commonware.log.getLogger('z.github')
+
+
+class GithubCallback(object):
+
+    def __init__(self, data):
+        if data['type'] != 'github':
+            raise ValueError('Not a github callback.')
+        self.data = data
+
+    def get(self):
+        log.info('Getting zip from github: {}'.format(self.data['zip_url']))
+        with statsd.timer('github.zip'):
+            res = requests.get(self.data['zip_url'])
+            res.raise_for_status()
+        return res
+
+    def post(self, url, data):
+        msg = data.get('state', 'comment')
+        log.info('Setting github to: {} at: {}'.format(msg, url))
+        with statsd.timer('github.{}'.format(msg)):
+            data['context'] = 'addons/linter'
+            log.info('Body: {}'.format(data))
+            res = requests.post(
+                url,
+                json=data,
+                auth=(settings.GITHUB_API_USER, settings.GITHUB_API_TOKEN))
+            log.info('Response: {}'.format(res.content))
+            res.raise_for_status()
+
+    def pending(self):
+        self.post(self.data['status_url'], data={'state': 'pending'})
+
+    def success(self, url):
+        self.post(self.data['status_url'], data={
+            'state': 'success',
+            'target_url': url
+        })
+
+    def error(self, url):
+        self.post(self.data['status_url'], data={
+            'state': 'error',
+            # Not localising because we aren't sure what locale to localise to.
+            # I would like to pass a longer string here that shows more details
+            # however, we are limited to "A short description of the status."
+            # Which means all the fancy things I wanted to do got truncated.
+            'description': 'This add-on did not validate.',
+            'target_url': url
+        })
+
+    def failure(self):
+        data = {
+            'state': 'failure',
+            # Not localising because we aren't sure what locale to localise to.
+            'description': 'The validator failed to run correctly.'
+        }
+        self.post(self.data['status_url'], data=data)
+
+
+class GithubRequest(forms.Form):
+    status_url = forms.URLField(required=False)
+    zip_url = forms.URLField(required=False)
+    sha = forms.CharField(required=False)
+
+    @property
+    def repo(self):
+        return self.data['pull_request']['head']['repo']
+
+    @property
+    def sha(self):
+        return self.data['pull_request']['head']['sha']
+
+    def get_status(self):
+        return self.repo['statuses_url'].replace('{sha}', self.sha)
+
+    def get_zip(self):
+        return (
+            self.repo['archive_url']
+            .replace('{archive_format}', 'zipball')
+            .replace('{/ref}', '/' + self.sha))
+
+    def validate_url(self, url):
+        if not url.startswith('https://api.github.com/'):
+            raise forms.ValidationError('Invalid URL: {}'.format(url))
+        return url
+
+    def clean(self):
+        fields = (
+            ('status_url', self.get_status),
+            ('zip_url', self.get_zip),
+        )
+        for url, method in fields:
+            try:
+                self.cleaned_data[url] = self.validate_url(method())
+            except:
+                log.error('Invalid data in processing JSON')
+                raise forms.ValidationError('Invalid data')
+
+        self.cleaned_data['sha'] = self.data['pull_request']['head']['sha']
+        self.cleaned_data['type'] = 'github'
+        return self.cleaned_data
+
+
+def rezip_file(response, pk):
+    # An .xpi does not have a directory inside the zip, yet zips from github
+    # do, so we'll need to rezip the file before passing it through to the
+    # validator.
+    loc = os.path.join(user_media_path('addons'), 'temp', uuid.uuid4().hex)
+    old_filename = '{}_github_webhook.zip'.format(pk)
+    old_path = os.path.join(loc, old_filename)
+
+    with storage.open(old_path, 'wb') as old:
+        old.write(response.content)
+
+    new_filename = '{}_github_webhook.xpi'.format(pk)
+    new_path = os.path.join(loc, new_filename)
+
+    old_zip = SafeUnzip(old_path)
+    if not old_zip.is_valid():
+        raise
+
+    with storage.open(new_path, 'w') as new:
+        new_zip = zipfile.ZipFile(new, 'w')
+
+        for obj in old_zip.filelist:
+            # Basically strip off the leading directory.
+            new_filename = obj.filename.partition('/')[-1]
+            if not new_filename:
+                continue
+            new_zip.writestr(new_filename, old_zip.read(obj.filename))
+
+        new_zip.close()
+
+    old_zip.close()
+    return new_path

src/olympia/github/views.py

@@ -0,0 +1,32 @@
+import commonware
+
+from rest_framework.views import APIView
+from rest_framework.response import Response
+
+from olympia.files.models import FileUpload
+from olympia.github.tasks import process_webhook
+from olympia.github.utils import GithubRequest, GithubCallback
+
+log = commonware.log.getLogger('z.github')
+
+
+class GithubView(APIView):
+
+    def post(self, request):
+        if request.META.get('HTTP_X_GITHUB_EVENT') != 'pull_request':
+            # That's ok, we are just going to ignore it, we'll return a 2xx
+            # response so github doesn't report it as an error.
+            return Response({}, status=200)
+
+        github = GithubRequest(data=request.data)
+        if not github.is_valid():
+            return Response({}, status=422)
+
+        data = github.cleaned_data
+        upload = FileUpload.objects.create()
+        log.info('Created FileUpload from github api: {}'.format(upload.pk))
+        github = GithubCallback(data)
+        github.pending()
+
+        process_webhook.delay(upload.pk, data)
+        return Response({}, status=201)

src/olympia/lib/settings_base.py

@@ -430,6 +430,7 @@ INSTALLED_APPS = (
     'olympia.discovery',
     'olympia.editors',
     'olympia.files',
+    'olympia.github',
     'olympia.internal_tools',
     'olympia.legacy_api',
     'olympia.legacy_discovery',
@@ -1194,6 +1195,10 @@ CELERY_ROUTES = {
     'olympia.zadmin.tasks.notify_compatibility': {'queue': 'zadmin'},
     'olympia.zadmin.tasks.notify_compatibility_chunk': {'queue': 'zadmin'},
     'olympia.zadmin.tasks.update_maxversions': {'queue': 'zadmin'},
+
+    # Github API
+    'olympia.github.tasks.process_results': {'queue': 'devhub'},
+    'olympia.github.tasks.process_webhook': {'queue': 'devhub'},
 }