Fix 163: make credentials save/load thread safe

.pre-commit-config.yaml

@@ -2,7 +2,7 @@ repos:
 - hooks:
   - id: black
     language_version: python3
-  repo: https://github.com/ambv/black
+  repo: https://github.com/psf/black
   rev: 22.3.0
 - hooks:
   - id: flake8

pydrive2/auth.py

@@ -103,6 +103,7 @@ def _decorated(self, *args, **kwargs):
         elif self.access_token_expired:
             self.Refresh()
             dirty = True
+        self.credentials.set_store(self._default_storage)
         if dirty and save_credentials:
             self.SaveCredentials()
 
@@ -133,6 +134,7 @@ def _decorated(self, *args, **kwargs):
                 dirty = True
         if code is not None:
             self.Auth(code)
+        self.credentials.set_store(self._default_storage)
         if dirty and save_credentials:
             self.SaveCredentials()
 
@@ -192,6 +194,9 @@ def __init__(self, settings_file="settings.yaml", http_timeout=None):
                 self.settings = self.DEFAULT_SETTINGS
             else:
                 ValidateSettings(self.settings)
+        self._storages = self._InitializeStoragesFromSettings()
+        # Only one (`file`) backend is supported now
+        self._default_storage = self._storages["file"]
 
     @property
     def access_token_expired(self):
@@ -321,6 +326,23 @@ def ServiceAuth(self):
                 sub=user_email
             )
 
+    def _InitializeStoragesFromSettings(self):
+        result = {"file": None}
+        backend = self.settings.get("save_credentials_backend")
+        save_credentials = self.settings.get("save_credentials")
+        if backend == "file":
+            credentials_file = self.settings.get("save_credentials_file")
+            if credentials_file is None:
+                raise InvalidConfigError(
+                    "Please specify credentials file to read"
+                )
+            result[backend] = Storage(credentials_file)
+        elif save_credentials:
+            raise InvalidConfigError(
+                "Unknown save_credentials_backend: %s" % backend
+            )
+        return result
+
     def LoadCredentials(self, backend=None):
         """Loads credentials or create empty credentials if it doesn't exist.
 
@@ -347,19 +369,26 @@ def LoadCredentialsFile(self, credentials_file=None):
         :raises: InvalidConfigError, InvalidCredentialsError
         """
         if credentials_file is None:
-            credentials_file = self.settings.get("save_credentials_file")
-            if credentials_file is None:
+            self._default_storage = self._storages["file"]
+            if self._default_storage is None:
                 raise InvalidConfigError(
-                    "Please specify credentials file to read"
+                    "Backend `file` is not configured, specify "
+                    "credentials file to read in the settings "
+                    "file or pass an explicit value"
                 )
+        else:
+            self._default_storage = Storage(credentials_file)
+
         try:
-            storage = Storage(credentials_file)
-            self.credentials = storage.get()
+            self.credentials = self._default_storage.get()
         except IOError:
             raise InvalidCredentialsError(
                 "Credentials file cannot be symbolic link"
             )
 
+        if self.credentials:
+            self.credentials.set_store(self._default_storage)
+
     def SaveCredentials(self, backend=None):
         """Saves credentials according to specified backend.
 
@@ -388,16 +417,20 @@ def SaveCredentialsFile(self, credentials_file=None):
         """
         if self.credentials is None:
             raise InvalidCredentialsError("No credentials to save")
+
         if credentials_file is None:
-            credentials_file = self.settings.get("save_credentials_file")
-            if credentials_file is None:
+            storage = self._storages["file"]
+            if storage is None:
                 raise InvalidConfigError(
-                    "Please specify credentials file to read"
+                    "Backend `file` is not configured, specify "
+                    "credentials file to read in the settings "
+                    "file or pass an explicit value"
                 )
-        try:
+        else:
             storage = Storage(credentials_file)
+
+        try:
             storage.put(self.credentials)
-            self.credentials.set_store(storage)
         except IOError:
             raise InvalidCredentialsError(
                 "Credentials file cannot be symbolic link"

pydrive2/test/README.rst

@@ -22,15 +22,10 @@ Run tests locally
       sidebar, and select **Service Accounts**. Click **+ CREATE SERVICE
       ACCOUNT**, on the next screen, enter **Service account name** e.g. "PyDrive
       tests", and click **Create**. Select **Continue** at the next **Service
-      account permissions** page, click at **+ CREATE KEY**, select **P12** and
-      **Create**. Save generated :code:`.p12` key file at your local disk.
-    - Copy downloaded :code:`p.12` file to :code:`pydrive2/test` directory.
-      Edit files :code:`pydrive2/test/settings/local/default.yaml` and
-      :code:`pydrive2/test/settings/local/test_oauth_test_06.yaml` by replacing
-      **your-service-account-email** with email of your new created service account
-      and by replacing **your-file-path.p12** with name of copied :code:`.p12` key
-      file, for example :code:`pydrive-test-270414-581c887879a3.p12`. Value for
-      **client_user_email** should be left blank.
+      account permissions** page, click at **+ CREATE KEY**, select **JSON** and
+      **Create**. Save generated :code:`.json` key file at your local disk.
+    - Copy downloaded :code:`json` file to :code:`/tmp/pydrive2/credentials.json`
+      directory.
 
 3. Optional. If you would like to use your own an OAuth client ID follow the steps:
     - Under `Google API Console <https://console.developers.google.com>`_ select
@@ -63,7 +58,7 @@ Run tests locally
 
 ::
 
-    pip install -e .[tests]
+    pip install -e .[tests,fsspec]
 
 
 5. Run tests:

pydrive2/test/settings/test_oauth_test_08.yaml

@@ -0,0 +1,8 @@
+client_config_backend: service
+service_config:
+  client_json_file_path: /tmp/pydrive2/credentials.json
+
+save_credentials: False
+
+oauth_scope:
+  - https://www.googleapis.com/auth/drive

pydrive2/test/settings/test_oauth_test_09.yaml

@@ -0,0 +1,10 @@
+client_config_backend: service
+service_config:
+  client_json_file_path: /tmp/pydrive2/credentials.json
+
+save_credentials: True
+save_credentials_backend: file
+save_credentials_file: credentials/9.dat
+
+oauth_scope:
+  - https://www.googleapis.com/auth/drive

pydrive2/test/test_oauth.py

@@ -1,5 +1,4 @@
 import os
-import unittest
 import time
 import pytest
 
@@ -9,98 +8,124 @@
     delete_file,
     settings_file_path,
 )
-
-
-class GoogleAuthTest(unittest.TestCase):
-    """Tests basic OAuth2 operations of auth.GoogleAuth."""
-
-    @classmethod
-    def setup_class(cls):
-        setup_credentials()
-
-    @pytest.mark.manual
-    def test_01_LocalWebserverAuthWithClientConfigFromFile(self):
-        # Delete old credentials file
-        delete_file("credentials/1.dat")
-        # Test if authentication works with config read from file
-        ga = GoogleAuth(settings_file_path("test_oauth_test_01.yaml"))
-        ga.LocalWebserverAuth()
-        self.assertEqual(ga.access_token_expired, False)
-        # Test if correct credentials file is created
-        self.CheckCredentialsFile("credentials/1.dat")
-        time.sleep(1)
-
-    @pytest.mark.manual
-    def test_02_LocalWebserverAuthWithClientConfigFromSettings(self):
-        # Delete old credentials file
-        delete_file("credentials/2.dat")
-        # Test if authentication works with config read from settings
-        ga = GoogleAuth(settings_file_path("test_oauth_test_02.yaml"))
-        ga.LocalWebserverAuth()
-        self.assertEqual(ga.access_token_expired, False)
-        # Test if correct credentials file is created
-        self.CheckCredentialsFile("credentials/2.dat")
-        time.sleep(1)
-
-    @pytest.mark.manual
-    def test_03_LocalWebServerAuthWithNoCredentialsSaving(self):
-        # Delete old credentials file
-        delete_file("credentials/3.dat")
-        # Provide wrong credentials file
-        ga = GoogleAuth(settings_file_path("test_oauth_test_03.yaml"))
-        ga.LocalWebserverAuth()
-        self.assertEqual(ga.access_token_expired, False)
-        # Test if correct credentials file is created
-        self.CheckCredentialsFile("credentials/3.dat", no_file=True)
-        time.sleep(1)
-
-    @pytest.mark.manual
-    def test_04_CommandLineAuthWithClientConfigFromFile(self):
-        # Delete old credentials file
-        delete_file("credentials/4.dat")
-        # Test if authentication works with config read from file
-        ga = GoogleAuth(settings_file_path("test_oauth_test_04.yaml"))
-        ga.CommandLineAuth()
-        self.assertEqual(ga.access_token_expired, False)
-        # Test if correct credentials file is created
-        self.CheckCredentialsFile("credentials/4.dat")
-        time.sleep(1)
-
-    @pytest.mark.manual
-    def test_05_ConfigFromSettingsWithoutOauthScope(self):
-        # Test if authentication works without oauth_scope
-        ga = GoogleAuth(settings_file_path("test_oauth_test_05.yaml"))
-        ga.LocalWebserverAuth()
-        self.assertEqual(ga.access_token_expired, False)
-        time.sleep(1)
-
-    @pytest.mark.skip(reason="P12 authentication is deprecated")
-    def test_06_ServiceAuthFromSavedCredentialsP12File(self):
-        setup_credentials("credentials/6.dat")
-        ga = GoogleAuth(settings_file_path("test_oauth_test_06.yaml"))
-        ga.ServiceAuth()
-        self.assertEqual(ga.access_token_expired, False)
-        time.sleep(1)
-
-    def test_07_ServiceAuthFromSavedCredentialsJsonFile(self):
-        # Have an initial auth so that credentials/7.dat gets saved
-        ga = GoogleAuth(settings_file_path("test_oauth_test_07.yaml"))
-        ga.ServiceAuth()
-        self.assertTrue(os.path.exists(ga.settings["save_credentials_file"]))
-
-        # Secondary auth should be made only using the previously saved
-        # login info
-        ga = GoogleAuth(settings_file_path("test_oauth_test_07.yaml"))
-        ga.ServiceAuth()
-
-        self.assertEqual(ga.access_token_expired, False)
-        time.sleep(1)
-
-    def CheckCredentialsFile(self, credentials, no_file=False):
-        ga = GoogleAuth(settings_file_path("test_oauth_default.yaml"))
-        ga.LoadCredentialsFile(credentials)
-        self.assertEqual(ga.access_token_expired, no_file)
-
-
-if __name__ == "__main__":
-    unittest.main()
+from oauth2client.file import Storage
+
+
+def setup_module(module):
+    setup_credentials()
+
+
+@pytest.mark.manual
+def test_01_LocalWebserverAuthWithClientConfigFromFile():
+    # Delete old credentials file
+    delete_file("credentials/1.dat")
+    # Test if authentication works with config read from file
+    ga = GoogleAuth(settings_file_path("test_oauth_test_01.yaml"))
+    ga.LocalWebserverAuth()
+    assert not ga.access_token_expired
+    # Test if correct credentials file is created
+    CheckCredentialsFile("credentials/1.dat")
+    time.sleep(1)
+
+
+@pytest.mark.manual
+def test_02_LocalWebserverAuthWithClientConfigFromSettings():
+    # Delete old credentials file
+    delete_file("credentials/2.dat")
+    # Test if authentication works with config read from settings
+    ga = GoogleAuth(settings_file_path("test_oauth_test_02.yaml"))
+    ga.LocalWebserverAuth()
+    assert not ga.access_token_expired
+    # Test if correct credentials file is created
+    CheckCredentialsFile("credentials/2.dat")
+    time.sleep(1)
+
+
+@pytest.mark.manual
+def test_03_LocalWebServerAuthWithNoCredentialsSaving():
+    # Delete old credentials file
+    delete_file("credentials/3.dat")
+    ga = GoogleAuth(settings_file_path("test_oauth_test_03.yaml"))
+    assert not ga.settings["save_credentials"]
+    ga.LocalWebserverAuth()
+    assert not ga.access_token_expired
+    time.sleep(1)
+
+
+@pytest.mark.manual
+def test_04_CommandLineAuthWithClientConfigFromFile():
+    # Delete old credentials file
+    delete_file("credentials/4.dat")
+    # Test if authentication works with config read from file
+    ga = GoogleAuth(settings_file_path("test_oauth_test_04.yaml"))
+    ga.CommandLineAuth()
+    assert not ga.access_token_expired
+    # Test if correct credentials file is created
+    CheckCredentialsFile("credentials/4.dat")
+    time.sleep(1)
+
+
+@pytest.mark.manual
+def test_05_ConfigFromSettingsWithoutOauthScope():
+    # Test if authentication works without oauth_scope
+    ga = GoogleAuth(settings_file_path("test_oauth_test_05.yaml"))
+    ga.LocalWebserverAuth()
+    assert not ga.access_token_expired
+    time.sleep(1)
+
+
+@pytest.mark.skip(reason="P12 authentication is deprecated")
+def test_06_ServiceAuthFromSavedCredentialsP12File():
+    setup_credentials("credentials/6.dat")
+    ga = GoogleAuth(settings_file_path("test_oauth_test_06.yaml"))
+    ga.ServiceAuth()
+    assert not ga.access_token_expired
+    time.sleep(1)
+
+
+def test_07_ServiceAuthFromSavedCredentialsJsonFile():
+    # Have an initial auth so that credentials/7.dat gets saved
+    ga = GoogleAuth(settings_file_path("test_oauth_test_07.yaml"))
+    credentials_file = ga.settings["save_credentials_file"]
+    # Delete old credentials file
+    delete_file(credentials_file)
+    assert not os.path.exists(credentials_file)
+    ga.ServiceAuth()
+    assert os.path.exists(credentials_file)
+    # Secondary auth should be made only using the previously saved
+    # login info
+    ga = GoogleAuth(settings_file_path("test_oauth_test_07.yaml"))
+    ga.ServiceAuth()
+    assert not ga.access_token_expired
+    time.sleep(1)
+
+
+def test_08_ServiceAuthFromJsonFileNoCredentialsSaving():
+    # Test that no credentials are saved and API is still functional
+    # We are testing that there are no exceptions at least
+    ga = GoogleAuth(settings_file_path("test_oauth_test_08.yaml"))
+    assert not ga.settings["save_credentials"]
+    ga.ServiceAuth()
+    time.sleep(1)
+
+
+def test_09_SaveLoadCredentialsUsesDefaultStorage(mocker):
+    # Test fix for https://github.com/iterative/PyDrive2/issues/163
+    # Make sure that Load and Save credentials by default reuse the
+    # same Storage (since it defined lock which make it TS)
+    ga = GoogleAuth(settings_file_path("test_oauth_test_09.yaml"))
+    credentials_file = ga.settings["save_credentials_file"]
+    # Delete old credentials file
+    delete_file(credentials_file)
+    assert not os.path.exists(credentials_file)
+    spy = mocker.spy(Storage, "__init__")
+    ga.ServiceAuth()
+    ga.LoadCredentials()
+    ga.SaveCredentials()
+    assert spy.call_count == 0
+
+
+def CheckCredentialsFile(credentials, no_file=False):
+    ga = GoogleAuth(settings_file_path("test_oauth_default.yaml"))
+    ga.LoadCredentialsFile(credentials)
+    assert ga.access_token_expired == no_file