Only bulk sign recent firefox files (bug 1157444)

lib/crypto/tasks.py

@@ -4,15 +4,23 @@
 import shutil
 import zipfile
 
+from django.db.models import Q
+
 from celeryutils import task
 from lxml import etree
 
 import amo
 from versions.models import Version
 from lib.crypto.packaged import sign_file
+from versions.compare import version_int
 
 log = logging.getLogger('z.task')
 
+# Minimum Firefox version for default to compatible addons.
+MIN_D2C_VERSION = '4'
+# Minimum Firefox version for not default to compatible addons.
+MIN_NOT_D2C_VERSION = '37'
+
 
 @task
 def sign_addons(addon_ids, force=False, **kw):
@@ -26,12 +34,30 @@ def sign_addons(addon_ids, force=False, **kw):
     installs it.
     """
     log.info('[{0}] Signing addons.'.format(len(addon_ids)))
+
+    def file_supports_firefox(version):
+        """Return a Q object: files supporting at least a firefox version."""
+        return Q(version__apps__max__application=amo.FIREFOX.id,
+                 version__apps__max__version_int__gte=version_int(version))
+
+    is_default_compatible = Q(binary_components=False,
+                              strict_compatibility=False)
+    # We only want to sign files that are at least compatible with Firefox
+    # MIN_D2C_VERSION, or Firefox MIN_NOT_D2C_VERSION if they are not default
+    # to compatible.
+    # The signing feature should be supported from Firefox 40 and above, but
+    # we're still signing some files that are a bit older just in case.
+    ff_version_filter = (
+        (is_default_compatible & file_supports_firefox(MIN_D2C_VERSION)) |
+        (~is_default_compatible & file_supports_firefox(MIN_NOT_D2C_VERSION)))
+
     for version in Version.objects.filter(addon_id__in=addon_ids,
                                           addon__type=amo.ADDON_EXTENSION):
+        to_sign = version.files.filter(ff_version_filter)
         if force:
-            to_sign = version.files.all()
+            to_sign = to_sign.all()
         else:
-            to_sign = [f for f in version.files.all() if not f.is_signed]
+            to_sign = [f for f in to_sign.all() if not f.is_signed]
         if not to_sign:
             log.info('Not signing addon {0}, version {1} (no files or already '
                      'signed)'.format(version.addon, version))
@@ -40,7 +66,7 @@ def sign_addons(addon_ids, force=False, **kw):
                                                          version))
         bump_version = False  # Did we sign at least one file?
         for file_obj in to_sign:
-            if not os.path.exists(file_obj.file_path):
+            if not os.path.isfile(file_obj.file_path):
                 log.info('File {0} does not exist, skip'.format(file_obj.pk))
                 continue
             # Save the original file, before bumping the version.

lib/crypto/tests.py

@@ -12,6 +12,7 @@
 import amo.tests
 from files.utils import parse_xpi
 from lib.crypto import packaged, tasks
+from versions.compare import version_int
 
 
 @override_settings(SIGNING_SERVER='http://full',
@@ -130,6 +131,10 @@ def setUp(self):
         super(TestTasks, self).setUp()
         self.addon = amo.tests.addon_factory(version_kw={'version': '1.3'})
         self.version = self.addon.current_version
+        # Make sure our file/version is at least compatible with FF
+        # MIN_NOT_D2C_VERSION.
+        self.max_appversion = self.version.apps.first().max
+        self.set_max_appversion(tasks.MIN_NOT_D2C_VERSION)
         self.file_ = self.version.all_files[0]
         self.file_.update(filename='jetpack.xpi')
         self.backup_file_path = '{0}.backup_signature'.format(
@@ -140,6 +145,11 @@ def tearDown(self):
             os.unlink(self.backup_file_path)
         super(TestTasks, self).tearDown()
 
+    def set_max_appversion(self, version):
+        """Set self.max_appversion to the given version."""
+        self.max_appversion.update(version=version,
+                                   version_int=version_int(version))
+
     def assert_backup(self):
         """Make sure there's a backup file."""
         assert os.path.exists(self.backup_file_path)
@@ -175,6 +185,71 @@ def test_bump_version_in_model(self, mock_sign_file):
             if os.path.exists(backup_file2_path):
                 os.unlink(backup_file2_path)
 
+    @mock.patch('lib.crypto.tasks.sign_file')
+    def test_dont_sign_dont_bump_old_versions(self, mock_sign_file):
+        """Don't sign files which are too old, or not default to compatible."""
+        def not_signed():
+            assert not mock_sign_file.called
+            self.version.reload()
+            assert self.version.version == '1.3'
+            assert file_hash == self.file_.generate_hash()
+            self.assert_no_backup()
+
+        with amo.tests.copy_file('apps/files/fixtures/files/jetpack.xpi',
+                                 self.file_.file_path):
+            file_hash = self.file_.generate_hash()
+            assert self.version.version == '1.3'
+
+            # Too old, don't sign.
+            self.set_max_appversion('1')  # Very very old.
+            tasks.sign_addons([self.addon.pk])
+            not_signed()
+
+            # MIN_D2C_VERSION, but strict compat: don't sign.
+            self.set_max_appversion(tasks.MIN_D2C_VERSION)
+            self.file_.update(strict_compatibility=True)
+            tasks.sign_addons([self.addon.pk])
+            not_signed()
+
+            # MIN_D2C_VERSION, but binary component: don't sign.
+            self.file_.update(strict_compatibility=False,
+                              binary_components=True)
+            tasks.sign_addons([self.addon.pk])
+            not_signed()
+
+    @mock.patch('lib.crypto.tasks.sign_file')
+    def test_sign_bump_old_versions_default_compat(self, mock_sign_file):
+        """Sign files which are old, but default to compatible."""
+        with amo.tests.copy_file(
+                'apps/files/fixtures/files/new-addon-signature.xpi',
+                self.file_.file_path):
+            file_hash = self.file_.generate_hash()
+            assert self.version.version == '1.3'
+            self.set_max_appversion(tasks.MIN_D2C_VERSION)
+            tasks.sign_addons([self.addon.pk], force=True)
+            assert mock_sign_file.called
+            self.version.reload()
+            assert self.version.version == '1.3.1-signed'
+            assert file_hash != self.file_.generate_hash()
+            self.assert_backup()
+
+    @mock.patch('lib.crypto.tasks.sign_file')
+    def test_sign_bump_new_versions_not_default_compat(self, mock_sign_file):
+        """Sign files which are recent, event if not default to compatible."""
+        with amo.tests.copy_file(
+                'apps/files/fixtures/files/new-addon-signature.xpi',
+                self.file_.file_path):
+            file_hash = self.file_.generate_hash()
+            assert self.version.version == '1.3'
+            self.file_.update(binary_components=True,
+                              strict_compatibility=True)
+            tasks.sign_addons([self.addon.pk], force=True)
+            assert mock_sign_file.called
+            self.version.reload()
+            assert self.version.version == '1.3.1-signed'
+            assert file_hash != self.file_.generate_hash()
+            self.assert_backup()
+
     @mock.patch('lib.crypto.tasks.sign_file')
     def test_dont_resign_dont_bump_version_in_model(self, mock_sign_file):
         with amo.tests.copy_file(