Reland "macOS Signing Scripts: Move the NotarizationAndStapleLevel in…

…to Config"

This is a reland of commit 1dcdf9f

Original change's description:
> macOS Signing Scripts: Move the NotarizationAndStapleLevel into Config
>
> Bug: 1442256
> Change-Id: I0a174190e10dc57269791da947d24439e0d6bba6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4517690
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1142069}

Bug: 1442256
Change-Id: Id0db58295b5ee99a815d6b0cc05496915fd4ff58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4538252
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145411}

chrome/installer/mac/signing/config.py

@@ -4,7 +4,7 @@
 
 import os.path
 
-from .model import Distribution, NotarizationTool
+from .model import Distribution, NotarizeAndStapleLevel, NotarizationTool
 
 
 class ConfigError(Exception):
@@ -37,7 +37,8 @@ def __init__(self,
                  notary_asc_provider=None,
                  notary_team_id=None,
                  codesign_requirements_basic='',
-                 notarization_tool=None):
+                 notarization_tool=None,
+                 notarize=NotarizeAndStapleLevel.STAPLE):
         """Creates a CodeSignConfig that will sign the product using the static
         properties on the class, using the code signing identity passed to the
         constructor.
@@ -68,6 +69,7 @@ def __init__(self,
                 otherwise.
             notarization_tool: The tool to use to communicate with the Apple
                 notary service. If None, the config will choose a default.
+            notarize: The |model.NotarizeAndStapleLevel|.
         """
         assert identity is not None
         assert type(identity) is str
@@ -79,6 +81,7 @@ def __init__(self,
         self._codesign_requirements_basic = codesign_requirements_basic
         self._notary_team_id = notary_team_id
         self._notarization_tool = notarization_tool
+        self._notarize = notarize
 
     @staticmethod
     def is_chrome_branded():
@@ -153,6 +156,13 @@ def notarization_tool_path(self):
         """
         return None
 
+    @property
+    def notarize(self):
+        """Returns the |model.NotarizeAndStapleLevel| that controls how, if
+        at all, notarization and stapling of CodeSignedProducts should occur.
+        """
+        return self._notarize
+
     @property
     def app_product(self):
         """Returns the product name that is used for the outer .app bundle.

chrome/installer/mac/signing/driver.py

@@ -144,9 +144,10 @@ def main(args):
     parser.add_argument(
         '--notarize',
         nargs='?',
-        choices=model.NotarizeAndStapleLevel.valid_strings(),
+        choices=list(model.NotarizeAndStapleLevel),
         const='staple',
         default='none',
+        type=model.NotarizeAndStapleLevel.from_string,
         help='Specifies the requested notarization actions to be taken. '
         '`none` causes no notarization tasks to be performed. '
         '`nowait` submits the signed application and packaging to Apple for '
@@ -164,12 +165,6 @@ def main(args):
 
     args = parser.parse_args(args)
 
-    notarization = model.NotarizeAndStapleLevel.from_string(args.notarize)
-    if notarization.should_notarize():
-        if not args.notary_user or not args.notary_password:
-            parser.error('The `--notary-user` and `--notary-password` '
-                         'arguments are required if notarizing.')
-
     config = _create_config(
         model.pick(args, (
             'identity',
@@ -179,8 +174,14 @@ def main(args):
             'notary_asc_provider',
             'notary_team_id',
             'notarization_tool',
+            'notarize',
         )), args.development)
 
+    if config.notarize.should_notarize():
+        if not args.notary_user or not args.notary_password:
+            parser.error('The `--notary-user` and `--notary-password` '
+                         'arguments are required if notarizing.')
+
     if config.notarization_tool == model.NotarizationTool.NOTARYTOOL:
         # Let the config override notary_team_id, including a potentially
         # unspecified argument.
@@ -199,6 +200,5 @@ def main(args):
         paths,
         config,
         disable_packaging=args.disable_packaging,
-        notarization=notarization,
         skip_brands=args.skip_brands,
         channels=args.channels)

chrome/installer/mac/signing/driver_test.py

@@ -35,7 +35,6 @@ def test_main_flow(self, *args):
                 mock.ANY,
                 mock.ANY,
                 disable_packaging=mock.ANY,
-                notarization=mock.ANY,
                 skip_brands=mock.ANY,
                 channels=mock.ANY),
         ])
@@ -97,8 +96,7 @@ def test_simple_invoke(self, sign_all, **kwargs):
         self.assertTrue(config.run_spctl_assess)
         self.assertFalse(config.inject_get_task_allow_entitlement)
         self.assertFalse(sign_all.call_args.kwargs['disable_packaging'])
-        self.assertEquals(model.NotarizeAndStapleLevel.NONE,
-                          sign_all.call_args.kwargs['notarization'])
+        self.assertEquals(model.NotarizeAndStapleLevel.NONE, config.notarize)
         self.assertEquals([], sign_all.call_args.kwargs['skip_brands'])
         self.assertEquals([], sign_all.call_args.kwargs['channels'])
 
@@ -163,8 +161,7 @@ def test_notarize_unspecified(self, sign_all, **kwargs):
         ])
         self.assertEquals(1, sign_all.call_count)
         config = sign_all.call_args.args[1]
-        self.assertEquals(model.NotarizeAndStapleLevel.STAPLE,
-                          sign_all.call_args.kwargs['notarization'])
+        self.assertEquals(model.NotarizeAndStapleLevel.STAPLE, config.notarize)
         self.assertEquals('Notary-User', config.notary_user)
         self.assertEquals('@env:NOTARY', config.notary_password)
 
@@ -176,8 +173,7 @@ def test_notarize_specific(self, sign_all, **kwargs):
         ])
         self.assertEquals(1, sign_all.call_count)
         config = sign_all.call_args.args[1]
-        self.assertEquals(model.NotarizeAndStapleLevel.NOWAIT,
-                          sign_all.call_args.kwargs['notarization'])
+        self.assertEquals(model.NotarizeAndStapleLevel.NOWAIT, config.notarize)
         self.assertEquals('Notary-User', config.notary_user)
         self.assertEquals('@env:NOTARY', config.notary_password)
 
@@ -213,8 +209,7 @@ def test_notarize_notarytool(self, sign_all, **kwargs):
         ])
         self.assertEquals(1, sign_all.call_count)
         config = sign_all.call_args.args[1]
-        self.assertEquals(model.NotarizeAndStapleLevel.STAPLE,
-                          sign_all.call_args.kwargs['notarization'])
+        self.assertEquals(model.NotarizeAndStapleLevel.STAPLE, config.notarize)
         self.assertEquals('Notary-User', config.notary_user)
         self.assertEquals('@env:NOTARY', config.notary_password)
         self.assertEquals('Team1', config.notary_team_id)

chrome/installer/mac/signing/model.py

@@ -216,13 +216,15 @@ def should_wait(self):
     def should_staple(self):
         return self.value > self.WAIT_NOSTAPLE.value
 
-    @classmethod
-    def valid_strings(cls):
-        return tuple(level.name.lower().replace('_', '-') for level in cls)
+    def __str__(self):
+        return self.name.lower().replace('_', '-')
 
     @classmethod
     def from_string(cls, str):
-        return cls[str.upper().replace('-', '_')]
+        try:
+            return cls[str.upper().replace('-', '_')]
+        except KeyError:
+            raise ValueError(f'Invalid NotarizeAndStapleLevel: {str}')
 
 
 class NotarizationTool(enum.Enum):

chrome/installer/mac/signing/pipeline.py

@@ -372,7 +372,7 @@ def _package_and_sign_pkg(paths, dist_config):
             distribution_path, '--package-path', pkg_paths.work, '--sign',
             dist_config.installer_identity
         ]
-        if dist_config.notary_user:
+        if dist_config.notarize.should_notarize():
             # Assume if the config has notary authentication information that
             # the products will be notarized, which requires a secure
             # timestamp.
@@ -645,7 +645,6 @@ def include_channel(dist):
 def sign_all(orig_paths,
              config,
              disable_packaging=False,
-             notarization=model.NotarizeAndStapleLevel.STAPLE,
              skip_brands=[],
              channels=[]):
     """For each distribution in |config|, performs customization, signing, and
@@ -659,9 +658,6 @@ def sign_all(orig_paths,
             unpackaged signed app bundle will be copied to |paths.output|. If
             False, the packaging specified in the distribution will be
             performed.
-        notarization: The level of notarization to be performed. If
-            |disable_packaging| is False, the packages (dmg/pkg) will undergo
-            the same notarization.
         skip_brands: A list of brand code strings. If a distribution has a brand
             code in this list, or if a distribution has a brand code and
             |skip_brands| contains *, that distribution will be skipped.
@@ -687,7 +683,7 @@ def sign_all(orig_paths,
 
                 # If not packaging and not notarizing, then simply drop the
                 # signed bundle in the output directory when done signing.
-                if not do_packaging and not notarization.should_notarize():
+                if not do_packaging and not config.notarize.should_notarize():
                     dest_dir = paths.output
                 else:
                     dest_dir = notary_paths.work
@@ -708,7 +704,7 @@ def sign_all(orig_paths,
 
                 # If the build products are to be notarized, ZIP the app bundle
                 # and submit it for notarization.
-                if notarization.should_notarize():
+                if config.notarize.should_notarize():
                     zip_file = os.path.join(
                         notary_paths.work,
                         dist_config.packaging_basename + '.zip')
@@ -722,10 +718,10 @@ def sign_all(orig_paths,
 
         # If needed, wait for app notarization results to come back, and staple
         # if required.
-        if notarization.should_wait():
+        if config.notarize.should_wait():
             for result in notarize.wait_for_results(uuids_to_config.keys(),
                                                     config):
-                if notarization.should_staple():
+                if config.notarize.should_staple():
                     dist_config = uuids_to_config[result]
                     dest_dir = os.path.join(
                         notary_paths.work,
@@ -754,23 +750,23 @@ def sign_all(orig_paths,
                 if dist.package_as_dmg:
                     dmg_path = _package_and_sign_dmg(paths, dist_config)
 
-                    if notarization.should_notarize():
+                    if config.notarize.should_notarize():
                         uuid = notarize.submit(dmg_path, dist_config)
                         uuids_to_package_path[uuid] = dmg_path
 
                 if dist.package_as_pkg:
                     pkg_path = _package_and_sign_pkg(paths, dist_config)
 
-                    if notarization.should_notarize():
+                    if config.notarize.should_notarize():
                         uuid = notarize.submit(pkg_path, dist_config)
                         uuids_to_package_path[uuid] = pkg_path
 
             # If needed, wait for package notarization results to come back, and
             # staple if required.
-            if notarization.should_wait():
+            if config.notarize.should_wait():
                 for result in notarize.wait_for_results(
                         uuids_to_package_path.keys(), config):
-                    if notarization.should_staple():
+                    if config.notarize.should_staple():
                         package_path = uuids_to_package_path[result]
                         notarize.staple(package_path)
 

chrome/installer/mac/signing/pipeline_test.py

@@ -1319,11 +1319,9 @@ def test_sign_notarize_no_wait(self, **kwargs):
         kwargs[
             '_package_and_sign_dmg'].return_value = '/$O/AppProduct-99.0.9999.99.dmg'
 
-        config = test_config.TestConfig()
-        pipeline.sign_all(
-            self.paths,
-            config,
-            notarization=model.NotarizeAndStapleLevel.NOWAIT)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.NOWAIT)
+        pipeline.sign_all(self.paths, config)
 
         self.assertEqual(1, kwargs['_package_installer_tools'].call_count)
 
@@ -1366,11 +1364,9 @@ def test_sign_notarize_wait_no_staple(self, **kwargs):
         kwargs[
             '_package_and_sign_dmg'].return_value = '/$O/AppProduct-99.0.9999.99.dmg'
 
-        config = test_config.TestConfig()
-        pipeline.sign_all(
-            self.paths,
-            config,
-            notarization=model.NotarizeAndStapleLevel.WAIT_NOSTAPLE)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.WAIT_NOSTAPLE)
+        pipeline.sign_all(self.paths, config)
 
         self.assertEqual(1, kwargs['_package_installer_tools'].call_count)
 
@@ -1406,9 +1402,9 @@ def test_sign_no_notarization(self, **kwargs):
         for attr in kwargs:
             manager.attach_mock(kwargs[attr], attr)
 
-        config = test_config.TestConfig()
-        pipeline.sign_all(
-            self.paths, config, notarization=model.NotarizeAndStapleLevel.NONE)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.NONE)
+        pipeline.sign_all(self.paths, config)
 
         self.assertEqual(1, kwargs['_package_installer_tools'].call_count)
 
@@ -1431,12 +1427,9 @@ def test_sign_no_packaging_no_notarization(self, **kwargs):
         for attr in kwargs:
             manager.attach_mock(kwargs[attr], attr)
 
-        config = test_config.TestConfig()
-        pipeline.sign_all(
-            self.paths,
-            config,
-            disable_packaging=True,
-            notarization=model.NotarizeAndStapleLevel.NONE)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.NONE)
+        pipeline.sign_all(self.paths, config, disable_packaging=True)
 
         manager.assert_has_calls([
             # First customize the distribution and sign it.
@@ -1480,9 +1473,8 @@ def distributions(self):
                         package_as_pkg=True),
                 ]
 
-        config = Config()
-        pipeline.sign_all(
-            self.paths, config, notarization=model.NotarizeAndStapleLevel.NONE)
+        config = Config(notarize=model.NotarizeAndStapleLevel.NONE)
+        pipeline.sign_all(self.paths, config)
 
         self.assertEqual(1, kwargs['_package_installer_tools'].call_count)
         self.assertEqual(3, kwargs['_customize_and_sign_chrome'].call_count)
@@ -1534,11 +1526,10 @@ def distributions(self):
                     model.Distribution(),
                 ]
 
-        config = Config()
+        config = Config(notarize=model.NotarizeAndStapleLevel.NONE)
         pipeline.sign_all(
             self.paths,
             config,
-            notarization=model.NotarizeAndStapleLevel.NONE,
             skip_brands=skip_brands,
             channels=include_channels)
 

chrome/installer/mac/signing/signing.py

@@ -73,9 +73,9 @@ def sign_part(paths, config, part):
     path = os.path.join(paths.work, part.path)
     if _linker_signed_arm64_needs_force(path):
         command.append('--force')
-    if config.notary_user:
-        # Assume if the config has notary authentication information that the
-        # products will be notarized, which requires a secure timestamp.
+    if config.notarize.should_notarize():
+        # If the products will be notarized, the signature requires a secure
+        # timestamp.
         command.append('--timestamp')
     if part.sign_with_identifier:
         command.extend(['--identifier', part.identifier])

chrome/installer/mac/signing/signing_test.py

@@ -112,7 +112,6 @@ def test_sign_part_with_requirements(self, run_command,
             '/$W/Test.app'
         ])
 
-
     def test_sign_part_needs_force(self, run_command,
                                    linker_signed_arm64_needs_force):
         linker_signed_arm64_needs_force.return_value = True
@@ -138,7 +137,8 @@ def test_sign_part_with_requirements_needs_force(
 
     def test_sign_part_no_notary(self, run_command,
                                  linker_signed_arm64_needs_force):
-        config = test_config.TestConfig(notary_user=None, notary_password=None)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.NONE)
         part = model.CodeSignedProduct('Test.app', 'test.signing.app')
         signing.sign_part(self.paths, config, part)
         run_command.assert_called_once_with(