Improvement: Add option to specify allowed apps for keychain add-cert…

…ificates (#69)

* Add options to keychain add-certificates to specify which applications have access to the imported certificate

* Remove reference to undefined fixture

* Accept just executable name for allowed app

* Update docs

* Move method definition within the same class

CHANGELOG.md

@@ -1,3 +1,10 @@
+Version 0.4.5
+-------------
+
+**Improvements**
+
+- Improvement: Add options to `keychain add-certificates` to specify which applications have access to the imported certificate without warning.
+
 Version 0.4.4
 -------------
 

doc.py

@@ -88,6 +88,8 @@ def _process_choice(choices):
         def _process_default(default):
             if not default:
                 return ''
+            if isinstance(default, tuple) and len(default) > 1 and isinstance(default[0], Path):
+                default = ', '.join(str(p).replace(str(Path.home()), '$HOME') for p in default)
             if isinstance(default, tuple) and isinstance(default[0], Path):
                 default = default[0]
             if isinstance(default, Path):

docs/keychain/add-certificates.md

@@ -10,6 +10,9 @@ keychain add-certificates [-h] [--log-stream STREAM] [--no-color] [--version] [-
     [-p PATH]
     [-c CERTIFICATE_PATHS]
     [--certificate-password CERTIFICATE_PASSWORD]
+    [-a ALLOWED_APPLICATIONS]
+    [-A]
+    [-D]
 ```
 ### Optional arguments for action `add-certificates`
 
@@ -21,6 +24,18 @@ Path to pkcs12 certificate. Can be either a path literal, or a glob pattern to m
 
 
 Encrypted p12 certificate password. Alternatively to entering CERTIFICATE_PASSWORD in plaintext, it may also be specified using a "@env:" prefix followed by a environment variable name, or "@file:" prefix followed by a path to the file containing the value. Example: "@env:<variable>" uses the value in the environment variable named "<variable>", and "@file:<file_path>" uses the value from file at "<file_path>". [Default: '']
+##### `-a, --allow-app=ALLOWED_APPLICATIONS`
+
+
+Specify an application which may access the imported key without warning. Multiple arguments. Default:&nbsp;`codesign, productsign`
+##### `-A, --allow-all-applications`
+
+
+Allow any application to access the imported key without warning
+##### `-D, --disallow-all-applications`
+
+
+Do not allow any applications to access the imported key without warning
 ### Optional arguments for command `keychain`
 
 ##### `-p, --path=PATH`

src/codemagic/__version__.py

@@ -1,5 +1,5 @@
 __title__ = "codemagic-cli-tools"
 __description__ = "CLI tools used in Codemagic builds"
-__version__ = "0.4.4"
+__version__ = "0.4.5"
 __url__ = 'https://github.com/codemagic-ci-cd/cli-tools'
 __licence__ = 'GNU General Public License v3.0'

src/codemagic/cli/argument/argument.py

@@ -15,6 +15,10 @@
 
 class Argument(ArgumentProperties, enum.Enum):
 
+    @property
+    def flag(self) -> str:
+        return sorted(self.value.flags, key=len, reverse=True)[0]
+
     def register(self, argument_group: argparse._ArgumentGroup):
         kwargs = self.value.argparse_kwargs or {}
         if 'action' not in kwargs:

src/codemagic/tools/keychain.py

@@ -6,6 +6,7 @@
 import pathlib
 import shutil
 from tempfile import NamedTemporaryFile
+from typing import Iterable
 from typing import List
 from typing import Optional
 from typing import Sequence
@@ -77,6 +78,32 @@ class KeychainArgument(cli.Argument):
         description='Encrypted p12 certificate password',
         argparse_kwargs={'required': False, 'default': ''},
     )
+    ALLOWED_APPLICATIONS = cli.ArgumentProperties(
+        flags=('-a', '--allow-app'),
+        key='allowed_applications',
+        description='Specify an application which may access the imported key without warning',
+        type=pathlib.Path,
+        argparse_kwargs={
+            'required': False,
+            'default': (pathlib.Path('codesign'), pathlib.Path('productsign')),
+            'nargs': '+',
+            'metavar': 'allowed-app',
+        },
+    )
+    ALLOW_ALL_APPLICATIONS = cli.ArgumentProperties(
+        flags=('-A', '--allow-all-applications'),
+        key='allow_all_applications',
+        type=bool,
+        description='Allow any application to access the imported key without warning',
+        argparse_kwargs={'required': False, 'action': 'store_true'},
+    )
+    DISALLOW_ALL_APPLICATIONS = cli.ArgumentProperties(
+        flags=('-D', '--disallow-all-applications'),
+        key='disallow_all_applications',
+        type=bool,
+        description='Do not allow any applications to access the imported key without warning',
+        argparse_kwargs={'required': False, 'action': 'store_true'},
+    )
 
 
 @cli.common_arguments(KeychainArgument.PATH)
@@ -245,24 +272,58 @@ def _generate_path(self):
 
     @cli.action('add-certificates',
                 KeychainArgument.CERTIFICATE_PATHS,
-                KeychainArgument.CERTIFICATE_PASSWORD)
-    def add_certificates(self,
-                         certificate_path_patterns: Sequence[pathlib.Path],
-                         certificate_password: Password = Password('')):
+                KeychainArgument.CERTIFICATE_PASSWORD,
+                KeychainArgument.ALLOWED_APPLICATIONS,
+                KeychainArgument.ALLOW_ALL_APPLICATIONS,
+                KeychainArgument.DISALLOW_ALL_APPLICATIONS)
+    def add_certificates(
+            self,
+            certificate_path_patterns: Sequence[pathlib.Path],
+            certificate_password: Password = Password(''),
+            allowed_applications: Sequence[pathlib.Path] = KeychainArgument.ALLOWED_APPLICATIONS.get_default(),
+            allow_all_applications: Optional[bool] = KeychainArgument.ALLOW_ALL_APPLICATIONS.get_default(),
+            disallow_all_applications: Optional[bool] = KeychainArgument.DISALLOW_ALL_APPLICATIONS.get_default()):
         """
         Add p12 certificate to specified keychain.
         """
 
-        self.logger.info(f'Add certificates to keychain {self.path}')
+        add_for_all_apps = False
+        add_for_apps: List[str] = []
+        if allow_all_applications and disallow_all_applications:
+            raise KeychainArgument.ALLOW_ALL_APPLICATIONS.raise_argument_error(
+                f'Using mutually exclusive options '
+                f'{KeychainArgument.ALLOWED_APPLICATIONS.flag!r} and '
+                f'{KeychainArgument.DISALLOW_ALL_APPLICATIONS.flag!r}')
+        elif allow_all_applications:
+            add_for_all_apps = True
+        elif not disallow_all_applications:
+            add_for_apps = list(self._get_certificate_allowed_applications(allowed_applications))
+
+        self.logger.info('Add certificates to keychain %s', self.path)
         certificate_paths = list(self.find_paths(*certificate_path_patterns))
         if not certificate_paths:
             raise KeychainError('Did not find any certificates from specified locations')
         for certificate_path in certificate_paths:
-            self._add_certificate(certificate_path, certificate_password)
+            self._add_certificate(certificate_path, certificate_password, add_for_all_apps, add_for_apps)
+
+    @classmethod
+    def _get_certificate_allowed_applications(
+            cls, given_allowed_applications: Sequence[pathlib.Path]) -> Iterable[str]:
+        for application in given_allowed_applications:
+            resolved_path = shutil.which(application)
+            if resolved_path is None:
+                # Only raise exception if user-specified path is not present
+                if application not in KeychainArgument.ALLOWED_APPLICATIONS.get_default():
+                    raise KeychainArgument.ALLOWED_APPLICATIONS.raise_argument_error(
+                        f'Application "{application}" does not exist or is not in PATH')
+            else:
+                yield resolved_path
 
     def _add_certificate(self,
                          certificate_path: pathlib.Path,
-                         certificate_password: Optional[Password] = None):
+                         certificate_password: Optional[Password] = None,
+                         allow_for_all_apps: bool = False,
+                         allowed_applications: Sequence[str] = tuple()):
         self.logger.info(f'Add certificate {certificate_path} to keychain {self.path}')
         # If case of no password, we need to explicitly set -P '' flag. Otherwise,
         # security tries to open an interactive dialog to prompt the user for a password,
@@ -273,13 +334,18 @@ def _add_certificate(self,
             certificate_password = Password('')
             obfuscate_patterns = []
 
-        process = self.execute([
+        import_cmd = [
             'security', 'import', certificate_path,
             '-f', "pkcs12",
             '-k', self.path,
-            '-T', shutil.which('codesign'),
             '-P', certificate_password.value,
-        ], obfuscate_patterns=obfuscate_patterns)
+        ]
+        if allow_for_all_apps:
+            import_cmd.append('-A')
+        for allowed_application in allowed_applications:
+            import_cmd.extend(['-T', allowed_application])
+
+        process = self.execute(import_cmd, obfuscate_patterns=obfuscate_patterns)
 
         if process.returncode != 0:
             if 'The specified item already exists in the keychain' in process.stderr:

tests/tools/test_keychain.py

@@ -0,0 +1,58 @@
+import argparse
+import pathlib
+from unittest import mock
+from unittest.mock import Mock
+
+import pytest
+
+from codemagic.cli import CliProcess
+from codemagic.tools.keychain import Keychain
+from codemagic.tools.keychain import KeychainArgument
+
+
+def mock_find_paths(_self, *args):
+    return [*args]
+
+
+@pytest.mark.parametrize('allowed_apps, allow_all, disallow_all, expected_args, not_expected_args', [
+    (['app1', 'app2'], False, False, ['app1', 'app2'], ['-A']),  # Only allow specified apps
+    (['app1', 'app2'], True, False, ['-A'], ['app1', 'app2']),  # Allow all apps
+    (['app1', 'app2'], False, True, [], ['-A', 'app1', 'app2']),  # Do not allow any apps
+])
+@mock.patch.object(Keychain, 'find_paths', mock_find_paths)
+def test_add_certificates_allowed_applications(
+        allowed_apps, allow_all, disallow_all, expected_args, not_expected_args, cli_argument_group):
+    KeychainArgument.ALLOWED_APPLICATIONS.register(cli_argument_group)
+    keychain = Keychain(path=pathlib.Path('/tmp/keychain'))
+
+    with mock.patch.object(keychain, 'execute') as mock_execute, \
+            mock.patch('codemagic.tools.keychain.shutil') as mock_shutil:
+        mock_shutil.which = lambda path: str(path)
+        mock_execute.return_value = Mock(returncode=0, spec=CliProcess)
+        keychain.add_certificates(
+            [pathlib.Path('*.p12')],
+            allowed_applications=list(map(pathlib.Path, allowed_apps)),
+            allow_all_applications=allow_all,
+            disallow_all_applications=disallow_all,
+        )
+        mock_call_args = mock_execute.call_args[0][0]
+
+    for arg in expected_args:
+        assert arg in mock_call_args
+    for arg in not_expected_args:
+        assert arg not in mock_call_args
+
+
+@mock.patch.object(Keychain, 'find_paths', mock_find_paths)
+def test_add_certificates_all_apps_allowed_and_disallowed(cli_argument_group):
+    KeychainArgument.ALLOW_ALL_APPLICATIONS.register(cli_argument_group)
+    KeychainArgument.DISALLOW_ALL_APPLICATIONS.register(cli_argument_group)
+    with pytest.raises(argparse.ArgumentError) as argument_error:
+        Keychain(path=pathlib.Path('/tmp/keychain')).add_certificates(
+            [pathlib.Path('*.p12')],
+            allowed_applications=[],
+            allow_all_applications=True,
+            disallow_all_applications=True,
+        )
+    assert KeychainArgument.ALLOW_ALL_APPLICATIONS.flag in argument_error.value.argument_name
+    assert 'mutually exclusive options' in argument_error.value.message