Validate and save packaged app origin (bug 878101, 878103)

apps/addons/models.py

@@ -458,7 +458,9 @@ class Addon(amo.models.OnChangeMixin, amo.models.ModelBase):
             addon.default_locale = to_language(translation.get_language())
         if addon.is_webapp():
             addon.is_packaged = is_packaged
-            if not is_packaged:
+            if is_packaged:
+                addon.app_domain = data.get('origin')
+            else:
                 addon.manifest_url = upload.name
                 addon.app_domain = addon.domain_from_url(addon.manifest_url)
         addon.save()

apps/files/utils.py

@@ -268,7 +268,8 @@ class WebAppParser(object):
                 'developer_name': developer_name,
                 'summary': self.trans_all_locales(localized_descr),
                 'version': data.get('version', '1.0'),
-                'default_locale': default_locale}
+                'default_locale': default_locale,
+                'origin': data.get('origin')}
 
     def trans_locale(self, locale):
         return to_language(settings.SHORTER_LANGUAGES.get(locale, locale))

mkt/developers/forms.py

@@ -2,6 +2,7 @@
 import json
 import os
 from datetime import datetime
+from urlparse import urlparse, urlunparse
 
 from django import forms
 from django.conf import settings
@@ -180,6 +181,24 @@ def trap_duplicate(request, manifest_url):
         return msg % (app.name, error_url)
 
 
+def validate_origin(origin):
+    """
+    Validates that an origin looks like a url.
+
+    Returns origin stripped of path, params, query strings, and fragments.
+    """
+    parts = urlparse(origin)
+    if not parts.scheme.startswith('app'):
+        raise forms.ValidationError(
+            _('Origin must start with either "app://".'))
+
+    if not '.' in parts.netloc:
+        raise forms.ValidationError(
+            _('Origin must be a fully qualified domain name.'))
+
+    return urlunparse((parts.scheme, parts.netloc, '', '', '', ''))
+
+
 def verify_app_domain(manifest_url, exclude=None):
     if waffle.switch_is_active('webapps-unique-by-domain'):
         domain = Webapp.domain_from_url(manifest_url)

mkt/developers/tests/test_forms.py

@@ -5,6 +5,7 @@ import shutil
 from django.conf import settings
 from django.core.files.storage import default_storage as storage
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.forms import ValidationError
 
 import mock
 from nose.tools import eq_
@@ -347,6 +348,32 @@ class TestNewManifestForm(amo.tests.TestCase):
         assert not _verify_app_domain.called
 
 
+class TestValidateOrigin(amo.tests.TestCase):
+
+    def test_invalid_origins(self):
+        origins = [
+            'this-is-not-an-origin',
+            'ftp://domain.com',
+            'mail:someone@somewhere.com',
+            '//domain.com',
+            'http://domain.com',
+            'https://domain.com',
+        ]
+        for origin in origins:
+            with self.assertRaises(ValidationError):
+                forms.validate_origin(origin)
+
+    def test_valid_origins(self):
+        origins = [
+            'app://domain.com',
+            'app://domain.com/with/path.exe?q=yo',
+            # TODO: Should that be valid? ^
+        ]
+        for origin in origins:
+            origin = forms.validate_origin(origin)
+            assert origin, 'Origin invalid: %s' % origin
+
+
 class TestPackagedAppForm(amo.tests.AMOPaths, amo.tests.WebappTestCase):
 
     def setUp(self):

mkt/submit/forms.py

@@ -24,6 +24,7 @@ from mkt.constants import (APP_FEATURES_DESCRIPTIONS, FREE_PLATFORMS,
                            PAID_PLATFORMS)
 from mkt.site.forms import AddonChoiceField, APP_PUBLIC_CHOICES
 from mkt.webapps.models import AppFeatures
+from mkt.developers.forms import validate_origin, verify_app_domain
 
 
 def mark_for_rereview(addon, added_devices, removed_devices):
@@ -150,7 +151,18 @@ class NewWebappVersionForm(happyforms.Form):
         queryset=FileUpload.objects.filter(valid=True),
         error_messages={'invalid_choice': upload_error})
 
+    def __init__(self, *args, **kw):
+        request = kw.pop('request', None)
+        self.addon = kw.pop('addon', None)
+        self._is_packaged = kw.pop('is_packaged', False)
+        super(NewWebappVersionForm, self).__init__(*args, **kw)
+
+        if (not waffle.flag_is_active(request, 'allow-b2g-paid-submission')
+            and 'paid_platforms' in self.fields):
+            del self.fields['paid_platforms']
+
     def clean(self):
+
         data = self.cleaned_data
         if 'upload' not in self.cleaned_data:
             self._errors['upload'] = self.upload_error
@@ -171,11 +183,22 @@ class NewWebappVersionForm(happyforms.Form):
                 self.addon.versions.filter(version=ver).exists()):
                 self._errors['upload'] = _(u'Version %s already exists') % ver
                 return
+
+            origin = pkg.get('origin')
+            if origin:
+                try:
+                    validate_origin(origin)
+                    origin = verify_app_domain(origin)
+                except forms.ValidationError, e:
+                    self._errors['upload'] = self.error_class(e.messages)
+                    return
+                if origin:
+                    data['origin'] = origin
+
         else:
             # Throw an error if this is a dupe.
             # (JS sets manifest as `upload.name`.)
             try:
-                from mkt.developers.forms import verify_app_domain
                 verify_app_domain(data['upload'].name)
             except forms.ValidationError, e:
                 self._errors['upload'] = self.error_class(e.messages)
@@ -186,16 +209,6 @@ class NewWebappVersionForm(happyforms.Form):
     def is_packaged(self):
         return self._is_packaged
 
-    def __init__(self, *args, **kw):
-        request = kw.pop('request', None)
-        self.addon = kw.pop('addon', None)
-        self._is_packaged = kw.pop('is_packaged', False)
-        super(NewWebappVersionForm, self).__init__(*args, **kw)
-
-        if (not waffle.flag_is_active(request, 'allow-b2g-paid-submission')
-            and 'paid_platforms' in self.fields):
-            del self.fields['paid_platforms']
-
 
 class NewWebappForm(DeviceTypeForm, NewWebappVersionForm):
     upload = forms.ModelChoiceField(widget=forms.HiddenInput,

mkt/submit/tests/test_forms.py

@@ -98,6 +98,7 @@ class TestNewWebappForm(amo.tests.TestCase):
 
     @mock.patch('mkt.submit.forms.parse_addon')
     def test_packaged_allowed(self, parse_addon):
+        parse_addon.return_value = {}
         form = forms.NewWebappForm({'free_platforms': ['free-firefoxos'],
                                     'upload': self.file.uuid,
                                     'packaged': True})
@@ -106,6 +107,7 @@ class TestNewWebappForm(amo.tests.TestCase):
 
     @mock.patch('mkt.submit.forms.parse_addon')
     def test_packaged_allowed_android(self, parse_addon):
+        parse_addon.return_value = {}
         form = forms.NewWebappForm({'free_platforms': ['free-android-mobile'],
                                     'upload': self.file.uuid,
                                     'packaged': True})
@@ -122,6 +124,29 @@ class TestNewWebappForm(amo.tests.TestCase):
         eq_(form.ERRORS['packaged'], form.errors['paid_platforms'])
 
 
+class TestNewWebappVersionForm(amo.tests.TestCase):
+
+    def setUp(self):
+        self.request = RequestFactory().get('/')
+        self.file = FileUpload.objects.create(valid=True)
+
+    def test_no_upload(self):
+        form = forms.NewWebappVersionForm(request=self.request,
+                                          is_packaged=True)
+        assert not form.is_valid(), form.errors
+
+    @mock.patch('mkt.submit.forms.parse_addon',
+                lambda *args: {"origin": "app://hy.fr"})
+    @mock.patch('mkt.submit.forms.validate_origin')
+    def test_validate_origin_called(self, _validate):
+        self.create_switch('webapps-unique-by-domain')
+        form = forms.NewWebappVersionForm({'upload': self.file.uuid},
+                                          request=self.request,
+                                          is_packaged=True)
+        assert form.is_valid(), form.errors
+        assert _validate.called
+
+
 class TestAppDetailsBasicForm(amo.tests.TestCase):
     fixtures = fixture('user_999')
 

mkt/submit/tests/test_views.py

@@ -501,13 +501,15 @@ class BasePackagedAppTest(BaseUploadTest, UploadAddon, amo.tests.TestCase):
 
 class TestCreatePackagedApp(BasePackagedAppTest):
 
-    def test_post_app_redirect(self):
+    @mock.patch('mkt.webapps.models.Webapp.get_cached_manifest')
+    def test_post_app_redirect(self, _mock):
         res = self.post()
         webapp = Webapp.objects.order_by('-created')[0]
         self.assert3xx(res,
             reverse('submit.app.details', args=[webapp.app_slug]))
 
-    def test_app_from_uploaded_package(self):
+    @mock.patch('mkt.webapps.models.Webapp.get_cached_manifest')
+    def test_app_from_uploaded_package(self, _mock):
         addon = self.post_addon(
             data={'packaged': True, 'free_platforms': ['free-firefoxos']})
         eq_(addon.type, amo.ADDON_WEBAPP)
@@ -520,17 +522,19 @@ class TestCreatePackagedApp(BasePackagedAppTest):
         eq_(addon.app_slug, u'packaged-mozillaball-ょ')
         eq_(addon.summary, u'Exciting Open Web development action!')
         eq_(addon.manifest_url, None)
-        eq_(addon.app_domain, None)
+        eq_(addon.app_domain, 'app://hy.fr')
         eq_(Translation.objects.get(id=addon.summary.id, locale='it'),
             u'Azione aperta emozionante di sviluppo di fotoricettore!')
         eq_(addon.current_version.developer_name, 'Mozilla Labs')
 
     @mock.patch('mkt.developers.forms.verify_app_domain')
-    def test_packaged_app_not_unique_by_domain(self, _verify):
+    @mock.patch('mkt.webapps.models.Webapp.get_cached_manifest')
+    def test_packaged_app_not_unique_by_domain(self, _verify, _mock):
         self.post(
             data={'packaged': True, 'free_platforms': ['free-firefoxos']})
-        assert not _verify.called, (
-            '`verify_app_domain` should not be called for packaged apps.')
+        assert _verify.called, (
+            '`verify_app_domain` should be called for packaged apps with '
+            'origins.')
 
 
 class TestDetails(TestSubmit):