Switch to google-auth lib for authentication

This is not backwards-compatible: we now require the service account credentials in json format instead of PKCS#12 (and as a result the username is no longer passed separately, since it is directly available in the json file) Fixes #251
mozilla-releng · Nov 13, 2023 · 4e2ccf9 · 4e2ccf9
1 parent aba32a2
commit 4e2ccf9
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 77 deletions.
diff --git a/README.md b/README.md
@@ -37,27 +37,17 @@ source venv/bin/activate
             * `sudo ln -s /usr/local/Cellar/openssl/1.0.2j/include/openssl/ /usr/local/include/openssl`
         5. Restore original permissions on /usr/local/bin:
             * `sudo chown root:wheel /usr/local`
-1. Some errors might happen when executing `mozapkpublisher/push_apk.py`
-    1. You might have errors like
-        * Errors in from_p12_keyfile in oauth2client/service_account.py or
-        * ImportError: cannot import name `_openssl_crypt`
-            * `pip uninstall oauth2client`
-            * `pip install oauth2client==2.0.0`
-            * `pip install google-api-python-client==1.5.0`
-    1. Symbol not found: `_BIO_new_CMS`
-        * `pip uninstall cryptography`
-        * `LDFLAGS="-L/usr/local/opt/openssl/lib" pip install cryptography --no-use-wheel`
 
 ## What to do when pushapk_scriptworker doesn't work?
 
 > A guide to manually publish APKs onto Google Play Store
 
-1. Generate a Google Play Store p12 certificate. This certificate needs to have write access to the app you want to publish. In this context, "app" means Fennec, Fennec Beta or Fennec Nightly.
+1. Generate a Google Play Store json certificate. This certificate needs to have write access to the app you want to publish. In this context, "app" means Fennec, Fennec Beta or Fennec Nightly.
 1. Execute the steps defined in the section above.
 1. Download the latest signed builds. For instance, for Fennec Nightly: `./mozapkpublisher/get_apk.py --latest-nightly`
 1. 
 ```sh
-./mozapkpublisher/push_apk.py --no-gp-string-update --track beta --credentials /path/to/your/googleplay/creds.p12 --service-account [email protected] x86.apk arm.apk
+./mozapkpublisher/push_apk.py --no-gp-string-update --track beta --credentials /path/to/your/googleplay/creds.json x86.apk arm.apk
 ```
 
   * Note `beta` track on Google Play, that's our way to show to people on Play Store that it's not a finished product. We don't use the "production" track for Nightly, unlike beta and release.

diff --git a/mozapkpublisher/check_rollout.py b/mozapkpublisher/check_rollout.py
@@ -40,8 +40,7 @@ def main():
                         type=int, default=7)
     config = parser.parse_args()
 
-    with GooglePlayEdit.transaction(config.service_account,
-                                    config.google_play_credentials_filename,
+    with GooglePlayEdit.transaction(config.google_play_credentials_filename,
                                     'org.mozilla.firefox', contact_server=True, dry_run=True) as edit:
         for (release, age) in check_rollout(edit, config.days):
             print('fennec {} is on staged rollout at {}% but it shipped {} days ago'.format(

diff --git a/mozapkpublisher/common/store.py b/mozapkpublisher/common/store.py
@@ -1,12 +1,14 @@
 from contextlib import contextmanager
 
-import httplib2
 import json
 import logging
 
+import google.auth
+import httplib2
+
 from apiclient.discovery import build
-from oauth2client.service_account import ServiceAccountCredentials
 from googleapiclient.errors import HttpError
+from google.oauth2 import service_account
 # HACK: importing mock in production is useful for option `--do-not-contact-google-play`
 from unittest.mock import MagicMock
 
@@ -16,16 +18,15 @@
 
 
 def add_general_google_play_arguments(parser):
-    parser.add_argument('--service-account', help='The service account email', required=True)
     parser.add_argument('--credentials', dest='google_play_credentials_filename',
-                        help='The p12 authentication file', required=True)
+                        help='The json authentication file', required=True)
 
     parser.add_argument('--commit', action='store_true',
                         help='Commit changes onto Google Play. This action cannot be reverted.')
     parser.add_argument('--do-not-contact-google-play', action='store_false', dest='contact_google_play',
                         help='''Prevent any request to reach Google Play. Use this option if you want to run the script
 without any valid credentials nor valid APKs. In fact, Google Play may error out at the first invalid piece of data sent.
---service-account and --credentials must still be provided (you can just fill them with random string and file).''')
+--credentials must still be provided (you can pass a random file name).''')
 
 
 class _ExecuteDummy:
@@ -186,8 +187,8 @@ def update_whats_new(self, language, apk_version_code, whats_new):
 
     @staticmethod
     @contextmanager
-    def transaction(service_account, credentials_file_name, package_name, *, contact_server, dry_run):
-        edit_resource = _create_google_edit_resource(contact_server, service_account, credentials_file_name)
+    def transaction(credentials_file_name, package_name, *, contact_server, dry_run):
+        edit_resource = _create_google_edit_resource(contact_server, credentials_file_name)
         edit_id = edit_resource.insert(body={}, packageName=package_name).execute()['id']
         google_play = GooglePlayEdit(edit_resource, edit_id, package_name)
         yield google_play
@@ -199,23 +200,16 @@ def transaction(service_account, credentials_file_name, package_name, *, contact
             logger.warning('Transaction not committed, since `dry_run` was `True`')
 
 
-def _create_google_edit_resource(contact_google_play, service_account, credentials_file_name):
+def _create_google_edit_resource(contact_google_play, credentials_file_name):
     if contact_google_play:
-        # Create an httplib2.Http object to handle our HTTP requests an
-        # authorize it with the Credentials. Note that the first parameter,
-        # service_account_name, is the Email address created for the Service
-        # account. It must be the email address associated with the key that
-        # was created.
         scope = 'https://www.googleapis.com/auth/androidpublisher'
-        credentials = ServiceAccountCredentials.from_p12_keyfile(
-            service_account,
+        credentials = service_account.Credentials.from_service_account_file(
             credentials_file_name,
-            scopes=scope
+            scopes=[scope],
         )
-        http = httplib2.Http()
-        http = credentials.authorize(http)
 
-        service = build(serviceName='androidpublisher', version='v3', http=http,
+        service = build(serviceName='androidpublisher', version='v3',
+                        credentials=credentials,
                         cache_discovery=False)
 
         return service.edits()

diff --git a/mozapkpublisher/common/utils.py b/mozapkpublisher/common/utils.py
@@ -47,14 +47,12 @@ def is_firefox_version_nightly(firefox_version):
 
 
 def add_push_arguments(parser):
-    parser.add_argument('--username', required=True,
-                        help='Google service account')
     parser.add_argument('--secret', required=True,
-                        help='File that contains google credentials')
+                        help='File that contains google credentials (json)')
     parser.add_argument('--do-not-contact-server', action='store_false', dest='contact_server',
                         help='''Prevent any request to reach the APK server. Use this option if
-you want to run the script without any valid credentials nor valid APKs. --service-account and
---credentials must still be provided (you can just fill them with random string and file).''')
+you want to run the script without any valid credentials nor valid APKs. --credentials must
+still be provided (you can pass a random file name).''')
     parser.add_argument('track', help='Track on which to upload')
     parser.add_argument(
         '--rollout-percentage',

diff --git a/mozapkpublisher/push_aab.py b/mozapkpublisher/push_aab.py
@@ -13,7 +13,6 @@
 
 def push_aab(
     aabs,
-    username,
     secret,
     track,
     rollout_percentage=None,
@@ -23,8 +22,7 @@ def push_aab(
     """
     Args:
         aabs: list of AAB files
-        username (str): Google Play service account
-        secret (str): Filename of Google Play Credentials file
+        secret (str): Filename of Google Play Credentials file (json)
         track (str): Google Play track to deploy to (e.g.: "nightly"). If "rollout" is chosen,
             the parameter `rollout_percentage` must be specified as well
         rollout_percentage (int): percentage of users to roll out this update to. Must be a number
@@ -51,7 +49,7 @@ def push_aab(
     # by package name here.
     aabs_by_package_name = metadata_by_package_name(aabs_metadata_per_paths)
     for package_name, extracted_aabs in aabs_by_package_name.items():
-        with GooglePlayEdit.transaction(username, secret, package_name, contact_server=contact_server,
+        with GooglePlayEdit.transaction(secret, package_name, contact_server=contact_server,
                                         dry_run=dry_run) as edit:
             edit.update_aab(extracted_aabs, **update_aab_kwargs)
 
@@ -64,7 +62,6 @@ def main():
 
     push_aab(
         config.aabs,
-        config.username,
         config.secret,
         config.track,
         config.rollout_percentage,

diff --git a/mozapkpublisher/push_apk.py b/mozapkpublisher/push_apk.py
@@ -13,7 +13,6 @@
 
 def push_apk(
     apks,
-    username,
     secret,
     expected_package_names,
     track,
@@ -28,8 +27,7 @@ def push_apk(
     """
     Args:
         apks: list of APK files
-        username (str): Google Play service account
-        secret (str): Filename of Google Play Credentials file
+        secret (str): Filename of Google Play Credentials file (json)
         expected_package_names (list of str): defines what the expected package names must be.
         track (str): Google Play track to deploy to (e.g.: "nightly"). If "rollout" is chosen, the parameter
             `rollout_percentage` must be specified as well
@@ -69,7 +67,7 @@ def push_apk(
     # by package name here.
     apks_by_package_name = metadata_by_package_name(apks_metadata_per_paths)
     for package_name, extracted_apks in apks_by_package_name.items():
-        with GooglePlayEdit.transaction(username, secret, package_name, contact_server=contact_server,
+        with GooglePlayEdit.transaction(secret, package_name, contact_server=contact_server,
                                         dry_run=dry_run) as edit:
             edit.update_app(extracted_apks, **update_app_kwargs)
 
@@ -82,7 +80,6 @@ def main():
 
     push_apk(
         config.apks,
-        config.username,
         config.secret,
         config.expected_package_names,
         config.track,

diff --git a/mozapkpublisher/test/common/test_store.py b/mozapkpublisher/test/common/test_store.py
@@ -21,24 +21,23 @@ def test_add_general_google_play_arguments():
     add_general_google_play_arguments(parser)
 
     config = parser.parse_args([
-        '--service-account', 'dummy@dummy', '--credentials', 'credentials.p12'
+        '--credentials', 'credentials.json'
     ])
-    assert config.google_play_credentials_filename == 'credentials.p12'
-    assert config.service_account == 'dummy@dummy'
+    assert config.google_play_credentials_filename == 'credentials.json'
 
 
 def test_google_edit_resource_for_options_contact(monkeypatch):
     service_mock = MagicMock()
     service_mock.edits.return_value = 'edit resource'
-    monkeypatch.setattr(store.ServiceAccountCredentials, 'from_p12_keyfile',
+    monkeypatch.setattr(store.service_account.Credentials, 'from_service_account_file',
                         lambda *args, **kwargs: MagicMock())
     monkeypatch.setattr(store, 'build', lambda *args, **kwargs: service_mock)
-    edit_resource = _create_google_edit_resource(True, 'account', 'credentials_filename')
+    edit_resource = _create_google_edit_resource(True, 'credentials_filename')
     assert edit_resource == 'edit resource'
 
 
 def test_google_edit_resource_for_options_do_not_contact():
-    edit_resource = _create_google_edit_resource(False, None, None)
+    edit_resource = _create_google_edit_resource(False, None)
     assert isinstance(edit_resource, MagicMock)
 
 
@@ -54,7 +53,7 @@ def edit_resource_mock():
 
 def test_google_rollout_without_rollout_percentage():
     # Note: specifying "track='rollout'" (even with a valid percentage) is currently deprecated
-    with GooglePlayEdit.transaction(None, None, 'dummy_package_name', contact_server=False,
+    with GooglePlayEdit.transaction(None, 'dummy_package_name', contact_server=False,
                                     dry_run=True) as edit:
         with pytest.raises(WrongArgumentGiven):
             edit._update_track('rollout', [1], None)
@@ -64,7 +63,7 @@ def test_google_rollout_without_rollout_percentage():
 def test_google_valid_rollout_percentage_with_track_rollout(create_edit_resource):
     mock_edits_resource = MagicMock()
     create_edit_resource.return_value = mock_edits_resource
-    with GooglePlayEdit.transaction(None, None, 'dummy_package_name', contact_server=False,
+    with GooglePlayEdit.transaction(None, 'dummy_package_name', contact_server=False,
                                     dry_run=True) as edit:
         edit._update_track('rollout', [1], 50)
 
@@ -84,7 +83,7 @@ def test_google_valid_rollout_percentage_with_track_rollout(create_edit_resource
 def test_google_valid_rollout_percentage_with_real_track(create_edit_resource):
     mock_edits_resource = MagicMock()
     create_edit_resource.return_value = mock_edits_resource
-    with GooglePlayEdit.transaction(None, None, 'dummy_package_name', contact_server=False,
+    with GooglePlayEdit.transaction(None, 'dummy_package_name', contact_server=False,
                                     dry_run=True) as edit:
         edit._update_track('beta', [1, 2], 20)
 
@@ -104,7 +103,7 @@ def test_google_valid_rollout_percentage_with_real_track(create_edit_resource):
 def test_google_play_edit_commit_transaction(create_edit_resource):
     mock_edits_resource = MagicMock()
     create_edit_resource.return_value = mock_edits_resource
-    with GooglePlayEdit.transaction(None, None, 'dummy_package_name', contact_server=False,
+    with GooglePlayEdit.transaction(None, 'dummy_package_name', contact_server=False,
                                     dry_run=False) as _:
         pass
 
@@ -115,7 +114,7 @@ def test_google_play_edit_commit_transaction(create_edit_resource):
 def test_google_play_edit_no_commit_transaction(create_edit_resource):
     mock_edits_resource = MagicMock()
     create_edit_resource.return_value = mock_edits_resource
-    with GooglePlayEdit.transaction(None, None, 'dummy_package_name', contact_server=False,
+    with GooglePlayEdit.transaction(None, 'dummy_package_name', contact_server=False,
                                     dry_run=True) as _:
         pass
 

diff --git a/mozapkpublisher/test/test_push_aab.py b/mozapkpublisher/test/test_push_aab.py
@@ -24,9 +24,6 @@
 aab2 = NamedTemporaryFile()
 
 AABS = [aab1, aab2]
-SERVICE_ACCOUNT = '[email protected]'
-CLIENT_ID = 'client'
-CLIENT_SECRET = 'secret'
 
 
 def patch_extract_metadata(monkeypatch):
@@ -54,7 +51,7 @@ def patch_store_transaction(monkeypatch_, patch_target):
     mock_edit = create_autospec(patch_target)
 
     @contextmanager
-    def fake_transaction(_, __, ___, *, contact_server, dry_run):
+    def fake_transaction(_, __, *, contact_server, dry_run):
         yield mock_edit
 
     monkeypatch_.setattr(patch_target, 'transaction', fake_transaction)
@@ -64,7 +61,7 @@ def fake_transaction(_, __, ___, *, contact_server, dry_run):
 def test_google(monkeypatch):
     mock_metadata = patch_extract_metadata(monkeypatch)
     edit_mock = patch_store_transaction(monkeypatch, store.GooglePlayEdit)
-    push_aab(AABS, SERVICE_ACCOUNT, credentials, 'production', rollout_percentage=50,
+    push_aab(AABS, credentials, 'production', rollout_percentage=50,
              contact_server=False)
     edit_mock.update_aab.assert_called_once_with([
         (aab1, mock_metadata[aab1]),
@@ -78,7 +75,7 @@ def test_push_aab_tunes_down_logs(monkeypatch):
     monkeypatch.setattr('mozapkpublisher.push_aab.extract_aabs_metadata', MagicMock())
     monkeypatch.setattr('mozapkpublisher.common.utils.metadata_by_package_name', MagicMock())
 
-    push_aab(AABS, SERVICE_ACCOUNT, credentials, 'alpha', contact_server=False)
+    push_aab(AABS, credentials, 'alpha', contact_server=False)
 
     main_logging_mock.init.assert_called_once_with()
 
@@ -94,7 +91,6 @@ def test_main_google(monkeypatch):
     file = os.path.join(os.path.dirname(__file__), 'data', 'blob')
     fail_manual_validation_args = [
         'script',
-        '--username', '[email protected]',
         '--secret', file,
         'alpha',
         file,
@@ -106,7 +102,6 @@ def test_main_google(monkeypatch):
 
         mock_push_aab.assert_called_once_with(
             ANY,
-            '[email protected]',
             file,
             'alpha',
             None,

diff --git a/mozapkpublisher/test/test_push_apk.py b/mozapkpublisher/test/test_push_apk.py
@@ -24,9 +24,6 @@
 apk_arm = NamedTemporaryFile()
 
 APKS = [apk_x86, apk_arm]
-SERVICE_ACCOUNT = '[email protected]'
-CLIENT_ID = 'client'
-CLIENT_SECRET = 'secret'
 
 
 def patch_extract_metadata(monkeypatch):
@@ -78,7 +75,7 @@ def patch_store_transaction(monkeypatch_, patch_target):
     mock_edit = create_autospec(patch_target)
 
     @contextmanager
-    def fake_transaction(_, __, ___, *, contact_server, dry_run):
+    def fake_transaction(_, __, *, contact_server, dry_run):
         yield mock_edit
 
     monkeypatch_.setattr(patch_target, 'transaction', fake_transaction)
@@ -88,7 +85,7 @@ def fake_transaction(_, __, ___, *, contact_server, dry_run):
 def test_google(monkeypatch):
     mock_metadata = patch_extract_metadata(monkeypatch)
     edit_mock = patch_store_transaction(monkeypatch, store.GooglePlayEdit)
-    push_apk(APKS, SERVICE_ACCOUNT, credentials, [], 'rollout', rollout_percentage=50,
+    push_apk(APKS, credentials, [], 'rollout', rollout_percentage=50,
              contact_server=False)
     edit_mock.update_app.assert_called_once_with([
         (apk_arm, mock_metadata[apk_arm]),
@@ -102,7 +99,7 @@ def test_push_apk_tunes_down_logs(monkeypatch):
     monkeypatch.setattr('mozapkpublisher.push_apk.extract_and_check_apks_metadata', MagicMock())
     monkeypatch.setattr('mozapkpublisher.common.utils.metadata_by_package_name', MagicMock())
 
-    push_apk(APKS, SERVICE_ACCOUNT, credentials, [], 'alpha', contact_server=False)
+    push_apk(APKS, credentials, [], 'alpha', contact_server=False)
 
     main_logging_mock.init.assert_called_once_with()
 
@@ -118,7 +115,6 @@ def test_main_google(monkeypatch):
     file = os.path.join(os.path.dirname(__file__), 'data', 'blob')
     fail_manual_validation_args = [
         'script',
-        '--username', '[email protected]',
         '--secret', file,
         'alpha',
         file,
@@ -131,7 +127,6 @@ def test_main_google(monkeypatch):
 
         mock_push_apk.assert_called_once_with(
             ANY,
-            '[email protected]',
             file,
             ['org.mozilla.fennec_aurora'],
             'alpha',

diff --git a/requirements/base.in b/requirements/base.in
@@ -1,9 +1,8 @@
 aiohttp
 androguard
 google-api-python-client
+google-auth
 humanize
 mozilla-version
-oauth2client
-pyOpenSSL   # Even not imported once in mozapkpublisher, it's needed to open p12 files
 requests
 voluptuous